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/dmem: implement dmem.high soft limit via prioritized eviction Date: Thu, 04 Jun 2026 15:03:19 +1000 Message-ID: In-Reply-To: <20260531-feature-dmem-high-v5-1-1c6c532b26a9@gmail.com> References: <20260531-feature-dmem-high-v5-1-1c6c532b26a9@gmail.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Overall Series Review Subject: cgroup/dmem: implement dmem.high soft limit via prioritized evicti= on Author: Qiliang Yuan Patches: 1 Reviewed: 2026-06-04T15:03:19.411766 --- This is a single-patch v5 series adding a `dmem.high` soft limit to the dme= m cgroup v2 controller. The concept is sound: when a hard `max` limit is hi= t and eviction is triggered, BOs from cgroups exceeding their `dmem.high` a= re targeted first via a new "Pass 1" in `ttm_bo_evict_alloc()`. This provid= es a useful middle ground between no limit and hard failure. The patch has gone through significant iteration (v1=E2=86=92v5) addressing= NULL-pointer issues, deadlock risks, and control flow problems. The v5 cha= nges are clearly motivated by real bugs found in v4. However, several issue= s remain: **Key concerns:** 1. **Missing `elow` check in the `try_high` path** =E2=80=94 The try_high b= lock respects `emin` but silently ignores `elow`, meaning BOs protected by = `dmem.low` can be evicted during the high-priority pass. 2. **No documentation** =E2=80=94 The patch adds a new cgroupfs control fil= e but provides no documentation update (e.g., `Documentation/admin-guide/cg= roup-v2.rst` or similar). 3. **The `try_high` path for `limit_pool =3D=3D test_pool` skips `emin` pro= tection** =E2=80=94 When a cgroup hits its own limit and the same pool is b= eing tested, the code only checks `used > high` but does not check against = `emin`, unlike the cross-cgroup path. 4. **The 3-pass strategy has a subtle interaction** =E2=80=94 Pass 1 uses b= locking locks when a ticket is available, but Pass 2 always uses trylock. I= f Pass 1 fails to evict anything (all over-high BOs were locked and no tick= et), and Pass 2 also fails to evict, the code falls through to Pass 3 which= is the existing properly-locked path. This seems correct but could benefit= from a comment about the fallback chain. **Minor issues:** - The cover letter text in the commit message body is duplicative with the = commit message itself. - Typo in the v2 changelog: "above-elow" (line 199). --- --- Generated by Claude Code Patch Reviewer