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: Split cgroup charge and resource allocation Date: Sat, 14 Mar 2026 07:10:16 +1000 Message-ID: In-Reply-To: <20260313-dmemcg-aggressive-protect-v6-4-7c71cc1492db@gmx.de> References: <20260313-dmemcg-aggressive-protect-v6-0-7c71cc1492db@gmx.de> <20260313-dmemcg-aggressive-protect-v6-4-7c71cc1492db@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 This is the most complex refactoring patch. It splits `ttm_resource_alloc()= ` into `ttm_resource_try_charge()` + `ttm_resource_alloc()`, keeping the ch= arge alive across eviction retries. **Key design**: `charge_pool` is charged once, and ownership transfers to t= he `ttm_resource` on success (set to NULL in alloc_state). On failure to pl= ace, the charge is explicitly undone with `dmem_cgroup_uncharge()`. **The `in_evict` flag**: prevents recursive eviction =E2=80=94 `may_evict` = is forced false during the evict callback. This is important since `ttm_bo_= alloc_at_place` is now called from `ttm_bo_evict_cb`. **Review of uncharge paths in `ttm_bo_alloc_resource()`**: - `-ENOSPC` path: uncharges + puts limit_pool + `continue` =E2=9C=93 - `-EBUSY` path with evict failure: uncharges + `continue` or `return` =E2= =9C=93 =20 - Other error path: uncharges + puts limit_pool + `return` =E2=9C=93 - Success path: charge transferred to resource =E2=9C=93 This looks correct. The `ttm_resource_alloc()` signature change from `**ret= _limit_pool` to `*charge_pool` is clean. **One concern**: In the evict callback path (`ttm_bo_evict_cb`), `ttm_bo_al= loc_at_place` is called with `force_space=3Dfalse`. Since `charge_pool` is = already set (charged earlier), the `if (!alloc_state->charge_pool)` block i= s skipped, going straight to `ttm_resource_alloc()`. This is correct =E2=80= =94 we don't want to re-charge. But `may_evict` will be false (both `in_evi= ct` prevents it and `force_space` is false), so allocation failure returns = the raw error. Since `lret` in the callback treats `0` as success and negat= ive as "keep trying", this should be fine. --- Generated by Claude Code Patch Reviewer