On Thu, Feb 19, 2026 at 10:54:35AM +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 <[email protected]>
> Suggested-by: Matthew Brost <[email protected]>
> Cc: Thomas Hellström <[email protected]>
> Cc: Michal Wajdeczko <[email protected]>
> Cc: Matthew Auld <[email protected]>
> Cc: Christian König <[email protected]>
> Cc: [email protected]
> Cc: Maarten Lankhorst <[email protected]>
>
> ---
> V6 -> V7:
> - Dropped R-B to review again with the new changes.
> - Dropped drm_suballoc_release() which was introduced in this patch.
> (Maarten).
>
> V5 -> V6:
> - Renamed drm_suballoc_init() to drm_suballoc_insert() (Maarten).
>
> V4 -> V5:
> - None.
>
> V3 -> V4:
> - None.
>
> 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 | 105 ++++++++++++++++++++++++++-------
> include/drm/drm_suballoc.h | 6 ++
> 2 files changed, 91 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_suballoc.c b/drivers/gpu/drm/drm_suballoc.c
> index 879ea33dbbc4..1acb5363b65d 100644
> --- 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,47 @@ drm_suballoc_new(struct drm_suballoc_manager
> *sa_manager, size_t size,
> } while (!r);
>
> spin_unlock(&sa_manager->wq.lock);
You need to set 'sa->manager = NULL' here for drm_suballoc_free() to
work as expected.
Matt
> - kfree(sa);
> - return ERR_PTR(r);
> + 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 +465,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 7ba72a81a808..29befdda35d2 100644
> --- 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);
> --
> 2.43.0
>
