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: Thu, 23 Apr 2026 08:36:40 +1000 [thread overview]
Message-ID: <review-patch1-20260421-panthor-bo-reclaim-observability-v2-1-c9135eedfb6f@collabora.com> (raw)
In-Reply-To: <20260421-panthor-bo-reclaim-observability-v2-1-c9135eedfb6f@collabora.com>
Patch Review
This patch adds `DRM_GEM_OBJECT_EVICTED = BIT(3)` to the status enum, an `evicted` field to `drm_memory_stats`, and wires them into the fdinfo printing/accumulation paths.
**Issue: Missing drm-usage-stats.rst documentation.**
The new `drm-evicted-<region>` key needs an entry in `Documentation/gpu/drm-usage-stats.rst` alongside the existing documented keys. This file explicitly says driver-specific stats "should be documented above and where possible, aligned with other drivers." Since this is being added to the core DRM framework for any driver to use, it's especially important to document the semantics — for example, that evicted is a subset of non-resident memory.
**Minor observation on semantics in `drm_show_memory_stats()`:**
The existing code clears `DRM_GEM_OBJECT_PURGEABLE` when the object is not resident:
```c
if (s & DRM_GEM_OBJECT_RESIDENT) {
status.resident += add_size;
} else {
s &= ~DRM_GEM_OBJECT_PURGEABLE;
}
```
The new `DRM_GEM_OBJECT_EVICTED` is accumulated without any filtering. This is correct — evicted objects are by definition not resident, so the semantics are different from purgeable (which is defined as resident + idle + marked). However, the core code doesn't enforce mutual exclusivity between RESIDENT and EVICTED. If a driver were to erroneously return both, both counters would be incremented. This is arguably fine (it's the driver's job to set bits correctly), but a brief comment on the enum doc noting that EVICTED implies non-resident would be helpful.
**Code is correct as written.** The additions to `drm_memory_stats_is_zero()`, `drm_print_memory_stats()`, and `drm_show_memory_stats()` all follow the established pattern.
---
Generated by Claude Code Patch Reviewer
next prev parent reply other threads:[~2026-04-22 22:36 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-21 10:45 [PATCH v2 0/3] Let userspace know about swapped out panthor GEM objects Nicolas Frattaroli
2026-04-21 10:45 ` [PATCH v2 1/3] drm/fdinfo: Add "evicted" memory accounting Nicolas Frattaroli
2026-04-22 10:59 ` Steven Price
2026-04-22 22:36 ` Claude Code Review Bot [this message]
2026-04-21 10:45 ` [PATCH v2 2/3] drm/panthor: Implement evicted status for GEM objects Nicolas Frattaroli
2026-04-22 22:36 ` Claude review: " Claude Code Review Bot
2026-04-21 10:45 ` [PATCH v2 3/3] drm/panthor: Reduce padding in gems debugfs for refcount Nicolas Frattaroli
2026-04-22 22:36 ` Claude review: " Claude Code Review Bot
2026-04-22 22:36 ` Claude review: Let userspace know about swapped out panthor GEM objects Claude Code Review Bot
-- strict thread matches above, loose matches on Subject: below --
2026-04-20 15:46 [PATCH 0/2] " Nicolas Frattaroli
2026-04-20 15:46 ` [PATCH 1/2] drm/fdinfo: Add "evicted" memory accounting Nicolas Frattaroli
2026-04-22 23:29 ` Claude review: " Claude Code Review Bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=review-patch1-20260421-panthor-bo-reclaim-observability-v2-1-c9135eedfb6f@collabora.com \
--to=claude-review@example.com \
--cc=dri-devel-reviews@example.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox