From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: cgroup,cgroup/dmem: Add (dmem_)cgroup_common_ancestor helper Date: Tue, 03 Mar 2026 13:29:38 +1000 Message-ID: In-Reply-To: <20260302-dmemcg-aggressive-protect-v5-2-ffd3a2602309@gmx.de> References: <20260302-dmemcg-aggressive-protect-v5-0-ffd3a2602309@gmx.de> <20260302-dmemcg-aggressive-protect-v5-2-ffd3a2602309@gmx.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Purpose:** Adds `cgroup_common_ancestor()` (generic cgroup helper) and `d= mem_cgroup_common_ancestor()` (dmem-specific wrapper), plus refactors `get_= cg_pool_unlocked()` into a lookup-only `find_cg_pool_unlocked()`. **Bug =E2=80=94 doc comment copy-paste error:** ```c /** * dmem_cgroup_common_ancestor(): Find the first common ancestor of two poo= ls. * @a: First pool to find the common ancestor of. * @b: First pool to find the common ancestor of. ``` `@b` should say **"Second pool"**, not "First pool." **`cgroup_common_ancestor()` correctness:** The algorithm is correct =E2=80= =94 walking from `min(a->level, b->level)` down to 0, comparing the `ancest= ors[]` arrays. Since all cgroups in the same hierarchy share the root at le= vel 0, this will always find an ancestor (never return NULL) for cgroups in= the same subsystem. **Reference counting concern:** `dmem_cgroup_common_ancestor()` calls `find= _cg_pool_unlocked()` which performs an RCU lookup. If this returns a pool p= ointer without grabbing a reference, then patch 6's `dmem_cgroup_pool_state= _put(ancestor)` would be unbalanced. The diff context doesn't show enough o= f the original `get_cg_pool_unlocked()` to confirm whether `find_cg_pool_un= locked()` grabs a `css_tryget()` reference. Please verify that `find_cg_poo= l_unlocked()` returns a properly referenced pool, or add explicit reference= counting in `dmem_cgroup_common_ancestor()`. The refactoring of `get_cg_pool_unlocked()` into `find_cg_pool_unlocked()` = + `get_cg_pool_unlocked()` is clean =E2=80=94 the "find" variant does the l= ookup without allocation, and the "get" variant retains the allocate-on-mis= s behavior. --- Generated by Claude Code Patch Reviewer