Re: [PATCH v8 5/6] drm/ttm: Be more aggressive when allocating below protection limit

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Allocations protected by the dmem cgroup `min` limit will incorrectly evict their own cgroup's buffers during the early eviction pass, preventing the cgroup from reaching its `min` guarantee.
--

--- Patch [5]: [PATCH v8 5/6] drm/ttm: Be more aggressive when allocating below protection limit ---
commit 30d3ed5047eb42c7b72fcb6f1d1973210be68163
Author: Natalie Vock <[email protected]>

drm/ttm: Be more aggressive when allocating below protection limit
    
When the cgroup's memory usage is below the low/min limit and allocation
fails, try evicting some unprotected buffers to make space. Otherwise,
application buffers may be forced to go into GTT even though usage is
below the corresponding low/min limit, if other applications filled VRAM
with their allocations first.

> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 3becddd2c43b8..6bbea148838e4 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
[ ... ]
> @@ -537,12 +541,50 @@ static int ttm_bo_alloc_at_place(struct ttm_buffer_object *bo,
[ ... ]
> +	/*
> +	 * Buffers protected by 'min' will never be considered for eviction
> +	 * anyway, so the regular eviction path should be triggered here.
> +	 * Buffers protected by 'low' but not 'min' will take a special
> +	 * eviction path that only evicts buffers covered by neither 'low' or
> +	 * 'min' protections.
> +	 */
> +	if (!alloc_state->in_evict) {
> +		may_evict |= dmem_cgroup_below_min(NULL, alloc_state->charge_pool);
> +		alloc_state->may_try_low = may_evict;
> +
> +		may_evict |= dmem_cgroup_below_low(NULL, alloc_state->charge_pool);
> +	}

[Severity: High]
Does this code inadvertently allow a cgroup to evict its own buffers when it is
below the min limit?

When dmem_cgroup_below_min() returns true, alloc_state->may_try_low is set
to true here.

Later in ttm_bo_evict_cb(), this causes the self-eviction skip check to be
bypassed:

	if (!evict_walk->alloc_state->may_try_low &&
			bo->resource->css == evict_walk->alloc_state->charge_pool)
		return 0;

Because dmem_cgroup_state_evict_valuable() allows a pool to evict from itself,
this means the cgroup will evict its own buffers instead of unprotected buffers
from other cgroups. Will this prevent the cgroup from reaching its min limit
guarantee?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5