Re: [Intel-gfx] [PATCH 2/7] drm/selftests: add drm buddy alloc limit testcase

[Arunpravin <arunpravin.paneerselvam@amd.com>]

On 08/02/22 3:10 pm, Matthew Auld wrote:
> On 03/02/2022 13:32, Arunpravin wrote:
>> add a test to check the maximum allocation limit
>>
>> Signed-off-by: Arunpravin <Arunpravin.PaneerSelvam@amd.com>
>> ---
>>   .../gpu/drm/selftests/drm_buddy_selftests.h   |  1 +
>>   drivers/gpu/drm/selftests/test-drm_buddy.c    | 60 +++++++++++++++++++
>>   2 files changed, 61 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/selftests/drm_buddy_selftests.h b/drivers/gpu/drm/selftests/drm_buddy_selftests.h
>> index a4bcf3a6dfe3..ebe16162762f 100644
>> --- a/drivers/gpu/drm/selftests/drm_buddy_selftests.h
>> +++ b/drivers/gpu/drm/selftests/drm_buddy_selftests.h
>> @@ -7,3 +7,4 @@
>>    * Tests are executed in order by igt/drm_buddy
>>    */
>>   selftest(sanitycheck, igt_sanitycheck) /* keep first (selfcheck for igt) */
>> +selftest(buddy_alloc_limit, igt_buddy_alloc_limit)
>> diff --git a/drivers/gpu/drm/selftests/test-drm_buddy.c b/drivers/gpu/drm/selftests/test-drm_buddy.c
>> index 51e4d393d22c..fd7d1a112458 100644
>> --- a/drivers/gpu/drm/selftests/test-drm_buddy.c
>> +++ b/drivers/gpu/drm/selftests/test-drm_buddy.c
>> @@ -16,6 +16,66 @@
>>   
>>   static unsigned int random_seed;
>>   
>> +static int igt_buddy_alloc_limit(void *arg)
>> +{
>> +	u64 end, size = U64_MAX, start = 0;
>> +	struct drm_buddy_block *block;
>> +	unsigned long flags = 0;
>> +	LIST_HEAD(allocated);
>> +	struct drm_buddy mm;
>> +	int err;
>> +
>> +	size = end = round_down(size, 4096);
>> +	err = drm_buddy_init(&mm, size, PAGE_SIZE);
>> +	if (err)
>> +		return err;
>> +
>> +	if (mm.max_order != DRM_BUDDY_MAX_ORDER) {
>> +		pr_err("mm.max_order(%d) != %d\n",
>> +		       mm.max_order, DRM_BUDDY_MAX_ORDER);
>> +		err = -EINVAL;
>> +		goto out_fini;
>> +	}
>> +
>> +	err = drm_buddy_alloc_blocks(&mm, start, end, size,
>> +				     PAGE_SIZE, &allocated, flags);
>> +
>> +	if (unlikely(err))
>> +		goto out_free;
>> +
>> +	block = list_first_entry_or_null(&allocated,
>> +					 struct drm_buddy_block,
>> +					 link);
>> +
>> +	if (!block)
> 
> err = -EINVAL;
> 
>> +		goto out_fini;
>> +
>> +	if (drm_buddy_block_order(block) != mm.max_order) {
>> +		pr_err("block order(%d) != %d\n",
>> +		       drm_buddy_block_order(block), mm.max_order);
>> +		err = -EINVAL;
>> +		goto out_free;
>> +	}
>> +
>> +	if (drm_buddy_block_size(&mm, block) !=
>> +	    BIT_ULL(mm.max_order) * PAGE_SIZE) {
>> +		pr_err("block size(%llu) != %llu\n",
>> +		       drm_buddy_block_size(&mm, block),
>> +		       BIT_ULL(mm.max_order) * PAGE_SIZE);
>> +		err = -EINVAL;
>> +		goto out_free;
>> +	}
>> +
>> +	if (!err)
> 
> Always true AFAICT?
> 
>> +		pr_info("%s - succeeded\n", __func__);
> 
> I guess this could be made part of the run_selftests()? It looks like it 
> already prints the current test, perhaps that is already enough?

removed all unnecessary succeeded prints

Thanks,
Arun
> 
> With the err = -EINVAL change, feel free to add,
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> 
>> +
>> +out_free:
>> +	drm_buddy_free_list(&mm, &allocated);
>> +out_fini:
>> +	drm_buddy_fini(&mm);
>> +	return err;
>> +}
>> +
>>   static int igt_sanitycheck(void *ignored)
>>   {
>>   	pr_info("%s - ok!\n", __func__);