* [PATCH] ttm/pool: Use sentinels in debugfs
@ 2026-04-27 16:22 Mario Limonciello
2026-04-28 4:24 ` Claude review: " Claude Code Review Bot
2026-04-28 4:24 ` Claude Code Review Bot
0 siblings, 2 replies; 3+ messages in thread
From: Mario Limonciello @ 2026-04-27 16:22 UTC (permalink / raw)
To: Christian König; +Cc: dri-devel, Mario Limonciello
The values in the `page_pool_shrink` debugfs file should recognize
the sentinels rather than causing an underflow.
Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/5218
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/gpu/drm/ttm/ttm_pool.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index 26a3689e5fd90..4e83f4978b136 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -1335,15 +1335,22 @@ static int ttm_pool_debugfs_shrink_show(struct seq_file *m, void *data)
.gfp_mask = GFP_NOFS,
.nr_to_scan = TTM_SHRINKER_BATCH,
};
- unsigned long count;
+ unsigned long count, scanned;
int nid;
fs_reclaim_acquire(GFP_KERNEL);
for_each_node(nid) {
sc.nid = nid;
count = ttm_pool_shrinker_count(mm_shrinker, &sc);
- seq_printf(m, "%d: %lu/%lu\n", nid, count,
- ttm_pool_shrinker_scan(mm_shrinker, &sc));
+ scanned = ttm_pool_shrinker_scan(mm_shrinker, &sc);
+
+ /* Convert shrinker API sentinel values to 0 for debugfs output */
+ if (count == SHRINK_EMPTY)
+ count = 0;
+ if (scanned == SHRINK_STOP)
+ scanned = 0;
+
+ seq_printf(m, "%d: %lu/%lu\n", nid, count, scanned);
}
fs_reclaim_release(GFP_KERNEL);
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Claude review: ttm/pool: Use sentinels in debugfs
2026-04-27 16:22 [PATCH] ttm/pool: Use sentinels in debugfs Mario Limonciello
@ 2026-04-28 4:24 ` Claude Code Review Bot
2026-04-28 4:24 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-04-28 4:24 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: ttm/pool: Use sentinels in debugfs
Author: Mario Limonciello <mario.limonciello@amd.com>
Patches: 1
Reviewed: 2026-04-28T14:24:08.932872
---
This is a single-patch fix for the TTM pool debugfs output. The problem is straightforward: `ttm_pool_shrinker_count()` returns `SHRINK_EMPTY` (~0UL - 1) when there are no pages, and `ttm_pool_shrinker_scan()` returns `SHRINK_STOP` (~0UL) when nothing was freed. These are enormous unsigned long sentinel values that would be printed verbatim in the `page_pool_shrink` debugfs file, appearing as nonsensical huge numbers (e.g., 18446744073709551614/18446744073709551615) rather than meaningful zeros.
The fix is correct and minimal.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
* Claude review: ttm/pool: Use sentinels in debugfs
2026-04-27 16:22 [PATCH] ttm/pool: Use sentinels in debugfs Mario Limonciello
2026-04-28 4:24 ` Claude review: " Claude Code Review Bot
@ 2026-04-28 4:24 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-04-28 4:24 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correctness: Good.** The sentinel checks match exactly what the underlying functions can return:
- `ttm_pool_shrinker_count()` at line 1222 returns `SHRINK_EMPTY` when `num_pages` is 0 — the patch correctly checks `count == SHRINK_EMPTY`.
- `ttm_pool_shrinker_scan()` at line 1213 returns `SHRINK_STOP` when `num_freed` is 0 — the patch correctly checks `scanned == SHRINK_STOP`.
**Minor observations:**
1. **Scan is unconditional even when count shows empty.** The code calls `ttm_pool_shrinker_scan()` even when `count == SHRINK_EMPTY` (i.e., there are zero pages on this node). The scan will iterate and find nothing to free, returning `SHRINK_STOP`. This is harmless since this is a debugfs path (not performance-critical), and the pre-existing code had the same behavior. Not a blocking concern, but a `if (count != SHRINK_EMPTY)` guard could avoid the pointless scan call.
2. **The comment is reasonable.** The inline comment `/* Convert shrinker API sentinel values to 0 for debugfs output */` explains the *why* clearly enough.
3. **The variable split is good practice.** Separating `scanned = ttm_pool_shrinker_scan(...)` out of the `seq_printf` call improves readability and enables the sentinel checks without evaluating side effects in a printf argument list.
**No correctness issues found.** The patch is a clean, minimal fix for a real debugfs output bug.
Reviewed-by: looks appropriate.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-28 4:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-27 16:22 [PATCH] ttm/pool: Use sentinels in debugfs Mario Limonciello
2026-04-28 4:24 ` Claude review: " Claude Code Review Bot
2026-04-28 4:24 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox