* [PATCH] gpu/buddy: Introduce gpu_buddy_assert() for kunit-aware assertions
@ 2026-02-24 13:25 Sanjay Yadav
2026-02-25 11:19 ` Arunpravin Paneer Selvam
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Sanjay Yadav @ 2026-02-24 13:25 UTC (permalink / raw)
To: dri-devel
Cc: intel-xe, Christian König, Arunpravin Paneer Selvam,
Matthew Auld
Introduce gpu_buddy_assert(), a small helper that wraps WARN_ON() and,
when CONFIG_KUNIT is enabled, also calls kunit_fail_current_test() so
that any active KUnit test is marked as failed. In non-KUnit builds
the macro reduces to WARN_ON(), preserving existing behaviour.
Stringify the asserted condition in the failure message to make it easy
to identify which assertion fired. Leave the WARN_ON() in
gpu_buddy_block_trim() unchanged, as it returns -EINVAL and the caller
already observes the failure via the return code.
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>
---
drivers/gpu/buddy.c | 36 ++++++++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
index b27761246d4b..da5a1222f46b 100644
--- a/drivers/gpu/buddy.c
+++ b/drivers/gpu/buddy.c
@@ -3,8 +3,7 @@
* Copyright © 2021 Intel Corporation
*/
-#include <kunit/test-bug.h>
-
+#include <linux/bug.h>
#include <linux/export.h>
#include <linux/kmemleak.h>
#include <linux/module.h>
@@ -12,6 +11,28 @@
#include <linux/gpu_buddy.h>
+/**
+ * gpu_buddy_assert - assert a condition in the buddy allocator
+ * @condition: condition expected to be true
+ *
+ * When CONFIG_KUNIT is enabled, evaluates @condition and, if false, triggers
+ * a WARN_ON() and also calls kunit_fail_current_test() so that any running
+ * kunit test is properly marked as failed. The stringified condition is
+ * included in the failure message for easy identification.
+ *
+ * When CONFIG_KUNIT is not enabled, this reduces to WARN_ON() so production
+ * builds retain the same warning semantics as before.
+ */
+#if IS_ENABLED(CONFIG_KUNIT)
+#include <kunit/test-bug.h>
+#define gpu_buddy_assert(condition) do { \
+ if (WARN_ON(!(condition))) \
+ kunit_fail_current_test("gpu_buddy_assert(" #condition ")"); \
+} while (0)
+#else
+#define gpu_buddy_assert(condition) WARN_ON(!(condition))
+#endif
+
static struct kmem_cache *slab_blocks;
static unsigned int
@@ -268,8 +289,8 @@ static int __force_merge(struct gpu_buddy *mm,
if (!gpu_buddy_block_is_free(buddy))
continue;
- WARN_ON(gpu_buddy_block_is_clear(block) ==
- gpu_buddy_block_is_clear(buddy));
+ gpu_buddy_assert(gpu_buddy_block_is_clear(block) !=
+ gpu_buddy_block_is_clear(buddy));
/*
* Advance to the next node when the current node is the buddy,
@@ -415,8 +436,7 @@ void gpu_buddy_fini(struct gpu_buddy *mm)
start = gpu_buddy_block_offset(mm->roots[i]);
__force_merge(mm, start, start + size, order);
- if (WARN_ON(!gpu_buddy_block_is_free(mm->roots[i])))
- kunit_fail_current_test("buddy_fini() root");
+ gpu_buddy_assert(gpu_buddy_block_is_free(mm->roots[i]));
gpu_block_free(mm, mm->roots[i]);
@@ -424,7 +444,7 @@ void gpu_buddy_fini(struct gpu_buddy *mm)
size -= root_size;
}
- WARN_ON(mm->avail != mm->size);
+ gpu_buddy_assert(mm->avail == mm->size);
for_each_free_tree(i)
kfree(mm->free_trees[i]);
@@ -541,7 +561,7 @@ static void __gpu_buddy_free_list(struct gpu_buddy *mm,
{
struct gpu_buddy_block *block, *on;
- WARN_ON(mark_dirty && mark_clear);
+ gpu_buddy_assert(!(mark_dirty && mark_clear));
list_for_each_entry_safe(block, on, objects, link) {
if (mark_clear)
--
2.52.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] gpu/buddy: Introduce gpu_buddy_assert() for kunit-aware assertions
2026-02-24 13:25 [PATCH] gpu/buddy: Introduce gpu_buddy_assert() for kunit-aware assertions Sanjay Yadav
@ 2026-02-25 11:19 ` Arunpravin Paneer Selvam
2026-02-25 13:20 ` Yadav, Sanjay Kumar
2026-02-27 5:09 ` Claude review: " Claude Code Review Bot
2026-02-27 5:09 ` Claude Code Review Bot
2 siblings, 1 reply; 5+ messages in thread
From: Arunpravin Paneer Selvam @ 2026-02-25 11:19 UTC (permalink / raw)
To: Sanjay Yadav, dri-devel; +Cc: intel-xe, Christian König, Matthew Auld
Reviewed-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
On 2/24/2026 6:55 PM, Sanjay Yadav wrote:
> Introduce gpu_buddy_assert(), a small helper that wraps WARN_ON() and,
> when CONFIG_KUNIT is enabled, also calls kunit_fail_current_test() so
> that any active KUnit test is marked as failed. In non-KUnit builds
> the macro reduces to WARN_ON(), preserving existing behaviour.
>
> Stringify the asserted condition in the failure message to make it easy
> to identify which assertion fired. Leave the WARN_ON() in
> gpu_buddy_block_trim() unchanged, as it returns -EINVAL and the caller
> already observes the failure via the return code.
>
> 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>
> ---
> drivers/gpu/buddy.c | 36 ++++++++++++++++++++++++++++--------
> 1 file changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
> index b27761246d4b..da5a1222f46b 100644
> --- a/drivers/gpu/buddy.c
> +++ b/drivers/gpu/buddy.c
> @@ -3,8 +3,7 @@
> * Copyright © 2021 Intel Corporation
> */
>
> -#include <kunit/test-bug.h>
> -
> +#include <linux/bug.h>
> #include <linux/export.h>
> #include <linux/kmemleak.h>
> #include <linux/module.h>
> @@ -12,6 +11,28 @@
>
> #include <linux/gpu_buddy.h>
>
> +/**
> + * gpu_buddy_assert - assert a condition in the buddy allocator
> + * @condition: condition expected to be true
> + *
> + * When CONFIG_KUNIT is enabled, evaluates @condition and, if false, triggers
> + * a WARN_ON() and also calls kunit_fail_current_test() so that any running
> + * kunit test is properly marked as failed. The stringified condition is
> + * included in the failure message for easy identification.
> + *
> + * When CONFIG_KUNIT is not enabled, this reduces to WARN_ON() so production
> + * builds retain the same warning semantics as before.
> + */
> +#if IS_ENABLED(CONFIG_KUNIT)
> +#include <kunit/test-bug.h>
> +#define gpu_buddy_assert(condition) do { \
> + if (WARN_ON(!(condition))) \
> + kunit_fail_current_test("gpu_buddy_assert(" #condition ")"); \
> +} while (0)
> +#else
> +#define gpu_buddy_assert(condition) WARN_ON(!(condition))
> +#endif
> +
> static struct kmem_cache *slab_blocks;
>
> static unsigned int
> @@ -268,8 +289,8 @@ static int __force_merge(struct gpu_buddy *mm,
> if (!gpu_buddy_block_is_free(buddy))
> continue;
>
> - WARN_ON(gpu_buddy_block_is_clear(block) ==
> - gpu_buddy_block_is_clear(buddy));
> + gpu_buddy_assert(gpu_buddy_block_is_clear(block) !=
> + gpu_buddy_block_is_clear(buddy));
>
> /*
> * Advance to the next node when the current node is the buddy,
> @@ -415,8 +436,7 @@ void gpu_buddy_fini(struct gpu_buddy *mm)
> start = gpu_buddy_block_offset(mm->roots[i]);
> __force_merge(mm, start, start + size, order);
>
> - if (WARN_ON(!gpu_buddy_block_is_free(mm->roots[i])))
> - kunit_fail_current_test("buddy_fini() root");
> + gpu_buddy_assert(gpu_buddy_block_is_free(mm->roots[i]));
>
> gpu_block_free(mm, mm->roots[i]);
>
> @@ -424,7 +444,7 @@ void gpu_buddy_fini(struct gpu_buddy *mm)
> size -= root_size;
> }
>
> - WARN_ON(mm->avail != mm->size);
> + gpu_buddy_assert(mm->avail == mm->size);
>
> for_each_free_tree(i)
> kfree(mm->free_trees[i]);
> @@ -541,7 +561,7 @@ static void __gpu_buddy_free_list(struct gpu_buddy *mm,
> {
> struct gpu_buddy_block *block, *on;
>
> - WARN_ON(mark_dirty && mark_clear);
> + gpu_buddy_assert(!(mark_dirty && mark_clear));
>
> list_for_each_entry_safe(block, on, objects, link) {
> if (mark_clear)
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] gpu/buddy: Introduce gpu_buddy_assert() for kunit-aware assertions
2026-02-25 11:19 ` Arunpravin Paneer Selvam
@ 2026-02-25 13:20 ` Yadav, Sanjay Kumar
0 siblings, 0 replies; 5+ messages in thread
From: Yadav, Sanjay Kumar @ 2026-02-25 13:20 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, dri-devel
Cc: intel-xe, Christian König, Matthew Auld
On 25-02-2026 16:49, Arunpravin Paneer Selvam wrote:
> Reviewed-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Thanks for your prompt review, Arun!
-Sanjay
>
> On 2/24/2026 6:55 PM, Sanjay Yadav wrote:
>> Introduce gpu_buddy_assert(), a small helper that wraps WARN_ON() and,
>> when CONFIG_KUNIT is enabled, also calls kunit_fail_current_test() so
>> that any active KUnit test is marked as failed. In non-KUnit builds
>> the macro reduces to WARN_ON(), preserving existing behaviour.
>>
>> Stringify the asserted condition in the failure message to make it easy
>> to identify which assertion fired. Leave the WARN_ON() in
>> gpu_buddy_block_trim() unchanged, as it returns -EINVAL and the caller
>> already observes the failure via the return code.
>>
>> 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>
>> ---
>> drivers/gpu/buddy.c | 36 ++++++++++++++++++++++++++++--------
>> 1 file changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
>> index b27761246d4b..da5a1222f46b 100644
>> --- a/drivers/gpu/buddy.c
>> +++ b/drivers/gpu/buddy.c
>> @@ -3,8 +3,7 @@
>> * Copyright © 2021 Intel Corporation
>> */
>> -#include <kunit/test-bug.h>
>> -
>> +#include <linux/bug.h>
>> #include <linux/export.h>
>> #include <linux/kmemleak.h>
>> #include <linux/module.h>
>> @@ -12,6 +11,28 @@
>> #include <linux/gpu_buddy.h>
>> +/**
>> + * gpu_buddy_assert - assert a condition in the buddy allocator
>> + * @condition: condition expected to be true
>> + *
>> + * When CONFIG_KUNIT is enabled, evaluates @condition and, if false,
>> triggers
>> + * a WARN_ON() and also calls kunit_fail_current_test() so that any
>> running
>> + * kunit test is properly marked as failed. The stringified
>> condition is
>> + * included in the failure message for easy identification.
>> + *
>> + * When CONFIG_KUNIT is not enabled, this reduces to WARN_ON() so
>> production
>> + * builds retain the same warning semantics as before.
>> + */
>> +#if IS_ENABLED(CONFIG_KUNIT)
>> +#include <kunit/test-bug.h>
>> +#define gpu_buddy_assert(condition) do { \
>> + if (WARN_ON(!(condition))) \
>> + kunit_fail_current_test("gpu_buddy_assert(" #condition
>> ")"); \
>> +} while (0)
>> +#else
>> +#define gpu_buddy_assert(condition) WARN_ON(!(condition))
>> +#endif
>> +
>> static struct kmem_cache *slab_blocks;
>> static unsigned int
>> @@ -268,8 +289,8 @@ static int __force_merge(struct gpu_buddy *mm,
>> if (!gpu_buddy_block_is_free(buddy))
>> continue;
>> - WARN_ON(gpu_buddy_block_is_clear(block) ==
>> - gpu_buddy_block_is_clear(buddy));
>> + gpu_buddy_assert(gpu_buddy_block_is_clear(block) !=
>> + gpu_buddy_block_is_clear(buddy));
>> /*
>> * Advance to the next node when the current node
>> is the buddy,
>> @@ -415,8 +436,7 @@ void gpu_buddy_fini(struct gpu_buddy *mm)
>> start = gpu_buddy_block_offset(mm->roots[i]);
>> __force_merge(mm, start, start + size, order);
>> - if (WARN_ON(!gpu_buddy_block_is_free(mm->roots[i])))
>> - kunit_fail_current_test("buddy_fini() root");
>> + gpu_buddy_assert(gpu_buddy_block_is_free(mm->roots[i]));
>> gpu_block_free(mm, mm->roots[i]);
>> @@ -424,7 +444,7 @@ void gpu_buddy_fini(struct gpu_buddy *mm)
>> size -= root_size;
>> }
>> - WARN_ON(mm->avail != mm->size);
>> + gpu_buddy_assert(mm->avail == mm->size);
>> for_each_free_tree(i)
>> kfree(mm->free_trees[i]);
>> @@ -541,7 +561,7 @@ static void __gpu_buddy_free_list(struct
>> gpu_buddy *mm,
>> {
>> struct gpu_buddy_block *block, *on;
>> - WARN_ON(mark_dirty && mark_clear);
>> + gpu_buddy_assert(!(mark_dirty && mark_clear));
>> list_for_each_entry_safe(block, on, objects, link) {
>> if (mark_clear)
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Claude review: gpu/buddy: Introduce gpu_buddy_assert() for kunit-aware assertions
2026-02-24 13:25 [PATCH] gpu/buddy: Introduce gpu_buddy_assert() for kunit-aware assertions Sanjay Yadav
2026-02-25 11:19 ` Arunpravin Paneer Selvam
@ 2026-02-27 5:09 ` Claude Code Review Bot
2026-02-27 5:09 ` Claude Code Review Bot
2 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-02-27 5:09 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Positive aspects:**
- The macro is well-documented with a kernel-doc comment.
- Correctly avoids double-evaluation of the condition — it's evaluated once inside `WARN_ON(!(condition))`.
- The decision to leave the `WARN_ON` in `gpu_buddy_block_trim()` alone is sensible and explicitly called out in the commit message.
- The `#include <kunit/test-bug.h>` is now properly guarded behind `IS_ENABLED(CONFIG_KUNIT)`, which avoids pulling in kunit headers in production builds.
**Minor observations:**
1. **Return value of WARN_ON discarded in non-KUNIT path**: In the `#else` branch:
```c
#define gpu_buddy_assert(condition) WARN_ON(!(condition))
```
`WARN_ON()` returns a value (the condition), but all call sites discard it (the macro is used as a statement, not in an `if`). This is fine for the current usage, but if someone later tries to use `if (gpu_buddy_assert(...))`, the KUNIT path (wrapped in `do { } while(0)`) would not compile while the non-KUNIT path would. This asymmetry is minor since the macro name `_assert` doesn't suggest a return value, but for consistency you could wrap the `#else` case in `do { } while(0)` as well:
```c
#define gpu_buddy_assert(condition) do { WARN_ON(!(condition)); } while (0)
```
2. **`#include <linux/bug.h>` addition**: The old code included `<kunit/test-bug.h>` unconditionally, which transitively provided `WARN_ON` via its include chain. Now that `kunit/test-bug.h` is conditional, adding `<linux/bug.h>` explicitly is correct. However, it's worth noting that `WARN_ON` is likely already available through other transitive includes (e.g., `<linux/slab.h>` via `<linux/module.h>`), so the explicit include is defensive — which is good practice.
3. **Stringification readability for the negated case**: At the `__gpu_buddy_free_list` site:
```c
gpu_buddy_assert(!(mark_dirty && mark_clear));
```
The kunit failure message will read `"gpu_buddy_assert(!(mark_dirty && mark_clear))"`, which is clear enough, though slightly verbose. This is a trivial aesthetic point.
4. **Scope of the macro**: The macro is defined locally in `buddy.c` rather than in the `<linux/gpu_buddy.h>` header. This is appropriate since all current users are in this single file, following the principle of narrowest scope.
**Correctness of each hunk:**
- **`__force_merge`**: `WARN_ON(clear(block) == clear(buddy))` → `gpu_buddy_assert(clear(block) != clear(buddy))` — logically equivalent, correct.
- **`gpu_buddy_fini` root check**: `if (WARN_ON(!is_free(...))) kunit_fail_current_test(...)` → `gpu_buddy_assert(is_free(...))` — equivalent, cleaner.
- **`gpu_buddy_fini` avail check**: `WARN_ON(mm->avail != mm->size)` → `gpu_buddy_assert(mm->avail == mm->size)` — correct inversion, adds kunit reporting that was previously missing.
- **`__gpu_buddy_free_list`**: `WARN_ON(mark_dirty && mark_clear)` → `gpu_buddy_assert(!(mark_dirty && mark_clear))` — correct inversion, adds kunit reporting that was previously missing.
**Verdict:** The patch is straightforward, correct, and improves consistency. The only actionable suggestion is wrapping the `#else` path in `do { } while(0)` for macro hygiene symmetry. Otherwise:
Reviewed-by worthy with the minor `do { } while(0)` nit optionally addressed.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 5+ messages in thread* Claude review: gpu/buddy: Introduce gpu_buddy_assert() for kunit-aware assertions
2026-02-24 13:25 [PATCH] gpu/buddy: Introduce gpu_buddy_assert() for kunit-aware assertions Sanjay Yadav
2026-02-25 11:19 ` Arunpravin Paneer Selvam
2026-02-27 5:09 ` Claude review: " Claude Code Review Bot
@ 2026-02-27 5:09 ` Claude Code Review Bot
2 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-02-27 5:09 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: gpu/buddy: Introduce gpu_buddy_assert() for kunit-aware assertions
Author: Sanjay Yadav <sanjay.kumar.yadav@intel.com>
Patches: 3
Reviewed: 2026-02-27T15:09:14.390252
---
This is a single-patch series introducing a `gpu_buddy_assert()` macro to the gpu buddy allocator (a renamed/refactored version of `drm_buddy`). The macro unifies the existing ad-hoc pattern of `WARN_ON()` + optional `kunit_fail_current_test()` into a consistent helper. The patch is clean, well-motivated, and the commit message clearly explains the rationale including why `gpu_buddy_block_trim()`'s WARN_ON is left untouched.
The condition inversions at each call site are all correct, the `#include` restructuring is appropriate, and the macro avoids double-evaluation. No functional issues found.
A few minor observations below, but nothing blocking.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-02-27 5:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-24 13:25 [PATCH] gpu/buddy: Introduce gpu_buddy_assert() for kunit-aware assertions Sanjay Yadav
2026-02-25 11:19 ` Arunpravin Paneer Selvam
2026-02-25 13:20 ` Yadav, Sanjay Kumar
2026-02-27 5:09 ` Claude review: " Claude Code Review Bot
2026-02-27 5:09 ` 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