[PATCH v3 1/3] drm/sa: Split drm_suballoc_new() into SA alloc and init helpers

[PATCH v3 1/3] drm/sa: Split drm_suballoc_new() into SA alloc and init helpers

[Satyanarayana K V P]

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

---
V2 -> V3:
- Updated commit message (Matt, Thomas & Christian).
- Removed timeout logic from drm_suballoc_init(). (Thomas & Christian).

V1 -> V2:
- Splitted drm_suballoc_new() into drm_suballoc_alloc() and
drm_suballoc_init() (Thomas).
---
 drivers/gpu/drm/drm_suballoc.c | 110 ++++++++++++++++++++++++++-------
 include/drm/drm_suballoc.h     |   8 +++
 2 files changed, 97 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_suballoc.c b/drivers/gpu/drm/drm_suballoc.c
index 879ea33dbbc4..b97ffcd98d45 100644
--- a/drivers/gpu/drm/drm_suballoc.c
+++ b/drivers/gpu/drm/drm_suballoc.c
@@ -123,7 +123,7 @@ static void drm_suballoc_remove_locked(struct drm_suballoc *sa)
 	list_del_init(&sa->olist);
 	list_del_init(&sa->flist);
 	dma_fence_put(sa->fence);
-	kfree(sa);
+	drm_suballoc_release(sa);
 }
 
 static void drm_suballoc_try_free(struct drm_suballoc_manager *sa_manager)
@@ -293,45 +293,74 @@ 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_release 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);
+
+	return sa;
+}
+EXPORT_SYMBOL(drm_suballoc_alloc);
+
+/**
+ * drm_suballoc_release() - Release memory for suballocation.
+ * @sa: The struct drm_suballoc.
+ */
+void drm_suballoc_release(struct drm_suballoc *sa)
+{
+	kfree(sa);
+}
+EXPORT_SYMBOL(drm_suballoc_release);
+
+/**
+ * drm_suballoc_init() - Initialize a suballocation.
  * @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_init(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 +377,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 +414,47 @@ 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);
+	return r;
+}
+EXPORT_SYMBOL(drm_suballoc_init);
+
+/**
+ * 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_init(sa_manager, sa, size, intr, align);
+	if (err) {
+		drm_suballoc_release(sa);
+		return ERR_PTR(err);
+	}
+
+	return sa;
 }
 EXPORT_SYMBOL(drm_suballoc_new);
 
diff --git a/include/drm/drm_suballoc.h b/include/drm/drm_suballoc.h
index 7ba72a81a808..b8d1d5449fd8 100644
--- a/include/drm/drm_suballoc.h
+++ b/include/drm/drm_suballoc.h
@@ -53,6 +53,14 @@ 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);
+
+void drm_suballoc_release(struct drm_suballoc *sa);
+
+int drm_suballoc_init(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);
-- 
2.43.0

Re: [PATCH v3 1/3] drm/sa: Split drm_suballoc_new() into SA alloc and init helpers

[Thomas Hellström]

On Tue, 2026-02-10 at 10:59 +0000, Satyanarayana K V P wrote:
> 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

LGTM.
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

> 
> ---
> V2 -> V3:
> - Updated commit message (Matt, Thomas & Christian).
> - Removed timeout logic from drm_suballoc_init(). (Thomas &
> Christian).
> 
> V1 -> V2:
> - Splitted drm_suballoc_new() into drm_suballoc_alloc() and
> drm_suballoc_init() (Thomas).
> ---
>  drivers/gpu/drm/drm_suballoc.c | 110 ++++++++++++++++++++++++++-----
> --
>  include/drm/drm_suballoc.h     |   8 +++
>  2 files changed, 97 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_suballoc.c
> b/drivers/gpu/drm/drm_suballoc.c
> index 879ea33dbbc4..b97ffcd98d45 100644
> --- a/drivers/gpu/drm/drm_suballoc.c
> +++ b/drivers/gpu/drm/drm_suballoc.c
> @@ -123,7 +123,7 @@ static void drm_suballoc_remove_locked(struct
> drm_suballoc *sa)
>  	list_del_init(&sa->olist);
>  	list_del_init(&sa->flist);
>  	dma_fence_put(sa->fence);
> -	kfree(sa);
> +	drm_suballoc_release(sa);
>  }
>  
>  static void drm_suballoc_try_free(struct drm_suballoc_manager
> *sa_manager)
> @@ -293,45 +293,74 @@ 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_release 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);
> +
> +	return sa;
> +}
> +EXPORT_SYMBOL(drm_suballoc_alloc);
> +
> +/**
> + * drm_suballoc_release() - Release memory for suballocation.
> + * @sa: The struct drm_suballoc.
> + */
> +void drm_suballoc_release(struct drm_suballoc *sa)
> +{
> +	kfree(sa);
> +}
> +EXPORT_SYMBOL(drm_suballoc_release);
> +
> +/**
> + * drm_suballoc_init() - Initialize a suballocation.
>   * @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_init(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 +377,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 +414,47 @@ 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);
> +	return r;
> +}
> +EXPORT_SYMBOL(drm_suballoc_init);
> +
> +/**
> + * 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_init(sa_manager, sa, size, intr, align);
> +	if (err) {
> +		drm_suballoc_release(sa);
> +		return ERR_PTR(err);
> +	}
> +
> +	return sa;
>  }
>  EXPORT_SYMBOL(drm_suballoc_new);
>  
> diff --git a/include/drm/drm_suballoc.h b/include/drm/drm_suballoc.h
> index 7ba72a81a808..b8d1d5449fd8 100644
> --- a/include/drm/drm_suballoc.h
> +++ b/include/drm/drm_suballoc.h
> @@ -53,6 +53,14 @@ 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);
> +
> +void drm_suballoc_release(struct drm_suballoc *sa);
> +
> +int drm_suballoc_init(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);

Re: [PATCH v3 1/3] drm/sa: Split drm_suballoc_new() into SA alloc and init helpers

[Christian König]

On 2/10/26 13:09, Thomas Hellström wrote:
> On Tue, 2026-02-10 at 10:59 +0000, Satyanarayana K V P wrote:
>> 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
> 
> LGTM.
> Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

Where are patches 2 and 3 in that series?

Regards,
Christian.

> 
>>
>> ---
>> V2 -> V3:
>> - Updated commit message (Matt, Thomas & Christian).
>> - Removed timeout logic from drm_suballoc_init(). (Thomas &
>> Christian).
>>
>> V1 -> V2:
>> - Splitted drm_suballoc_new() into drm_suballoc_alloc() and
>> drm_suballoc_init() (Thomas).
>> ---
>>  drivers/gpu/drm/drm_suballoc.c | 110 ++++++++++++++++++++++++++-----
>> --
>>  include/drm/drm_suballoc.h     |   8 +++
>>  2 files changed, 97 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_suballoc.c
>> b/drivers/gpu/drm/drm_suballoc.c
>> index 879ea33dbbc4..b97ffcd98d45 100644
>> --- a/drivers/gpu/drm/drm_suballoc.c
>> +++ b/drivers/gpu/drm/drm_suballoc.c
>> @@ -123,7 +123,7 @@ static void drm_suballoc_remove_locked(struct
>> drm_suballoc *sa)
>>  	list_del_init(&sa->olist);
>>  	list_del_init(&sa->flist);
>>  	dma_fence_put(sa->fence);
>> -	kfree(sa);
>> +	drm_suballoc_release(sa);
>>  }
>>  
>>  static void drm_suballoc_try_free(struct drm_suballoc_manager
>> *sa_manager)
>> @@ -293,45 +293,74 @@ 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_release 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);
>> +
>> +	return sa;
>> +}
>> +EXPORT_SYMBOL(drm_suballoc_alloc);
>> +
>> +/**
>> + * drm_suballoc_release() - Release memory for suballocation.
>> + * @sa: The struct drm_suballoc.
>> + */
>> +void drm_suballoc_release(struct drm_suballoc *sa)
>> +{
>> +	kfree(sa);
>> +}
>> +EXPORT_SYMBOL(drm_suballoc_release);
>> +
>> +/**
>> + * drm_suballoc_init() - Initialize a suballocation.
>>   * @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_init(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 +377,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 +414,47 @@ 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);
>> +	return r;
>> +}
>> +EXPORT_SYMBOL(drm_suballoc_init);
>> +
>> +/**
>> + * 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_init(sa_manager, sa, size, intr, align);
>> +	if (err) {
>> +		drm_suballoc_release(sa);
>> +		return ERR_PTR(err);
>> +	}
>> +
>> +	return sa;
>>  }
>>  EXPORT_SYMBOL(drm_suballoc_new);
>>  
>> diff --git a/include/drm/drm_suballoc.h b/include/drm/drm_suballoc.h
>> index 7ba72a81a808..b8d1d5449fd8 100644
>> --- a/include/drm/drm_suballoc.h
>> +++ b/include/drm/drm_suballoc.h
>> @@ -53,6 +53,14 @@ 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);
>> +
>> +void drm_suballoc_release(struct drm_suballoc *sa);
>> +
>> +int drm_suballoc_init(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);

Re: [PATCH v3 1/3] drm/sa: Split drm_suballoc_new() into SA alloc and init helpers

[Matthew Brost]

On Tue, Feb 10, 2026 at 10:59:31AM +0000, Satyanarayana K V P wrote:
> 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>

Reviewed-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
> 
> ---
> V2 -> V3:
> - Updated commit message (Matt, Thomas & Christian).
> - Removed timeout logic from drm_suballoc_init(). (Thomas & Christian).
> 
> V1 -> V2:
> - Splitted drm_suballoc_new() into drm_suballoc_alloc() and
> drm_suballoc_init() (Thomas).
> ---
>  drivers/gpu/drm/drm_suballoc.c | 110 ++++++++++++++++++++++++++-------
>  include/drm/drm_suballoc.h     |   8 +++
>  2 files changed, 97 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_suballoc.c b/drivers/gpu/drm/drm_suballoc.c
> index 879ea33dbbc4..b97ffcd98d45 100644
> --- a/drivers/gpu/drm/drm_suballoc.c
> +++ b/drivers/gpu/drm/drm_suballoc.c
> @@ -123,7 +123,7 @@ static void drm_suballoc_remove_locked(struct drm_suballoc *sa)
>  	list_del_init(&sa->olist);
>  	list_del_init(&sa->flist);
>  	dma_fence_put(sa->fence);
> -	kfree(sa);
> +	drm_suballoc_release(sa);
>  }
>  
>  static void drm_suballoc_try_free(struct drm_suballoc_manager *sa_manager)
> @@ -293,45 +293,74 @@ 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_release 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);
> +
> +	return sa;
> +}
> +EXPORT_SYMBOL(drm_suballoc_alloc);
> +
> +/**
> + * drm_suballoc_release() - Release memory for suballocation.
> + * @sa: The struct drm_suballoc.
> + */
> +void drm_suballoc_release(struct drm_suballoc *sa)
> +{
> +	kfree(sa);
> +}
> +EXPORT_SYMBOL(drm_suballoc_release);
> +
> +/**
> + * drm_suballoc_init() - Initialize a suballocation.
>   * @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_init(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 +377,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 +414,47 @@ 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);
> +	return r;
> +}
> +EXPORT_SYMBOL(drm_suballoc_init);
> +
> +/**
> + * 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_init(sa_manager, sa, size, intr, align);
> +	if (err) {
> +		drm_suballoc_release(sa);
> +		return ERR_PTR(err);
> +	}
> +
> +	return sa;
>  }
>  EXPORT_SYMBOL(drm_suballoc_new);
>  
> diff --git a/include/drm/drm_suballoc.h b/include/drm/drm_suballoc.h
> index 7ba72a81a808..b8d1d5449fd8 100644
> --- a/include/drm/drm_suballoc.h
> +++ b/include/drm/drm_suballoc.h
> @@ -53,6 +53,14 @@ 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);
> +
> +void drm_suballoc_release(struct drm_suballoc *sa);
> +
> +int drm_suballoc_init(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);
> -- 
> 2.43.0
>