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] Reviewed-by: Thomas Hellström <[email protected]> Reviewed-by: Matthew Brost <[email protected]> Reviewed-by: Christian König <[email protected]> --- 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
