drm/sa: Split drm_suballoc_new() into SA alloc and init helpers

drm_suballoc_new() currently both allocates the SA object using kmalloc()
and searches for a suitable hole in the sub-allocator for the requested
size. If SA allocation is done by holding sub-allocator mutex, this design
can lead to reclaim safety issues.

By splitting the kmalloc() step outside of the critical section, we allow
the memory allocation to use GFP_KERNEL (reclaim-safe) while ensuring that
the initialization step that holds reclaim-tainted locks (sub-allocator
mutex) operates in a reclaim-unsafe context with pre-allocated memory.

This separation prevents potential deadlocks where memory reclaim could
attempt to acquire locks that are already held during the sub-allocator
operations.

Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: dri-devel@lists.freedesktop.org
Cc: Maarten Lankhorst <dev@lankhorst.se>
Reviewed-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Acked-by: Maarten Lankhorst <dev@lankhorst.se>
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
Link: https://patch.msgid.link/20260220055519.2485681-6-satyanarayana.k.v.p@intel.com

diff --git a/drivers/gpu/drm/drm_suballoc.c b/drivers/gpu/drm/drm_suballoc.c
index 879ea33dbbc4356ef339bea372517fd362938c95..dc9bef3c0419d75a63ea62e263d70692cb679f5a 100644 (file)
--- a/drivers/gpu/drm/drm_suballoc.c
+++ b/drivers/gpu/drm/drm_suballoc.c
@@ -293,45 +293,66 @@ static bool drm_suballoc_next_hole(struct drm_suballoc_manager *sa_manager,
 }
 
 /**
- * drm_suballoc_new() - Make a suballocation.
+ * drm_suballoc_alloc() - Allocate uninitialized suballoc object.
+ * @gfp: gfp flags used for memory allocation.
+ *
+ * Allocate memory for an uninitialized suballoc object. Intended usage is
+ * allocate memory for suballoc object outside of a reclaim tainted context
+ * and then be initialized at a later time in a reclaim tainted context.
+ *
+ * @drm_suballoc_free() should be used to release the memory if returned
+ * suballoc object is in uninitialized state.
+ *
+ * Return: a new uninitialized suballoc object, or an ERR_PTR(-ENOMEM).
+ */
+struct drm_suballoc *drm_suballoc_alloc(gfp_t gfp)
+{
+       struct drm_suballoc *sa;
+
+       sa = kmalloc(sizeof(*sa), gfp);
+       if (!sa)
+               return ERR_PTR(-ENOMEM);
+
+       sa->manager = NULL;
+
+       return sa;
+}
+EXPORT_SYMBOL(drm_suballoc_alloc);
+
+/**
+ * drm_suballoc_insert() - Initialize a suballocation and insert a hole.
  * @sa_manager: pointer to the sa_manager
+ * @sa: The struct drm_suballoc.
  * @size: number of bytes we want to suballocate.
- * @gfp: gfp flags used for memory allocation. Typically GFP_KERNEL but
- *       the argument is provided for suballocations from reclaim context or
- *       where the caller wants to avoid pipelining rather than wait for
- *       reclaim.
  * @intr: Whether to perform waits interruptible. This should typically
  *        always be true, unless the caller needs to propagate a
  *        non-interruptible context from above layers.
  * @align: Alignment. Must not exceed the default manager alignment.
  *         If @align is zero, then the manager alignment is used.
  *
- * Try to make a suballocation of size @size, which will be rounded
- * up to the alignment specified in specified in drm_suballoc_manager_init().
+ * Try to make a suballocation on a pre-allocated suballoc object of size @size,
+ * which will be rounded up to the alignment specified in specified in
+ * drm_suballoc_manager_init().
  *
- * Return: a new suballocated bo, or an ERR_PTR.
+ * Return: zero on success, errno on failure.
  */
-struct drm_suballoc *
-drm_suballoc_new(struct drm_suballoc_manager *sa_manager, size_t size,
-                gfp_t gfp, bool intr, size_t align)
+int drm_suballoc_insert(struct drm_suballoc_manager *sa_manager,
+                       struct drm_suballoc *sa, size_t size,
+                       bool intr, size_t align)
 {
        struct dma_fence *fences[DRM_SUBALLOC_MAX_QUEUES];
        unsigned int tries[DRM_SUBALLOC_MAX_QUEUES];
        unsigned int count;
        int i, r;
-       struct drm_suballoc *sa;
 
        if (WARN_ON_ONCE(align > sa_manager->align))
-               return ERR_PTR(-EINVAL);
+               return -EINVAL;
        if (WARN_ON_ONCE(size > sa_manager->size || !size))
-               return ERR_PTR(-EINVAL);
+               return -EINVAL;
 
        if (!align)
                align = sa_manager->align;
 
-       sa = kmalloc(sizeof(*sa), gfp);
-       if (!sa)
-               return ERR_PTR(-ENOMEM);
        sa->manager = sa_manager;
        sa->fence = NULL;
        INIT_LIST_HEAD(&sa->olist);
@@ -348,7 +369,7 @@ drm_suballoc_new(struct drm_suballoc_manager *sa_manager, size_t size,
                        if (drm_suballoc_try_alloc(sa_manager, sa,
                                                   size, align)) {
                                spin_unlock(&sa_manager->wq.lock);
-                               return sa;
+                               return 0;
                        }
 
                        /* see if we can skip over some allocations */
@@ -385,8 +406,48 @@ drm_suballoc_new(struct drm_suballoc_manager *sa_manager, size_t size,
        } while (!r);
 
        spin_unlock(&sa_manager->wq.lock);
-       kfree(sa);
-       return ERR_PTR(r);
+       sa->manager = NULL;
+       return r;
+}
+EXPORT_SYMBOL(drm_suballoc_insert);
+
+/**
+ * drm_suballoc_new() - Make a suballocation.
+ * @sa_manager: pointer to the sa_manager
+ * @size: number of bytes we want to suballocate.
+ * @gfp: gfp flags used for memory allocation. Typically GFP_KERNEL but
+ *       the argument is provided for suballocations from reclaim context or
+ *       where the caller wants to avoid pipelining rather than wait for
+ *       reclaim.
+ * @intr: Whether to perform waits interruptible. This should typically
+ *        always be true, unless the caller needs to propagate a
+ *        non-interruptible context from above layers.
+ * @align: Alignment. Must not exceed the default manager alignment.
+ *         If @align is zero, then the manager alignment is used.
+ *
+ * Try to make a suballocation of size @size, which will be rounded
+ * up to the alignment specified in specified in drm_suballoc_manager_init().
+ *
+ * Return: a new suballocated bo, or an ERR_PTR.
+ */
+struct drm_suballoc *
+drm_suballoc_new(struct drm_suballoc_manager *sa_manager, size_t size,
+                gfp_t gfp, bool intr, size_t align)
+{
+       struct drm_suballoc *sa;
+       int err;
+
+       sa = drm_suballoc_alloc(gfp);
+       if (IS_ERR(sa))
+               return sa;
+
+       err = drm_suballoc_insert(sa_manager, sa, size, intr, align);
+       if (err) {
+               drm_suballoc_free(sa, NULL);
+               return ERR_PTR(err);
+       }
+
+       return sa;
 }
 EXPORT_SYMBOL(drm_suballoc_new);
 
@@ -405,6 +466,11 @@ void drm_suballoc_free(struct drm_suballoc *suballoc,
        if (!suballoc)
                return;
 
+       if (!suballoc->manager) {
+               kfree(suballoc);
+               return;
+       }
+
        sa_manager = suballoc->manager;
 
        spin_lock(&sa_manager->wq.lock);

diff --git a/include/drm/drm_suballoc.h b/include/drm/drm_suballoc.h
index 7ba72a81a808f4133d763e5826c9826bc4cb34f0..29befdda35d230bd005abb34c7c8cd09eb3ecc12 100644 (file)
--- a/include/drm/drm_suballoc.h
+++ b/include/drm/drm_suballoc.h
@@ -53,6 +53,12 @@ void drm_suballoc_manager_init(struct drm_suballoc_manager *sa_manager,
 
 void drm_suballoc_manager_fini(struct drm_suballoc_manager *sa_manager);
 
+struct drm_suballoc *drm_suballoc_alloc(gfp_t gfp);
+
+int drm_suballoc_insert(struct drm_suballoc_manager *sa_manager,
+                       struct drm_suballoc *sa, size_t size, bool intr,
+                       size_t align);
+
 struct drm_suballoc *
 drm_suballoc_new(struct drm_suballoc_manager *sa_manager, size_t size,
                 gfp_t gfp, bool intr, size_t align);