Commit 16843e66 authored by Satyanarayana K V P's avatar Satyanarayana K V P Committed by Matthew Brost
Browse files

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: default avatarSatyanarayana K V P <satyanarayana.k.v.p@intel.com>
Suggested-by: default avatarMatthew 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: default avatarChristian König <christian.koenig@amd.com>
Reviewed-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
Reviewed-by: default avatarMatthew Brost <matthew.brost@intel.com>
Acked-by: default avatarMaarten Lankhorst <dev@lankhorst.se>
Signed-off-by: default avatarMatthew Brost <matthew.brost@intel.com>
Link: https://patch.msgid.link/20260220055519.2485681-6-satyanarayana.k.v.p@intel.com
parent a5d5634c

drivers/gpu/drm/drm_suballoc.c

+86 −20
Original line number Diff line number Diff line
@@ -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);

include/drm/drm_suballoc.h

+6 −0
Original line number Diff line number Diff line
@@ -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);