From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/panthor: Implement evicted status for GEM objects
Date: Mon, 25 May 2026 21:52:46 +1000
Message-ID: <review-patch2-20260520-panthor-bo-reclaim-observability-v4-2-a47ab61cb80d@collabora.com>
In-Reply-To: <20260520-panthor-bo-reclaim-observability-v4-2-a47ab61cb80d@collabora.com>
References: <20260520-panthor-bo-reclaim-observability-v4-0-a47ab61cb80d@collabora.com> <20260520-panthor-bo-reclaim-observability-v4-2-a47ab61cb80d@collabora.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

**Overall:** Well-designed implementation using `atomic_t` for the reclaime=
d counter with INT_MAX saturation.

**`atomic_add_unless` for saturation:** Correct approach.

```c
atomic_add_unless(&bo->reclaimed_count, 1, INT_MAX);
```

This atomically increments unless the value is already INT_MAX, providing c=
lean saturation semantics. Placed correctly before the actual eviction clea=
nup.

**Status callback logic:**

```c
if (drm_gem_is_imported(&bo->base) || bo->backing.pages)
    res |=3D DRM_GEM_OBJECT_RESIDENT;
else if (atomic_read(&bo->reclaimed_count))
    res |=3D DRM_GEM_OBJECT_EVICTED;
```

The `else if` correctly makes RESIDENT and EVICTED mutually exclusive. An o=
bject that is swapped back in (`backing.pages` non-NULL again) will show as=
 RESIDENT, not EVICTED =E2=80=94 which is the correct current-state semanti=
cs. The `reclaimed_count` provides the historical record. The distinction b=
etween "never had pages" (reclaimed_count =3D=3D 0, neither RESIDENT nor EV=
ICTED) and "had pages but were evicted" (reclaimed_count > 0, EVICTED) is e=
xactly the gap this series fills.

**Race between `backing.pages` and `reclaimed_count`:** These are read with=
out holding locks in the status callback (called under `file->table_lock` s=
pinlock, not dma_resv). A concurrent shrinker could evict the object betwee=
n reading `backing.pages` and `reclaimed_count`. This is acceptable =E2=80=
=94 fdinfo is inherently racy and will be correct on the next poll.

**Zero-initialization:** `reclaimed_count` is implicitly zero-initialized v=
ia `kzalloc_obj(*bo)` in `panthor_gem_alloc_object()`, so newly allocated o=
bjects correctly start with count 0.

**debugfs state flag:**

```c
if (drm_gem_is_imported(&bo->base))
    gem_state_flags |=3D PANTHOR_DEBUGFS_GEM_STATE_FLAG_IMPORTED;
else if (!resident_size && reclaimed_count)
    gem_state_flags |=3D PANTHOR_DEBUGFS_GEM_STATE_FLAG_EVICTED;
```

The `else if` is correct =E2=80=94 imported BOs are always considered resid=
ent by panthor and are not managed by the shrinker, so they can never be in=
 the evicted state.

**Format string update:** The new `%-11d` for `reclaimed_count` is correct =
(INT_MAX =3D 2147483647, 10 digits + 1 space =3D 11 chars). The column head=
er "evictions" (9 chars) fits within the 11-char field.

**No issues with:** enum definitions, kerneldoc for `reclaimed_count`, debu=
gfs flag name table update.

---

---
Generated by Claude Code Patch Reviewer