Re: [PATCH 02/10] ttm: add a memcg accounting flag to the alloc/populate APIs

[sashiko-bot@kernel.org]

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 <airlied@redhat.com>

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/20260706052330.1110909-1-airlied@gmail.com?part=2