public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tursulin@ursulin.net>
To: Natalie Vock <natalie.vock@gmx.de>,
	Maarten Lankhorst <dev@lankhorst.se>,
	Maxime Ripard <mripard@kernel.org>, Tejun Heo <tj@kernel.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Michal Koutný <mkoutny@suse.com>,
	Christian Koenig <christian.koenig@amd.com>,
	Huang Rui <ray.huang@amd.com>,
	Matthew Auld <matthew.auld@intel.com>,
	Matthew Brost <matthew.brost@intel.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>
Cc: cgroups@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v6 5/6] drm/ttm: Be more aggressive when allocating below protection limit
Date: Fri, 13 Mar 2026 13:29:07 +0000	[thread overview]
Message-ID: <c1b8d304-09d3-47cf-a314-4a8777b85e6f@ursulin.net> (raw)
In-Reply-To: <20260313-dmemcg-aggressive-protect-v6-5-7c71cc1492db@gmx.de>


On 13/03/2026 11:40, Natalie Vock wrote:
> 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.
> 
> Signed-off-by: Natalie Vock <natalie.vock@gmx.de>
> ---
>   drivers/gpu/drm/ttm/ttm_bo.c | 51 +++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 48 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 4adc9b80cba4a..7300b91b77dd3 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -496,6 +496,10 @@ struct ttm_bo_alloc_state {
>   	struct dmem_cgroup_pool_state *limit_pool;
>   	/** @in_evict: Whether we are currently evicting buffers */
>   	bool in_evict;
> +	/** @may_try_low: If only unprotected BOs, i.e. BOs whose cgroup
> +	 *  is exceeding its dmem low/min protection, should be considered for eviction
> +	 */
> +	bool may_try_low;
>   };
>   
>   /**
> @@ -545,6 +549,42 @@ static int ttm_bo_alloc_at_place(struct ttm_buffer_object *bo,
>   		}
>   	}
>   
> +	/*
> +	 * cgroup protection plays a special role in eviction.
> +	 * Conceptually, protection of memory via the dmem cgroup controller
> +	 * entitles the protected cgroup to use a certain amount of memory.
> +	 * There are two types of protection - the 'low' limit is a
> +	 * "best-effort" protection, whereas the 'min' limit provides a hard
> +	 * guarantee that memory within the cgroup's allowance will not be
> +	 * evicted under any circumstance.
> +	 *
> +	 * To faithfully model this concept in TTM, we also need to take cgroup
> +	 * protection into account when allocating. When allocation in one
> +	 * place fails, TTM will default to trying other places first before
> +	 * evicting.
> +	 * If the allocation is covered by dmem cgroup protection, however,
> +	 * this prevents the allocation from using the memory it is "entitled"
> +	 * to. To make sure unprotected allocations cannot push new protected
> +	 * allocations out of places they are "entitled" to use, we should
> +	 * evict buffers not covered by any cgroup protection, if this
> +	 * allocation is covered by cgroup protection.
> +	 *
> +	 * Buffers covered by 'min' protection are a special case - the 'min'
> +	 * limit is a stronger guarantee than 'low', and thus buffers protected
> +	 * by 'low' but not 'min' should also be considered for eviction.
> +	 * 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);

For some value of optimisation you could combine the two calls of 
dmem_cgroup_below_min/low into a single helper which returns both min 
and low, given how it is the only callers, saving a pair of function 
calls and tree traversing calculations. Or.. I am not sure if simply 
exporting dmem_cgroup_calculate_protection() and making it return 
page_counters would be okay, or a copy of them, or a new data structure 
would need to be added.

Anyway, that can be discussed separately. In the meantime this patch 
looks good to me:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

Regards,

Tvrtko

> +	}
> +
>   	ret = ttm_resource_alloc(bo, place, res, alloc_state->charge_pool);
>   	if (ret) {
>   		if (ret == -ENOSPC && may_evict)
> @@ -657,8 +697,12 @@ static int ttm_bo_evict_alloc(struct ttm_device *bdev,
>   	evict_walk.walk.arg.trylock_only = true;
>   	lret = ttm_lru_walk_for_evict(&evict_walk.walk, bdev, man, 1);
>   
> -	/* One more attempt if we hit low limit? */
> -	if (!lret && evict_walk.hit_low) {
> +	/* If we failed to find enough BOs to evict, but we skipped over
> +	 * some BOs because they were covered by dmem low protection, retry
> +	 * evicting these protected BOs too, except if we're told not to
> +	 * consider protected BOs at all.
> +	 */
> +	if (!lret && evict_walk.hit_low && state->may_try_low) {
>   		evict_walk.try_low = true;
>   		lret = ttm_lru_walk_for_evict(&evict_walk.walk, bdev, man, 1);
>   	}
> @@ -679,7 +723,8 @@ static int ttm_bo_evict_alloc(struct ttm_device *bdev,
>   	} while (!lret && evict_walk.evicted);
>   
>   	/* We hit the low limit? Try once more */
> -	if (!lret && evict_walk.hit_low && !evict_walk.try_low) {
> +	if (!lret && evict_walk.hit_low && !evict_walk.try_low &&
> +			state->may_try_low) {
>   		evict_walk.try_low = true;
>   		goto retry;
>   	}
> 


  reply	other threads:[~2026-03-13 13:29 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-13 11:39 [PATCH v6 0/6] cgroup/dmem,drm/ttm: Improve protection in contended cases Natalie Vock
2026-03-13 11:40 ` [PATCH v6 1/6] cgroup/dmem: Add queries for protection values Natalie Vock
2026-03-13 21:10   ` Claude review: " Claude Code Review Bot
2026-03-13 11:40 ` [PATCH v6 2/6] cgroup,cgroup/dmem: Add (dmem_)cgroup_common_ancestor helper Natalie Vock
2026-03-13 14:16   ` Michal Koutný
2026-03-13 21:10   ` Claude review: " Claude Code Review Bot
2026-03-13 11:40 ` [PATCH v6 3/6] drm/ttm: Extract code for attempting allocation in a place Natalie Vock
2026-03-13 21:10   ` Claude review: " Claude Code Review Bot
2026-03-13 11:40 ` [PATCH v6 4/6] drm/ttm: Split cgroup charge and resource allocation Natalie Vock
2026-03-13 12:53   ` Tvrtko Ursulin
2026-03-13 21:10   ` Claude review: " Claude Code Review Bot
2026-03-13 11:40 ` [PATCH v6 5/6] drm/ttm: Be more aggressive when allocating below protection limit Natalie Vock
2026-03-13 13:29   ` Tvrtko Ursulin [this message]
2026-03-13 21:10   ` Claude review: " Claude Code Review Bot
2026-03-13 11:40 ` [PATCH v6 6/6] drm/ttm: Use common ancestor of evictor and evictee as limit pool Natalie Vock
2026-03-13 14:11   ` Tvrtko Ursulin
2026-03-13 14:16   ` Michal Koutný
2026-03-13 21:10   ` Claude review: " Claude Code Review Bot
2026-03-13 21:10 ` Claude review: cgroup/dmem,drm/ttm: Improve protection in contended cases Claude Code Review Bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c1b8d304-09d3-47cf-a314-4a8777b85e6f@ursulin.net \
    --to=tursulin@ursulin.net \
    --cc=airlied@gmail.com \
    --cc=cgroups@vger.kernel.org \
    --cc=christian.koenig@amd.com \
    --cc=dev@lankhorst.se \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hannes@cmpxchg.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=matthew.auld@intel.com \
    --cc=matthew.brost@intel.com \
    --cc=mkoutny@suse.com \
    --cc=mripard@kernel.org \
    --cc=natalie.vock@gmx.de \
    --cc=ray.huang@amd.com \
    --cc=simona@ffwll.ch \
    --cc=tj@kernel.org \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox