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/ttm: Fix GPU MM stats during pool shrinking
Date: Tue, 05 May 2026 09:02:45 +1000
Message-ID: <review-patch1-20260501223003.2648450-1-matthew.brost@intel.com>
In-Reply-To: <20260501223003.2648450-1-matthew.brost@intel.com>
References: <20260501223003.2648450-1-matthew.brost@intel.com> <20260501223003.2648450-1-matthew.brost@intel.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

**Correctness: Good.**

The patch introduces:
```c
static void __free_pages_gpu_account(struct page *p, unsigned int order,
				     bool reclaim)
{
	mod_lruvec_page_state(p, reclaim ? NR_GPU_RECLAIM : NR_GPU_ACTIVE,
			      -(1 << order));
	__free_pages(p, order);
}
```

And applies it to all three `__free_pages` call sites:

1. **`ttm_pool_free_page()` (line 225)** -- Pure refactor, replaces identical inline code. Already guarded by `if (!pool || !ttm_pool_uses_dma_alloc(pool))`, so it only runs for non-DMA pages that were accounted at allocation. Correct.

2. **`ttm_pool_restore_commit()` (line 609)** -- This frees an old retained page after splitting and copying to a new allocation. These pages were previously allocated through `ttm_pool_alloc_page()` which accounts them as `NR_GPU_ACTIVE` (line 175 in the current tree). Passing `reclaim=false` correctly decrements `NR_GPU_ACTIVE`. This was a real leak.

3. **`ttm_pool_backup()` (line 1071)** -- The purge path frees pages directly from `tt->pages`. These are actively-used pages counted in `NR_GPU_ACTIVE` (either from initial allocation or from `ttm_pool_type_take()` which moves pages back to ACTIVE state). Passing `reclaim=false` is correct. This was the main leak reported by the user.

**Coverage check:** I verified that these are the only three `__free_pages` call sites in `ttm_pool.c`:
- Line 228: handled (refactored)
- Line 609: handled (bug fix)
- Line 1071: handled (bug fix)

The shrinker dispose path (`ttm_pool_dispose_list` -> `ttm_pool_free_page` with `reclaim=true`) was already correct since it went through `ttm_pool_free_page()`.

**One question worth confirming:** At lines 609 and 1071, the `__free_pages()` call is not guarded by a `!ttm_pool_uses_dma_alloc()` check (unlike `ttm_pool_free_page` which has that guard). If pages from a DMA-alloc pool could reach these paths, calling `__free_pages_gpu_account()` would undercount `NR_GPU_ACTIVE` (since DMA-alloc pages never increment it at allocation). However, this would also mean the pre-existing `__free_pages()` call was itself wrong for DMA pages (leaking the DMA allocation), so this patch doesn't make it worse -- it's a separate pre-existing concern. It's worth confirming that these paths are indeed only reachable for non-DMA pools, or if not, whether there's a separate fix needed for the DMA-alloc case.

---
Generated by Claude Code Patch Reviewer