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

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

[Matthew Auld]

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.

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);
 }
 
 /**
-- 
2.56.0

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

[Thomas Hellström]

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);
>  }
>  
>  /**

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

[Matthew Auld]

Hi,

On 22/10/2026 19:29, Thomas Hellström wrote:
> 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.

Maybe. This is the xserver, so I assume this is very old and not some 
new thing. I feel like even in this situation the kernel should be able 
to make some forward progress in the ioctl.

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

At least for the copy/clear, it looks like there is already an 
uninterruptible wait on the err path if fence != NULL, which we trigger 
on a signal.

> 
> 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?

Other idea is to expose the intr, and then maybe plumb down the 
ctx->interruptible somehow, which would then cover the shrinker case and 
maybe other places like that which explicitly asked for that? For the 
clear case (gem_create) maybe we check if the fence != NULL first and 
then set intr accordingly? If we have a pending signal it would bail if 
we have not started GPU clearing yet, but if we have started then we try 
to finish? The error path will currently do an uninterruptible wait 
anyway, and that one could be a lot worse for a big buffer, since it 
drains ALL prior jobs, instead of just waiting for the oldest bb. And 
here it might be that we got right to the tail end of the clears but we 
got a signal, so we throw away all of that work.

Increasing the sa bo is also an option (maybe both). Although I guess 
there will likely still be something too big, and I guess we might still 
see issues if the pool is still busy with other pending work?

> 
> /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);
>>   }
>>   
>>   /**

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

[Matthew Auld]

On 23/10/2026 10:32, Matthew Auld wrote:
> Hi,
> 
> On 22/10/2026 19:29, Thomas Hellström wrote:
>> 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.
> 
> Maybe. This is the xserver, so I assume this is very old and not some 
> new thing. I feel like even in this situation the kernel should be able 
> to make some forward progress in the ioctl.
> 
>>
>> Given our pipelining abilities, isn't there a risk that we delay signal
>> delivery with up to 10s?
> 
> At least for the copy/clear, it looks like there is already an 
> uninterruptible wait on the err path if fence != NULL, which we trigger 
> on a signal.
> 
>>
>> 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?
> 
> Other idea is to expose the intr, and then maybe plumb down the ctx- 
>  >interruptible somehow, which would then cover the shrinker case and 
> maybe other places like that which explicitly asked for that? For the 
> clear case (gem_create) maybe we check if the fence != NULL first and 
> then set intr accordingly? If we have a pending signal it would bail if 
> we have not started GPU clearing yet, but if we have started then we try 
> to finish? The error path will currently do an uninterruptible wait 
> anyway, and that one could be a lot worse for a big buffer, since it 
> drains ALL prior jobs, instead of just waiting for the oldest bb. And 
> here it might be that we got right to the tail end of the clears but we 
> got a signal, so we throw away all of that work.
> 
> Increasing the sa bo is also an option (maybe both). Although I guess 
> there will likely still be something too big, and I guess we might still 
> see issues if the pool is still busy with other pending work?

Some more thoughts. Since Matt nuked the vm_bind GPU path, I don't think 
there is much contention on the reserved bcs any more since there is 
just one migrate queue (?), so maybe we can also:

1. Bump the VRAM clear + identity path. 8M is maybe very pessimistic 
now? Maybe we could push a lot more work per MEM_SET + page mode here? 
This could also give more perf, with a lot fewer jobs for 1G+ buffers. I 
have seen llama.py with 5G+ gemma model use 1G+ buffer on my BMG, for 
example.

2. Maybe also pack more work per-bb, at least for identity? Each new bb 
eats up a bunch of padding for pre-fetch etc.

> 
>>
>> /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);
>>>   }
>>>   /**
>