On 29/03/22 4:54 pm, Matthew Auld wrote: > On Tue, 29 Mar 2022 at 12:17, Arunpravin Paneer Selvam > <arunpravin.paneerselvam@xxxxxxx> wrote: >> >> >> >> 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@xxxxxxx> >>>> Suggested-by: Matthew Auld <matthew.auld@xxxxxxxxx> >>>> --- >>>> 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. > > We could also move the WARN_ON() into i915 as a separate patch, and > just change the default buddy behaviour to transparently handle the > rounding + trim, if you prefer. I don't have a strong opinion. ok, I sent a patch handling rounding + trim in drm_buddy_alloc_blocks() function. Please review the patch. > >> >> 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? >>> >>> Regards, >>> Christian. >>> >>>> /* Actual range allocation */ >>>> if (start + size == end) >>>> return __drm_buddy_alloc_range(mm, start, size, blocks); >>>> >>>> base-commit: 056d47eaf6ea753fa2e21da31f9cbd8b721bbb7b >>>
