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/fdinfo: Add "evicted" memory accounting
Date: Mon, 25 May 2026 21:52:46 +1000
Message-ID: <review-patch1-20260520-panthor-bo-reclaim-observability-v4-1-a47ab61cb80d@collabora.com>
In-Reply-To: <20260520-panthor-bo-reclaim-observability-v4-1-a47ab61cb80d@collabora.com>
References: <20260520-panthor-bo-reclaim-observability-v4-0-a47ab61cb80d@collabora.com> <20260520-panthor-bo-reclaim-observability-v4-1-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:** Clean framework extension that follows the existing patterns f=
or RESIDENT, PURGEABLE, and ACTIVE perfectly.

**Design concern =E2=80=94 `add_size` vs `obj->size` for evicted objects:**

In `drm_show_memory_stats()`, the new evicted accounting uses `add_size`:

```c
if (s & DRM_GEM_OBJECT_EVICTED)
    status.evicted +=3D add_size;
```

where `add_size` is computed as:

```c
size_t add_size =3D (obj->funcs && obj->funcs->rss) ?
    obj->funcs->rss(obj) : obj->size;
```

For panthor (no `rss` callback), `add_size =3D obj->size`, which is correct=
. However, for any future driver that implements `rss()`, an evicted object=
 would have `rss() =3D=3D 0` (since the pages have been freed), making the =
evicted counter always zero =E2=80=94 defeating its purpose. Consider using=
 `obj->size` instead of `add_size` for the evicted counter, since the inten=
t is to report how much data has been evicted, not how much of it is still =
physically present (which is by definition none).

This isn't a blocker since panthor is the only user, but it would be a trap=
 for the next driver that adds eviction support.

**Documentation:** The new `drm-evicted-<region>` documentation is clear an=
d correctly states the semantics ("Only present if there are buffers that a=
re currently evicted, and if the driver implements reporting of this type o=
f memory."). The v4 change to avoid explicitly mentioning swap is appropria=
te since eviction is a broader concept.

**Correctness of interaction with existing flags:** The existing framework =
code already clears `DRM_GEM_OBJECT_PURGEABLE` for non-resident objects (li=
ne 944 in the `else` branch), so an evicted object won't double-count as pu=
rgeable. Good.

**No issues with:**
- `drm_memory_stats_is_zero()` update =E2=80=94 correct addition of `evicte=
d =3D=3D 0` check.
- `drm_print_memory_stats()` =E2=80=94 follows the same conditional pattern=
 as purgeable/active.
- Enum value `BIT(3)` =E2=80=94 correct, no collision with existing BIT(0..=
2).

---

---
Generated by Claude Code Patch Reviewer