Claude review: rust: gpu: Add GPU buddy allocator bindings

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

Patch Review

**Positive aspects:**
- Clean separation of allocation mode (enum) vs modifier flags (bitflags) — this is good Rust API design.
- Proper pin-init usage throughout, with correct `#[pin_data(PinnedDrop)]` annotations.
- The `Arc<GpuBuddyInner>` sharing between `GpuBuddy` and `AllocatedBlocks` correctly ensures the allocator outlives its allocations.
- Doctests are thorough, covering simple, top-down, range, fragmented, and contiguous-failure cases.
- SAFETY comments are present and generally adequate.

**Observations:**

1. **`base_offset` is never passed to C:** The `GpuBuddyParams::base_offset` field is stored in the Rust wrapper and used to adjust offsets returned by `AllocatedBlock::offset()`, but it is never passed to `gpu_buddy_init()`. This means the C allocator always thinks it starts at 0, and the Rust side adds the offset. This works for offset reporting, but if the caller ever passes a range to `GpuBuddyAllocMode::Range`, they would need to know that the range is relative to the buddy's internal 0-based space, not the base-offset-adjusted space. The doctest examples all use `base_offset: 0`, so this isn't tested. Consider documenting this contract explicitly, or adjusting `Range` start/end by subtracting `base_offset` before passing to C.

2. **Range validation is incomplete:** The `alloc_blocks` method checks `range.is_empty()` for `Range` mode but doesn't validate that the range falls within the allocator's `[0, size)` bounds. The C side may handle this, but a Rust-side bounds check would provide a clearer error:

   ```rust
   if let GpuBuddyAllocMode::Range(range) = &mode {
       if range.is_empty() {
           Err::<(), Error>(EINVAL)?;
       }
   }
   ```

3. **`try_pin_init!` field reference across init:** In `alloc_blocks`, the `try_pin_init!` block references `buddy` and `list` inside the unnamed `_:` initializer field:

   ```rust
   try_pin_init!(AllocatedBlocks {
       buddy: buddy_arc,
       list <- CListHead::new(),
       _: {
           ...
           let guard = buddy.lock();
           ...
           list.as_raw(),
           ...
       }
   })
   ```

   This relies on the `try_pin_init!` macro making previously-initialized fields available by name in subsequent field initializers. This is a known feature of the kernel's pin-init macros, but it's subtle — a brief comment noting this would help future readers.

4. **`gpu_buddy_block_size` helper is unused:** The C helper `rust_helper_gpu_buddy_block_size` is defined in `rust/helpers/gpu.c` but never called from Rust. The Rust `AllocatedBlock::size()` computes the size itself:

   ```rust
   pub fn size(&self) -> u64 {
       (self.blocks.buddy.params.chunk_size.as_usize() as u64) << self.this.order()
   }
   ```

   This duplicates the logic in `gpu_buddy_block_size()`. Consider either using the C helper (passing the `gpu_buddy*` pointer) for consistency, or removing the unused helper to avoid dead code. Using the C helper would also be more robust if the size computation ever changes in the C side.

5. **`Block::offset` takes `const` pointer, `Block::order` does not:** Looking at the C header:
   - `gpu_buddy_block_offset(const struct gpu_buddy_block *block)` — const
   - `gpu_buddy_block_order(struct gpu_buddy_block *block)` — non-const

   The Rust helpers match this, but both are logically read-only. The non-const on `order` in the C side looks like an oversight in the C header (since it only reads `block->header`), but that's not something for this patch to fix.

6. **MAINTAINERS dual-listing of `rust/helpers/gpu.c`:** The patch adds `F: rust/helpers/gpu.c`, `F: rust/kernel/gpu.rs`, and `F: rust/kernel/gpu/` to both the "DRM DRIVERS AND GPU SUBSYSTEM - RUST" section and the "GPU BUDDY ALLOCATOR" section. This dual listing is intentional (both groups should review changes), but worth confirming this is the desired arrangement.

7. **`#[cfg(CONFIG_GPU_BUDDY = "y")]` vs `IS_ENABLED`:** The Rust module uses `CONFIG_GPU_BUDDY = "y"` (built-in only), while the C bindings_helper.h uses `IS_ENABLED(CONFIG_GPU_BUDDY)` (built-in or module). If `CONFIG_GPU_BUDDY` is ever `=m`, the C constants would be compiled but the Rust module wouldn't. This is probably fine for now (GPU_BUDDY is likely always `=y` when selected by a built-in driver like nova-core), but the inconsistency is worth noting.

8. **Minor nit — comment style:** The `GpuBuddyAllocMode` method comments use `//` instead of `///`:

   ```rust
   // Returns the C flags corresponding to the allocation mode.
   fn as_flags(&self) -> usize {
   ```

   Since these are private methods, `//` is fine, but `///` would generate internal docs that could be useful during development.

---
Generated by Claude Code Patch Reviewer