From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: gpu/buddy: Track per-order used blocks with a scoreboard Date: Sat, 16 May 2026 14:58:08 +1000 Message-ID: In-Reply-To: <20260511164217.150237-4-francois.dugast@intel.com> References: <20260511164217.150237-1-francois.dugast@intel.com> <20260511164217.150237-4-francois.dugast@intel.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Assessment: Correct, with one observation.** The used scoreboard has fewer update sites than the free one because blocks= can only become ALLOCATED through `mark_allocated()` and can only leave AL= LOCATED through `mark_free()` or direct `gpu_block_free()` during coalescin= g. | Site | Operation | Guard | Correct? | |------|-----------|-------|----------| | `mark_allocated()` | `used_scoreboard[order]++` | Always (block is becomi= ng allocated) | Yes | | `mark_free()` | `used_scoreboard[order]--` | `gpu_buddy_block_is_allocate= d(block)` | Yes | | `__gpu_buddy_free()` coalesce | `used_scoreboard[order]--` | `gpu_buddy_b= lock_is_allocated(block)` | Yes | The guard in `mark_free()` is needed because `mark_free()` is called on blo= cks that were SPLIT (during `__force_merge` cleanup), not just ALLOCATED. T= he guard in `__gpu_buddy_free()` is needed because the block entering the c= oalesce loop can be either ALLOCATED (from `gpu_buddy_free_block`) or FREE = (from err_undo / __force_merge paths). I traced the full lifecycle of `gpu_buddy_free_block()` =E2=86=92 `__gpu_bu= ddy_free()`: - If no coalescing: the decrement happens in `mark_free()` (at the bottom o= f `__gpu_buddy_free`) via the `is_allocated` guard. Correct. - If coalescing happens: the decrement happens in the new code inside the c= oalesce loop body. After `gpu_block_free(block)`, `block =3D parent` (SPLIT= state), so subsequent iterations and the final `mark_free()` do NOT double= -decrement. Correct. **Print format change:** The threshold for KiB vs MiB formatting changes fr= om `if (free < SZ_1M)` to `if (block_size < SZ_1M)`. This is a good change = =E2=80=94 it makes the unit consistent per-order regardless of how many blo= cks exist, and ensures `free` and `used` values in the same line always use= the same unit. But this is a user-visible output change in debugfs, so any= one scraping `/sys/kernel/debug/dri/0/tile0/vram_mm` with scripts may notic= e. **Label naming nit:** `out_free_free_scoreboard` is a bit stutter-y. Someth= ing like `out_free_sb` or `out_scoreboard` would read better, but this is p= urely cosmetic. **Type choice:** Both scoreboards use `u64`. Given that the maximum number = of blocks at any order is bounded by `size / chunk_size` (which is also `u6= 4`), this is appropriate and overflow-safe. --- Generated by Claude Code Patch Reviewer