* [PATCH v2 1/2] drm/buddy: fix issue that force_merge cannot free all roots
@ 2024-12-26 7:01 Arunpravin Paneer Selvam
2024-12-26 7:01 ` [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini Arunpravin Paneer Selvam
0 siblings, 1 reply; 8+ messages in thread
From: Arunpravin Paneer Selvam @ 2024-12-26 7:01 UTC (permalink / raw)
To: dri-devel, amd-gfx, matthew.auld
Cc: christian.koenig, alexander.deucher, Arunpravin Paneer Selvam,
Lin . Cao
From: Lin.Cao <lincao12@amd.com>
If buddy manager have more than one roots and each root have sub-block
need to be free. When drm_buddy_fini called, the first loop of
force_merge will merge and free all of the sub block of first root,
which offset is 0x0 and size is biggest(more than have of the mm size).
In subsequent force_merge rounds, if we use 0 as start and use remaining
mm size as end, the block of other roots will be skipped in
__force_merge function. It will cause the other roots can not be freed.
Solution: use roots' offset as the start could fix this issue.
Signed-off-by: Lin.Cao <lincao12@amd.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/drm_buddy.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 103c185bb1c8..ca42e6081d27 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -324,7 +324,7 @@ EXPORT_SYMBOL(drm_buddy_init);
*/
void drm_buddy_fini(struct drm_buddy *mm)
{
- u64 root_size, size;
+ u64 root_size, size, start;
unsigned int order;
int i;
@@ -332,7 +332,8 @@ void drm_buddy_fini(struct drm_buddy *mm)
for (i = 0; i < mm->n_roots; ++i) {
order = ilog2(size) - ilog2(mm->chunk_size);
- __force_merge(mm, 0, size, order);
+ start = drm_buddy_block_offset(mm->roots[i]);
+ __force_merge(mm, start, start + size, order);
WARN_ON(!drm_buddy_block_is_free(mm->roots[i]));
drm_block_free(mm, mm->roots[i]);
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini
2024-12-26 7:01 [PATCH v2 1/2] drm/buddy: fix issue that force_merge cannot free all roots Arunpravin Paneer Selvam
@ 2024-12-26 7:01 ` Arunpravin Paneer Selvam
2025-01-06 9:51 ` Matthew Auld
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Arunpravin Paneer Selvam @ 2024-12-26 7:01 UTC (permalink / raw)
To: dri-devel, amd-gfx, matthew.auld
Cc: christian.koenig, alexander.deucher, Arunpravin Paneer Selvam,
Lin . Cao
- Added a testcase to verify the multiroot force merge fini.
- Added a new field in_use to track the mm freed status.
v2:(Matthew)
- Add kunit_fail_current_test() when WARN_ON is true.
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Signed-off-by: Lin.Cao <lincao12@amd.com>
---
drivers/gpu/drm/drm_buddy.c | 6 +++++-
drivers/gpu/drm/tests/drm_buddy_test.c | 29 ++++++++++++++++++--------
2 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index ca42e6081d27..241c855f891f 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -3,6 +3,8 @@
* Copyright © 2021 Intel Corporation
*/
+#include <kunit/test-bug.h>
+
#include <linux/kmemleak.h>
#include <linux/module.h>
#include <linux/sizes.h>
@@ -335,7 +337,9 @@ void drm_buddy_fini(struct drm_buddy *mm)
start = drm_buddy_block_offset(mm->roots[i]);
__force_merge(mm, start, start + size, order);
- WARN_ON(!drm_buddy_block_is_free(mm->roots[i]));
+ if (WARN_ON(!drm_buddy_block_is_free(mm->roots[i])))
+ kunit_fail_current_test("buddy_fini() root");
+
drm_block_free(mm, mm->roots[i]);
root_size = mm->chunk_size << order;
diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c
index 9662c949d0e3..4b5818f9f2a9 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -385,17 +385,28 @@ static void drm_test_buddy_alloc_clear(struct kunit *test)
drm_buddy_fini(&mm);
/*
- * Create a new mm with a non power-of-two size. Allocate a random size, free as
- * cleared and then call fini. This will ensure the multi-root force merge during
- * fini.
+ * Create a new mm with a non power-of-two size. Allocate a random size from each
+ * root, free as cleared and then call fini. This will ensure the multi-root
+ * force merge during fini.
*/
- mm_size = 12 * SZ_4K;
- size = max(round_up(prandom_u32_state(&prng) % mm_size, ps), ps);
+ mm_size = (SZ_4K << max_order) + (SZ_4K << (max_order - 2));
+
KUNIT_EXPECT_FALSE(test, drm_buddy_init(&mm, mm_size, ps));
- KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, mm_size,
- size, ps, &allocated,
- DRM_BUDDY_TOPDOWN_ALLOCATION),
- "buddy_alloc hit an error size=%u\n", size);
+ KUNIT_EXPECT_EQ(test, mm.max_order, max_order);
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, SZ_4K << max_order,
+ 4 * ps, ps, &allocated,
+ DRM_BUDDY_RANGE_ALLOCATION),
+ "buddy_alloc hit an error size=%lu\n", 4 * ps);
+ drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, SZ_4K << max_order,
+ 2 * ps, ps, &allocated,
+ DRM_BUDDY_CLEAR_ALLOCATION),
+ "buddy_alloc hit an error size=%lu\n", 2 * ps);
+ drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, SZ_4K << max_order, mm_size,
+ ps, ps, &allocated,
+ DRM_BUDDY_RANGE_ALLOCATION),
+ "buddy_alloc hit an error size=%lu\n", ps);
drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
drm_buddy_fini(&mm);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini
2024-12-26 7:01 ` [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini Arunpravin Paneer Selvam
@ 2025-01-06 9:51 ` Matthew Auld
2025-01-06 12:37 ` Paneer Selvam, Arunpravin
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Matthew Auld @ 2025-01-06 9:51 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, dri-devel, amd-gfx
Cc: christian.koenig, alexander.deucher, Lin . Cao
On 26/12/2024 07:01, Arunpravin Paneer Selvam wrote:
> - Added a testcase to verify the multiroot force merge fini.
> - Added a new field in_use to track the mm freed status.
I guess this is out of date now?
>
> v2:(Matthew)
> - Add kunit_fail_current_test() when WARN_ON is true.
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
> Signed-off-by: Lin.Cao <lincao12@amd.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini
2024-12-26 7:01 ` [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini Arunpravin Paneer Selvam
2025-01-06 9:51 ` Matthew Auld
@ 2025-01-06 12:37 ` Paneer Selvam, Arunpravin
2025-01-06 12:40 ` Matthew Auld
2025-01-15 11:38 ` Jani Nikula
2025-01-15 12:12 ` [v2,2/2] " Hellstrom, Thomas
3 siblings, 1 reply; 8+ messages in thread
From: Paneer Selvam, Arunpravin @ 2025-01-06 12:37 UTC (permalink / raw)
To: dri-devel, amd-gfx, matthew.auld
Cc: christian.koenig, alexander.deucher, Lin . Cao
Hi Matthew,
Ping?
Regards,
Arun.
On 12/26/2024 12:31 PM, Arunpravin Paneer Selvam wrote:
> - Added a testcase to verify the multiroot force merge fini.
> - Added a new field in_use to track the mm freed status.
>
> v2:(Matthew)
> - Add kunit_fail_current_test() when WARN_ON is true.
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
> Signed-off-by: Lin.Cao <lincao12@amd.com>
> ---
> drivers/gpu/drm/drm_buddy.c | 6 +++++-
> drivers/gpu/drm/tests/drm_buddy_test.c | 29 ++++++++++++++++++--------
> 2 files changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
> index ca42e6081d27..241c855f891f 100644
> --- a/drivers/gpu/drm/drm_buddy.c
> +++ b/drivers/gpu/drm/drm_buddy.c
> @@ -3,6 +3,8 @@
> * Copyright © 2021 Intel Corporation
> */
>
> +#include <kunit/test-bug.h>
> +
> #include <linux/kmemleak.h>
> #include <linux/module.h>
> #include <linux/sizes.h>
> @@ -335,7 +337,9 @@ void drm_buddy_fini(struct drm_buddy *mm)
> start = drm_buddy_block_offset(mm->roots[i]);
> __force_merge(mm, start, start + size, order);
>
> - WARN_ON(!drm_buddy_block_is_free(mm->roots[i]));
> + if (WARN_ON(!drm_buddy_block_is_free(mm->roots[i])))
> + kunit_fail_current_test("buddy_fini() root");
> +
> drm_block_free(mm, mm->roots[i]);
>
> root_size = mm->chunk_size << order;
> diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c
> index 9662c949d0e3..4b5818f9f2a9 100644
> --- a/drivers/gpu/drm/tests/drm_buddy_test.c
> +++ b/drivers/gpu/drm/tests/drm_buddy_test.c
> @@ -385,17 +385,28 @@ static void drm_test_buddy_alloc_clear(struct kunit *test)
> drm_buddy_fini(&mm);
>
> /*
> - * Create a new mm with a non power-of-two size. Allocate a random size, free as
> - * cleared and then call fini. This will ensure the multi-root force merge during
> - * fini.
> + * Create a new mm with a non power-of-two size. Allocate a random size from each
> + * root, free as cleared and then call fini. This will ensure the multi-root
> + * force merge during fini.
> */
> - mm_size = 12 * SZ_4K;
> - size = max(round_up(prandom_u32_state(&prng) % mm_size, ps), ps);
> + mm_size = (SZ_4K << max_order) + (SZ_4K << (max_order - 2));
> +
> KUNIT_EXPECT_FALSE(test, drm_buddy_init(&mm, mm_size, ps));
> - KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, mm_size,
> - size, ps, &allocated,
> - DRM_BUDDY_TOPDOWN_ALLOCATION),
> - "buddy_alloc hit an error size=%u\n", size);
> + KUNIT_EXPECT_EQ(test, mm.max_order, max_order);
> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, SZ_4K << max_order,
> + 4 * ps, ps, &allocated,
> + DRM_BUDDY_RANGE_ALLOCATION),
> + "buddy_alloc hit an error size=%lu\n", 4 * ps);
> + drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, SZ_4K << max_order,
> + 2 * ps, ps, &allocated,
> + DRM_BUDDY_CLEAR_ALLOCATION),
> + "buddy_alloc hit an error size=%lu\n", 2 * ps);
> + drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, SZ_4K << max_order, mm_size,
> + ps, ps, &allocated,
> + DRM_BUDDY_RANGE_ALLOCATION),
> + "buddy_alloc hit an error size=%lu\n", ps);
> drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
> drm_buddy_fini(&mm);
> }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini
2025-01-06 12:37 ` Paneer Selvam, Arunpravin
@ 2025-01-06 12:40 ` Matthew Auld
0 siblings, 0 replies; 8+ messages in thread
From: Matthew Auld @ 2025-01-06 12:40 UTC (permalink / raw)
To: Paneer Selvam, Arunpravin, dri-devel, amd-gfx
Cc: christian.koenig, alexander.deucher, Lin . Cao
Hey,
On 06/01/2025 12:37, Paneer Selvam, Arunpravin wrote:
> Hi Matthew,
>
> Ping?
Just got back from time off. I already slapped an r-b here:
https://lore.kernel.org/dri-devel/aa27455c-04b8-494c-a3b5-c4385a7748bc@intel.com/
>
> Regards,
> Arun.
>
> On 12/26/2024 12:31 PM, Arunpravin Paneer Selvam wrote:
>> - Added a testcase to verify the multiroot force merge fini.
>> - Added a new field in_use to track the mm freed status.
>>
>> v2:(Matthew)
>> - Add kunit_fail_current_test() when WARN_ON is true.
>>
>> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
>> Signed-off-by: Lin.Cao <lincao12@amd.com>
>> ---
>> drivers/gpu/drm/drm_buddy.c | 6 +++++-
>> drivers/gpu/drm/tests/drm_buddy_test.c | 29 ++++++++++++++++++--------
>> 2 files changed, 25 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
>> index ca42e6081d27..241c855f891f 100644
>> --- a/drivers/gpu/drm/drm_buddy.c
>> +++ b/drivers/gpu/drm/drm_buddy.c
>> @@ -3,6 +3,8 @@
>> * Copyright © 2021 Intel Corporation
>> */
>> +#include <kunit/test-bug.h>
>> +
>> #include <linux/kmemleak.h>
>> #include <linux/module.h>
>> #include <linux/sizes.h>
>> @@ -335,7 +337,9 @@ void drm_buddy_fini(struct drm_buddy *mm)
>> start = drm_buddy_block_offset(mm->roots[i]);
>> __force_merge(mm, start, start + size, order);
>> - WARN_ON(!drm_buddy_block_is_free(mm->roots[i]));
>> + if (WARN_ON(!drm_buddy_block_is_free(mm->roots[i])))
>> + kunit_fail_current_test("buddy_fini() root");
>> +
>> drm_block_free(mm, mm->roots[i]);
>> root_size = mm->chunk_size << order;
>> diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/
>> tests/drm_buddy_test.c
>> index 9662c949d0e3..4b5818f9f2a9 100644
>> --- a/drivers/gpu/drm/tests/drm_buddy_test.c
>> +++ b/drivers/gpu/drm/tests/drm_buddy_test.c
>> @@ -385,17 +385,28 @@ static void drm_test_buddy_alloc_clear(struct
>> kunit *test)
>> drm_buddy_fini(&mm);
>> /*
>> - * Create a new mm with a non power-of-two size. Allocate a
>> random size, free as
>> - * cleared and then call fini. This will ensure the multi-root
>> force merge during
>> - * fini.
>> + * Create a new mm with a non power-of-two size. Allocate a
>> random size from each
>> + * root, free as cleared and then call fini. This will ensure the
>> multi-root
>> + * force merge during fini.
>> */
>> - mm_size = 12 * SZ_4K;
>> - size = max(round_up(prandom_u32_state(&prng) % mm_size, ps), ps);
>> + mm_size = (SZ_4K << max_order) + (SZ_4K << (max_order - 2));
>> +
>> KUNIT_EXPECT_FALSE(test, drm_buddy_init(&mm, mm_size, ps));
>> - KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, mm_size,
>> - size, ps, &allocated,
>> - DRM_BUDDY_TOPDOWN_ALLOCATION),
>> - "buddy_alloc hit an error size=%u\n", size);
>> + KUNIT_EXPECT_EQ(test, mm.max_order, max_order);
>> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, SZ_4K
>> << max_order,
>> + 4 * ps, ps, &allocated,
>> + DRM_BUDDY_RANGE_ALLOCATION),
>> + "buddy_alloc hit an error size=%lu\n", 4 * ps);
>> + drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
>> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, SZ_4K
>> << max_order,
>> + 2 * ps, ps, &allocated,
>> + DRM_BUDDY_CLEAR_ALLOCATION),
>> + "buddy_alloc hit an error size=%lu\n", 2 * ps);
>> + drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
>> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, SZ_4K <<
>> max_order, mm_size,
>> + ps, ps, &allocated,
>> + DRM_BUDDY_RANGE_ALLOCATION),
>> + "buddy_alloc hit an error size=%lu\n", ps);
>> drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
>> drm_buddy_fini(&mm);
>> }
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini
2024-12-26 7:01 ` [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini Arunpravin Paneer Selvam
2025-01-06 9:51 ` Matthew Auld
2025-01-06 12:37 ` Paneer Selvam, Arunpravin
@ 2025-01-15 11:38 ` Jani Nikula
2025-01-15 13:53 ` Paneer Selvam, Arunpravin
2025-01-15 12:12 ` [v2,2/2] " Hellstrom, Thomas
3 siblings, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2025-01-15 11:38 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, dri-devel, amd-gfx, matthew.auld
Cc: christian.koenig, alexander.deucher, Arunpravin Paneer Selvam,
Lin . Cao
On Thu, 26 Dec 2024, Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com> wrote:
> - Added a testcase to verify the multiroot force merge fini.
> - Added a new field in_use to track the mm freed status.
>
> v2:(Matthew)
> - Add kunit_fail_current_test() when WARN_ON is true.
This i.e. commit 8cb3a1e2b350 ("drm/buddy: Add a testcase to verify the
multiroot fini") fails drm-tip build for me with:
In file included from ../drivers/gpu/drm/tests/drm_buddy_test.c:15:
../drivers/gpu/drm/tests/drm_buddy_test.c: In function ‘drm_test_buddy_alloc_clear’:
../drivers/gpu/drm/tests/drm_buddy_test.c:264:23: error: unused variable ‘prng’ [-Werror=unused-variable]
264 | DRM_RND_STATE(prng, random_seed);
| ^~~~
../drivers/gpu/drm/tests/../lib/drm_random.h:18:26: note: in definition of macro ‘DRM_RND_STATE’
18 | struct rnd_state name__ = DRM_RND_STATE_INITIALIZER(seed__)
| ^~~~~~
cc1: all warnings being treated as errors
BR,
Jani.
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
> Signed-off-by: Lin.Cao <lincao12@amd.com>
> ---
> drivers/gpu/drm/drm_buddy.c | 6 +++++-
> drivers/gpu/drm/tests/drm_buddy_test.c | 29 ++++++++++++++++++--------
> 2 files changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
> index ca42e6081d27..241c855f891f 100644
> --- a/drivers/gpu/drm/drm_buddy.c
> +++ b/drivers/gpu/drm/drm_buddy.c
> @@ -3,6 +3,8 @@
> * Copyright © 2021 Intel Corporation
> */
>
> +#include <kunit/test-bug.h>
> +
> #include <linux/kmemleak.h>
> #include <linux/module.h>
> #include <linux/sizes.h>
> @@ -335,7 +337,9 @@ void drm_buddy_fini(struct drm_buddy *mm)
> start = drm_buddy_block_offset(mm->roots[i]);
> __force_merge(mm, start, start + size, order);
>
> - WARN_ON(!drm_buddy_block_is_free(mm->roots[i]));
> + if (WARN_ON(!drm_buddy_block_is_free(mm->roots[i])))
> + kunit_fail_current_test("buddy_fini() root");
> +
> drm_block_free(mm, mm->roots[i]);
>
> root_size = mm->chunk_size << order;
> diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c
> index 9662c949d0e3..4b5818f9f2a9 100644
> --- a/drivers/gpu/drm/tests/drm_buddy_test.c
> +++ b/drivers/gpu/drm/tests/drm_buddy_test.c
> @@ -385,17 +385,28 @@ static void drm_test_buddy_alloc_clear(struct kunit *test)
> drm_buddy_fini(&mm);
>
> /*
> - * Create a new mm with a non power-of-two size. Allocate a random size, free as
> - * cleared and then call fini. This will ensure the multi-root force merge during
> - * fini.
> + * Create a new mm with a non power-of-two size. Allocate a random size from each
> + * root, free as cleared and then call fini. This will ensure the multi-root
> + * force merge during fini.
> */
> - mm_size = 12 * SZ_4K;
> - size = max(round_up(prandom_u32_state(&prng) % mm_size, ps), ps);
> + mm_size = (SZ_4K << max_order) + (SZ_4K << (max_order - 2));
> +
> KUNIT_EXPECT_FALSE(test, drm_buddy_init(&mm, mm_size, ps));
> - KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, mm_size,
> - size, ps, &allocated,
> - DRM_BUDDY_TOPDOWN_ALLOCATION),
> - "buddy_alloc hit an error size=%u\n", size);
> + KUNIT_EXPECT_EQ(test, mm.max_order, max_order);
> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, SZ_4K << max_order,
> + 4 * ps, ps, &allocated,
> + DRM_BUDDY_RANGE_ALLOCATION),
> + "buddy_alloc hit an error size=%lu\n", 4 * ps);
> + drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, SZ_4K << max_order,
> + 2 * ps, ps, &allocated,
> + DRM_BUDDY_CLEAR_ALLOCATION),
> + "buddy_alloc hit an error size=%lu\n", 2 * ps);
> + drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, SZ_4K << max_order, mm_size,
> + ps, ps, &allocated,
> + DRM_BUDDY_RANGE_ALLOCATION),
> + "buddy_alloc hit an error size=%lu\n", ps);
> drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
> drm_buddy_fini(&mm);
> }
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [v2,2/2] drm/buddy: Add a testcase to verify the multiroot fini
2024-12-26 7:01 ` [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini Arunpravin Paneer Selvam
` (2 preceding siblings ...)
2025-01-15 11:38 ` Jani Nikula
@ 2025-01-15 12:12 ` Hellstrom, Thomas
3 siblings, 0 replies; 8+ messages in thread
From: Hellstrom, Thomas @ 2025-01-15 12:12 UTC (permalink / raw)
To: dri-devel@lists.freedesktop.org, Arunpravin.PaneerSelvam@amd.com,
amd-gfx@lists.freedesktop.org, Auld, Matthew
Cc: alexander.deucher@amd.com, christian.koenig@amd.com,
lincao12@amd.com
Hi!
On Thu, 2024-12-26 at 12:31 +0530, Arunpravin Paneer Selvam wrote:
> - Added a testcase to verify the multiroot force merge fini.
> - Added a new field in_use to track the mm freed status.
>
> v2:(Matthew)
> - Add kunit_fail_current_test() when WARN_ON is true.
>
> Signed-off-by: Arunpravin Paneer Selvam
> <Arunpravin.PaneerSelvam@amd.com>
> Signed-off-by: Lin.Cao <lincao12@amd.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> ---
> drivers/gpu/drm/drm_buddy.c | 6 +++++-
> drivers/gpu/drm/tests/drm_buddy_test.c | 29 ++++++++++++++++++------
> --
> 2 files changed, 25 insertions(+), 10 deletions(-)
It appears this patch breaks drm-tip:
drivers/gpu/drm/tests/drm_buddy_test.c: In function
‘drm_test_buddy_alloc_clear’:
drivers/gpu/drm/tests/drm_buddy_test.c:264:23: error: unused variable
‘prng’ [-Werror=unused-variable]
264 | DRM_RND_STATE(prng, random_seed);
| ^~~~
drivers/gpu/drm/tests/../lib/drm_random.h:18:26: note: in definition of
macro ‘DRM_RND_STATE’
18 | struct rnd_state name__ =
DRM_RND_STATE_INITIALIZER(seed__)
| ^~~~~~
Thanks,
Thomas
>
> diff --git a/drivers/gpu/drm/drm_buddy.c
> b/drivers/gpu/drm/drm_buddy.c
> index ca42e6081d27..241c855f891f 100644
> --- a/drivers/gpu/drm/drm_buddy.c
> +++ b/drivers/gpu/drm/drm_buddy.c
> @@ -3,6 +3,8 @@
> * Copyright © 2021 Intel Corporation
> */
>
> +#include <kunit/test-bug.h>
> +
> #include <linux/kmemleak.h>
> #include <linux/module.h>
> #include <linux/sizes.h>
> @@ -335,7 +337,9 @@ void drm_buddy_fini(struct drm_buddy *mm)
> start = drm_buddy_block_offset(mm->roots[i]);
> __force_merge(mm, start, start + size, order);
>
> - WARN_ON(!drm_buddy_block_is_free(mm->roots[i]));
> + if (WARN_ON(!drm_buddy_block_is_free(mm->roots[i])))
> + kunit_fail_current_test("buddy_fini()
> root");
> +
> drm_block_free(mm, mm->roots[i]);
>
> root_size = mm->chunk_size << order;
> diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c
> b/drivers/gpu/drm/tests/drm_buddy_test.c
> index 9662c949d0e3..4b5818f9f2a9 100644
> --- a/drivers/gpu/drm/tests/drm_buddy_test.c
> +++ b/drivers/gpu/drm/tests/drm_buddy_test.c
> @@ -385,17 +385,28 @@ static void drm_test_buddy_alloc_clear(struct
> kunit *test)
> drm_buddy_fini(&mm);
>
> /*
> - * Create a new mm with a non power-of-two size. Allocate a
> random size, free as
> - * cleared and then call fini. This will ensure the multi-
> root force merge during
> - * fini.
> + * Create a new mm with a non power-of-two size. Allocate a
> random size from each
> + * root, free as cleared and then call fini. This will
> ensure the multi-root
> + * force merge during fini.
> */
> - mm_size = 12 * SZ_4K;
> - size = max(round_up(prandom_u32_state(&prng) % mm_size, ps),
> ps);
> + mm_size = (SZ_4K << max_order) + (SZ_4K << (max_order - 2));
> +
> KUNIT_EXPECT_FALSE(test, drm_buddy_init(&mm, mm_size, ps));
> - KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0,
> mm_size,
> - size,
> ps, &allocated,
> -
> DRM_BUDDY_TOPDOWN_ALLOCATION),
> - "buddy_alloc hit an error
> size=%u\n", size);
> + KUNIT_EXPECT_EQ(test, mm.max_order, max_order);
> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0,
> SZ_4K << max_order,
> + 4 * ps,
> ps, &allocated,
> +
> DRM_BUDDY_RANGE_ALLOCATION),
> + "buddy_alloc hit an error
> size=%lu\n", 4 * ps);
> + drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0,
> SZ_4K << max_order,
> + 2 * ps,
> ps, &allocated,
> +
> DRM_BUDDY_CLEAR_ALLOCATION),
> + "buddy_alloc hit an error
> size=%lu\n", 2 * ps);
> + drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm,
> SZ_4K << max_order, mm_size,
> + ps, ps,
> &allocated,
> +
> DRM_BUDDY_RANGE_ALLOCATION),
> + "buddy_alloc hit an error
> size=%lu\n", ps);
> drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
> drm_buddy_fini(&mm);
> }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini
2025-01-15 11:38 ` Jani Nikula
@ 2025-01-15 13:53 ` Paneer Selvam, Arunpravin
0 siblings, 0 replies; 8+ messages in thread
From: Paneer Selvam, Arunpravin @ 2025-01-15 13:53 UTC (permalink / raw)
To: Jani Nikula, dri-devel, amd-gfx, matthew.auld
Cc: christian.koenig, alexander.deucher, Lin . Cao
Hi Jani,
I merged the below patch into drm-misc-next, Please try rebuilding the
drm-tip.
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=00728273bdf1001f8d2f7b65bc398000d7defe0b
Thanks,
Arun.
On 1/15/2025 5:08 PM, Jani Nikula wrote:
> On Thu, 26 Dec 2024, Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com> wrote:
>> - Added a testcase to verify the multiroot force merge fini.
>> - Added a new field in_use to track the mm freed status.
>>
>> v2:(Matthew)
>> - Add kunit_fail_current_test() when WARN_ON is true.
> This i.e. commit 8cb3a1e2b350 ("drm/buddy: Add a testcase to verify the
> multiroot fini") fails drm-tip build for me with:
>
> In file included from ../drivers/gpu/drm/tests/drm_buddy_test.c:15:
> ../drivers/gpu/drm/tests/drm_buddy_test.c: In function ‘drm_test_buddy_alloc_clear’:
> ../drivers/gpu/drm/tests/drm_buddy_test.c:264:23: error: unused variable ‘prng’ [-Werror=unused-variable]
> 264 | DRM_RND_STATE(prng, random_seed);
> | ^~~~
> ../drivers/gpu/drm/tests/../lib/drm_random.h:18:26: note: in definition of macro ‘DRM_RND_STATE’
> 18 | struct rnd_state name__ = DRM_RND_STATE_INITIALIZER(seed__)
> | ^~~~~~
> cc1: all warnings being treated as errors
>
>
> BR,
> Jani.
>
>
>> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
>> Signed-off-by: Lin.Cao <lincao12@amd.com>
>> ---
>> drivers/gpu/drm/drm_buddy.c | 6 +++++-
>> drivers/gpu/drm/tests/drm_buddy_test.c | 29 ++++++++++++++++++--------
>> 2 files changed, 25 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
>> index ca42e6081d27..241c855f891f 100644
>> --- a/drivers/gpu/drm/drm_buddy.c
>> +++ b/drivers/gpu/drm/drm_buddy.c
>> @@ -3,6 +3,8 @@
>> * Copyright © 2021 Intel Corporation
>> */
>>
>> +#include <kunit/test-bug.h>
>> +
>> #include <linux/kmemleak.h>
>> #include <linux/module.h>
>> #include <linux/sizes.h>
>> @@ -335,7 +337,9 @@ void drm_buddy_fini(struct drm_buddy *mm)
>> start = drm_buddy_block_offset(mm->roots[i]);
>> __force_merge(mm, start, start + size, order);
>>
>> - WARN_ON(!drm_buddy_block_is_free(mm->roots[i]));
>> + if (WARN_ON(!drm_buddy_block_is_free(mm->roots[i])))
>> + kunit_fail_current_test("buddy_fini() root");
>> +
>> drm_block_free(mm, mm->roots[i]);
>>
>> root_size = mm->chunk_size << order;
>> diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c
>> index 9662c949d0e3..4b5818f9f2a9 100644
>> --- a/drivers/gpu/drm/tests/drm_buddy_test.c
>> +++ b/drivers/gpu/drm/tests/drm_buddy_test.c
>> @@ -385,17 +385,28 @@ static void drm_test_buddy_alloc_clear(struct kunit *test)
>> drm_buddy_fini(&mm);
>>
>> /*
>> - * Create a new mm with a non power-of-two size. Allocate a random size, free as
>> - * cleared and then call fini. This will ensure the multi-root force merge during
>> - * fini.
>> + * Create a new mm with a non power-of-two size. Allocate a random size from each
>> + * root, free as cleared and then call fini. This will ensure the multi-root
>> + * force merge during fini.
>> */
>> - mm_size = 12 * SZ_4K;
>> - size = max(round_up(prandom_u32_state(&prng) % mm_size, ps), ps);
>> + mm_size = (SZ_4K << max_order) + (SZ_4K << (max_order - 2));
>> +
>> KUNIT_EXPECT_FALSE(test, drm_buddy_init(&mm, mm_size, ps));
>> - KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, mm_size,
>> - size, ps, &allocated,
>> - DRM_BUDDY_TOPDOWN_ALLOCATION),
>> - "buddy_alloc hit an error size=%u\n", size);
>> + KUNIT_EXPECT_EQ(test, mm.max_order, max_order);
>> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, SZ_4K << max_order,
>> + 4 * ps, ps, &allocated,
>> + DRM_BUDDY_RANGE_ALLOCATION),
>> + "buddy_alloc hit an error size=%lu\n", 4 * ps);
>> + drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
>> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, SZ_4K << max_order,
>> + 2 * ps, ps, &allocated,
>> + DRM_BUDDY_CLEAR_ALLOCATION),
>> + "buddy_alloc hit an error size=%lu\n", 2 * ps);
>> + drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
>> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, SZ_4K << max_order, mm_size,
>> + ps, ps, &allocated,
>> + DRM_BUDDY_RANGE_ALLOCATION),
>> + "buddy_alloc hit an error size=%lu\n", ps);
>> drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
>> drm_buddy_fini(&mm);
>> }
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-01-15 14:00 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-26 7:01 [PATCH v2 1/2] drm/buddy: fix issue that force_merge cannot free all roots Arunpravin Paneer Selvam
2024-12-26 7:01 ` [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini Arunpravin Paneer Selvam
2025-01-06 9:51 ` Matthew Auld
2025-01-06 12:37 ` Paneer Selvam, Arunpravin
2025-01-06 12:40 ` Matthew Auld
2025-01-15 11:38 ` Jani Nikula
2025-01-15 13:53 ` Paneer Selvam, Arunpravin
2025-01-15 12:12 ` [v2,2/2] " Hellstrom, Thomas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.