Claude review: gpu/buddy: Introduce gpu_buddy_assert() for kunit-aware assertions

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

Patch Review

**Positive aspects:**
- The macro is well-documented with a kernel-doc comment.
- Correctly avoids double-evaluation of the condition — it's evaluated once inside `WARN_ON(!(condition))`.
- The decision to leave the `WARN_ON` in `gpu_buddy_block_trim()` alone is sensible and explicitly called out in the commit message.
- The `#include <kunit/test-bug.h>` is now properly guarded behind `IS_ENABLED(CONFIG_KUNIT)`, which avoids pulling in kunit headers in production builds.

**Minor observations:**

1. **Return value of WARN_ON discarded in non-KUNIT path**: In the `#else` branch:
```c
#define gpu_buddy_assert(condition) WARN_ON(!(condition))
```
`WARN_ON()` returns a value (the condition), but all call sites discard it (the macro is used as a statement, not in an `if`). This is fine for the current usage, but if someone later tries to use `if (gpu_buddy_assert(...))`, the KUNIT path (wrapped in `do { } while(0)`) would not compile while the non-KUNIT path would. This asymmetry is minor since the macro name `_assert` doesn't suggest a return value, but for consistency you could wrap the `#else` case in `do { } while(0)` as well:
```c
#define gpu_buddy_assert(condition) do { WARN_ON(!(condition)); } while (0)
```

2. **`#include <linux/bug.h>` addition**: The old code included `<kunit/test-bug.h>` unconditionally, which transitively provided `WARN_ON` via its include chain. Now that `kunit/test-bug.h` is conditional, adding `<linux/bug.h>` explicitly is correct. However, it's worth noting that `WARN_ON` is likely already available through other transitive includes (e.g., `<linux/slab.h>` via `<linux/module.h>`), so the explicit include is defensive — which is good practice.

3. **Stringification readability for the negated case**: At the `__gpu_buddy_free_list` site:
```c
gpu_buddy_assert(!(mark_dirty && mark_clear));
```
The kunit failure message will read `"gpu_buddy_assert(!(mark_dirty && mark_clear))"`, which is clear enough, though slightly verbose. This is a trivial aesthetic point.

4. **Scope of the macro**: The macro is defined locally in `buddy.c` rather than in the `<linux/gpu_buddy.h>` header. This is appropriate since all current users are in this single file, following the principle of narrowest scope.

**Correctness of each hunk:**

- **`__force_merge`**: `WARN_ON(clear(block) == clear(buddy))` → `gpu_buddy_assert(clear(block) != clear(buddy))` — logically equivalent, correct.
- **`gpu_buddy_fini` root check**: `if (WARN_ON(!is_free(...))) kunit_fail_current_test(...)` → `gpu_buddy_assert(is_free(...))` — equivalent, cleaner.
- **`gpu_buddy_fini` avail check**: `WARN_ON(mm->avail != mm->size)` → `gpu_buddy_assert(mm->avail == mm->size)` — correct inversion, adds kunit reporting that was previously missing.
- **`__gpu_buddy_free_list`**: `WARN_ON(mark_dirty && mark_clear)` → `gpu_buddy_assert(!(mark_dirty && mark_clear))` — correct inversion, adds kunit reporting that was previously missing.

**Verdict:** The patch is straightforward, correct, and improves consistency. The only actionable suggestion is wrapping the `#else` path in `do { } while(0)` for macro hygiene symmetry. Otherwise:

Reviewed-by worthy with the minor `do { } while(0)` nit optionally addressed.

---
Generated by Claude Code Patch Reviewer