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 free blocks with a scoreboard Date: Sat, 16 May 2026 14:58:08 +1000 Message-ID: In-Reply-To: <20260511164217.150237-3-francois.dugast@intel.com> References: <20260511164217.150237-1-francois.dugast@intel.com> <20260511164217.150237-3-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 accounting, clean implementation.** The scoreboard is maintained at all six state-transition points: | Site | Operation | Correct? | |------|-----------|----------| | `mark_free()` | `free_scoreboard[order]++` | Yes =E2=80=94 block enters F= REE | | `mark_allocated()` | `free_scoreboard[order]--` | Yes =E2=80=94 block lea= ves FREE | | `mark_split()` | `free_scoreboard[order]--` | Yes =E2=80=94 block leaves = FREE | | `__gpu_buddy_free()` coalesce loop | `free_scoreboard[buddy_order]--` | Y= es =E2=80=94 buddy removed from tree | | `__force_merge()` | `free_scoreboard[order]--` | Yes =E2=80=94 block remo= ved from tree | | Four err_undo sites | `free_scoreboard[order]--` | Yes =E2=80=94 block re= moved from tree | The coalesce loop in `__gpu_buddy_free` only decrements for the buddy (not = the block), because the block was already accounted for by the caller (eith= er the err_undo decrement or `mark_free`=E2=86=92`mark_allocated` transitio= n). The final `mark_free(mm, block)` at the bottom of `__gpu_buddy_free` th= en increments the scoreboard for the coalesced (higher-order) block. This i= s correct. **Memory management** is handled properly: - `gpu_buddy_init()`: `kcalloc` with proper cleanup label `out_free_scorebo= ard` on the error path - `gpu_buddy_fini()`: `kfree(mm->free_scoreboard)` **Minor nit on the header comment:** ```c + /* + * Per-order free block scoreboard: free_scoreboard[order] holds the + * number of blocks of that order currently in the free state. + * Incremented in mark_free(), decremented wherever rbtree_remove() is + * called on a free block. + */ ``` This is a good comment for an internal field. But the "decremented wherever= rbtree_remove() is called on a free block" formulation is slightly impreci= se =E2=80=94 it's also decremented in `mark_split()` which does its own `rb= tree_remove` internally. The wording is fine in practice though; it capture= s the invariant well enough. --- Generated by Claude Code Patch Reviewer