From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/ttm: Use common ancestor of evictor and evictee as limit pool Date: Fri, 27 Feb 2026 13:36:29 +1000 Message-ID: In-Reply-To: <20260225-dmemcg-aggressive-protect-v4-6-de847ab35184@gmx.de> References: <20260225-dmemcg-aggressive-protect-v4-0-de847ab35184@gmx.de> <20260225-dmemcg-aggressive-protect-v4-6-de847ab35184@gmx.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review This correctly implements recursive protection semantics for sibling cgroup competition. The commit message explanation is excellent. The self-eviction guard is important and well-commented: ```c + if (evict_walk->alloc_state->only_evict_unprotected && + bo->resource->css == evict_walk->alloc_state->charge_pool) + return 0; ``` Without this, `dmem_cgroup_state_evict_valuable` would allow a cgroup to evict from itself (since a cgroup is always allowed to self-evict), which would defeat the purpose of `only_evict_unprotected`. The common ancestor fallback logic is correct: ```c + if (!limit_pool) + limit_pool = dmem_cgroup_common_ancestor(bo->resource->css, + evict_walk->alloc_state->charge_pool); ``` The extensive comment explaining why the common ancestor matters for `effective_protection` calculation is very helpful. **One observation**: The `below_min`/`below_low` checks in Patch 5 use `NULL` (global root) while eviction decisions in this patch use the common ancestor. These are at different logical levels - the global check determines the "policy" (should we attempt aggressive eviction?) while the per-candidate check determines the "mechanism" (is this specific buffer evictable?). This design makes sense. No blocking issues with this patch. --- Generated by Claude Code Patch Reviewer