public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] drm/buddy: Documentation and internal helper cleanup
@ 2026-02-11  5:31 Sanjay Yadav
  2026-02-11  5:31 ` [PATCH v2 1/2] drm/buddy: Add kernel-doc for allocator structures and flags Sanjay Yadav
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Sanjay Yadav @ 2026-02-11  5:31 UTC (permalink / raw)
  To: dri-devel
  Cc: intel-xe, Christian König, Arunpravin Paneer Selvam,
	Matthew Auld

This series improves the GPU buddy allocator by adding missing
documentation and cleaning up internal helpers.

It adds kernel-doc for the allocator structures and flags, covering
gpu_buddy, gpu_buddy_block, and the allocation flags (RANGE, TOPDOWN,
CONTIGUOUS, CLEAR, TRIM_DISABLE) to make the allocator’s design and usage
clearer for readers and contributors.

It also reduces the header surface by moving internal helpers from
gpu_buddy.h into buddy.c as static functions, since they have no
external users, and removes gpu_get_buddy(), an unused exported wrapper
around the internal __get_buddy() helper. This clarifies the intended API
and avoids exporting unused symbols.

No functional changes.

v2:
- Rebased after DRM buddy allocator moved to drivers/gpu/
- Keep gpu_buddy_block_is_free() in header since it's now
  used by drm_buddy.c
- Corrected GPU_BUDDY_CLEAR_TREE and GPU_BUDDY_DIRTY_TREE index values (Arun)
- Updated commit message and cover letter to reflect changes

Cc: Christian König <christian.koenig@amd.com>
Cc: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Suggested-by: Matthew Auld <matthew.auld@intel.com>

Sanjay Yadav (2):
  drm/buddy: Add kernel-doc for allocator structures and flags
  drm/buddy: Move internal helpers to buddy.c

 drivers/gpu/buddy.c       |  35 ++++-----
 include/linux/gpu_buddy.h | 147 +++++++++++++++++++++++++++-----------
 2 files changed, 122 insertions(+), 60 deletions(-)

-- 
2.52.0


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

* [PATCH v2 1/2] drm/buddy: Add kernel-doc for allocator structures and flags
  2026-02-11  5:31 [PATCH v2 0/2] drm/buddy: Documentation and internal helper cleanup Sanjay Yadav
@ 2026-02-11  5:31 ` Sanjay Yadav
  2026-02-11  6:07   ` Claude review: " Claude Code Review Bot
  2026-02-12  7:38   ` [PATCH v2 1/2] " Arunpravin Paneer Selvam
  2026-02-11  5:31 ` [PATCH v2 2/2] drm/buddy: Move internal helpers to buddy.c Sanjay Yadav
  2026-02-11  6:07 ` Claude review: drm/buddy: Documentation and internal helper cleanup Claude Code Review Bot
  2 siblings, 2 replies; 9+ messages in thread
From: Sanjay Yadav @ 2026-02-11  5:31 UTC (permalink / raw)
  To: dri-devel
  Cc: intel-xe, Christian König, Arunpravin Paneer Selvam,
	Matthew Auld

Add missing kernel-doc for GPU buddy allocator flags,
gpu_buddy_block, and gpu_buddy. The documentation covers block
header fields, allocator roots, free trees, and allocation flags
such as RANGE, TOPDOWN, CONTIGUOUS, CLEAR, and TRIM_DISABLE.
Private members are marked with kernel-doc private markers
and documented with regular comments.

No functional changes.

v2:
- Corrected GPU_BUDDY_CLEAR_TREE and GPU_BUDDY_DIRTY_TREE index values (Arun)
- Rebased after DRM buddy allocator moved to drivers/gpu/
- Updated commit message

Cc: Christian König <christian.koenig@amd.com>
Cc: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Suggested-by: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Sanjay Yadav <sanjay.kumar.yadav@intel.com>
---
 include/linux/gpu_buddy.h | 122 +++++++++++++++++++++++++++++++-------
 1 file changed, 102 insertions(+), 20 deletions(-)

diff --git a/include/linux/gpu_buddy.h b/include/linux/gpu_buddy.h
index 07ac65db6d2e..b06241c78437 100644
--- a/include/linux/gpu_buddy.h
+++ b/include/linux/gpu_buddy.h
@@ -12,11 +12,58 @@
 #include <linux/sched.h>
 #include <linux/rbtree.h>
 
+/**
+ * GPU_BUDDY_RANGE_ALLOCATION - Allocate within a specific address range
+ *
+ * When set, allocation is restricted to the range [start, end) specified
+ * in gpu_buddy_alloc_blocks(). Without this flag, start/end are ignored
+ * and allocation can use any free space.
+ */
 #define GPU_BUDDY_RANGE_ALLOCATION		BIT(0)
+
+/**
+ * GPU_BUDDY_TOPDOWN_ALLOCATION - Allocate from top of address space
+ *
+ * Allocate starting from high addresses and working down. Useful for
+ * separating different allocation types (e.g., kernel vs userspace)
+ * to reduce fragmentation.
+ */
 #define GPU_BUDDY_TOPDOWN_ALLOCATION		BIT(1)
+
+/**
+ * GPU_BUDDY_CONTIGUOUS_ALLOCATION - Require physically contiguous blocks
+ *
+ * The allocation must be satisfied with a single contiguous block.
+ * If the requested size cannot be allocated contiguously, the
+ * allocation fails with -ENOSPC.
+ */
 #define GPU_BUDDY_CONTIGUOUS_ALLOCATION		BIT(2)
+
+/**
+ * GPU_BUDDY_CLEAR_ALLOCATION - Prefer pre-cleared (zeroed) memory
+ *
+ * Attempt to allocate from the clear tree first. If insufficient clear
+ * memory is available, falls back to dirty memory. Useful when the
+ * caller needs zeroed memory and wants to avoid GPU clear operations.
+ */
 #define GPU_BUDDY_CLEAR_ALLOCATION		BIT(3)
+
+/**
+ * GPU_BUDDY_CLEARED - Mark returned blocks as cleared
+ *
+ * Used with gpu_buddy_free_list() to indicate that the memory being
+ * freed has been cleared (zeroed). The blocks will be placed in the
+ * clear tree for future GPU_BUDDY_CLEAR_ALLOCATION requests.
+ */
 #define GPU_BUDDY_CLEARED			BIT(4)
+
+/**
+ * GPU_BUDDY_TRIM_DISABLE - Disable automatic block trimming
+ *
+ * By default, if an allocation is smaller than the allocated block,
+ * excess memory is trimmed and returned to the free pool. This flag
+ * disables trimming, keeping the full power-of-two block size.
+ */
 #define GPU_BUDDY_TRIM_DISABLE			BIT(5)
 
 enum gpu_buddy_free_tree {
@@ -28,7 +75,27 @@ enum gpu_buddy_free_tree {
 #define for_each_free_tree(tree) \
 	for ((tree) = 0; (tree) < GPU_BUDDY_MAX_FREE_TREES; (tree)++)
 
+/**
+ * struct gpu_buddy_block - Block within a buddy allocator
+ *
+ * Each block in the buddy allocator is represented by this structure.
+ * Blocks are organized in a binary tree where each parent block can be
+ * split into two children (left and right buddies). The allocator manages
+ * blocks at various orders (power-of-2 sizes) from chunk_size up to the
+ * largest contiguous region.
+ *
+ * @private: Private data owned by the allocator user (e.g., driver-specific data)
+ * @link: List node for user ownership while block is allocated
+ */
 struct gpu_buddy_block {
+/* private: */
+	/*
+	 * Header bit layout:
+	 * - Bits 63:12: block offset within the address space
+	 * - Bits 11:10: state (ALLOCATED, FREE, or SPLIT)
+	 * - Bit 9: clear bit (1 if memory is zeroed)
+	 * - Bits 5:0: order (log2 of size relative to chunk_size)
+	 */
 #define GPU_BUDDY_HEADER_OFFSET GENMASK_ULL(63, 12)
 #define GPU_BUDDY_HEADER_STATE  GENMASK_ULL(11, 10)
 #define   GPU_BUDDY_ALLOCATED	   (1 << 10)
@@ -43,7 +110,7 @@ struct gpu_buddy_block {
 	struct gpu_buddy_block *left;
 	struct gpu_buddy_block *right;
 	struct gpu_buddy_block *parent;
-
+/* public: */
 	void *private; /* owned by creator */
 
 	/*
@@ -53,43 +120,58 @@ struct gpu_buddy_block {
 	 * gpu_buddy_free* ownership is given back to the mm.
 	 */
 	union {
+/* private: */
 		struct rb_node rb;
+/* public: */
 		struct list_head link;
 	};
-
+/* private: */
 	struct list_head tmp_link;
 };
 
 /* Order-zero must be at least SZ_4K */
 #define GPU_BUDDY_MAX_ORDER (63 - 12)
 
-/*
- * Binary Buddy System.
+/**
+ * struct gpu_buddy - GPU binary buddy allocator
+ *
+ * The buddy allocator provides efficient power-of-two memory allocation
+ * with fast allocation and free operations. It is commonly used for GPU
+ * memory management where allocations can be split into power-of-two
+ * block sizes.
  *
- * Locking should be handled by the user, a simple mutex around
- * gpu_buddy_alloc* and gpu_buddy_free* should suffice.
+ * Locking should be handled by the user; a simple mutex around
+ * gpu_buddy_alloc_blocks() and gpu_buddy_free_block()/gpu_buddy_free_list()
+ * should suffice.
+ *
+ * @n_roots: Number of root blocks in the roots array.
+ * @max_order: Maximum block order (log2 of largest block size / chunk_size).
+ * @chunk_size: Minimum allocation granularity in bytes. Must be at least SZ_4K.
+ * @size: Total size of the address space managed by this allocator in bytes.
+ * @avail: Total free space currently available for allocation in bytes.
+ * @clear_avail: Free space available in the clear tree (zeroed memory) in bytes.
+ *               This is a subset of @avail.
  */
 struct gpu_buddy {
-	/* Maintain a free list for each order. */
-	struct rb_root **free_trees;
-
+/* private: */
 	/*
-	 * Maintain explicit binary tree(s) to track the allocation of the
-	 * address space. This gives us a simple way of finding a buddy block
-	 * and performing the potentially recursive merge step when freeing a
-	 * block.  Nodes are either allocated or free, in which case they will
-	 * also exist on the respective free list.
+	 * Array of red-black trees for free block management.
+	 * Indexed as free_trees[clear/dirty][order] where:
+	 * - Index 0 (GPU_BUDDY_CLEAR_TREE): blocks with zeroed content
+	 * - Index 1 (GPU_BUDDY_DIRTY_TREE): blocks with unknown content
+	 * Each tree holds free blocks of the corresponding order.
 	 */
-	struct gpu_buddy_block **roots;
-
+	struct rb_root **free_trees;
 	/*
-	 * Anything from here is public, and remains static for the lifetime of
-	 * the mm. Everything above is considered do-not-touch.
+	 * Array of root blocks representing the top-level blocks of the
+	 * binary tree(s). Multiple roots exist when the total size is not
+	 * a power of two, with each root being the largest power-of-two
+	 * that fits in the remaining space.
 	 */
+	struct gpu_buddy_block **roots;
+/* public: */
 	unsigned int n_roots;
 	unsigned int max_order;
-
-	/* Must be at least SZ_4K */
 	u64 chunk_size;
 	u64 size;
 	u64 avail;
-- 
2.52.0


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

* [PATCH v2 2/2] drm/buddy: Move internal helpers to buddy.c
  2026-02-11  5:31 [PATCH v2 0/2] drm/buddy: Documentation and internal helper cleanup Sanjay Yadav
  2026-02-11  5:31 ` [PATCH v2 1/2] drm/buddy: Add kernel-doc for allocator structures and flags Sanjay Yadav
@ 2026-02-11  5:31 ` Sanjay Yadav
  2026-02-11  6:07   ` Claude review: " Claude Code Review Bot
  2026-02-11  6:07 ` Claude review: drm/buddy: Documentation and internal helper cleanup Claude Code Review Bot
  2 siblings, 1 reply; 9+ messages in thread
From: Sanjay Yadav @ 2026-02-11  5:31 UTC (permalink / raw)
  To: dri-devel
  Cc: intel-xe, Christian König, Arunpravin Paneer Selvam,
	Matthew Auld

Move gpu_buddy_block_state(), gpu_buddy_block_is_allocated(),
and gpu_buddy_block_is_split() from gpu_buddy.h to gpu_buddy.c
as static functions since they have no external callers.

Remove gpu_get_buddy() as it was an unused exported wrapper
around the internal __get_buddy().

No functional changes.

v2:
- Rebased after DRM buddy allocator moved to drivers/gpu/
- Keep gpu_buddy_block_is_free() in header since it's now
  used by drm_buddy.c
- Updated commit message

Cc: Christian König <christian.koenig@amd.com>
Cc: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Suggested-by: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Sanjay Yadav <sanjay.kumar.yadav@intel.com>
Reviewed-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
 drivers/gpu/buddy.c       | 35 ++++++++++++++++++-----------------
 include/linux/gpu_buddy.h | 25 ++-----------------------
 2 files changed, 20 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
index 603c59a2013a..b27761246d4b 100644
--- a/drivers/gpu/buddy.c
+++ b/drivers/gpu/buddy.c
@@ -14,6 +14,24 @@
 
 static struct kmem_cache *slab_blocks;
 
+static unsigned int
+gpu_buddy_block_state(struct gpu_buddy_block *block)
+{
+	return block->header & GPU_BUDDY_HEADER_STATE;
+}
+
+static bool
+gpu_buddy_block_is_allocated(struct gpu_buddy_block *block)
+{
+	return gpu_buddy_block_state(block) == GPU_BUDDY_ALLOCATED;
+}
+
+static bool
+gpu_buddy_block_is_split(struct gpu_buddy_block *block)
+{
+	return gpu_buddy_block_state(block) == GPU_BUDDY_SPLIT;
+}
+
 static struct gpu_buddy_block *gpu_block_alloc(struct gpu_buddy *mm,
 					       struct gpu_buddy_block *parent,
 					       unsigned int order,
@@ -449,23 +467,6 @@ static int split_block(struct gpu_buddy *mm,
 	return 0;
 }
 
-/**
- * gpu_get_buddy - get buddy address
- *
- * @block: GPU buddy block
- *
- * Returns the corresponding buddy block for @block, or NULL
- * if this is a root block and can't be merged further.
- * Requires some kind of locking to protect against
- * any concurrent allocate and free operations.
- */
-struct gpu_buddy_block *
-gpu_get_buddy(struct gpu_buddy_block *block)
-{
-	return __get_buddy(block);
-}
-EXPORT_SYMBOL(gpu_get_buddy);
-
 /**
  * gpu_buddy_reset_clear - reset blocks clear state
  *
diff --git a/include/linux/gpu_buddy.h b/include/linux/gpu_buddy.h
index b06241c78437..4d68445ac16d 100644
--- a/include/linux/gpu_buddy.h
+++ b/include/linux/gpu_buddy.h
@@ -190,16 +190,10 @@ gpu_buddy_block_order(struct gpu_buddy_block *block)
 	return block->header & GPU_BUDDY_HEADER_ORDER;
 }
 
-static inline unsigned int
-gpu_buddy_block_state(struct gpu_buddy_block *block)
-{
-	return block->header & GPU_BUDDY_HEADER_STATE;
-}
-
 static inline bool
-gpu_buddy_block_is_allocated(struct gpu_buddy_block *block)
+gpu_buddy_block_is_free(struct gpu_buddy_block *block)
 {
-	return gpu_buddy_block_state(block) == GPU_BUDDY_ALLOCATED;
+	return (block->header & GPU_BUDDY_HEADER_STATE) == GPU_BUDDY_FREE;
 }
 
 static inline bool
@@ -208,18 +202,6 @@ gpu_buddy_block_is_clear(struct gpu_buddy_block *block)
 	return block->header & GPU_BUDDY_HEADER_CLEAR;
 }
 
-static inline bool
-gpu_buddy_block_is_free(struct gpu_buddy_block *block)
-{
-	return gpu_buddy_block_state(block) == GPU_BUDDY_FREE;
-}
-
-static inline bool
-gpu_buddy_block_is_split(struct gpu_buddy_block *block)
-{
-	return gpu_buddy_block_state(block) == GPU_BUDDY_SPLIT;
-}
-
 static inline u64
 gpu_buddy_block_size(struct gpu_buddy *mm,
 		     struct gpu_buddy_block *block)
@@ -231,9 +213,6 @@ int gpu_buddy_init(struct gpu_buddy *mm, u64 size, u64 chunk_size);
 
 void gpu_buddy_fini(struct gpu_buddy *mm);
 
-struct gpu_buddy_block *
-gpu_get_buddy(struct gpu_buddy_block *block);
-
 int gpu_buddy_alloc_blocks(struct gpu_buddy *mm,
 			   u64 start, u64 end, u64 size,
 			   u64 min_page_size,
-- 
2.52.0



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

* Claude review: drm/buddy: Documentation and internal helper cleanup
  2026-02-11  5:31 [PATCH v2 0/2] drm/buddy: Documentation and internal helper cleanup Sanjay Yadav
  2026-02-11  5:31 ` [PATCH v2 1/2] drm/buddy: Add kernel-doc for allocator structures and flags Sanjay Yadav
  2026-02-11  5:31 ` [PATCH v2 2/2] drm/buddy: Move internal helpers to buddy.c Sanjay Yadav
@ 2026-02-11  6:07 ` Claude Code Review Bot
  2 siblings, 0 replies; 9+ messages in thread
From: Claude Code Review Bot @ 2026-02-11  6:07 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/buddy: Documentation and internal helper cleanup
Author: Sanjay Yadav <sanjay.kumar.yadav@intel.com>
Patches: 3
Reviewed: 2026-02-11T16:07:51.694362

---

This v2 patch series improves the GPU buddy allocator by adding comprehensive kernel-doc documentation and cleaning up the internal API surface. The series addresses feedback from v1, including correcting documentation for tree index values and adapting to the recent code relocation to `drivers/gpu/`.

**Series Structure:** The two-patch split is logical - documentation additions first, followed by code cleanup. This ordering makes sense as it documents the API before narrowing it.

**Scope:** The series claims "No functional changes" which appears accurate. The changes are purely documentation and code organization improvements.

**Version Changes:** The v2 changelog indicates:
- Rebase after DRM buddy allocator moved to `drivers/gpu/`
- Retained `gpu_buddy_block_is_free()` in header (now used by `drm_buddy.c`)
- Fixed tree index documentation (responding to Arun's feedback)

**Review Status:** Patch 2/2 already has `Reviewed-by: Arunpravin Paneer Selvam`.

**Concerns:**
1. The series references `drm_buddy.c` using `gpu_buddy_block_is_free()`, but this file isn't shown in the diff context. Need to verify this external dependency.
2. The header bit layout documentation in Patch 1 should be verified against actual usage.

---

---
Generated by Claude Code Patch Reviewer

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

* Claude review: drm/buddy: Add kernel-doc for allocator structures and flags
  2026-02-11  5:31 ` [PATCH v2 1/2] drm/buddy: Add kernel-doc for allocator structures and flags Sanjay Yadav
@ 2026-02-11  6:07   ` Claude Code Review Bot
  2026-02-12  7:38   ` [PATCH v2 1/2] " Arunpravin Paneer Selvam
  1 sibling, 0 replies; 9+ messages in thread
From: Claude Code Review Bot @ 2026-02-11  6:07 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Overall Assessment:** This patch adds much-needed documentation to the GPU buddy allocator. The kernel-doc formatting appears correct, and the descriptions provide useful context for developers.

**Detailed Comments:**

#### Allocation Flags Documentation

```c
+/**
+ * GPU_BUDDY_RANGE_ALLOCATION - Allocate within a specific address range
+ *
+ * When set, allocation is restricted to the range [start, end) specified
+ * in gpu_buddy_alloc_blocks(). Without this flag, start/end are ignored
+ * and allocation can use any free space.
+ */
```

**Good:** Clear explanation of the flag's behavior and the mathematical notation `[start, end)` correctly indicates start-inclusive, end-exclusive.

**Suggestion:** Consider mentioning error behavior when range allocation fails (returns `-ENOSPC`?).

```c
+/**
+ * GPU_BUDDY_TOPDOWN_ALLOCATION - Allocate from top of address space
+ *
+ * Allocate starting from high addresses and working down. Useful for
+ * separating different allocation types (e.g., kernel vs userspace)
+ * to reduce fragmentation.
+ */
```

**Good:** Explains both mechanism and use case.

**Question:** Is the fragmentation claim always true? Bottom-up vs top-down affects fragmentation patterns differently depending on allocation patterns.

```c
+/**
+ * GPU_BUDDY_CONTIGUOUS_ALLOCATION - Require physically contiguous blocks
+ *
+ * The allocation must be satisfied with a single contiguous block.
+ * If the requested size cannot be allocated contiguously, the
+ * allocation fails with -ENOSPC.
+ */
```

**Issue:** Says "physically contiguous" but buddy allocators typically manage virtual address spaces or GPU aperture ranges, not physical memory directly. Should this say "contiguous blocks in the managed address space" instead?

```c
+/**
+ * GPU_BUDDY_CLEAR_ALLOCATION - Prefer pre-cleared (zeroed) memory
+ *
+ * Attempt to allocate from the clear tree first. If insufficient clear
+ * memory is available, falls back to dirty memory. Useful when the
+ * caller needs zeroed memory and wants to avoid GPU clear operations.
+ */
```

**Good:** Clear fallback behavior documentation.

**Nit:** "Zeroed memory" - does this mean GPU-side zeroing or CPU-side? May be worth clarifying.

```c
+/**
+ * GPU_BUDDY_CLEARED - Mark returned blocks as cleared
+ *
+ * Used with gpu_buddy_free_list() to indicate that the memory being
+ * freed has been cleared (zeroed). The blocks will be placed in the
+ * clear tree for future GPU_BUDDY_CLEAR_ALLOCATION requests.
+ */
```

**Good:** Explains the cleared/dirty tree relationship.

```c
+/**
+ * GPU_BUDDY_TRIM_DISABLE - Disable automatic block trimming
+ *
+ * By default, if an allocation is smaller than the allocated block,
+ * excess memory is trimmed and returned to the free pool. This flag
+ * disables trimming, keeping the full power-of-two block size.
+ */
```

**Good:** Clear explanation of default behavior vs flagged behavior.

#### Tree Index Documentation

```c
 enum gpu_buddy_free_tree {
+	GPU_BUDDY_CLEAR_TREE = 0,
+	GPU_BUDDY_DIRTY_TREE = 1,
```

**From v2 changelog:** "Corrected GPU_BUDDY_CLEAR_TREE and GPU_BUDDY_DIRTY_TREE index values (Arun)"

**Issue:** The patch doesn't show these as changes in the diff. Were these already correct in v1 and only the documentation comment was fixed? If these enum values changed, this would be a functional change contradicting the commit message.

**Need clarification:** What exactly was corrected - the enum values or just the comment?

#### Block Structure Documentation

```c
+/**
+ * struct gpu_buddy_block - Block within a buddy allocator
+ *
+ * Each block in the buddy allocator is represented by this structure.
+ * Blocks are organized in a binary tree where each parent block can be
+ * split into two children (left and right buddies). The allocator manages
+ * blocks at various orders (power-of-2 sizes) from chunk_size up to the
+ * largest contiguous region.
```

**Good:** Solid high-level overview of the data structure.

```c
+/* private: */
+	/*
+	 * Header bit layout:
+	 * - Bits 63:12: block offset within the address space
+	 * - Bits 11:10: state (ALLOCATED, FREE, or SPLIT)
+	 * - Bit 9: clear bit (1 if memory is zeroed)
+	 * - Bits 5:0: order (log2 of size relative to chunk_size)
+	 */
```

**Issue:** This documents bits 63:12, 11:10, 9, and 5:0. What about bits 8:6? Are they unused/reserved? Should be documented explicitly.

**Issue:** Maximum order is 63 (GPU_BUDDY_MAX_ORDER is 63-12=51 based on line 326), but header bits 5:0 can only hold values 0-63. Is this sufficient? This seems fine since max_order would be 51.

**Verification needed:** Does the actual code match this bit layout? Check GPU_BUDDY_HEADER_* macros:
- `GPU_BUDDY_HEADER_OFFSET GENMASK_ULL(63, 12)` ✓
- `GPU_BUDDY_HEADER_STATE GENMASK_ULL(11, 10)` ✓
- `GPU_BUDDY_HEADER_CLEAR BIT(9)` ✓ (implied by "Bit 9")
- `GPU_BUDDY_HEADER_ORDER` should be GENMASK(5, 0) - need to verify

#### Main Structure Documentation

```c
+/**
+ * struct gpu_buddy - GPU binary buddy allocator
+ *
+ * The buddy allocator provides efficient power-of-two memory allocation
+ * with fast allocation and free operations. It is commonly used for GPU
+ * memory management where allocations can be split into power-of-two
+ * block sizes.
```

**Good:** Clear purpose statement.

```c
+ * Locking should be handled by the user; a simple mutex around
+ * gpu_buddy_alloc_blocks() and gpu_buddy_free_block()/gpu_buddy_free_list()
+ * should suffice.
```

**Good:** Updated from original comment to reference specific function names. More helpful.

**Question:** What about `gpu_buddy_reset_clear()` and other exported functions? Do they need the same locking?

```c
+	/*
+	 * Array of red-black trees for free block management.
+	 * Indexed as free_trees[clear/dirty][order] where:
+	 * - Index 0 (GPU_BUDDY_CLEAR_TREE): blocks with zeroed content
+	 * - Index 1 (GPU_BUDDY_DIRTY_TREE): blocks with unknown content
+	 * Each tree holds free blocks of the corresponding order.
+	 */
```

**Good:** Clear explanation of the 2D array indexing.

**Summary for Patch 1:**
- Documentation quality is generally good
- Need clarification on what exactly was "corrected" in v2
- Missing documentation for header bits 8:6
- "Physically contiguous" terminology may be imprecise
- Overall: Good improvement to code documentation

---

---
Generated by Claude Code Patch Reviewer

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

* Claude review: drm/buddy: Move internal helpers to buddy.c
  2026-02-11  5:31 ` [PATCH v2 2/2] drm/buddy: Move internal helpers to buddy.c Sanjay Yadav
@ 2026-02-11  6:07   ` Claude Code Review Bot
  0 siblings, 0 replies; 9+ messages in thread
From: Claude Code Review Bot @ 2026-02-11  6:07 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Overall Assessment:** This patch appropriately hides implementation details by moving unused-externally functions from the header to the source file. The claim of "no functional changes" appears accurate.

**Detailed Comments:**

#### Functions Being Made Static

```c
+static unsigned int
+gpu_buddy_block_state(struct gpu_buddy_block *block)
+{
+	return block->header & GPU_BUDDY_HEADER_STATE;
+}
```

**Good:** Reasonable to make static if only used internally.

**Code style:** Kernel style typically puts return type on same line as function name for static functions, though this appears consistent with existing code style in this file.

```c
+static bool
+gpu_buddy_block_is_allocated(struct gpu_buddy_block *block)
+{
+	return gpu_buddy_block_state(block) == GPU_BUDDY_ALLOCATED;
+}
+
+static bool
+gpu_buddy_block_is_split(struct gpu_buddy_block *block)
+{
+	return gpu_buddy_block_state(block) == GPU_BUDDY_SPLIT;
+}
```

**Good:** These are simple wrappers that were unused externally.

#### Function Being Removed

```c
-/**
- * gpu_get_buddy - get buddy address
- *
- * @block: GPU buddy block
- *
- * Returns the corresponding buddy block for @block, or NULL
- * if this is a root block and can't be merged further.
- * Requires some kind of locking to protect against
- * any concurrent allocate and free operations.
- */
-struct gpu_buddy_block *
-gpu_get_buddy(struct gpu_buddy_block *block)
-{
-	return __get_buddy(block);
-}
-EXPORT_SYMBOL(gpu_get_buddy);
```

**Critical Question:** This removes an EXPORT_SYMBOL(). Was this symbol truly unused? A simple wrapper removal seems fine, but removing an exported symbol requires verification:

1. Was this checked with `git grep gpu_get_buddy` across the entire kernel tree?
2. Are there any out-of-tree drivers that might use this?
3. Should there be a deprecation period?

**If truly unused:** Removing unused exports is good hygiene.

**If potentially used:** This would break external modules and needs coordination.

#### Function Being Kept

```c
 static inline bool
-gpu_buddy_block_is_allocated(struct gpu_buddy_block *block)
+gpu_buddy_block_is_free(struct gpu_buddy_block *block)
 {
-	return gpu_buddy_block_state(block) == GPU_BUDDY_ALLOCATED;
+	return (block->header & GPU_BUDDY_HEADER_STATE) == GPU_BUDDY_FREE;
 }
```

**From v2 changelog:** "Keep gpu_buddy_block_is_free() in header since it's now used by drm_buddy.c"

**Good:** The commit message explains why this stays in the header.

**Code change:** The implementation changed from calling `gpu_buddy_block_state()` to directly accessing the header. This is necessary since `gpu_buddy_block_state()` is now static.

**Question:** Since this is inlined and used externally, is there any performance impact from the more complex expression `(block->header & GPU_BUDDY_HEADER_STATE) == GPU_BUDDY_FREE` vs the old two-step approach? Probably not, the compiler should optimize identically.

#### Header Declaration Removal

```c
-struct gpu_buddy_block *
-gpu_get_buddy(struct gpu_buddy_block *block);
```

**Good:** Correctly removes the declaration matching the removed export.

**Summary for Patch 2:**
- The internal API cleanup is sound
- Moving internal functions to .c file is appropriate
- **Critical concern:** Need confirmation that `gpu_get_buddy()` removal won't break anything
- The `gpu_buddy_block_is_free()` handling is well-explained
- Code changes match commit message

---

## FINAL RECOMMENDATIONS

**Patch 1:**
1. Clarify what was "corrected" regarding tree index values
2. Document unused header bits 8:6 explicitly
3. Consider revising "physically contiguous" terminology
4. Minor: Clarify what "cleared/zeroed" means in GPU context

**Patch 2:**
1. **Critical:** Confirm `gpu_get_buddy()` has no users (in-tree or out-of-tree considerations)
2. Consider whether ABI compatibility concerns exist

**Series:**
- Generally well-executed cleanup
- Documentation is a valuable addition
- Code movement is appropriate
- Version 2 changes address reviewer feedback appropriately

---
Generated by Claude Code Patch Reviewer

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

* Re: [PATCH v2 1/2] drm/buddy: Add kernel-doc for allocator structures and flags
  2026-02-11  5:31 ` [PATCH v2 1/2] drm/buddy: Add kernel-doc for allocator structures and flags Sanjay Yadav
  2026-02-11  6:07   ` Claude review: " Claude Code Review Bot
@ 2026-02-12  7:38   ` Arunpravin Paneer Selvam
  2026-02-12  7:40     ` Arunpravin Paneer Selvam
  1 sibling, 1 reply; 9+ messages in thread
From: Arunpravin Paneer Selvam @ 2026-02-12  7:38 UTC (permalink / raw)
  To: Sanjay Yadav, dri-devel; +Cc: intel-xe, Christian König, Matthew Auld

[-- Attachment #1: Type: text/plain, Size: 7907 bytes --]



On 2/11/2026 11:01 AM, Sanjay Yadav wrote:
> Add missing kernel-doc for GPU buddy allocator flags,
> gpu_buddy_block, and gpu_buddy. The documentation covers block
> header fields, allocator roots, free trees, and allocation flags
> such as RANGE, TOPDOWN, CONTIGUOUS, CLEAR, and TRIM_DISABLE.
> Private members are marked with kernel-doc private markers
> and documented with regular comments.
>
> No functional changes.
>
> v2:
> - Corrected GPU_BUDDY_CLEAR_TREE and GPU_BUDDY_DIRTY_TREE index values (Arun)
> - Rebased after DRM buddy allocator moved to drivers/gpu/
> - Updated commit message
>
> Cc: Christian König<christian.koenig@amd.com>
> Cc: Arunpravin Paneer Selvam<Arunpravin.PaneerSelvam@amd.com>
> Suggested-by: Matthew Auld<matthew.auld@intel.com>
> Signed-off-by: Sanjay Yadav<sanjay.kumar.yadav@intel.com>
> ---
>   include/linux/gpu_buddy.h | 122 +++++++++++++++++++++++++++++++-------
>   1 file changed, 102 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/gpu_buddy.h b/include/linux/gpu_buddy.h
> index 07ac65db6d2e..b06241c78437 100644
> --- a/include/linux/gpu_buddy.h
> +++ b/include/linux/gpu_buddy.h
> @@ -12,11 +12,58 @@
>   #include <linux/sched.h>
>   #include <linux/rbtree.h>
>   
> +/**
> + * GPU_BUDDY_RANGE_ALLOCATION - Allocate within a specific address range
> + *
> + * When set, allocation is restricted to the range [start, end) specified
> + * in gpu_buddy_alloc_blocks(). Without this flag, start/end are ignored
> + * and allocation can use any free space.
> + */
>   #define GPU_BUDDY_RANGE_ALLOCATION		BIT(0)
> +
> +/**
> + * GPU_BUDDY_TOPDOWN_ALLOCATION - Allocate from top of address space
> + *
> + * Allocate starting from high addresses and working down. Useful for
> + * separating different allocation types (e.g., kernel vs userspace)
> + * to reduce fragmentation.
> + */
>   #define GPU_BUDDY_TOPDOWN_ALLOCATION		BIT(1)
> +
> +/**
> + * GPU_BUDDY_CONTIGUOUS_ALLOCATION - Require physically contiguous blocks
> + *
> + * The allocation must be satisfied with a single contiguous block.
> + * If the requested size cannot be allocated contiguously, the
> + * allocation fails with -ENOSPC.
> + */
>   #define GPU_BUDDY_CONTIGUOUS_ALLOCATION		BIT(2)
> +
> +/**
> + * GPU_BUDDY_CLEAR_ALLOCATION - Prefer pre-cleared (zeroed) memory
> + *
> + * Attempt to allocate from the clear tree first. If insufficient clear
> + * memory is available, falls back to dirty memory. Useful when the
> + * caller needs zeroed memory and wants to avoid GPU clear operations.
> + */
>   #define GPU_BUDDY_CLEAR_ALLOCATION		BIT(3)
> +
> +/**
> + * GPU_BUDDY_CLEARED - Mark returned blocks as cleared
> + *
> + * Used with gpu_buddy_free_list() to indicate that the memory being
> + * freed has been cleared (zeroed). The blocks will be placed in the
> + * clear tree for future GPU_BUDDY_CLEAR_ALLOCATION requests.
> + */
>   #define GPU_BUDDY_CLEARED			BIT(4)
> +
> +/**
> + * GPU_BUDDY_TRIM_DISABLE - Disable automatic block trimming
> + *
> + * By default, if an allocation is smaller than the allocated block,
> + * excess memory is trimmed and returned to the free pool. This flag
> + * disables trimming, keeping the full power-of-two block size.
> + */
>   #define GPU_BUDDY_TRIM_DISABLE			BIT(5)
>   
>   enum gpu_buddy_free_tree {
> @@ -28,7 +75,27 @@ enum gpu_buddy_free_tree {
>   #define for_each_free_tree(tree) \
>   	for ((tree) = 0; (tree) < GPU_BUDDY_MAX_FREE_TREES; (tree)++)
>   
> +/**
> + * struct gpu_buddy_block - Block within a buddy allocator
> + *
> + * Each block in the buddy allocator is represented by this structure.
> + * Blocks are organized in a binary tree where each parent block can be
> + * split into two children (left and right buddies). The allocator manages
> + * blocks at various orders (power-of-2 sizes) from chunk_size up to the
> + * largest contiguous region.
> + *
> + * @private: Private data owned by the allocator user (e.g., driver-specific data)
> + * @link: List node for user ownership while block is allocated
> + */
>   struct gpu_buddy_block {
> +/* private: */
> +	/*
> +	 * Header bit layout:
> +	 * - Bits 63:12: block offset within the address space
> +	 * - Bits 11:10: state (ALLOCATED, FREE, or SPLIT)
> +	 * - Bit 9: clear bit (1 if memory is zeroed)
I think we should add
- Bits 8:9: reserved

Apart from that, looks good to me.
Reviewed-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
> +	 * - Bits 5:0: order (log2 of size relative to chunk_size)
> +	 */
>   #define GPU_BUDDY_HEADER_OFFSET GENMASK_ULL(63, 12)
>   #define GPU_BUDDY_HEADER_STATE  GENMASK_ULL(11, 10)
>   #define   GPU_BUDDY_ALLOCATED	   (1 << 10)
> @@ -43,7 +110,7 @@ struct gpu_buddy_block {
>   	struct gpu_buddy_block *left;
>   	struct gpu_buddy_block *right;
>   	struct gpu_buddy_block *parent;
> -
> +/* public: */
>   	void *private; /* owned by creator */
>   
>   	/*
> @@ -53,43 +120,58 @@ struct gpu_buddy_block {
>   	 * gpu_buddy_free* ownership is given back to the mm.
>   	 */
>   	union {
> +/* private: */
>   		struct rb_node rb;
> +/* public: */
>   		struct list_head link;
>   	};
> -
> +/* private: */
>   	struct list_head tmp_link;
>   };
>   
>   /* Order-zero must be at least SZ_4K */
>   #define GPU_BUDDY_MAX_ORDER (63 - 12)
>   
> -/*
> - * Binary Buddy System.
> +/**
> + * struct gpu_buddy - GPU binary buddy allocator
> + *
> + * The buddy allocator provides efficient power-of-two memory allocation
> + * with fast allocation and free operations. It is commonly used for GPU
> + * memory management where allocations can be split into power-of-two
> + * block sizes.
>    *
> - * Locking should be handled by the user, a simple mutex around
> - * gpu_buddy_alloc* and gpu_buddy_free* should suffice.
> + * Locking should be handled by the user; a simple mutex around
> + * gpu_buddy_alloc_blocks() and gpu_buddy_free_block()/gpu_buddy_free_list()
> + * should suffice.
> + *
> + * @n_roots: Number of root blocks in the roots array.
> + * @max_order: Maximum block order (log2 of largest block size / chunk_size).
> + * @chunk_size: Minimum allocation granularity in bytes. Must be at least SZ_4K.
> + * @size: Total size of the address space managed by this allocator in bytes.
> + * @avail: Total free space currently available for allocation in bytes.
> + * @clear_avail: Free space available in the clear tree (zeroed memory) in bytes.
> + *               This is a subset of @avail.
>    */
>   struct gpu_buddy {
> -	/* Maintain a free list for each order. */
> -	struct rb_root **free_trees;
> -
> +/* private: */
>   	/*
> -	 * Maintain explicit binary tree(s) to track the allocation of the
> -	 * address space. This gives us a simple way of finding a buddy block
> -	 * and performing the potentially recursive merge step when freeing a
> -	 * block.  Nodes are either allocated or free, in which case they will
> -	 * also exist on the respective free list.
> +	 * Array of red-black trees for free block management.
> +	 * Indexed as free_trees[clear/dirty][order] where:
> +	 * - Index 0 (GPU_BUDDY_CLEAR_TREE): blocks with zeroed content
> +	 * - Index 1 (GPU_BUDDY_DIRTY_TREE): blocks with unknown content
> +	 * Each tree holds free blocks of the corresponding order.
>   	 */
> -	struct gpu_buddy_block **roots;
> -
> +	struct rb_root **free_trees;
>   	/*
> -	 * Anything from here is public, and remains static for the lifetime of
> -	 * the mm. Everything above is considered do-not-touch.
> +	 * Array of root blocks representing the top-level blocks of the
> +	 * binary tree(s). Multiple roots exist when the total size is not
> +	 * a power of two, with each root being the largest power-of-two
> +	 * that fits in the remaining space.
>   	 */
> +	struct gpu_buddy_block **roots;
> +/* public: */
>   	unsigned int n_roots;
>   	unsigned int max_order;
> -
> -	/* Must be at least SZ_4K */
>   	u64 chunk_size;
>   	u64 size;
>   	u64 avail;

[-- Attachment #2: Type: text/html, Size: 8482 bytes --]

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

* Re: [PATCH v2 1/2] drm/buddy: Add kernel-doc for allocator structures and flags
  2026-02-12  7:38   ` [PATCH v2 1/2] " Arunpravin Paneer Selvam
@ 2026-02-12  7:40     ` Arunpravin Paneer Selvam
  2026-02-12  9:23       ` Yadav, Sanjay Kumar
  0 siblings, 1 reply; 9+ messages in thread
From: Arunpravin Paneer Selvam @ 2026-02-12  7:40 UTC (permalink / raw)
  To: Sanjay Yadav, dri-devel; +Cc: intel-xe, Christian König, Matthew Auld



On 2/12/2026 1:08 PM, Arunpravin Paneer Selvam wrote:
>
>
> On 2/11/2026 11:01 AM, Sanjay Yadav wrote:
>> Add missing kernel-doc for GPU buddy allocator flags,
>> gpu_buddy_block, and gpu_buddy. The documentation covers block
>> header fields, allocator roots, free trees, and allocation flags
>> such as RANGE, TOPDOWN, CONTIGUOUS, CLEAR, and TRIM_DISABLE.
>> Private members are marked with kernel-doc private markers
>> and documented with regular comments.
>>
>> No functional changes.
>>
>> v2:
>> - Corrected GPU_BUDDY_CLEAR_TREE and GPU_BUDDY_DIRTY_TREE index values (Arun)
>> - Rebased after DRM buddy allocator moved to drivers/gpu/
>> - Updated commit message
>>
>> Cc: Christian König<christian.koenig@amd.com>
>> Cc: Arunpravin Paneer Selvam<Arunpravin.PaneerSelvam@amd.com>
>> Suggested-by: Matthew Auld<matthew.auld@intel.com>
>> Signed-off-by: Sanjay Yadav<sanjay.kumar.yadav@intel.com>
>> ---
>>   include/linux/gpu_buddy.h | 122 +++++++++++++++++++++++++++++++-------
>>   1 file changed, 102 insertions(+), 20 deletions(-)
>>
>> diff --git a/include/linux/gpu_buddy.h b/include/linux/gpu_buddy.h
>> index 07ac65db6d2e..b06241c78437 100644
>> --- a/include/linux/gpu_buddy.h
>> +++ b/include/linux/gpu_buddy.h
>> @@ -12,11 +12,58 @@
>>   #include <linux/sched.h>
>>   #include <linux/rbtree.h>
>>   
>> +/**
>> + * GPU_BUDDY_RANGE_ALLOCATION - Allocate within a specific address range
>> + *
>> + * When set, allocation is restricted to the range [start, end) specified
>> + * in gpu_buddy_alloc_blocks(). Without this flag, start/end are ignored
>> + * and allocation can use any free space.
>> + */
>>   #define GPU_BUDDY_RANGE_ALLOCATION		BIT(0)
>> +
>> +/**
>> + * GPU_BUDDY_TOPDOWN_ALLOCATION - Allocate from top of address space
>> + *
>> + * Allocate starting from high addresses and working down. Useful for
>> + * separating different allocation types (e.g., kernel vs userspace)
>> + * to reduce fragmentation.
>> + */
>>   #define GPU_BUDDY_TOPDOWN_ALLOCATION		BIT(1)
>> +
>> +/**
>> + * GPU_BUDDY_CONTIGUOUS_ALLOCATION - Require physically contiguous blocks
>> + *
>> + * The allocation must be satisfied with a single contiguous block.
>> + * If the requested size cannot be allocated contiguously, the
>> + * allocation fails with -ENOSPC.
>> + */
>>   #define GPU_BUDDY_CONTIGUOUS_ALLOCATION		BIT(2)
>> +
>> +/**
>> + * GPU_BUDDY_CLEAR_ALLOCATION - Prefer pre-cleared (zeroed) memory
>> + *
>> + * Attempt to allocate from the clear tree first. If insufficient clear
>> + * memory is available, falls back to dirty memory. Useful when the
>> + * caller needs zeroed memory and wants to avoid GPU clear operations.
>> + */
>>   #define GPU_BUDDY_CLEAR_ALLOCATION		BIT(3)
>> +
>> +/**
>> + * GPU_BUDDY_CLEARED - Mark returned blocks as cleared
>> + *
>> + * Used with gpu_buddy_free_list() to indicate that the memory being
>> + * freed has been cleared (zeroed). The blocks will be placed in the
>> + * clear tree for future GPU_BUDDY_CLEAR_ALLOCATION requests.
>> + */
>>   #define GPU_BUDDY_CLEARED			BIT(4)
>> +
>> +/**
>> + * GPU_BUDDY_TRIM_DISABLE - Disable automatic block trimming
>> + *
>> + * By default, if an allocation is smaller than the allocated block,
>> + * excess memory is trimmed and returned to the free pool. This flag
>> + * disables trimming, keeping the full power-of-two block size.
>> + */
>>   #define GPU_BUDDY_TRIM_DISABLE			BIT(5)
>>   
>>   enum gpu_buddy_free_tree {
>> @@ -28,7 +75,27 @@ enum gpu_buddy_free_tree {
>>   #define for_each_free_tree(tree) \
>>   	for ((tree) = 0; (tree) < GPU_BUDDY_MAX_FREE_TREES; (tree)++)
>>   
>> +/**
>> + * struct gpu_buddy_block - Block within a buddy allocator
>> + *
>> + * Each block in the buddy allocator is represented by this structure.
>> + * Blocks are organized in a binary tree where each parent block can be
>> + * split into two children (left and right buddies). The allocator manages
>> + * blocks at various orders (power-of-2 sizes) from chunk_size up to the
>> + * largest contiguous region.
>> + *
>> + * @private: Private data owned by the allocator user (e.g., driver-specific data)
>> + * @link: List node for user ownership while block is allocated
>> + */
>>   struct gpu_buddy_block {
>> +/* private: */
>> +	/*
>> +	 * Header bit layout:
>> +	 * - Bits 63:12: block offset within the address space
>> +	 * - Bits 11:10: state (ALLOCATED, FREE, or SPLIT)
>> +	 * - Bit 9: clear bit (1 if memory is zeroed)
> I think we should add
> - Bits 8:9: reserved
it should be -Bits 8:6: reserved
>
> Apart from that, looks good to me.
> Reviewed-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
>> +	 * - Bits 5:0: order (log2 of size relative to chunk_size)
>> +	 */
>>   #define GPU_BUDDY_HEADER_OFFSET GENMASK_ULL(63, 12)
>>   #define GPU_BUDDY_HEADER_STATE  GENMASK_ULL(11, 10)
>>   #define   GPU_BUDDY_ALLOCATED	   (1 << 10)
>> @@ -43,7 +110,7 @@ struct gpu_buddy_block {
>>   	struct gpu_buddy_block *left;
>>   	struct gpu_buddy_block *right;
>>   	struct gpu_buddy_block *parent;
>> -
>> +/* public: */
>>   	void *private; /* owned by creator */
>>   
>>   	/*
>> @@ -53,43 +120,58 @@ struct gpu_buddy_block {
>>   	 * gpu_buddy_free* ownership is given back to the mm.
>>   	 */
>>   	union {
>> +/* private: */
>>   		struct rb_node rb;
>> +/* public: */
>>   		struct list_head link;
>>   	};
>> -
>> +/* private: */
>>   	struct list_head tmp_link;
>>   };
>>   
>>   /* Order-zero must be at least SZ_4K */
>>   #define GPU_BUDDY_MAX_ORDER (63 - 12)
>>   
>> -/*
>> - * Binary Buddy System.
>> +/**
>> + * struct gpu_buddy - GPU binary buddy allocator
>> + *
>> + * The buddy allocator provides efficient power-of-two memory allocation
>> + * with fast allocation and free operations. It is commonly used for GPU
>> + * memory management where allocations can be split into power-of-two
>> + * block sizes.
>>    *
>> - * Locking should be handled by the user, a simple mutex around
>> - * gpu_buddy_alloc* and gpu_buddy_free* should suffice.
>> + * Locking should be handled by the user; a simple mutex around
>> + * gpu_buddy_alloc_blocks() and gpu_buddy_free_block()/gpu_buddy_free_list()
>> + * should suffice.
>> + *
>> + * @n_roots: Number of root blocks in the roots array.
>> + * @max_order: Maximum block order (log2 of largest block size / chunk_size).
>> + * @chunk_size: Minimum allocation granularity in bytes. Must be at least SZ_4K.
>> + * @size: Total size of the address space managed by this allocator in bytes.
>> + * @avail: Total free space currently available for allocation in bytes.
>> + * @clear_avail: Free space available in the clear tree (zeroed memory) in bytes.
>> + *               This is a subset of @avail.
>>    */
>>   struct gpu_buddy {
>> -	/* Maintain a free list for each order. */
>> -	struct rb_root **free_trees;
>> -
>> +/* private: */
>>   	/*
>> -	 * Maintain explicit binary tree(s) to track the allocation of the
>> -	 * address space. This gives us a simple way of finding a buddy block
>> -	 * and performing the potentially recursive merge step when freeing a
>> -	 * block.  Nodes are either allocated or free, in which case they will
>> -	 * also exist on the respective free list.
>> +	 * Array of red-black trees for free block management.
>> +	 * Indexed as free_trees[clear/dirty][order] where:
>> +	 * - Index 0 (GPU_BUDDY_CLEAR_TREE): blocks with zeroed content
>> +	 * - Index 1 (GPU_BUDDY_DIRTY_TREE): blocks with unknown content
>> +	 * Each tree holds free blocks of the corresponding order.
>>   	 */
>> -	struct gpu_buddy_block **roots;
>> -
>> +	struct rb_root **free_trees;
>>   	/*
>> -	 * Anything from here is public, and remains static for the lifetime of
>> -	 * the mm. Everything above is considered do-not-touch.
>> +	 * Array of root blocks representing the top-level blocks of the
>> +	 * binary tree(s). Multiple roots exist when the total size is not
>> +	 * a power of two, with each root being the largest power-of-two
>> +	 * that fits in the remaining space.
>>   	 */
>> +	struct gpu_buddy_block **roots;
>> +/* public: */
>>   	unsigned int n_roots;
>>   	unsigned int max_order;
>> -
>> -	/* Must be at least SZ_4K */
>>   	u64 chunk_size;
>>   	u64 size;
>>   	u64 avail;
>


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

* Re: [PATCH v2 1/2] drm/buddy: Add kernel-doc for allocator structures and flags
  2026-02-12  7:40     ` Arunpravin Paneer Selvam
@ 2026-02-12  9:23       ` Yadav, Sanjay Kumar
  0 siblings, 0 replies; 9+ messages in thread
From: Yadav, Sanjay Kumar @ 2026-02-12  9:23 UTC (permalink / raw)
  To: Arunpravin Paneer Selvam, dri-devel
  Cc: intel-xe, Christian König, Matthew Auld


On 12-02-2026 13:10, Arunpravin Paneer Selvam wrote:
>
>
> On 2/12/2026 1:08 PM, Arunpravin Paneer Selvam wrote:
>>
>>
>> On 2/11/2026 11:01 AM, Sanjay Yadav wrote:
>>> Add missing kernel-doc for GPU buddy allocator flags,
>>> gpu_buddy_block, and gpu_buddy. The documentation covers block
>>> header fields, allocator roots, free trees, and allocation flags
>>> such as RANGE, TOPDOWN, CONTIGUOUS, CLEAR, and TRIM_DISABLE.
>>> Private members are marked with kernel-doc private markers
>>> and documented with regular comments.
>>>
>>> No functional changes.
>>>
>>> v2:
>>> - Corrected GPU_BUDDY_CLEAR_TREE and GPU_BUDDY_DIRTY_TREE index 
>>> values (Arun)
>>> - Rebased after DRM buddy allocator moved to drivers/gpu/
>>> - Updated commit message
>>>
>>> Cc: Christian König<christian.koenig@amd.com>
>>> Cc: Arunpravin Paneer Selvam<Arunpravin.PaneerSelvam@amd.com>
>>> Suggested-by: Matthew Auld<matthew.auld@intel.com>
>>> Signed-off-by: Sanjay Yadav<sanjay.kumar.yadav@intel.com>
>>> ---
>>>   include/linux/gpu_buddy.h | 122 
>>> +++++++++++++++++++++++++++++++-------
>>>   1 file changed, 102 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/include/linux/gpu_buddy.h b/include/linux/gpu_buddy.h
>>> index 07ac65db6d2e..b06241c78437 100644
>>> --- a/include/linux/gpu_buddy.h
>>> +++ b/include/linux/gpu_buddy.h
>>> @@ -12,11 +12,58 @@
>>>   #include <linux/sched.h>
>>>   #include <linux/rbtree.h>
>>>   +/**
>>> + * GPU_BUDDY_RANGE_ALLOCATION - Allocate within a specific address 
>>> range
>>> + *
>>> + * When set, allocation is restricted to the range [start, end) 
>>> specified
>>> + * in gpu_buddy_alloc_blocks(). Without this flag, start/end are 
>>> ignored
>>> + * and allocation can use any free space.
>>> + */
>>>   #define GPU_BUDDY_RANGE_ALLOCATION        BIT(0)
>>> +
>>> +/**
>>> + * GPU_BUDDY_TOPDOWN_ALLOCATION - Allocate from top of address space
>>> + *
>>> + * Allocate starting from high addresses and working down. Useful for
>>> + * separating different allocation types (e.g., kernel vs userspace)
>>> + * to reduce fragmentation.
>>> + */
>>>   #define GPU_BUDDY_TOPDOWN_ALLOCATION        BIT(1)
>>> +
>>> +/**
>>> + * GPU_BUDDY_CONTIGUOUS_ALLOCATION - Require physically contiguous 
>>> blocks
>>> + *
>>> + * The allocation must be satisfied with a single contiguous block.
>>> + * If the requested size cannot be allocated contiguously, the
>>> + * allocation fails with -ENOSPC.
>>> + */
>>>   #define GPU_BUDDY_CONTIGUOUS_ALLOCATION        BIT(2)
>>> +
>>> +/**
>>> + * GPU_BUDDY_CLEAR_ALLOCATION - Prefer pre-cleared (zeroed) memory
>>> + *
>>> + * Attempt to allocate from the clear tree first. If insufficient 
>>> clear
>>> + * memory is available, falls back to dirty memory. Useful when the
>>> + * caller needs zeroed memory and wants to avoid GPU clear operations.
>>> + */
>>>   #define GPU_BUDDY_CLEAR_ALLOCATION        BIT(3)
>>> +
>>> +/**
>>> + * GPU_BUDDY_CLEARED - Mark returned blocks as cleared
>>> + *
>>> + * Used with gpu_buddy_free_list() to indicate that the memory being
>>> + * freed has been cleared (zeroed). The blocks will be placed in the
>>> + * clear tree for future GPU_BUDDY_CLEAR_ALLOCATION requests.
>>> + */
>>>   #define GPU_BUDDY_CLEARED            BIT(4)
>>> +
>>> +/**
>>> + * GPU_BUDDY_TRIM_DISABLE - Disable automatic block trimming
>>> + *
>>> + * By default, if an allocation is smaller than the allocated block,
>>> + * excess memory is trimmed and returned to the free pool. This flag
>>> + * disables trimming, keeping the full power-of-two block size.
>>> + */
>>>   #define GPU_BUDDY_TRIM_DISABLE            BIT(5)
>>>     enum gpu_buddy_free_tree {
>>> @@ -28,7 +75,27 @@ enum gpu_buddy_free_tree {
>>>   #define for_each_free_tree(tree) \
>>>       for ((tree) = 0; (tree) < GPU_BUDDY_MAX_FREE_TREES; (tree)++)
>>>   +/**
>>> + * struct gpu_buddy_block - Block within a buddy allocator
>>> + *
>>> + * Each block in the buddy allocator is represented by this structure.
>>> + * Blocks are organized in a binary tree where each parent block 
>>> can be
>>> + * split into two children (left and right buddies). The allocator 
>>> manages
>>> + * blocks at various orders (power-of-2 sizes) from chunk_size up 
>>> to the
>>> + * largest contiguous region.
>>> + *
>>> + * @private: Private data owned by the allocator user (e.g., 
>>> driver-specific data)
>>> + * @link: List node for user ownership while block is allocated
>>> + */
>>>   struct gpu_buddy_block {
>>> +/* private: */
>>> +    /*
>>> +     * Header bit layout:
>>> +     * - Bits 63:12: block offset within the address space
>>> +     * - Bits 11:10: state (ALLOCATED, FREE, or SPLIT)
>>> +     * - Bit 9: clear bit (1 if memory is zeroed)
>> I think we should add
>> - Bits 8:9: reserved
> it should be -Bits 8:6: reserved
>>
>> Apart from that, looks good to me.
>> Reviewed-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Thanks Arun for your prompt review, helpful comments, and your time.
>>> +     * - Bits 5:0: order (log2 of size relative to chunk_size)
>>> +     */
>>>   #define GPU_BUDDY_HEADER_OFFSET GENMASK_ULL(63, 12)
>>>   #define GPU_BUDDY_HEADER_STATE  GENMASK_ULL(11, 10)
>>>   #define   GPU_BUDDY_ALLOCATED       (1 << 10)
>>> @@ -43,7 +110,7 @@ struct gpu_buddy_block {
>>>       struct gpu_buddy_block *left;
>>>       struct gpu_buddy_block *right;
>>>       struct gpu_buddy_block *parent;
>>> -
>>> +/* public: */
>>>       void *private; /* owned by creator */
>>>         /*
>>> @@ -53,43 +120,58 @@ struct gpu_buddy_block {
>>>        * gpu_buddy_free* ownership is given back to the mm.
>>>        */
>>>       union {
>>> +/* private: */
>>>           struct rb_node rb;
>>> +/* public: */
>>>           struct list_head link;
>>>       };
>>> -
>>> +/* private: */
>>>       struct list_head tmp_link;
>>>   };
>>>     /* Order-zero must be at least SZ_4K */
>>>   #define GPU_BUDDY_MAX_ORDER (63 - 12)
>>>   -/*
>>> - * Binary Buddy System.
>>> +/**
>>> + * struct gpu_buddy - GPU binary buddy allocator
>>> + *
>>> + * The buddy allocator provides efficient power-of-two memory 
>>> allocation
>>> + * with fast allocation and free operations. It is commonly used 
>>> for GPU
>>> + * memory management where allocations can be split into power-of-two
>>> + * block sizes.
>>>    *
>>> - * Locking should be handled by the user, a simple mutex around
>>> - * gpu_buddy_alloc* and gpu_buddy_free* should suffice.
>>> + * Locking should be handled by the user; a simple mutex around
>>> + * gpu_buddy_alloc_blocks() and 
>>> gpu_buddy_free_block()/gpu_buddy_free_list()
>>> + * should suffice.
>>> + *
>>> + * @n_roots: Number of root blocks in the roots array.
>>> + * @max_order: Maximum block order (log2 of largest block size / 
>>> chunk_size).
>>> + * @chunk_size: Minimum allocation granularity in bytes. Must be at 
>>> least SZ_4K.
>>> + * @size: Total size of the address space managed by this allocator 
>>> in bytes.
>>> + * @avail: Total free space currently available for allocation in 
>>> bytes.
>>> + * @clear_avail: Free space available in the clear tree (zeroed 
>>> memory) in bytes.
>>> + *               This is a subset of @avail.
>>>    */
>>>   struct gpu_buddy {
>>> -    /* Maintain a free list for each order. */
>>> -    struct rb_root **free_trees;
>>> -
>>> +/* private: */
>>>       /*
>>> -     * Maintain explicit binary tree(s) to track the allocation of the
>>> -     * address space. This gives us a simple way of finding a buddy 
>>> block
>>> -     * and performing the potentially recursive merge step when 
>>> freeing a
>>> -     * block.  Nodes are either allocated or free, in which case 
>>> they will
>>> -     * also exist on the respective free list.
>>> +     * Array of red-black trees for free block management.
>>> +     * Indexed as free_trees[clear/dirty][order] where:
>>> +     * - Index 0 (GPU_BUDDY_CLEAR_TREE): blocks with zeroed content
>>> +     * - Index 1 (GPU_BUDDY_DIRTY_TREE): blocks with unknown content
>>> +     * Each tree holds free blocks of the corresponding order.
>>>        */
>>> -    struct gpu_buddy_block **roots;
>>> -
>>> +    struct rb_root **free_trees;
>>>       /*
>>> -     * Anything from here is public, and remains static for the 
>>> lifetime of
>>> -     * the mm. Everything above is considered do-not-touch.
>>> +     * Array of root blocks representing the top-level blocks of the
>>> +     * binary tree(s). Multiple roots exist when the total size is not
>>> +     * a power of two, with each root being the largest power-of-two
>>> +     * that fits in the remaining space.
>>>        */
>>> +    struct gpu_buddy_block **roots;
>>> +/* public: */
>>>       unsigned int n_roots;
>>>       unsigned int max_order;
>>> -
>>> -    /* Must be at least SZ_4K */
>>>       u64 chunk_size;
>>>       u64 size;
>>>       u64 avail;
>>
>

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

end of thread, other threads:[~2026-02-12  9:23 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-11  5:31 [PATCH v2 0/2] drm/buddy: Documentation and internal helper cleanup Sanjay Yadav
2026-02-11  5:31 ` [PATCH v2 1/2] drm/buddy: Add kernel-doc for allocator structures and flags Sanjay Yadav
2026-02-11  6:07   ` Claude review: " Claude Code Review Bot
2026-02-12  7:38   ` [PATCH v2 1/2] " Arunpravin Paneer Selvam
2026-02-12  7:40     ` Arunpravin Paneer Selvam
2026-02-12  9:23       ` Yadav, Sanjay Kumar
2026-02-11  5:31 ` [PATCH v2 2/2] drm/buddy: Move internal helpers to buddy.c Sanjay Yadav
2026-02-11  6:07   ` Claude review: " Claude Code Review Bot
2026-02-11  6:07 ` Claude review: drm/buddy: Documentation and internal helper cleanup 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