[PATCH v2 2/2] drm/tests: Add a unit test for range bias allocation

[Matthew Auld]

On 13/05/2024 16:11, Paneer Selvam, Arunpravin wrote:
> Hi Matthew,
> 
> On 5/13/2024 1:49 PM, Matthew Auld wrote:
>> On 12/05/2024 08:59, Arunpravin Paneer Selvam wrote:
>>> Allocate cleared blocks in the bias range when the DRM
>>> buddy's clear avail is zero. This will validate the bias
>>> range allocation in scenarios like system boot when no
>>> cleared blocks are available and exercise the fallback
>>> path too. The resulting blocks should always be dirty.
>>>
>>> Signed-off-by: Arunpravin Paneer Selvam 
>>> <Arunpravin.PaneerSelvam at amd.com>
>>> ---
>>>   drivers/gpu/drm/tests/drm_buddy_test.c | 35 ++++++++++++++++++++++++++
>>>   1 file changed, 35 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c 
>>> b/drivers/gpu/drm/tests/drm_buddy_test.c
>>> index e3b50e240d36..a194f271bc55 100644
>>> --- a/drivers/gpu/drm/tests/drm_buddy_test.c
>>> +++ b/drivers/gpu/drm/tests/drm_buddy_test.c
>>> @@ -26,6 +26,8 @@ static void drm_test_buddy_alloc_range_bias(struct 
>>> kunit *test)
>>>       u32 mm_size, ps, bias_size, bias_start, bias_end, bias_rem;
>>>       DRM_RND_STATE(prng, random_seed);
>>>       unsigned int i, count, *order;
>>> +    struct drm_buddy_block *block;
>>> +    unsigned long flags;
>>>       struct drm_buddy mm;
>>>       LIST_HEAD(allocated);
>>>   @@ -222,6 +224,39 @@ static void 
>>> drm_test_buddy_alloc_range_bias(struct kunit *test)
>>>         drm_buddy_free_list(&mm, &allocated, 0);
>>>       drm_buddy_fini(&mm);
>>> +
>>> +    /*
>>> +     * Allocate cleared blocks in the bias range when the DRM 
>>> buddy's clear avail is
>>> +     * zero. This will validate the bias range allocation in 
>>> scenarios like system boot
>>> +     * when no cleared blocks are available and exercise the 
>>> fallback path too. The resulting
>>> +     * blocks should always be dirty.
>>> +     */
>>> +
>>> +    KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_init(&mm, mm_size, ps),
>>> +                   "buddy_init failed\n");
>>> +    mm.clear_avail = 0;
>>
>> Should already be zero, right? Maybe make this an assert instead?
> No, since the mm declared as a local variable in the test case, 
> mm.clear_avail is not zero.

That sounds like a bug IMO. The init() should initialize it, like it 
does for mm.avail and everything else.

>>
>>> +
>>> +    bias_start = round_up(prandom_u32_state(&prng) % (mm_size - ps), 
>>> ps);
>>> +    bias_end = round_up(bias_start + prandom_u32_state(&prng) % 
>>> (mm_size - bias_start), ps);
>>> +    bias_end = max(bias_end, bias_start + ps);
>>> +    bias_rem = bias_end - bias_start;
>>> +
>>> +    flags = DRM_BUDDY_CLEAR_ALLOCATION | DRM_BUDDY_RANGE_ALLOCATION;
>>> +    u32 size = max(round_up(prandom_u32_state(&prng) % bias_rem, 
>>> ps), ps);
>>
>> u32 declaration should be moved to above?
> Sure.
> 
> Thanks,
> Arun.
>>
>> Otherwise,
>> Reviewed-by: Matthew Auld <matthew.auld at intel.com>
>>
>>> +
>>> +    KUNIT_ASSERT_FALSE_MSG(test,
>>> +                   drm_buddy_alloc_blocks(&mm, bias_start,
>>> +                              bias_end, size, ps,
>>> +                              &allocated,
>>> +                              flags),
>>> +                   "buddy_alloc failed with bias(%x-%x), size=%u, 
>>> ps=%u\n",
>>> +                   bias_start, bias_end, size, ps);
>>> +
>>> +    list_for_each_entry(block, &allocated, link)
>>> +        KUNIT_EXPECT_EQ(test, drm_buddy_block_is_clear(block), false);
>>> +
>>> +    drm_buddy_free_list(&mm, &allocated, 0);
>>> +    drm_buddy_fini(&mm);
>>>   }
>>>     static void drm_test_buddy_alloc_clear(struct kunit *test)
>