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

[Arunpravin]

On 15/03/22 2:35 pm, Paul Menzel wrote:
> Dear Arunpravin,
> 
> 
> Am 15.03.22 um 10:01 schrieb Arunpravin:
> 
>> On 15/03/22 1:49 pm, Paul Menzel wrote:
> 
>>> Am 14.03.22 um 20:40 schrieb Arunpravin:
>>>> handle a situation in the condition order-- == min_order,
>>>> when 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
>>>>
>>>> Signed-off-by: Arunpravin <arunpravin.paneersel...@amd.com>
>>>
>>> Please use your full name.
>> okay
> 
> You might also configure that in your email program.
yes
> 
>>>> ---
>>>>    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
>>>
>>> In what tree is that file?
>>>
>> drm-tip - 
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freedesktop.org%2Fdrm-tip%2Ftree%2F&amp;data=04%7C01%7CArunpravin.PaneerSelvam%40amd.com%7Cc456573102c04191cf9708da0662f798%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637829319396954551%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=5Bspe5QGjQ0KHfVI8%2F%2BXqxR45q6tOL4FE2fVD3uwL%2FM%3D&amp;reserved=0
>> drm-misc-next - 
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freedesktop.org%2Fdrm%2Fdrm-misc%2Ftree%2F&amp;data=04%7C01%7CArunpravin.PaneerSelvam%40amd.com%7Cc456573102c04191cf9708da0662f798%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637829319396954551%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=g2S14TfsHF5ORo9jTZ3uA0l1BH8mnAxk2OWYJeF5i8k%3D&amp;reserved=0
>>
>>>> @@ -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) {
>>>>                                    err = -ENOSPC;
>>>>                                    goto err_free;
>>>>                            }
> 
> Thank you for the hint. So the whole function is:
> 
>       do {
>               order = min(order, (unsigned int)fls(pages) - 1);
>               BUG_ON(order > mm->max_order);
>               BUG_ON(order < min_order);
> 
>               do {
>                       if (flags & DRM_BUDDY_RANGE_ALLOCATION)
>                               /* Allocate traversing within the range */
>                               block = alloc_range_bias(mm, start, end, order);
>                       else
>                               /* Allocate from freelist */
>                               block = alloc_from_freelist(mm, order, flags);
> 
>                       if (!IS_ERR(block))
>                               break;
> 
>                       if (order-- == min_order) {
>                               err = -ENOSPC;
>                               goto err_free;
>                       }
>               } while (1);
> 
>               mark_allocated(block);
>               mm->avail -= drm_buddy_block_size(mm, block);
>               kmemleak_update_trace(block);
>               list_add_tail(&block->link, &allocated);
> 
>               pages -= BIT(order);
> 
>               if (!pages)
>                       break;
>       } while (1);
> 
> Was the BUG_ON triggered for your case?
> 
>       BUG_ON(order < min_order);
no, this BUG_ON is not triggered for this bug
> 
> Please give more details.

there is a chance when there is no space to allocate, order value
decrements and reaches to 0 at one point, here we should exit the loop,
otherwise, further order value decrements to -1 and do..while loop
doesn't exit. Hence added a check to exit the loop if order value becomes 0.

Thanks,
Arun

> 
> 
> Kind regards,
> 
> Paul
>