[Intel-gfx] [PATCH] drm: Fix a infinite loop condition when order becomes 0

[Paul Menzel]

Dear Arunpravin,


Am 15.03.22 um 16:42 schrieb Arunpravin:

> On 15/03/22 2:35 pm, Paul Menzel wrote:

>> 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.PaneerSelvam at amd.com>
>>>>
>>>> Please use your full name.
>>> okay
>>
>> You might also configure that in your email program.
> yes

Not done yet though. ;-)

>>>>> ---
>>>>>     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&data=04%7C01%7CArunpravin.PaneerSelvam%40amd.com%7Cc456573102c04191cf9708da0662f798%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637829319396954551%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=5Bspe5QGjQ0KHfVI8%2F%2BXqxR45q6tOL4FE2fVD3uwL%2FM%3D&reserved=0
>>> drm-misc-next - https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freedesktop.org%2Fdrm%2Fdrm-misc%2Ftree%2F&data=04%7C01%7CArunpravin.PaneerSelvam%40amd.com%7Cc456573102c04191cf9708da0662f798%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637829319396954551%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=g2S14TfsHF5ORo9jTZ3uA0l1BH8mnAxk2OWYJeF5i8k%3D&reserved=0

Thank Outlook. Now everybody feels safe.

>>>>> @@ -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.

Sorry, I do not see it. How can that be with order ≥ min_order and the 
check `order-- == min_order`? Is min_order 0? Please explain that in the 
next commit message.


Kind regards,

Paul