public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] gpu/buddy: Per-order free and used block scoreboards
@ 2026-05-11 16:41 Francois Dugast
  2026-05-11 16:41 ` [PATCH v2 1/3] gpu/buddy: Fix use-after-free in split_block() call sites Francois Dugast
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Francois Dugast @ 2026-05-11 16:41 UTC (permalink / raw)
  To: intel-xe; +Cc: dri-devel, matthew.auld, Francois Dugast

drm_buddy_print() currently reports per-order free block counts by
walking all rbtrees, which is O(n) in the total number of free blocks
and holds the allocator lock for the duration. On large VRAM heaps with
many small fragments this becomes expensive.

This series replaces the rbtree walk with two lightweight scoreboard
arrays — free_scoreboard and used_scoreboard — indexed by order and
maintained incrementally at the points where block state transitions
occur. The print functions become simple array lookups, and drivers
reading debugfs (/sys/kernel/debug/dri/0/tile0/vram_mm) now get both
free and used counts per order at O(1) cost.

v2: Add first patch to fix bug reported by Sashiko [1] then update
    following patches accordingly

[1] https://sashiko.dev/#/patchset/20260504135343.1797869-1-francois.dugast%40intel.com

Francois Dugast (3):
  gpu/buddy: Fix use-after-free in split_block() call sites
  gpu/buddy: Track per-order free blocks with a scoreboard
  gpu/buddy: Track per-order used blocks with a scoreboard

 drivers/gpu/buddy.c         | 86 ++++++++++++++++++++++++++-----------
 drivers/gpu/drm/drm_buddy.c | 30 ++++++-------
 include/linux/gpu_buddy.h   | 15 +++++++
 3 files changed, 88 insertions(+), 43 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/3] gpu/buddy: Fix use-after-free in split_block() call sites
  2026-05-11 16:41 [PATCH v2 0/3] gpu/buddy: Per-order free and used block scoreboards Francois Dugast
@ 2026-05-11 16:41 ` Francois Dugast
  2026-05-16  4:58   ` Claude review: " Claude Code Review Bot
  2026-05-11 16:41 ` [PATCH v2 2/3] gpu/buddy: Track per-order free blocks with a scoreboard Francois Dugast
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Francois Dugast @ 2026-05-11 16:41 UTC (permalink / raw)
  To: intel-xe; +Cc: dri-devel, matthew.auld, Francois Dugast, Sashiko

When split_block() fails it returns before calling mark_split(), leaving
the block in the FREE state and still linked in the rbtree.  The four
err_undo paths then call __gpu_buddy_free() without first removing the
block from the tree, which leads to two distinct bugs:

 - If the buddy is also free, __gpu_buddy_free() merges the two siblings
   by calling gpu_block_free(mm, block) while block->rb is still linked
   in the tree.  Any subsequent rbtree traversal will follow the now-
   dangling pointer, causing a use-after-free.

 - In alloc_from_freetree(), where there is no buddy guard,
   __gpu_buddy_free() always reaches mark_free() -> rbtree_insert() with
   block still in the tree, corrupting the rbtree.

The same pattern is already used correctly in __force_merge(): call
rbtree_remove() to unlink the block before handing it to
__gpu_buddy_free().  Apply the same fix to all four err_undo sites.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Francois Dugast <francois.dugast@intel.com>
Assisted-by: GitHub Copilot:claude-sonnet-4.6
---
 drivers/gpu/buddy.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
index eb1457376307..dac2027bb64a 100644
--- a/drivers/gpu/buddy.c
+++ b/drivers/gpu/buddy.c
@@ -737,8 +737,10 @@ __alloc_range_bias(struct gpu_buddy *mm,
 	buddy = __get_buddy(block);
 	if (buddy &&
 	    (gpu_buddy_block_is_free(block) &&
-	     gpu_buddy_block_is_free(buddy)))
+	     gpu_buddy_block_is_free(buddy))) {
+		rbtree_remove(mm, block);
 		__gpu_buddy_free(mm, block, false);
+	}
 	return ERR_PTR(err);
 }
 
@@ -847,8 +849,10 @@ alloc_from_freetree(struct gpu_buddy *mm,
 	return block;
 
 err_undo:
-	if (tmp != order)
+	if (tmp != order) {
+		rbtree_remove(mm, block);
 		__gpu_buddy_free(mm, block, false);
+	}
 	return ERR_PTR(err);
 }
 
@@ -968,8 +972,10 @@ gpu_buddy_offset_aligned_allocation(struct gpu_buddy *mm,
 	buddy = __get_buddy(block);
 	if (buddy &&
 	    (gpu_buddy_block_is_free(block) &&
-	     gpu_buddy_block_is_free(buddy)))
+	     gpu_buddy_block_is_free(buddy))) {
+		rbtree_remove(mm, block);
 		__gpu_buddy_free(mm, block, false);
+	}
 	return ERR_PTR(err);
 }
 
@@ -1054,8 +1060,10 @@ static int __alloc_range(struct gpu_buddy *mm,
 	buddy = __get_buddy(block);
 	if (buddy &&
 	    (gpu_buddy_block_is_free(block) &&
-	     gpu_buddy_block_is_free(buddy)))
+	     gpu_buddy_block_is_free(buddy))) {
+		rbtree_remove(mm, block);
 		__gpu_buddy_free(mm, block, false);
+	}
 
 err_free:
 	if (err == -ENOSPC && total_allocated_on_err) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/3] gpu/buddy: Track per-order free blocks with a scoreboard
  2026-05-11 16:41 [PATCH v2 0/3] gpu/buddy: Per-order free and used block scoreboards Francois Dugast
  2026-05-11 16:41 ` [PATCH v2 1/3] gpu/buddy: Fix use-after-free in split_block() call sites Francois Dugast
@ 2026-05-11 16:41 ` Francois Dugast
  2026-05-15 15:50   ` Matthew Auld
  2026-05-16  4:58   ` Claude review: " Claude Code Review Bot
  2026-05-11 16:41 ` [PATCH v2 3/3] gpu/buddy: Track per-order used " Francois Dugast
  2026-05-16  4:58 ` Claude review: gpu/buddy: Per-order free and used block scoreboards Claude Code Review Bot
  3 siblings, 2 replies; 9+ messages in thread
From: Francois Dugast @ 2026-05-11 16:41 UTC (permalink / raw)
  To: intel-xe; +Cc: dri-devel, matthew.auld, Francois Dugast

Reporting per-order free block counts in drm_buddy_print() currently
requires walking all rbtrees, which is O(n) over the total number of
free blocks and holds the allocator lock for the duration. This becomes
expensive on large VRAM heaps with many small free fragments.

Maintain a free_scoreboard[] array indexed by order instead, so that
the count for any order is always available in O(1). The scoreboard is
kept accurate by hooking into the four places where a block's free state
changes: mark_free(), mark_allocated(), mark_split(), and the sites in
__gpu_buddy_free(), __force_merge(), and the four err_undo paths that
call rbtree_remove() directly on free blocks without going through
mark_*().

The print functions are simplified as a result: the rbtree traversal
is replaced by a direct array lookup.

v2: Update after fix for use-after-free in split_block() call sites

Signed-off-by: Francois Dugast <francois.dugast@intel.com>
Assisted-by: GitHub Copilot:claude-sonnet-4.6
---
 drivers/gpu/buddy.c         | 39 +++++++++++++++++++++++--------------
 drivers/gpu/drm/drm_buddy.c | 16 ++-------------
 include/linux/gpu_buddy.h   |  7 +++++++
 3 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
index dac2027bb64a..01247e3201fb 100644
--- a/drivers/gpu/buddy.c
+++ b/drivers/gpu/buddy.c
@@ -193,6 +193,8 @@ static void mark_allocated(struct gpu_buddy *mm,
 	block->header &= ~GPU_BUDDY_HEADER_STATE;
 	block->header |= GPU_BUDDY_ALLOCATED;
 
+	mm->free_scoreboard[gpu_buddy_block_order(block)]--;
+
 	rbtree_remove(mm, block);
 }
 
@@ -204,6 +206,8 @@ static void mark_free(struct gpu_buddy *mm,
 	block->header &= ~GPU_BUDDY_HEADER_STATE;
 	block->header |= GPU_BUDDY_FREE;
 
+	mm->free_scoreboard[gpu_buddy_block_order(block)]++;
+
 	tree = get_block_tree(block);
 	rbtree_insert(mm, block, tree);
 }
@@ -214,6 +218,8 @@ static void mark_split(struct gpu_buddy *mm,
 	block->header &= ~GPU_BUDDY_HEADER_STATE;
 	block->header |= GPU_BUDDY_SPLIT;
 
+	mm->free_scoreboard[gpu_buddy_block_order(block)]--;
+
 	rbtree_remove(mm, block);
 }
 
@@ -271,6 +277,7 @@ static unsigned int __gpu_buddy_free(struct gpu_buddy *mm,
 		}
 
 		rbtree_remove(mm, buddy);
+		mm->free_scoreboard[gpu_buddy_block_order(buddy)]--;
 		if (force_merge && gpu_buddy_block_is_clear(buddy))
 			mm->clear_avail -= gpu_buddy_block_size(mm, buddy);
 
@@ -335,6 +342,7 @@ static int __force_merge(struct gpu_buddy *mm,
 					iter = rb_prev(iter);
 
 				rbtree_remove(mm, block);
+				mm->free_scoreboard[gpu_buddy_block_order(block)]--;
 				if (gpu_buddy_block_is_clear(block))
 					mm->clear_avail -= gpu_buddy_block_size(mm, block);
 
@@ -384,11 +392,17 @@ int gpu_buddy_init(struct gpu_buddy *mm, u64 size, u64 chunk_size)
 
 	BUG_ON(mm->max_order > GPU_BUDDY_MAX_ORDER);
 
+	mm->free_scoreboard = kcalloc(mm->max_order + 1,
+				      sizeof(*mm->free_scoreboard),
+				      GFP_KERNEL);
+	if (!mm->free_scoreboard)
+		return -ENOMEM;
+
 	mm->free_trees = kmalloc_array(GPU_BUDDY_MAX_FREE_TREES,
 				       sizeof(*mm->free_trees),
 				       GFP_KERNEL);
 	if (!mm->free_trees)
-		return -ENOMEM;
+		goto out_free_scoreboard;
 
 	for_each_free_tree(i) {
 		mm->free_trees[i] = kmalloc_array(mm->max_order + 1,
@@ -450,6 +464,8 @@ int gpu_buddy_init(struct gpu_buddy *mm, u64 size, u64 chunk_size)
 	while (i--)
 		kfree(mm->free_trees[i]);
 	kfree(mm->free_trees);
+out_free_scoreboard:
+	kfree(mm->free_scoreboard);
 	return -ENOMEM;
 }
 EXPORT_SYMBOL(gpu_buddy_init);
@@ -488,6 +504,7 @@ void gpu_buddy_fini(struct gpu_buddy *mm)
 		kfree(mm->free_trees[i]);
 	kfree(mm->free_trees);
 	kfree(mm->roots);
+	kfree(mm->free_scoreboard);
 }
 EXPORT_SYMBOL(gpu_buddy_fini);
 
@@ -739,6 +756,7 @@ __alloc_range_bias(struct gpu_buddy *mm,
 	    (gpu_buddy_block_is_free(block) &&
 	     gpu_buddy_block_is_free(buddy))) {
 		rbtree_remove(mm, block);
+		mm->free_scoreboard[gpu_buddy_block_order(block)]--;
 		__gpu_buddy_free(mm, block, false);
 	}
 	return ERR_PTR(err);
@@ -851,6 +869,7 @@ alloc_from_freetree(struct gpu_buddy *mm,
 err_undo:
 	if (tmp != order) {
 		rbtree_remove(mm, block);
+		mm->free_scoreboard[gpu_buddy_block_order(block)]--;
 		__gpu_buddy_free(mm, block, false);
 	}
 	return ERR_PTR(err);
@@ -974,6 +993,7 @@ gpu_buddy_offset_aligned_allocation(struct gpu_buddy *mm,
 	    (gpu_buddy_block_is_free(block) &&
 	     gpu_buddy_block_is_free(buddy))) {
 		rbtree_remove(mm, block);
+		mm->free_scoreboard[gpu_buddy_block_order(block)]--;
 		__gpu_buddy_free(mm, block, false);
 	}
 	return ERR_PTR(err);
@@ -1062,6 +1082,7 @@ static int __alloc_range(struct gpu_buddy *mm,
 	    (gpu_buddy_block_is_free(block) &&
 	     gpu_buddy_block_is_free(buddy))) {
 		rbtree_remove(mm, block);
+		mm->free_scoreboard[gpu_buddy_block_order(block)]--;
 		__gpu_buddy_free(mm, block, false);
 	}
 
@@ -1498,21 +1519,9 @@ void gpu_buddy_print(struct gpu_buddy *mm)
 		mm->chunk_size >> 10, mm->size >> 20, mm->avail >> 20, mm->clear_avail >> 20);
 
 	for (order = mm->max_order; order >= 0; order--) {
-		struct gpu_buddy_block *block, *tmp;
-		struct rb_root *root;
-		u64 count = 0, free;
-		unsigned int tree;
-
-		for_each_free_tree(tree) {
-			root = &mm->free_trees[tree][order];
-
-			rbtree_postorder_for_each_entry_safe(block, tmp, root, rb) {
-				BUG_ON(!gpu_buddy_block_is_free(block));
-				count++;
-			}
-		}
+		u64 count = mm->free_scoreboard[order];
+		u64 free = count * (mm->chunk_size << order);
 
-		free = count * (mm->chunk_size << order);
 		if (free < SZ_1M)
 			pr_info("order-%2d free: %8llu KiB, blocks: %llu\n",
 				order, free >> 10, count);
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index faa025498de4..eef995e08a37 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -47,23 +47,11 @@ void drm_buddy_print(struct gpu_buddy *mm, struct drm_printer *p)
 		   mm->chunk_size >> 10, mm->size >> 20, mm->avail >> 20, mm->clear_avail >> 20);
 
 	for (order = mm->max_order; order >= 0; order--) {
-		struct gpu_buddy_block *block, *tmp;
-		struct rb_root *root;
-		u64 count = 0, free;
-		unsigned int tree;
-
-		for_each_free_tree(tree) {
-			root = &mm->free_trees[tree][order];
-
-			rbtree_postorder_for_each_entry_safe(block, tmp, root, rb) {
-				BUG_ON(!gpu_buddy_block_is_free(block));
-				count++;
-			}
-		}
+		u64 count = mm->free_scoreboard[order];
+		u64 free = count * (mm->chunk_size << order);
 
 		drm_printf(p, "order-%2d ", order);
 
-		free = count * (mm->chunk_size << order);
 		if (free < SZ_1M)
 			drm_printf(p, "free: %8llu KiB", free >> 10);
 		else
diff --git a/include/linux/gpu_buddy.h b/include/linux/gpu_buddy.h
index 71941a039648..a28f7d7637ca 100644
--- a/include/linux/gpu_buddy.h
+++ b/include/linux/gpu_buddy.h
@@ -173,6 +173,13 @@ struct gpu_buddy {
 	 * that fits in the remaining space.
 	 */
 	struct gpu_buddy_block **roots;
+	/*
+	 * 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.
+	 */
+	u64 *free_scoreboard;
 /* public: */
 	unsigned int n_roots;
 	unsigned int max_order;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/3] gpu/buddy: Track per-order used blocks with a scoreboard
  2026-05-11 16:41 [PATCH v2 0/3] gpu/buddy: Per-order free and used block scoreboards Francois Dugast
  2026-05-11 16:41 ` [PATCH v2 1/3] gpu/buddy: Fix use-after-free in split_block() call sites Francois Dugast
  2026-05-11 16:41 ` [PATCH v2 2/3] gpu/buddy: Track per-order free blocks with a scoreboard Francois Dugast
@ 2026-05-11 16:41 ` Francois Dugast
  2026-05-16  4:58   ` Claude review: " Claude Code Review Bot
  2026-05-16  4:58 ` Claude review: gpu/buddy: Per-order free and used block scoreboards Claude Code Review Bot
  3 siblings, 1 reply; 9+ messages in thread
From: Francois Dugast @ 2026-05-11 16:41 UTC (permalink / raw)
  To: intel-xe; +Cc: dri-devel, matthew.auld, Francois Dugast

Extend the scoreboard approach from the previous commit to used blocks,
so drm_buddy_print() can report per-order allocation pressure in O(1).

Unlike free blocks, an allocated block can leave the allocated state
through mark_free() (normal free and gpu_buddy_block_trim()) or be
consumed directly by gpu_block_free() during coalescing. Both sites are
guarded by gpu_buddy_block_is_allocated() and paired with the increment
in mark_allocated().

v2:
- Update after fix for use-after-free in split_block() call sites
- Change goto label to out_free_used_scoreboard for clarity
- Make drm_buddy_print() and gpu_buddy_print() symmetric for used and
  free

Signed-off-by: Francois Dugast <francois.dugast@intel.com>
Assisted-by: GitHub Copilot:claude-sonnet-4.6
---
 drivers/gpu/buddy.c         | 39 +++++++++++++++++++++++++++----------
 drivers/gpu/drm/drm_buddy.c | 18 +++++++++++------
 include/linux/gpu_buddy.h   |  8 ++++++++
 3 files changed, 49 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
index 01247e3201fb..daff39f50c40 100644
--- a/drivers/gpu/buddy.c
+++ b/drivers/gpu/buddy.c
@@ -194,6 +194,7 @@ static void mark_allocated(struct gpu_buddy *mm,
 	block->header |= GPU_BUDDY_ALLOCATED;
 
 	mm->free_scoreboard[gpu_buddy_block_order(block)]--;
+	mm->used_scoreboard[gpu_buddy_block_order(block)]++;
 
 	rbtree_remove(mm, block);
 }
@@ -203,6 +204,9 @@ static void mark_free(struct gpu_buddy *mm,
 {
 	enum gpu_buddy_free_tree tree;
 
+	if (gpu_buddy_block_is_allocated(block))
+		mm->used_scoreboard[gpu_buddy_block_order(block)]--;
+
 	block->header &= ~GPU_BUDDY_HEADER_STATE;
 	block->header |= GPU_BUDDY_FREE;
 
@@ -281,6 +285,9 @@ static unsigned int __gpu_buddy_free(struct gpu_buddy *mm,
 		if (force_merge && gpu_buddy_block_is_clear(buddy))
 			mm->clear_avail -= gpu_buddy_block_size(mm, buddy);
 
+		if (gpu_buddy_block_is_allocated(block))
+			mm->used_scoreboard[gpu_buddy_block_order(block)]--;
+
 		gpu_block_free(mm, block);
 		gpu_block_free(mm, buddy);
 
@@ -398,11 +405,17 @@ int gpu_buddy_init(struct gpu_buddy *mm, u64 size, u64 chunk_size)
 	if (!mm->free_scoreboard)
 		return -ENOMEM;
 
+	mm->used_scoreboard = kcalloc(mm->max_order + 1,
+				      sizeof(*mm->used_scoreboard),
+				      GFP_KERNEL);
+	if (!mm->used_scoreboard)
+		goto out_free_free_scoreboard;
+
 	mm->free_trees = kmalloc_array(GPU_BUDDY_MAX_FREE_TREES,
 				       sizeof(*mm->free_trees),
 				       GFP_KERNEL);
 	if (!mm->free_trees)
-		goto out_free_scoreboard;
+		goto out_free_used_scoreboard;
 
 	for_each_free_tree(i) {
 		mm->free_trees[i] = kmalloc_array(mm->max_order + 1,
@@ -464,7 +477,9 @@ int gpu_buddy_init(struct gpu_buddy *mm, u64 size, u64 chunk_size)
 	while (i--)
 		kfree(mm->free_trees[i]);
 	kfree(mm->free_trees);
-out_free_scoreboard:
+out_free_used_scoreboard:
+	kfree(mm->used_scoreboard);
+out_free_free_scoreboard:
 	kfree(mm->free_scoreboard);
 	return -ENOMEM;
 }
@@ -505,6 +520,7 @@ void gpu_buddy_fini(struct gpu_buddy *mm)
 	kfree(mm->free_trees);
 	kfree(mm->roots);
 	kfree(mm->free_scoreboard);
+	kfree(mm->used_scoreboard);
 }
 EXPORT_SYMBOL(gpu_buddy_fini);
 
@@ -1519,15 +1535,18 @@ void gpu_buddy_print(struct gpu_buddy *mm)
 		mm->chunk_size >> 10, mm->size >> 20, mm->avail >> 20, mm->clear_avail >> 20);
 
 	for (order = mm->max_order; order >= 0; order--) {
-		u64 count = mm->free_scoreboard[order];
-		u64 free = count * (mm->chunk_size << order);
-
-		if (free < SZ_1M)
-			pr_info("order-%2d free: %8llu KiB, blocks: %llu\n",
-				order, free >> 10, count);
+		u64 free_count = mm->free_scoreboard[order];
+		u64 used_count = mm->used_scoreboard[order];
+		u64 block_size = mm->chunk_size << order;
+		u64 free = free_count * block_size;
+		u64 used = used_count * block_size;
+
+		if (block_size < SZ_1M)
+			pr_info("order-%2d free: %8llu KiB, used: %8llu KiB, free_blocks: %llu, used_blocks: %llu\n",
+				order, free >> 10, used >> 10, free_count, used_count);
 		else
-			pr_info("order-%2d free: %8llu MiB, blocks: %llu\n",
-				order, free >> 20, count);
+			pr_info("order-%2d free: %8llu MiB, used: %8llu MiB, free_blocks: %llu, used_blocks: %llu\n",
+				order, free >> 20, used >> 20, free_count, used_count);
 	}
 }
 EXPORT_SYMBOL(gpu_buddy_print);
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index eef995e08a37..1536e59c6fe7 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -47,17 +47,23 @@ void drm_buddy_print(struct gpu_buddy *mm, struct drm_printer *p)
 		   mm->chunk_size >> 10, mm->size >> 20, mm->avail >> 20, mm->clear_avail >> 20);
 
 	for (order = mm->max_order; order >= 0; order--) {
-		u64 count = mm->free_scoreboard[order];
-		u64 free = count * (mm->chunk_size << order);
+		u64 free_count = mm->free_scoreboard[order];
+		u64 used_count = mm->used_scoreboard[order];
+		u64 block_size = mm->chunk_size << order;
+		u64 free = free_count * block_size;
+		u64 used = used_count * block_size;
 
 		drm_printf(p, "order-%2d ", order);
 
-		if (free < SZ_1M)
-			drm_printf(p, "free: %8llu KiB", free >> 10);
+		if (block_size < SZ_1M)
+			drm_printf(p, "free: %8llu KiB, used: %8llu KiB",
+				   free >> 10, used >> 10);
 		else
-			drm_printf(p, "free: %8llu MiB", free >> 20);
+			drm_printf(p, "free: %8llu MiB, used: %8llu MiB",
+				   free >> 20, used >> 20);
 
-		drm_printf(p, ", blocks: %llu\n", count);
+		drm_printf(p, ", free_blocks: %llu, used_blocks: %llu\n",
+			   free_count, used_count);
 	}
 }
 EXPORT_SYMBOL(drm_buddy_print);
diff --git a/include/linux/gpu_buddy.h b/include/linux/gpu_buddy.h
index a28f7d7637ca..e037714563d8 100644
--- a/include/linux/gpu_buddy.h
+++ b/include/linux/gpu_buddy.h
@@ -180,6 +180,14 @@ struct gpu_buddy {
 	 * called on a free block.
 	 */
 	u64 *free_scoreboard;
+	/*
+	 * Per-order used block scoreboard: used_scoreboard[order] holds the
+	 * number of blocks of that order currently in the allocated state.
+	 * Incremented in mark_allocated(), decremented in mark_free() (guarded
+	 * by gpu_buddy_block_is_allocated()) and in __gpu_buddy_free() when an
+	 * allocated block is consumed directly during buddy coalescing.
+	 */
+	u64 *used_scoreboard;
 /* public: */
 	unsigned int n_roots;
 	unsigned int max_order;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/3] gpu/buddy: Track per-order free blocks with a scoreboard
  2026-05-11 16:41 ` [PATCH v2 2/3] gpu/buddy: Track per-order free blocks with a scoreboard Francois Dugast
@ 2026-05-15 15:50   ` Matthew Auld
  2026-05-16  4:58   ` Claude review: " Claude Code Review Bot
  1 sibling, 0 replies; 9+ messages in thread
From: Matthew Auld @ 2026-05-15 15:50 UTC (permalink / raw)
  To: Francois Dugast, intel-xe; +Cc: dri-devel

On 11/05/2026 17:41, Francois Dugast wrote:
> Reporting per-order free block counts in drm_buddy_print() currently
> requires walking all rbtrees, which is O(n) over the total number of
> free blocks and holds the allocator lock for the duration. This becomes
> expensive on large VRAM heaps with many small free fragments.
> 
> Maintain a free_scoreboard[] array indexed by order instead, so that
> the count for any order is always available in O(1). The scoreboard is
> kept accurate by hooking into the four places where a block's free state
> changes: mark_free(), mark_allocated(), mark_split(), and the sites in
> __gpu_buddy_free(), __force_merge(), and the four err_undo paths that
> call rbtree_remove() directly on free blocks without going through
> mark_*().
> 
> The print functions are simplified as a result: the rbtree traversal
> is replaced by a direct array lookup.
> 
> v2: Update after fix for use-after-free in split_block() call sites
> 
> Signed-off-by: Francois Dugast <francois.dugast@intel.com>
> Assisted-by: GitHub Copilot:claude-sonnet-4.6
> ---
>   drivers/gpu/buddy.c         | 39 +++++++++++++++++++++++--------------
>   drivers/gpu/drm/drm_buddy.c | 16 ++-------------
>   include/linux/gpu_buddy.h   |  7 +++++++
>   3 files changed, 33 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
> index dac2027bb64a..01247e3201fb 100644
> --- a/drivers/gpu/buddy.c
> +++ b/drivers/gpu/buddy.c
> @@ -193,6 +193,8 @@ static void mark_allocated(struct gpu_buddy *mm,
>   	block->header &= ~GPU_BUDDY_HEADER_STATE;
>   	block->header |= GPU_BUDDY_ALLOCATED;
>   
> +	mm->free_scoreboard[gpu_buddy_block_order(block)]--;
> +
>   	rbtree_remove(mm, block);
>   }
>   
> @@ -204,6 +206,8 @@ static void mark_free(struct gpu_buddy *mm,
>   	block->header &= ~GPU_BUDDY_HEADER_STATE;
>   	block->header |= GPU_BUDDY_FREE;
>   
> +	mm->free_scoreboard[gpu_buddy_block_order(block)]++;
> +
>   	tree = get_block_tree(block);
>   	rbtree_insert(mm, block, tree);
>   }
> @@ -214,6 +218,8 @@ static void mark_split(struct gpu_buddy *mm,
>   	block->header &= ~GPU_BUDDY_HEADER_STATE;
>   	block->header |= GPU_BUDDY_SPLIT;
>   
> +	mm->free_scoreboard[gpu_buddy_block_order(block)]--;
> +
>   	rbtree_remove(mm, block);
>   }
>   
> @@ -271,6 +277,7 @@ static unsigned int __gpu_buddy_free(struct gpu_buddy *mm,
>   		}
>   
>   		rbtree_remove(mm, buddy);
> +		mm->free_scoreboard[gpu_buddy_block_order(buddy)]--;
>   		if (force_merge && gpu_buddy_block_is_clear(buddy))
>   			mm->clear_avail -= gpu_buddy_block_size(mm, buddy);
>   
> @@ -335,6 +342,7 @@ static int __force_merge(struct gpu_buddy *mm,
>   					iter = rb_prev(iter);
>   
>   				rbtree_remove(mm, block);
> +				mm->free_scoreboard[gpu_buddy_block_order(block)]--;
>   				if (gpu_buddy_block_is_clear(block))
>   					mm->clear_avail -= gpu_buddy_block_size(mm, block);
>   
> @@ -384,11 +392,17 @@ int gpu_buddy_init(struct gpu_buddy *mm, u64 size, u64 chunk_size)
>   
>   	BUG_ON(mm->max_order > GPU_BUDDY_MAX_ORDER);
>   
> +	mm->free_scoreboard = kcalloc(mm->max_order + 1,
> +				      sizeof(*mm->free_scoreboard),
> +				      GFP_KERNEL);
> +	if (!mm->free_scoreboard)
> +		return -ENOMEM;
> +
>   	mm->free_trees = kmalloc_array(GPU_BUDDY_MAX_FREE_TREES,
>   				       sizeof(*mm->free_trees),
>   				       GFP_KERNEL);
>   	if (!mm->free_trees)
> -		return -ENOMEM;
> +		goto out_free_scoreboard;
>   
>   	for_each_free_tree(i) {
>   		mm->free_trees[i] = kmalloc_array(mm->max_order + 1,
> @@ -450,6 +464,8 @@ int gpu_buddy_init(struct gpu_buddy *mm, u64 size, u64 chunk_size)
>   	while (i--)
>   		kfree(mm->free_trees[i]);
>   	kfree(mm->free_trees);
> +out_free_scoreboard:
> +	kfree(mm->free_scoreboard);
>   	return -ENOMEM;
>   }
>   EXPORT_SYMBOL(gpu_buddy_init);
> @@ -488,6 +504,7 @@ void gpu_buddy_fini(struct gpu_buddy *mm)
>   		kfree(mm->free_trees[i]);
>   	kfree(mm->free_trees);
>   	kfree(mm->roots);
> +	kfree(mm->free_scoreboard);
>   }
>   EXPORT_SYMBOL(gpu_buddy_fini);
>   
> @@ -739,6 +756,7 @@ __alloc_range_bias(struct gpu_buddy *mm,
>   	    (gpu_buddy_block_is_free(block) &&
>   	     gpu_buddy_block_is_free(buddy))) {
>   		rbtree_remove(mm, block);
> +		mm->free_scoreboard[gpu_buddy_block_order(block)]--;
>   		__gpu_buddy_free(mm, block, false);

Wondering if it now makes sense to refactor this and consolidate in one 
place? Maybe something like:

__gpu_buddy_undo_splits()
{
	if (is_free(block) && is_free(buddy) {
             rbtree_remove(mm, block);
             mm->free_scoreboard[gpu_buddy_block_order(block)]--;
             __gpu_buddy_free(mm, block, false);
         }
}

>   	}
>   	return ERR_PTR(err);
> @@ -851,6 +869,7 @@ alloc_from_freetree(struct gpu_buddy *mm,
>   err_undo:
>   	if (tmp != order) {
>   		rbtree_remove(mm, block);
> +		mm->free_scoreboard[gpu_buddy_block_order(block)]--;
>   		__gpu_buddy_free(mm, block, false);
>   	}
>   	return ERR_PTR(err);
> @@ -974,6 +993,7 @@ gpu_buddy_offset_aligned_allocation(struct gpu_buddy *mm,
>   	    (gpu_buddy_block_is_free(block) &&
>   	     gpu_buddy_block_is_free(buddy))) {
>   		rbtree_remove(mm, block);
> +		mm->free_scoreboard[gpu_buddy_block_order(block)]--;
>   		__gpu_buddy_free(mm, block, false);
>   	}
>   	return ERR_PTR(err);
> @@ -1062,6 +1082,7 @@ static int __alloc_range(struct gpu_buddy *mm,
>   	    (gpu_buddy_block_is_free(block) &&
>   	     gpu_buddy_block_is_free(buddy))) {
>   		rbtree_remove(mm, block);
> +		mm->free_scoreboard[gpu_buddy_block_order(block)]--;
>   		__gpu_buddy_free(mm, block, false);
>   	}
>   
> @@ -1498,21 +1519,9 @@ void gpu_buddy_print(struct gpu_buddy *mm)
>   		mm->chunk_size >> 10, mm->size >> 20, mm->avail >> 20, mm->clear_avail >> 20);
>   
>   	for (order = mm->max_order; order >= 0; order--) {
> -		struct gpu_buddy_block *block, *tmp;
> -		struct rb_root *root;
> -		u64 count = 0, free;
> -		unsigned int tree;
> -
> -		for_each_free_tree(tree) {
> -			root = &mm->free_trees[tree][order];
> -
> -			rbtree_postorder_for_each_entry_safe(block, tmp, root, rb) {
> -				BUG_ON(!gpu_buddy_block_is_free(block));
> -				count++;
> -			}
> -		}
> +		u64 count = mm->free_scoreboard[order];
> +		u64 free = count * (mm->chunk_size << order);
>   
> -		free = count * (mm->chunk_size << order);
>   		if (free < SZ_1M)
>   			pr_info("order-%2d free: %8llu KiB, blocks: %llu\n",
>   				order, free >> 10, count);
> diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
> index faa025498de4..eef995e08a37 100644
> --- a/drivers/gpu/drm/drm_buddy.c
> +++ b/drivers/gpu/drm/drm_buddy.c
> @@ -47,23 +47,11 @@ void drm_buddy_print(struct gpu_buddy *mm, struct drm_printer *p)
>   		   mm->chunk_size >> 10, mm->size >> 20, mm->avail >> 20, mm->clear_avail >> 20);
>   
>   	for (order = mm->max_order; order >= 0; order--) {
> -		struct gpu_buddy_block *block, *tmp;
> -		struct rb_root *root;
> -		u64 count = 0, free;
> -		unsigned int tree;
> -
> -		for_each_free_tree(tree) {
> -			root = &mm->free_trees[tree][order];
> -
> -			rbtree_postorder_for_each_entry_safe(block, tmp, root, rb) {
> -				BUG_ON(!gpu_buddy_block_is_free(block));
> -				count++;
> -			}
> -		}
> +		u64 count = mm->free_scoreboard[order];
> +		u64 free = count * (mm->chunk_size << order);
>   
>   		drm_printf(p, "order-%2d ", order);
>   
> -		free = count * (mm->chunk_size << order);
>   		if (free < SZ_1M)
>   			drm_printf(p, "free: %8llu KiB", free >> 10);
>   		else
> diff --git a/include/linux/gpu_buddy.h b/include/linux/gpu_buddy.h
> index 71941a039648..a28f7d7637ca 100644
> --- a/include/linux/gpu_buddy.h
> +++ b/include/linux/gpu_buddy.h
> @@ -173,6 +173,13 @@ struct gpu_buddy {
>   	 * that fits in the remaining space.
>   	 */
>   	struct gpu_buddy_block **roots;
> +	/*
> +	 * 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.
> +	 */
> +	u64 *free_scoreboard;
>   /* public: */
>   	unsigned int n_roots;
>   	unsigned int max_order;


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Claude review: gpu/buddy: Per-order free and used block scoreboards
  2026-05-11 16:41 [PATCH v2 0/3] gpu/buddy: Per-order free and used block scoreboards Francois Dugast
                   ` (2 preceding siblings ...)
  2026-05-11 16:41 ` [PATCH v2 3/3] gpu/buddy: Track per-order used " Francois Dugast
@ 2026-05-16  4:58 ` Claude Code Review Bot
  3 siblings, 0 replies; 9+ messages in thread
From: Claude Code Review Bot @ 2026-05-16  4:58 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: gpu/buddy: Per-order free and used block scoreboards
Author: Francois Dugast <francois.dugast@intel.com>
Patches: 5
Reviewed: 2026-05-16T14:58:07.515061

---

This is a well-structured 3-patch series against the `gpu_buddy` allocator. Patch 1 fixes a real use-after-free / rbtree corruption bug in four error-undo paths, patches 2 and 3 add per-order free and used block scoreboards to make `drm_buddy_print()` / `gpu_buddy_print()` O(1) instead of O(n).

The bug fix in patch 1 is correct and follows the established pattern from `__force_merge()`. The scoreboard accounting in patches 2 and 3 is carefully placed at every state-transition point and I was unable to find a path that would cause a missed increment, double-decrement, or underflow.

**Overall: series looks good, with minor nits below.**

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Claude review: gpu/buddy: Fix use-after-free in split_block() call sites
  2026-05-11 16:41 ` [PATCH v2 1/3] gpu/buddy: Fix use-after-free in split_block() call sites Francois Dugast
@ 2026-05-16  4:58   ` Claude Code Review Bot
  0 siblings, 0 replies; 9+ messages in thread
From: Claude Code Review Bot @ 2026-05-16  4:58 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Assessment: Correct and well-explained fix.**

The commit message clearly describes the two distinct failure modes: (1) use-after-free when the buddy is also free and coalescing frees a block whose `rb` is still linked, and (2) rbtree corruption when `mark_free()` → `rbtree_insert()` is called on a block that's already in the tree.

The root cause is that `split_block()` only calls `mark_split()` (which does `rbtree_remove()`) **after** both child allocations succeed (line 514 of the current tree). On failure, the block remains FREE and linked in the rbtree.

All four err_undo sites are fixed consistently:

```c
+		rbtree_remove(mm, block);
 		__gpu_buddy_free(mm, block, false);
```

This matches the existing pattern in `__force_merge()` (current tree line 337):
```c
				rbtree_remove(mm, block);
				...
				order = __gpu_buddy_free(mm, block, true);
```

**Nit:** The `alloc_from_freetree` err_undo path has no buddy guard — it unconditionally calls `__gpu_buddy_free` when `tmp != order`. This is fine because the block IS free (failed split left it free) and `__gpu_buddy_free` handles unmerge-able blocks correctly by just calling `mark_free()` at the bottom. But a brief comment like the other three sites' "make sure we merge everything back" would make the asymmetry less surprising.

**Tag concern:** `Assisted-by: GitHub Copilot:claude-sonnet-4.6` — this is not a recognized kernel tag. The kernel community generally uses `Reported-by`, `Suggested-by`, `Reviewed-by`, etc. Some maintainers may object to this. Consider whether this needs a different format or should be a `Link:` or trailer note instead.

**Cc: stable?** This is a real use-after-free bug. If it can be triggered from userspace (large alloc that forces split_block to fail due to memory pressure), it likely warrants a `Cc: stable@vger.kernel.org` tag and a `Fixes:` tag pointing to the commit that introduced the err_undo paths.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Claude review: gpu/buddy: Track per-order free blocks with a scoreboard
  2026-05-11 16:41 ` [PATCH v2 2/3] gpu/buddy: Track per-order free blocks with a scoreboard Francois Dugast
  2026-05-15 15:50   ` Matthew Auld
@ 2026-05-16  4:58   ` Claude Code Review Bot
  1 sibling, 0 replies; 9+ messages in thread
From: Claude Code Review Bot @ 2026-05-16  4:58 UTC (permalink / raw)
  To: dri-devel-reviews

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 — block enters FREE |
| `mark_allocated()` | `free_scoreboard[order]--` | Yes — block leaves FREE |
| `mark_split()` | `free_scoreboard[order]--` | Yes — block leaves FREE |
| `__gpu_buddy_free()` coalesce loop | `free_scoreboard[buddy_order]--` | Yes — buddy removed from tree |
| `__force_merge()` | `free_scoreboard[order]--` | Yes — block removed from tree |
| Four err_undo sites | `free_scoreboard[order]--` | Yes — block removed 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 (either the err_undo decrement or `mark_free`→`mark_allocated` transition). The final `mark_free(mm, block)` at the bottom of `__gpu_buddy_free` then increments the scoreboard for the coalesced (higher-order) block. This is correct.

**Memory management** is handled properly:
- `gpu_buddy_init()`: `kcalloc` with proper cleanup label `out_free_scoreboard` 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 imprecise — it's also decremented in `mark_split()` which does its own `rbtree_remove` internally. The wording is fine in practice though; it captures the invariant well enough.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Claude review: gpu/buddy: Track per-order used blocks with a scoreboard
  2026-05-11 16:41 ` [PATCH v2 3/3] gpu/buddy: Track per-order used " Francois Dugast
@ 2026-05-16  4:58   ` Claude Code Review Bot
  0 siblings, 0 replies; 9+ messages in thread
From: Claude Code Review Bot @ 2026-05-16  4:58 UTC (permalink / raw)
  To: dri-devel-reviews

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 ALLOCATED through `mark_free()` or direct `gpu_block_free()` during coalescing.

| Site | Operation | Guard | Correct? |
|------|-----------|-------|----------|
| `mark_allocated()` | `used_scoreboard[order]++` | Always (block is becoming allocated) | Yes |
| `mark_free()` | `used_scoreboard[order]--` | `gpu_buddy_block_is_allocated(block)` | Yes |
| `__gpu_buddy_free()` coalesce | `used_scoreboard[order]--` | `gpu_buddy_block_is_allocated(block)` | Yes |

The guard in `mark_free()` is needed because `mark_free()` is called on blocks that were SPLIT (during `__force_merge` cleanup), not just ALLOCATED. The guard in `__gpu_buddy_free()` is needed because the block entering the coalesce 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()` → `__gpu_buddy_free()`:
- If no coalescing: the decrement happens in `mark_free()` (at the bottom of `__gpu_buddy_free`) via the `is_allocated` guard. Correct.
- If coalescing happens: the decrement happens in the new code inside the coalesce loop body. After `gpu_block_free(block)`, `block = 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 from `if (free < SZ_1M)` to `if (block_size < SZ_1M)`. This is a good change — it makes the unit consistent per-order regardless of how many blocks 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 anyone scraping `/sys/kernel/debug/dri/0/tile0/vram_mm` with scripts may notice.

**Label naming nit:** `out_free_free_scoreboard` is a bit stutter-y. Something like `out_free_sb` or `out_scoreboard` would read better, but this is purely 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 `u64`), this is appropriate and overflow-safe.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-05-16  4:58 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-11 16:41 [PATCH v2 0/3] gpu/buddy: Per-order free and used block scoreboards Francois Dugast
2026-05-11 16:41 ` [PATCH v2 1/3] gpu/buddy: Fix use-after-free in split_block() call sites Francois Dugast
2026-05-16  4:58   ` Claude review: " Claude Code Review Bot
2026-05-11 16:41 ` [PATCH v2 2/3] gpu/buddy: Track per-order free blocks with a scoreboard Francois Dugast
2026-05-15 15:50   ` Matthew Auld
2026-05-16  4:58   ` Claude review: " Claude Code Review Bot
2026-05-11 16:41 ` [PATCH v2 3/3] gpu/buddy: Track per-order used " Francois Dugast
2026-05-16  4:58   ` Claude review: " Claude Code Review Bot
2026-05-16  4:58 ` Claude review: gpu/buddy: Per-order free and used block scoreboards Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox