Re: [PATCH v2] drm: Fix a infinite loop condition when order becomes 0

[Arunpravin Paneer Selvam <arunpravin.paneerselvam@xxxxxxx>]

On 16/03/22 6:02 pm, Christian König wrote:
> Am 16.03.22 um 12:31 schrieb Matthew Auld:
>> On 16/03/2022 06:34, Arunpravin Paneer Selvam wrote:
>>> handle a situation in the condition order-- == min_order,
>>> when order = 0 and min_order = 0, leading to order = -1,
>>> it now won't exit the loop. To avoid this problem,
>>> added a order check in the same condition, (i.e)
>>> when order is 0, we return -ENOSPC
>>>
>>> v2: use full name in email program and in Signed-off tag
>>>
>>> Signed-off-by: Arunpravin Paneer Selvam 
>>> <Arunpravin.PaneerSelvam@xxxxxxx>
>>> ---
>>>   drivers/gpu/drm/drm_buddy.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
>>> index 72f52f293249..5ab66aaf2bbd 100644
>>> --- a/drivers/gpu/drm/drm_buddy.c
>>> +++ b/drivers/gpu/drm/drm_buddy.c
>>> @@ -685,7 +685,7 @@ int drm_buddy_alloc_blocks(struct drm_buddy *mm,
>>>               if (!IS_ERR(block))
>>>                   break;
>>>   -            if (order-- == min_order) {
>>> +            if (!order || order-- == min_order) {
>>
>> It shouldn't be possible to enter an infinite loop here, without first 
>> tripping up the BUG_ON(order < min_order) further up, and for that, as 
>> we discussed here[1], it sounded like the conclusion was to rather add 
>> a simple check somewhere in drm_buddy_alloc_blocks() to reject any 
>> size not aligned to the min_page_size?
> 
> Sounds good to me as well.
> 
> And just to make it clear: I don't review the functionality here, that's 
> up to you guys.
> 
> Just giving my Ack on things like style and pushing the end result 
> upstream as necessary.
> 
> So let me know when you have settled on a solution.

I sent a drm buddy patch adding a simple check to verify the size
aligned to the min_page_size.

In amdgpu part patch, I have a check if size is not aligned to the
min_page_size, we are enabling the is_contiguous flag, therefore the
size value round up to the power of two and allocated, Later, the
allocated block trimmed to the original requested size.

> 
> Regards,
> Christian.
> 
>>
>> [1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F477414%2F%3Fseries%3D101108%26rev%3D1&amp;data=04%7C01%7CArunpravin.PaneerSelvam%40amd.com%7Cf03d93b31c7d47b1389c08da074915cf%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637830307722869478%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=i3Pw3Go9Niu20Z4CF0IiWKPgTYTBPyK0SJYVq02fx0A%3D&amp;reserved=0
>>
>>>                   err = -ENOSPC;
>>>                   goto err_free;
>>>               }
>>>
>>> base-commit: 3bd60c0259406c5ca3ce5cdc958fb910ad4b8175
>