Re: [PATCH 1/3] drm/sa: Scan all holes when fences are disabled

["Christian König" <christian.koenig@amd.com>]

On 3/11/26 16:52, Satyanarayana K V P wrote:
> The suballocator algorithm tracks a hole cursor at the last allocation
> and tries to allocate after it. This is optimized for fence-ordered
> progress, where older allocations are expected to become reusable first.
> 
> In fence-enabled mode, that ordering assumption holds. In fence-disabled
> mode, allocations may be freed in arbitrary order, so limiting allocation
> to the current hole window can miss valid free space and fail allocations
> despite sufficient total space.
> 
> Update the fence-disabled path to walk through all holes and select any
> fitting range, while keeping the existing fence-based behavior unchanged.

Mhm, that sounds like we are abusing the SA for an unfit use case to me.

The whole idea of the SA is to work with fence protected allocation, if you don't need that then you can also use the drm_mm for the job.

So why are you not using drm_mm for that?

Regards,
Christian.

> 
> Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Maarten Lankhorst <dev@lankhorst.se>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_suballoc.c | 90 ++++++++++++++++++++++++++++++++++
>  include/drm/drm_suballoc.h     |  5 ++
>  2 files changed, 95 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_suballoc.c b/drivers/gpu/drm/drm_suballoc.c
> index b74277bbc14b..307da191a855 100644
> --- a/drivers/gpu/drm/drm_suballoc.c
> +++ b/drivers/gpu/drm/drm_suballoc.c
> @@ -78,12 +78,33 @@ void drm_suballoc_manager_init(struct drm_suballoc_manager *sa_manager,
>  	sa_manager->size = size;
>  	sa_manager->align = align;
>  	sa_manager->hole = &sa_manager->olist;
> +	sa_manager->fence_disable = false;
>  	INIT_LIST_HEAD(&sa_manager->olist);
>  	for (i = 0; i < DRM_SUBALLOC_MAX_QUEUES; ++i)
>  		INIT_LIST_HEAD(&sa_manager->flist[i]);
>  }
>  EXPORT_SYMBOL(drm_suballoc_manager_init);
>  
> +/**
> + * drm_suballoc_manager_fence_disable() - Enable or disable fence tracking.
> + * @sa_manager: Pointer to the suballocation manager.
> + * @fence_disable: Whether to disable fence tracking for free suballocations.
> + *
> + * When @fence_disable is true allocation scans all holes without waiting on
> + * fences. When false, the manager tracks free suballocations behind fences and
> + * reuses them only after the fence signals.
> + *
> + * This should be called immediately after creating the suballocator and before
> + * any allocations are made. The behaviour is undefined if this is called after
> + * allocations have been made.
> + */
> +void drm_suballoc_manager_fence_disable(struct drm_suballoc_manager *sa_manager,
> +					bool fence_disable)
> +{
> +	sa_manager->fence_disable = fence_disable;
> +}
> +EXPORT_SYMBOL(drm_suballoc_manager_fence_disable);
> +
>  /**
>   * drm_suballoc_manager_fini() - Destroy the drm_suballoc_manager
>   * @sa_manager: pointer to the sa_manager
> @@ -110,6 +131,7 @@ void drm_suballoc_manager_fini(struct drm_suballoc_manager *sa_manager)
>  	}
>  
>  	sa_manager->size = 0;
> +	sa_manager->fence_disable = false;
>  }
>  EXPORT_SYMBOL(drm_suballoc_manager_fini);
>  
> @@ -161,6 +183,69 @@ static size_t drm_suballoc_hole_eoffset(struct drm_suballoc_manager *sa_manager)
>  	return sa_manager->size;
>  }
>  
> +/**
> + * drm_suballoc_hole_soffset_eoffset() - Find a hole fitting size and alignment.
> + * @sa_manager: Suballocator manager.
> + * @soffset: Start offset of the selected hole.
> + * @eoffset: End offset of the selected hole.
> + * @size: Size to be allocated.
> + * @align: Allocation alignment.
> + *
> + * This function is used when fences are disabled. It scans the holes starting
> + * at the current hole pointer and selects the first one that can satisfy @size
> + * once @align padding is applied. We may need to scan the entire list of holes
> + * and update soffset and eoffset for each hole to find a suitable one, as the
> + * current hole pointer may not be the same as sa_manager->hole.
> + *
> + * Return: true if a suitable hole is found, false otherwise.
> + */
> +static bool drm_suballoc_hole_soffset_eoffset(struct drm_suballoc_manager *sa_manager,
> +					      size_t *soffset, size_t *eoffset,
> +					      size_t size, size_t align)
> +{
> +	struct list_head *start, *hole;
> +
> +	if (!sa_manager->fence_disable)
> +		return true;
> +
> +	start = sa_manager->hole;
> +	hole = start;
> +
> +	do {
> +		size_t s, e, wasted;
> +
> +		/*
> +		 * We can't use drm_suballoc_hole_soffset() and
> +		 * drm_suballoc_hole_eoffset() here as the hole may not be same
> +		 * as sa_manager->hole.
> +		 */
> +		if (hole != &sa_manager->olist)
> +			s = list_entry(hole, struct drm_suballoc, olist)->eoffset;
> +		else
> +			s = 0;
> +
> +		if (hole->next != &sa_manager->olist)
> +			e = list_entry(hole->next, struct drm_suballoc, olist)->soffset;
> +		else
> +			e = sa_manager->size;
> +
> +		if (e < s || e == s || (e - s) < size) {
> +			hole = hole->next;
> +			continue;
> +		}
> +
> +		wasted = round_up(s, align) - s;
> +		if ((e - s) >= (size + wasted)) {
> +			*soffset = s;
> +			*eoffset = e;
> +			sa_manager->hole = hole;
> +			return true;
> +		}
> +	} while (hole != start);
> +
> +	return false;
> +}
> +
>  static bool drm_suballoc_try_alloc(struct drm_suballoc_manager *sa_manager,
>  				   struct drm_suballoc *sa,
>  				   size_t size, size_t align)
> @@ -169,6 +254,11 @@ static bool drm_suballoc_try_alloc(struct drm_suballoc_manager *sa_manager,
>  
>  	soffset = drm_suballoc_hole_soffset(sa_manager);
>  	eoffset = drm_suballoc_hole_eoffset(sa_manager);
> +
> +	if (!drm_suballoc_hole_soffset_eoffset(sa_manager, &soffset,
> +					       &eoffset, size, align))
> +		return false;
> +
>  	wasted = round_up(soffset, align) - soffset;
>  
>  	if ((eoffset - soffset) >= (size + wasted)) {
> diff --git a/include/drm/drm_suballoc.h b/include/drm/drm_suballoc.h
> index 29befdda35d2..066412196f8a 100644
> --- a/include/drm/drm_suballoc.h
> +++ b/include/drm/drm_suballoc.h
> @@ -20,6 +20,7 @@
>   * @flist: Array[fence context hash] of queues of fenced allocated ranges.
>   * @size: Size of the managed range.
>   * @align: Default alignment for the managed range.
> + * @fence_disable: Fences are disabled for this SA manager.
>   */
>  struct drm_suballoc_manager {
>  	wait_queue_head_t wq;
> @@ -28,6 +29,7 @@ struct drm_suballoc_manager {
>  	struct list_head flist[DRM_SUBALLOC_MAX_QUEUES];
>  	size_t size;
>  	size_t align;
> +	bool fence_disable;
>  };
>  
>  /**
> @@ -51,6 +53,9 @@ struct drm_suballoc {
>  void drm_suballoc_manager_init(struct drm_suballoc_manager *sa_manager,
>  			       size_t size, size_t align);
>  
> +void drm_suballoc_manager_fence_disable(struct drm_suballoc_manager
> +					*sa_manager, bool fence_disable);
> +
>  void drm_suballoc_manager_fini(struct drm_suballoc_manager *sa_manager);
>  
>  struct drm_suballoc *drm_suballoc_alloc(gfp_t gfp);