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/panthor: Add a GEM shrinker Date: Tue, 10 Mar 2026 12:15:40 +1000 Message-ID: In-Reply-To: <20260309151119.290217-1-boris.brezillon@collabora.com> References: <20260309151119.290217-1-boris.brezillon@collabora.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: drm/panthor: Add a GEM shrinker Author: Boris Brezillon Patches: 11 Reviewed: 2026-03-10T12:15:40.499562 --- This is a well-structured 9-patch series that adds GEM buffer reclaim (shri= nker) support to the panthor GPU driver. The series follows a logical progr= ession: first a small DRM core fix, then preparatory refactoring (moving co= de, splitting helpers), a major transition away from `drm_gem_shmem_object`= to a custom GEM implementation, lazy page allocation on mmap, mmap trackin= g, and finally the shrinker itself. The approach is sensible =E2=80=94 usin= g `_trylock()` throughout the reclaim path to avoid lock ordering issues is= pragmatic, even if it means missing some reclaim opportunities under conte= ntion. The series is at v5 and most patches have R-b tags from Steven Price and Li= viu Dudau. The overall design is sound but I have several concerns, primari= ly around locking subtleties in the shrinker and eviction paths. **Key concerns:** 1. The `panthor_vm_evict_bo_mappings_locked()` function does lock-then-brea= k with incomplete rollback =E2=80=94 if a `panthor_vm_lock_region()` fails = mid-way through VMAs, some VMAs are evicted and some are not, leaving the B= O in a partially-evicted state. 2. The `panthor_mmu_reclaim_priv_bos()` uses `vm->reclaim.lru_node.prev =3D= =3D &vms` as a check =E2=80=94 this is fragile and relies on list implement= ation details. 3. In patch 8 (mmap tracking), the `refcount_t` usage pattern with `refcoun= t_set(..., 1)` inside a double-check lock is unusual and deserves scrutiny. 4. The `gpu_mapped_count` is modified without holding the `reclaim.lock`, c= reating a potential race in `panthor_gem_shrinker_count()`. --- Generated by Claude Code Patch Reviewer