[PATCH v2 1/2] drm/buddy: fix issue that force_merge cannot free all roots

[PATCH v2 1/2] drm/buddy: fix issue that force_merge cannot free all roots

[Arunpravin Paneer Selvam]

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

[PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini

[Arunpravin Paneer Selvam]

- 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

Re: [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini

[Matthew Auld]

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>

Re: [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini

[Paneer Selvam, Arunpravin]

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);
>   }

Re: [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini

[Matthew Auld]

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);
>>   }
> 

Re: [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini

[Jani Nikula]

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

Re: [PATCH v2 2/2] drm/buddy: Add a testcase to verify the multiroot fini

[Paneer Selvam, Arunpravin]

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);
>>   }

Re: [v2,2/2] drm/buddy: Add a testcase to verify the multiroot fini

[Hellstrom, Thomas]

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);
>  }