Claude review: gpu/buddy: Track per-order used blocks with a scoreboard

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

Patch Review

**Correctness concern in `__gpu_buddy_free()` coalescing loop:**

```c
		if (gpu_buddy_block_is_allocated(block))
			mm->used_scoreboard[gpu_buddy_block_order(block)]--;

		gpu_block_free(mm, block);
		gpu_block_free(mm, buddy);

		block = parent;
```

On the **first iteration** of the loop, `block` can indeed be allocated (the normal path from `gpu_buddy_free_block()` which asserts `gpu_buddy_block_is_allocated(block)` at line 616). So the decrement is correct on the first iteration.

However, on **subsequent iterations** of the coalescing loop, `block = parent` from the previous iteration. The parent was previously in `GPU_BUDDY_SPLIT` state (that's how the buddy allocator tree works — when children exist, the parent is split). A split block is not allocated, so `gpu_buddy_block_is_allocated(block)` will be false and the decrement won't fire. This is actually correct behavior — split blocks shouldn't affect the used scoreboard.

But there's a subtler problem: **when `__gpu_buddy_free()` is called from `__force_merge()`** at line 356, the `block` passed in is **free** (it was just pulled from the free rbtree and had `rbtree_remove()` called on it, and its free_scoreboard already decremented). In this path, `block` is neither allocated nor split — it's free (though removed from the tree). So the `gpu_buddy_block_is_allocated()` check will correctly be false, and no spurious decrement happens. This path is safe.

Similarly, the error-path callers at lines 768, 878, 999 pass free blocks, so the guard is correct there too.

**Net assessment for this guard:** The `gpu_buddy_block_is_allocated()` check in `__gpu_buddy_free()` is **correct** — it only fires on the first iteration when called from `gpu_buddy_free_block()`. But the commit message says:

> "consumed directly by gpu_block_free() during coalescing"

This is somewhat misleading. The block isn't "consumed by gpu_block_free() during coalescing" in the sense of bypassing mark_free — `gpu_block_free()` just frees the slab memory. The used_scoreboard decrement is needed because the block is destroyed (merged into parent) without going through `mark_free()` first. The commit message and the comment in `gpu_buddy.h` (which says "decremented in gpu_buddy_free_block()") don't accurately describe this site — they should mention `__gpu_buddy_free()`'s coalescing loop.

**Observation on `mark_free()` used_scoreboard decrement:**

```c
static void mark_free(struct gpu_buddy *mm,
		      struct gpu_buddy_block *block)
{
	if (gpu_buddy_block_is_allocated(block))
		mm->used_scoreboard[gpu_buddy_block_order(block)]--;
```

`mark_free()` is called from:
1. `__gpu_buddy_free()` at line 298 — at the end, after the coalescing loop. At this point `block` could be the original allocated block (if no coalescing happened) or a parent that was split. Only in the "no coalescing" case is the block still allocated, so the guard is correct.
2. `gpu_buddy_init()` at line 455 — marking root blocks free during init. These are freshly allocated slab objects, not in ALLOCATED state. Safe.

This is correct.

**Naming nit:** `out_free_free_scoreboard` is an unfortunately stuttery label name. Something like `out_free_scoreboards` or `out_free_sb` would read better.

**Comment accuracy in `gpu_buddy.h`:**

```c
	 * Incremented in mark_allocated(), decremented in
	 * gpu_buddy_free_block() which is the sole entry point for freeing
	 * allocated blocks.
```

This comment says the decrement happens in `gpu_buddy_free_block()`, but the actual decrement happens in two places: `mark_free()` (which has the `gpu_buddy_block_is_allocated()` guard) and inside `__gpu_buddy_free()`'s coalescing loop. `gpu_buddy_free_block()` is the public entry point, but the decrements happen deeper. The comment should be updated to reflect the actual sites, or simplified to just say "decremented when blocks leave the allocated state."

**Print format change:** The output format changes from `blocks:` to `free_blocks:, used_blocks:` in both `gpu_buddy_print()` and `drm_buddy_print()`. This is a user-visible debugfs format change. If any tooling parses this output, it would break. This is probably fine for debugfs but worth noting.

**Summary:** The patch is functionally correct — the scoreboard accounting is right at all transition points. The main feedback is:
1. The header comment for `used_scoreboard` doesn't accurately describe where decrements happen.
2. The `out_free_free_scoreboard` label name could be improved.
3. The commit message's description of the decrement sites is slightly misleading.

---
Generated by Claude Code Patch Reviewer