Re: [Intel-gfx] [PATCH v2] drm: add a check to verify the size alignment

["Christian König" <christian.koenig@amd.com>]

Am 29.03.22 um 13:28 schrieb Arunpravin Paneer Selvam:
> On 23/03/22 1:15 pm, Christian König wrote:
>> Am 23.03.22 um 08:34 schrieb Arunpravin Paneer Selvam:
>>> Add a simple check to reject any size not aligned to the
>>> min_page_size.
>>>
>>> handle instances when size is not aligned with the min_page_size.
>>> Unigine Heaven has allocation requests for example required pages
>>> are 257 and alignment request is 256. To allocate the left over 1
>>> page, continues the iteration to find the order value which is 0
>>> and when it compares with min_order = 8, triggers the BUG_ON(order
>>> < min_order). To avoid this problem, we added a simple check to
>>> return -EINVAL if size is not aligned to the min_page_size.
>>>
>>> v2: Added more details to the commit description
>>>
>>> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
>>> Suggested-by: Matthew Auld <matthew.auld@intel.com>
>>> ---
>>>    drivers/gpu/drm/drm_buddy.c | 3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
>>> index 72f52f293249..b503c88786b0 100644
>>> --- a/drivers/gpu/drm/drm_buddy.c
>>> +++ b/drivers/gpu/drm/drm_buddy.c
>>> @@ -661,6 +661,9 @@ int drm_buddy_alloc_blocks(struct drm_buddy *mm,
>>>    	if (range_overflows(start, size, mm->size))
>>>    		return -EINVAL;
>>>    
>>> +	if (WARN_ON(!IS_ALIGNED(size, min_page_size)))
>>> +		return -EINVAL;
>>> +
>> I'm not that happy with the handling here.
>>
>> See a minimum page size larger than the requested size is perfectly
>> valid, it just means that the remaining pages needs to be trimmed.
>>
>> In other words when the request is to allocate 1 page with an alignment
>> of 256 we just need to give the remaining 255 pages back to the allocator.
> In one of the previous mail Matthew explained that i915 expects to
> return -EINVAL error code if size is not aligned to min_page_size.
>
> can we just modify in amdgpu code where we round_up the size to the
> min_page_size value and keep this error handling in drm_buddy.c?

Yeah, I'm fine with that as well now.

I realized that this is probably the easiest option to check if an 
allocation is contiguous or not between the alloc and trim.

So having two functions for that sounds like a good idea to me.

Thanks,
Christian.

>> Regards,
>> Christian.
>>
>>>    	/* Actual range allocation */
>>>    	if (start + size == end)
>>>    		return __drm_buddy_alloc_range(mm, start, size, blocks);
>>>
>>> base-commit: 056d47eaf6ea753fa2e21da31f9cbd8b721bbb7b