Re: [PATCH v7 3/3] drm/buddy: Add defragmentation support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx>
Suggested-by: Matthew Auld <matthew.auld@xxxxxxxxx>
---
  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,




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux