There's no need to explain every background detail behind the implementation, and the excessive verbosity makes it almost a bit annoying to read through.
Signed-off-by: Natalie Vock <[email protected]> --- I remember that there was some confusion about what the exact cgroup semantics were and how they interacted with TTM, and I think I might've gone this overboard with documenting literally everything in comments as a misguided attempt to remove that confusion. These comments even ended up sounding almost exactly like LLM slop, down to details like phrasing and excessive use of dashes. That's somewhere between impressive and disheartening if you consider that I actually wrote every word in these comments by hand. Either way, even though it wasn't generated by an LLM this time, the slop should still go. --- drivers/gpu/drm/ttm/ttm_bo.c | 56 +++++------------------------------- 1 file changed, 7 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 53c68e29ebc21..8f1b1981dd43f 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -532,14 +532,6 @@ static int ttm_bo_alloc_at_place(struct ttm_buffer_object *bo, force_space ? &alloc_state->limit_pool : NULL); if (ret) { - /* - * -EAGAIN means the charge failed, which we treat - * like an allocation failure. Therefore, return an - * error code indicating the allocation failed - - * either -EBUSY if the allocation should be - * retried with eviction, or -ENOSPC if there should - * be no second attempt. - */ if (!alloc_state->in_evict) alloc_state->may_try_low = may_evict; if (ret == -EAGAIN) @@ -549,33 +541,9 @@ 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 allocation fails and we back off to some other domain, that's technically a kind + * of eviction happening, so try harder to evict something if cgroup protections are + * supposed to prevent evictions. */ if (!alloc_state->in_evict) { may_evict |= dmem_cgroup_below_min(NULL, alloc_state->charge_pool); @@ -634,28 +602,18 @@ static s64 ttm_bo_evict_cb(struct ttm_lru_walk *walk, struct ttm_buffer_object * /* * If may_try_low is not set, then we're trying to evict unprotected - * buffers in favor of a protected allocation for charge_pool. Explicitly skip - * buffers belonging to the same cgroup here - that cgroup is definitely protected, - * even though dmem_cgroup_state_evict_valuable would allow the eviction because a - * cgroup is always allowed to evict from itself even if it is protected. + * buffers in favor of a protected allocation for charge_pool, so evicting + * from charge_pool itself makes no sense. */ if (!evict_walk->alloc_state->may_try_low && bo->resource->css == evict_walk->alloc_state->charge_pool) return 0; limit_pool = evict_walk->alloc_state->limit_pool; + /* * If there is no explicit limit pool, find the root of the shared subtree between - * evictor and evictee. This is important so that recursive protection rules can - * apply properly: Recursive protection distributes cgroup protection afforded - * to a parent cgroup but not used explicitly by a child cgroup between all child - * cgroups (see docs of effective_protection in mm/page_counter.c). However, when - * direct siblings compete for memory, siblings that were explicitly protected - * should get prioritized over siblings that weren't. This only happens correctly - * when the root of the shared subtree is passed to - * dmem_cgroup_state_evict_valuable. Otherwise, the effective-protection - * calculation cannot distinguish direct siblings from unrelated subtrees and the - * calculated protection ends up wrong. + * evictor and evictee so cgroup recursive protection semantics apply properly. */ if (!limit_pool) { ancestor = dmem_cgroup_get_common_ancestor(bo->resource->css, -- 2.55.0
