Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] The `memcg_account` flag is not plumbed through `ttm_tt_restore` and 
`ttm_pool_restore_and_alloc`, bypassing memory cgroup accounting for restored 
TTs.
--

commit b7727d0f7c7e75f86466225e149812b98610b27c
Author: Dave Airlie <[email protected]>

ttm: add a memcg accounting flag to the alloc/populate APIs

This flag does nothing yet, but this just changes the APIs to accept
it in the future across all users.

[ ... ]

> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
> index 278bbe7a11add..e4dbf4c93091c 100644
> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> +++ b/drivers/gpu/drm/ttm/ttm_pool.c

[ ... ]

> @@ -935,7 +937,7 @@ int ttm_pool_restore_and_alloc(struct ttm_pool *pool, 
> struct ttm_tt *tt,
>                       return 0;
>       }
>  
> -     return __ttm_pool_alloc(pool, tt, ctx, &alloc, restore);
> +     return __ttm_pool_alloc(pool, tt, false, ctx, &alloc, restore);
>  }

[Severity: High]
Should ttm_pool_restore_and_alloc() and ttm_tt_restore() also be updated to
accept the memcg_account flag?

By hardcoding false here, pages allocated to restore a backed-up TT will bypass
memory cgroup accounting. This could allow userspace to circumvent memory limits
by triggering swapout/backup and subsequently forcing a restore.

[ ... ]

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 4c80bac676229..20a10a174d1d3 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c

[ ... ]

> @@ -553,7 +554,7 @@ static int xe_ttm_tt_populate(struct ttm_device *ttm_dev, 
> struct ttm_tt *tt,
>               err = ttm_tt_restore(ttm_dev, tt, ctx);
>       } else {
>               ttm_tt_clear_backed_up(tt);
> -             err = ttm_pool_alloc(&ttm_dev->pool, tt, ctx);
> +             err = ttm_pool_alloc(&ttm_dev->pool, tt, memcg_account, ctx);
>       }
>       if (err)

[Severity: High]
Because ttm_tt_restore() wasn't updated to take the memcg_account parameter, the
flag passed into xe_ttm_tt_populate() is dropped when a backed-up buffer is
restored. Does this mean restored TTs in the xe driver will fail to be tracked
by their memory cgroup?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to