Skip to content

Commit 456b62e

Browse files
pixelclusterptr1337
authored andcommitted
drm/ttm: Use common ancestor of evictor and evictee as limit pool
When checking whether to skip certain buffers because they're protected by dmem.low, we're checking the effective protection of the evictee's cgroup, but depending on how the evictor's cgroup relates to the evictee's, the semantics of effective protection values change. When testing against cgroups from different subtrees, page_counter's recursive protection propagates memory protection afforded to a parent down to the child cgroups, even if the children were not explicitly protected. This prevents cgroups whose parents were afforded no protection from stealing memory from cgroups whose parents were afforded more protection, without users having to explicitly propagate this protection. However, if we always calculate protection from the root cgroup, this breaks prioritization of sibling cgroups: If one cgroup was explicitly protected and its siblings were not, the protected cgroup should get higher priority, i.e. the protected cgroup should be able to steal from unprotected siblings. This only works if we restrict the protection calculation to the subtree shared by evictor and evictee. Signed-off-by: Natalie Vock <natalie.vock@gmx.de>
1 parent 74145ca commit 456b62e

1 file changed

Lines changed: 40 additions & 3 deletions

File tree

drivers/gpu/drm/ttm/ttm_bo.c

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -628,11 +628,48 @@ static s64 ttm_bo_evict_cb(struct ttm_lru_walk *walk, struct ttm_buffer_object *
628628
{
629629
struct ttm_bo_evict_walk *evict_walk =
630630
container_of(walk, typeof(*evict_walk), walk);
631+
struct dmem_cgroup_pool_state *limit_pool, *ancestor = NULL;
632+
bool evict_valuable;
631633
s64 lret;
632634

633-
if (!dmem_cgroup_state_evict_valuable(evict_walk->alloc_state->limit_pool,
634-
bo->resource->css, evict_walk->try_low,
635-
&evict_walk->hit_low))
635+
/*
636+
* If may_try_low is not set, then we're trying to evict unprotected
637+
* buffers in favor of a protected allocation for charge_pool. Explicitly skip
638+
* buffers belonging to the same cgroup here - that cgroup is definitely protected,
639+
* even though dmem_cgroup_state_evict_valuable would allow the eviction because a
640+
* cgroup is always allowed to evict from itself even if it is protected.
641+
*/
642+
if (!evict_walk->alloc_state->may_try_low &&
643+
bo->resource->css == evict_walk->alloc_state->charge_pool)
644+
return 0;
645+
646+
limit_pool = evict_walk->alloc_state->limit_pool;
647+
/*
648+
* If there is no explicit limit pool, find the root of the shared subtree between
649+
* evictor and evictee. This is important so that recursive protection rules can
650+
* apply properly: Recursive protection distributes cgroup protection afforded
651+
* to a parent cgroup but not used explicitly by a child cgroup between all child
652+
* cgroups (see docs of effective_protection in mm/page_counter.c). However, when
653+
* direct siblings compete for memory, siblings that were explicitly protected
654+
* should get prioritized over siblings that weren't. This only happens correctly
655+
* when the root of the shared subtree is passed to
656+
* dmem_cgroup_state_evict_valuable. Otherwise, the effective-protection
657+
* calculation cannot distinguish direct siblings from unrelated subtrees and the
658+
* calculated protection ends up wrong.
659+
*/
660+
if (!limit_pool) {
661+
ancestor = dmem_cgroup_get_common_ancestor(bo->resource->css,
662+
evict_walk->alloc_state->charge_pool);
663+
limit_pool = ancestor;
664+
}
665+
666+
evict_valuable = dmem_cgroup_state_evict_valuable(limit_pool, bo->resource->css,
667+
evict_walk->try_low,
668+
&evict_walk->hit_low);
669+
if (ancestor)
670+
dmem_cgroup_pool_state_put(ancestor);
671+
672+
if (!evict_valuable)
636673
return 0;
637674

638675
if (bo->pin_count || !bo->bdev->funcs->eviction_valuable(bo, evict_walk->place))

0 commit comments

Comments
 (0)