Re: [PATCH 1/2] drm/xe/sa: make suballocator waits uninterruptible

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

Hi, Matt

On Thu, 2026-10-22 at 18:41 +0100, Matthew Auld wrote:
> Large BO clears and copies in xe_migrate_clear() and
> xe_migrate_copy()
> are split into multiple passes, each needing a batchbuffer from the
> fixed size kernel bb pool. Once the pool fills up, __xe_sa_bo_new()
> waits interruptibly for space, so a pending signal (e.g. Xorg's 5ms
> SIGALRM smart-scheduler timer) makes it fail mid-loop with -
> ERESTARTSYS.
> The partial progress is then thrown away and the restarted ioctl
> starts
> again from byte 0, including the memory allocation, which can
> livelock
> if the signal arrives faster than the whole operation completes. In
> the
> shrinker path the same error also aborts the shrinker walk, even
> though
> ctx->interruptible is false.
> 
> Being interruptible here doesn't seem to buy much anyway: on error
> the loop already waits uninterruptibly for all submitted passes
> before
> returning. So just make the suballocator waits uninterruptible,
> matching
> amdgpu and radeon. The other users (page reclaim from a kworker, and
> the
> GuC buffer cache) shouldn't really see any meaningful change.

This is a deviation from the "make all waits possible interruptible".
One might argue that user-space that interrupts kernel work with a 5ms
time only to restart it again and again is not well-behaving and
continously shoots itself in the foot.

Given our pipelining abilities, isn't there a risk that we delay signal
delivery with up to 10s?

In the worst case, is there a way we can periodically at least check
for pending signals or perhaps increase the sa bo size so that it
doesn't block on a single large buffer?

/Thomas

> 
> Assisted-by: LLM
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9644
> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel
> GPUs")
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: <stable@vger.kernel.org> # v6.12+
> ---
>  drivers/gpu/drm/xe/xe_sa.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_sa.c b/drivers/gpu/drm/xe/xe_sa.c
> index f32045f40b7a..d9c2b28b08c2 100644
> --- a/drivers/gpu/drm/xe/xe_sa.c
> +++ b/drivers/gpu/drm/xe/xe_sa.c
> @@ -165,7 +165,8 @@ void xe_sa_bo_sync_shadow(struct drm_suballoc
> *sa_bo)
>   * @size: number of bytes we want to suballocate
>   * @gfp: gfp flags used for memory allocation. Typically GFP_KERNEL.
>   *
> - * Try to make a suballocation of size @size.
> + * Try to make a suballocation of size @size. Any waits for space
> are
> + * uninterruptible.
>   *
>   * Return: a &drm_suballoc, or an ERR_PTR.
>   */
> @@ -178,7 +179,7 @@ struct drm_suballoc *__xe_sa_bo_new(struct
> xe_sa_manager *sa_manager, u32 size,
>  	if (size > sa_manager->base.size)
>  		return ERR_PTR(-ENOBUFS);
>  
> -	return drm_suballoc_new(&sa_manager->base, size, gfp, true,
> 0);
> +	return drm_suballoc_new(&sa_manager->base, size, gfp, false,
> 0);
>  }
>  
>  /**