Hey, This looks better.
Can be merged through drm-xe, as it's the only user of the new functions if you want. Acked-by: Maarten Lankhorst <[email protected]> Den 2026-02-20 kl. 07:15, skrev Matthew Brost: > On Fri, Feb 20, 2026 at 05:55:21AM +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]> > > Reviewed-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]> >> Reviewed-by: Christian König <[email protected]> >> Reviewed-by: Thomas Hellström <[email protected]> >> >> --- >> V7 -> V8: >> - Add missing sa->manager=NULL in the error flow in drm_suballoc_insert() >> (Matt) >> >> 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 | 106 ++++++++++++++++++++++++++------- >> include/drm/drm_suballoc.h | 6 ++ >> 2 files changed, 92 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_suballoc.c b/drivers/gpu/drm/drm_suballoc.c >> index 879ea33dbbc4..dc9bef3c0419 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,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 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 >>
