Claude review: gpu/tests/gpu_buddy: Add gpu_test_buddy_alloc_range for exact-range allocation

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

Patch Review

**Positive aspects:**

- Comprehensive coverage: basic full-mm, sub-range quarters, min-chunk at multiple offsets, non-power-of-two mm sizes (multi-root), randomized slices, negative overlap cases, checkerboard patterns, misalignment rejection, free/re-alloc cycling, and mixed power-of-two sizes.
- Negative tests correctly trigger the exact-range path (they satisfy `start + size == end`) and verify that the appropriate validation catches them.
- The randomized range generation maintains the invariant that `max_chunk >= ps > 0` (since both `remaining` and `(N_RAND_RANGES - 1 - i) * ps` are always multiples of `ps`), so there is no division-by-zero or underflow risk. I verified this carefully.

**Observations and minor issues:**

1. **Leaked blocks on unexpected negative-test success.** In several negative tests, the allocation result goes into `tmp`, but if the allocation unexpectedly *succeeds* (indicating an allocator bug), those blocks are never freed before `gpu_buddy_fini()`:

```c
ret = gpu_buddy_alloc_blocks(&mm, 0, mm_size, mm_size, ps, &tmp, 0);
KUNIT_EXPECT_NE_MSG(test, ret, 0,
		    "exact-range alloc should fail when range is partially used\n");
```

This is consistent with how `gpu_test_buddy_alloc_range_bias` handles its negative cases, so it's an existing pattern. But it means a failing EXPECT won't trigger the WARN in `gpu_buddy_fini()` about unreleased blocks — consider adding `gpu_buddy_free_list(&mm, &tmp, 0)` after each negative assertion, or at least before `gpu_buddy_fini()`, to make failures easier to debug.

2. **Function length.** At 327 lines, the function is quite long. The sub-test blocks are already scoped with `{ }` for variable locality, which is good, but consider whether splitting into separate `kunit_case` functions (e.g., `gpu_test_buddy_alloc_range_negative`, `gpu_test_buddy_alloc_range_random`) would improve readability and give finer-grained pass/fail reporting from KUnit. This is a stylistic suggestion, not a blocker.

3. **Test ordering in the case array.** The new test is inserted *before* `gpu_test_buddy_alloc_range_bias`:

```c
KUNIT_CASE(gpu_test_buddy_alloc_clear),
KUNIT_CASE(gpu_test_buddy_alloc_range),
KUNIT_CASE(gpu_test_buddy_alloc_range_bias),
```

This is a reasonable grouping. However, the name `gpu_test_buddy_alloc_range` is very similar to `gpu_test_buddy_alloc_range_bias` and could cause confusion since the former tests the "exact-range" path (flags=0, start+size==end) while the latter tests range-biased allocation (GPU_BUDDY_RANGE_ALLOCATION flag). A more descriptive name like `gpu_test_buddy_alloc_range_exact` would clarify intent and better distinguish the two.

4. **Negative misalignment tests.** The three misalignment sub-tests are good but it's worth noting they are all caught by the *general* alignment check at `buddy.c:1117`:

```c
if (!IS_ALIGNED(start | end | size, mm->chunk_size))
    return -EINVAL;
```

rather than the exact-range-specific check at line 1128. The test still validates correct rejection, but a comment clarifying this would help future maintainers understand which validation layer is being exercised. If the intent is to specifically test the exact-range alignment check on line 1128, you'd need start/end/size all aligned to `chunk_size` but start not aligned to `min_block_size` (when `min_block_size > chunk_size`).

5. **Minor style:** The commit subject line prefix `gpu/tests/gpu_buddy:` differs slightly from what you might see as a conventional `drm/buddy:` or `drm/tests:` prefix. Check the existing convention for this test file in the git log.

**Correctness verification of key test logic:**

- Quarter allocation: All four allocations go into the same `blocks` list; `gpu_buddy_free_list` correctly frees them all. ✓
- Checkerboard: Allocates every other page into `blocks`, tries exact-range over a partially-occupied pair, correctly verifies failure, then allocates the free page. The final `gpu_buddy_free_list(&mm, &blocks, 0)` frees both the checkerboard and the single free-page allocation. ✓
- Randomized: `round_up(prandom_u32_state(&prng) % max_chunk, ps)` — since `max_chunk` is always a multiple of `ps`, the rounded-up result never exceeds `max_chunk`, preserving the budget invariant. ✓
- Out-of-bounds negative test: caught by `end > mm->size` check at `buddy.c:1120`. ✓

**Verdict:** The patch is correct and provides valuable test coverage for the exact-range allocation path. The suggestions above (naming, tmp cleanup, possible split) are minor improvements. No blocking issues.

---
Generated by Claude Code Patch Reviewer