[PATCH v7 3/3] drm/buddy: Add defragmentation support
Matthew Auld
matthew.auld at intel.com
Mon Mar 4 12:36:36 UTC 2024
On 04/03/2024 12:22, Paneer Selvam, Arunpravin wrote:
> Hi Matthew,
>
> On 2/22/2024 12:12 AM, Matthew Auld wrote:
>> On 21/02/2024 12:18, Arunpravin Paneer Selvam wrote:
>>> Add a function to support defragmentation.
>>>
>>> v1:
>>> - Defragment the memory beginning from min_order
>>> till the required memory space is available.
>>>
>>> v2(Matthew):
>>> - add amdgpu user for defragmentation
>>> - add a warning if the two blocks are incompatible on
>>> defragmentation
>>> - call full defragmentation in the fini() function
>>> - place a condition to test if min_order is equal to 0
>>> - replace the list with safe_reverse() variant as we might
>>> remove the block from the list.
>>>
>>> Signed-off-by: Arunpravin Paneer Selvam
>>> <Arunpravin.PaneerSelvam at amd.com>
>>> Suggested-by: Matthew Auld <matthew.auld at intel.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 17 +++-
>>> drivers/gpu/drm/drm_buddy.c | 93 +++++++++++++++++---
>>> include/drm/drm_buddy.h | 3 +
>>> 3 files changed, 97 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>> index e494f5bf136a..cff8a526c622 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>> @@ -533,8 +533,21 @@ static int amdgpu_vram_mgr_new(struct
>>> ttm_resource_manager *man,
>>> min_block_size,
>>> &vres->blocks,
>>> vres->flags);
>>> - if (unlikely(r))
>>> - goto error_free_blocks;
>>> + if (unlikely(r)) {
>>> + if (r == -ENOSPC) {
>>> + drm_buddy_defrag(mm, min_block_size);
>>> + r = drm_buddy_alloc_blocks(mm, fpfn,
>>> + lpfn,
>>> + size,
>>> + min_block_size,
>>> + &vres->blocks,
>>> + vres->flags);
>>> + if (unlikely(r))
>>> + goto error_free_blocks;
>>> + } else {
>>> + goto error_free_blocks;
>>> + }
>>> + }
>>> if (size > remaining_size)
>>> remaining_size = 0;
>>> diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
>>> index 18e004fa39d3..56bd1560fbcd 100644
>>> --- a/drivers/gpu/drm/drm_buddy.c
>>> +++ b/drivers/gpu/drm/drm_buddy.c
>>> @@ -203,6 +203,8 @@ void drm_buddy_fini(struct drm_buddy *mm)
>>> drm_block_free(mm, mm->roots[i]);
>>> }
>>> + drm_buddy_defrag(mm, mm->chunk_size << mm->max_order);
>>
>> I think this needs to be called higher up, otherwise we blow up with
>> the WARN, plus we just freed the root(s). There is also the case with
>> non-power-of-two VRAM size, in which case you get multiple roots and
>> max_order is just the largest root and not entire address space. I
>> guess do this in the loop above and use the root order instead?
>>
>> Also this should be done as part of the first patch and then in this
>> patch it is just a case of exporting it. Every commit should ideally
>> be functional by itself.
> You mean we move the above change in drm_buddy_fini function and
> drm_buddy_defrag function as part of first patch.
> And just we add export function and add amdgpu user in this patch. Is my
> understanding correct?
Yeah, I think that makes sense.
>
> Thanks,
> Arun.
>>
>>> +
>>> WARN_ON(mm->avail != mm->size);
>>> kfree(mm->roots);
>>> @@ -276,25 +278,39 @@ drm_get_buddy(struct drm_buddy_block *block)
>>> }
>>> EXPORT_SYMBOL(drm_get_buddy);
>>> -static void __drm_buddy_free(struct drm_buddy *mm,
>>> - struct drm_buddy_block *block)
>>> +static unsigned int __drm_buddy_free(struct drm_buddy *mm,
>>> + struct drm_buddy_block *block,
>>> + bool defrag)
>>> {
>>> + unsigned int order, block_order;
>>> struct drm_buddy_block *parent;
>>> + block_order = drm_buddy_block_order(block);
>>> +
>>> while ((parent = block->parent)) {
>>> - struct drm_buddy_block *buddy;
>>> + struct drm_buddy_block *buddy = NULL;
>>> buddy = __get_buddy(block);
>>> if (!drm_buddy_block_is_free(buddy))
>>> break;
>>> - if (drm_buddy_block_is_clear(block) !=
>>> - drm_buddy_block_is_clear(buddy))
>>> - break;
>>> + if (!defrag) {
>>> + /*
>>> + * Check the block and its buddy clear state and exit
>>> + * the loop if they both have the dissimilar state.
>>> + */
>>> + if (drm_buddy_block_is_clear(block) !=
>>> + drm_buddy_block_is_clear(buddy))
>>> + break;
>>> - if (drm_buddy_block_is_clear(block))
>>> - mark_cleared(parent);
>>> + if (drm_buddy_block_is_clear(block))
>>> + mark_cleared(parent);
>>> + }
>>> +
>>> + WARN_ON(defrag &&
>>> + (drm_buddy_block_is_clear(block) ==
>>> + drm_buddy_block_is_clear(buddy)));
>>> list_del(&buddy->link);
>>> @@ -304,8 +320,57 @@ static void __drm_buddy_free(struct drm_buddy
>>> *mm,
>>> block = parent;
>>> }
>>> - mark_free(mm, block);
>>> + order = drm_buddy_block_order(block);
>>> + if (block_order != order)
>>> + mark_free(mm, block);
>>> +
>>> + return order;
>>> +}
>>> +
>>> +/**
>>> + * drm_buddy_defrag - Defragmentation routine
>>> + *
>>> + * @mm: DRM buddy manager
>>> + * @min_block_size: minimum size in bytes to begin
>>> + * the defragmentation process
>>> + *
>>> + * Driver calls the defragmentation function when the
>>> + * requested memory allocation returns -ENOSPC.
>>> + */
>>> +void drm_buddy_defrag(struct drm_buddy *mm,
>>> + unsigned int min_block_size)
>>
>> u64 min_block_size. Most cards have 4G+ of VRAM :)
>>
>>> +{
>>> + struct drm_buddy_block *block, *tmp;
>>> + unsigned int order, min_order;
>>> + struct list_head *list;
>>> + unsigned long pages;
>>> + int i;
>>> +
>>> + pages = min_block_size >> ilog2(mm->chunk_size);
>>> + min_order = fls(pages) - 1;
>>
>> I think min_block_size should be power-of-two, no?
>>
>>> +
>>> + if (!min_order)
>>> + return;
>>> +
>>> + if (min_order > mm->max_order)
>>> + return;
>>> +
>>> + for (i = min_order - 1; i >= 0; i--) {
>>> + list = &mm->free_list[i];
>>> + if (list_empty(list))
>>> + continue;
>>> +
>>> + list_for_each_entry_safe_reverse(block, tmp, list, link) {
>>> + if (!block->parent)
>>> + continue;
>>> +
>>> + order = __drm_buddy_free(mm, block, 1);
>>
>> s/1/true/
>>
>>> + if (order >= min_order)
>>> + return;
>>> + }
>>> + }
>>> }
>>> +EXPORT_SYMBOL(drm_buddy_defrag);
>>> /**
>>> * drm_buddy_free_block - free a block
>>> @@ -321,7 +386,7 @@ void drm_buddy_free_block(struct drm_buddy *mm,
>>> if (drm_buddy_block_is_clear(block))
>>> mm->clear_avail += drm_buddy_block_size(mm, block);
>>> - __drm_buddy_free(mm, block);
>>> + __drm_buddy_free(mm, block, 0);
>>> }
>>> EXPORT_SYMBOL(drm_buddy_free_block);
>>> @@ -468,7 +533,7 @@ __alloc_range_bias(struct drm_buddy *mm,
>>> if (buddy &&
>>> (drm_buddy_block_is_free(block) &&
>>> drm_buddy_block_is_free(buddy)))
>>> - __drm_buddy_free(mm, block);
>>> + __drm_buddy_free(mm, block, 0);
>>> return ERR_PTR(err);
>>> }
>>> @@ -586,7 +651,7 @@ alloc_from_freelist(struct drm_buddy *mm,
>>> err_undo:
>>> if (tmp != order)
>>> - __drm_buddy_free(mm, block);
>>> + __drm_buddy_free(mm, block, 0);
>>> return ERR_PTR(err);
>>> }
>>> @@ -666,7 +731,7 @@ static int __alloc_range(struct drm_buddy *mm,
>>> if (buddy &&
>>> (drm_buddy_block_is_free(block) &&
>>> drm_buddy_block_is_free(buddy)))
>>> - __drm_buddy_free(mm, block);
>>> + __drm_buddy_free(mm, block, 0);
>>> err_free:
>>> if (err == -ENOSPC && total_allocated_on_err) {
>>> @@ -828,7 +893,7 @@ EXPORT_SYMBOL(drm_buddy_block_trim);
>>> * @mm: DRM buddy manager to allocate from
>>> * @start: start of the allowed range for this block
>>> * @end: end of the allowed range for this block
>>> - * @size: size of the allocation
>>> + * @size: size of the allocation in bytes
>>> * @min_block_size: alignment of the allocation
>>> * @blocks: output list head to add allocated blocks
>>> * @flags: DRM_BUDDY_*_ALLOCATION flags
>>> diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h
>>> index 352a6364e26a..68a874846e78 100644
>>> --- a/include/drm/drm_buddy.h
>>> +++ b/include/drm/drm_buddy.h
>>> @@ -167,6 +167,9 @@ void drm_buddy_free_list(struct drm_buddy *mm,
>>> struct list_head *objects,
>>> unsigned int flags);
>>> +void drm_buddy_defrag(struct drm_buddy *mm,
>>> + unsigned int min_order);
>>> +
>>> void drm_buddy_print(struct drm_buddy *mm, struct drm_printer *p);
>>> void drm_buddy_block_print(struct drm_buddy *mm,
>>> struct drm_buddy_block *block,
>
More information about the amd-gfx
mailing list