Claude review: drm/ttm/pool: back up at native page order

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Build dependency — `__free_pages_gpu_account` does not exist**

The patch replaces the existing `put_page(page)` with:

```c
page->private = 0;
__free_pages_gpu_account(page, order, false);
```

This function does not exist anywhere in the drm-next tree. Grep across the entire kernel tree returns no matches. This is presumably introduced by the referenced series [1] (patchwork.freedesktop.org/series/165330/). The dependency should be stated explicitly in the commit message, or the patch should be rebased to use the existing free path. Note that the existing code's `put_page()` also lacks GPU page accounting (`mod_lruvec_page_state`), so if `__free_pages_gpu_account` wraps both the accounting and the free, this is actually an improvement over the original — but the dependency must be declared.

For a standalone fix targeting stable, you'd likely want:

```c
page->private = 0;
mod_lruvec_page_state(page, NR_GPU_ACTIVE, -(1 << order));
__free_pages(page, order);
```

to match the pattern in `ttm_pool_free_page()` at line 225-228 of the current tree.

**Fault injection error value**

```c
if (IS_ENABLED(CONFIG_FAULT_INJECTION) &&
    should_fail(&backup_fault_inject, 1))
	shandle = -1;
```

Using bare `-1` (which is `-EPERM`) is unconventional. The outer fault injection at the top of the function truncates `num_pages`, which is a cleaner injection mechanism. Here, `-ENOMEM` would be more self-documenting since a shmem backup failure is most likely an allocation failure. Minor nit, not a correctness issue since the caller only checks sign.

**Rollback correctness — looks correct**

The rollback path on per-subpage failure:

```c
for (k = 0; k < j; ++k) {
	handle = ttm_backup_page_ptr_to_handle(tt->pages[i + k]);
	ttm_backup_drop(backup, handle);
	tt->pages[i + k] = page + k;
}
goto out;
```

This correctly drops already-written handles and restores the original page pointers, preserving compound-level atomicity. On a retry, the compound will be attempted again from scratch. The `out:` label placement before the return is correct.

**Handle-slot iteration for previously backed-up compounds**

When re-entering after a partial backup, already-handled slots are detected:

```c
if (unlikely(ttm_backup_page_ptr_is_handle(page)))
	continue;
```

With `npages = 1` still set from the default, this iterates one slot at a time through backed-up compounds. This is correct but slightly less efficient than stepping by the original compound order. Acceptable since the retry case is uncommon.

**Skipping truncated compounds is safe**

```c
if (unlikely(i + npages > num_pages))
	break;
```

When fault injection truncates `num_pages` mid-compound, the entire compound is skipped rather than split. This is the right conservative choice and avoids introducing the fragmentation the patch is designed to prevent.

**`page->private = 0` only on head page — correct**

The split path clears `private` on all sub-pages, but when freeing at native order, only the head page's `private` needs to be cleared since the buddy allocator operates on the head page. This matches the purge path pattern at the current tree's line 1070.

**`set_pages_array_wb` interaction — no issue**

The existing `set_pages_array_wb(tt->pages, tt->num_pages)` call before the backup loop passes all sub-page pointers. Since `tt->pages[]` is populated with individual sub-page pointers (head, head+1, head+2, ...) during allocation via `ttm_pool_allocated_page_commit()`, the caching change applies correctly to each physical page regardless of whether they're later split or freed as a group.

**`shrunken` accounting change**

Old code: `shrunken++` per order-0 page. New code: `shrunken += npages` per compound. The return type is `long` and represents page count, so this is correct and consistent with the purge path.

**Summary**: The approach is sound and the rollback logic is well-constructed. The primary blocker is the unstated dependency on `__free_pages_gpu_account()`. If this is intended to be applied atop the referenced series, that should be explicit. For stable backport, the free call needs adaptation to use existing kernel APIs. The `Assisted-by: Claude` tag is noted.

---
Generated by Claude Code Patch Reviewer