Re: [PATCH 6/7] drm/xe: Don't block messages to the GPU scheduler

[Matthew Brost <matthew.brost@intel.com>]

On Mon, Oct 13, 2025 at 11:17:58AM -0600, Summers, Stuart wrote:
> On Mon, 2025-10-13 at 09:56 -0700, Matthew Brost wrote:
> > On Mon, Oct 13, 2025 at 04:25:03PM +0000, Stuart Summers wrote:
> > > Right now we are using the state of the GPU scheduler
> > > to determine whether we send and receive messages. There
> > > are some states, however, where we might intentionally
> > > pause the scheduler, like a device wedge, and expect that
> > > messages are resumed later once the user has taken the
> > > hardware state and is attempting to reset, like an unbind.
> > > 
> > > Remove these checks in the XeKMD and let the GPU scheduler
> > > handle state checks internally.
> > > 
> > 
> > We can't do this. The entire queue stop / starting mechanism relies
> > on
> > getting exclusive access to the queue by ensuring the scheduler is
> > fully
> > stopped - this includes messages. This will break job timeouts, GT
> > reset
> > flows, and VF migration.
> 
> I'm not sure I full understand here. The scheduler should be stopped as
> it was before, it just means we keep sending messages right? I can test
> the job timeout piece to make sure...
> 

This will show up as an obscure race condition — 99% of the time it will
work just fine, but I can assure you it will break the entire design of
submission.

> Basically I'm arguing the start/stop mechanics should be inside the
> scheduler and not in the calling driver.
> 

The message interface is built on top of the DRM scheduler, rather than
integrated into it. Originally, I had it built into the scheduler, but
based on feedback, I moved the channel to the driver side. Therefore, we
need to hook into the stopping mechanism on the driver side. The
layering could use some cleanup, but the functionality will remain on
the driver side.

> > 
> > What exactly is the problem you are trying to solve? The device is
> > wedged and queues are stopped, then an unbind occurs? That is
> > probably a
> > bug. IIRC even wedging a device / tearing down a queue we should
> > always
> > start the queue again. We could assert in guc_submit_wedged_fini that
> 
> I think there's basically a race between sending the cleanup message
> and stopping the scheduler. And once we send that message, we don't
> really track it on the xe side. So if we artificially pause things on
> the xe side (by adding the checks I'm removing in this patch), we can
> get into a scenario where the cleanup message is sent *after* the
> scheduler is paused and thus that cleanup message gets dropped, and we
> never issue the deregistration for that particular exec queue.
> 

That's not how stopping works. Stopping prevents future work items from
being queued and flushes all in-flight work items. These work items
include running jobs, freeing jobs, or processing messages. When stopped,
each of these interfaces can set up state so that when the scheduler is
started again, the work items are requeued for processing (e.g.,
messages are stored in a linked list). The key point is that when the
scheduler is stopped, work items that could be modifying the queue state
are not running, so the entity that stopped the scheduler has exclusive
access—without requiring any locks.

Matt

> Thanks,
> Stuart
> 
> > all queues are not paused.
> > 
> > Also if you having issues on unbind - there is this patch [1] which
> > fixes an issue too. I'm going to merge [1] now.
> > 
> > Matt
> > 
> > [1] https://patchwork.freedesktop.org/series/155417/
> > 
> > > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_gpu_scheduler.c | 6 +-----
> > >  1 file changed, 1 insertion(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_gpu_scheduler.c
> > > b/drivers/gpu/drm/xe/xe_gpu_scheduler.c
> > > index f91e06d03511..d9d6fb641188 100644
> > > --- a/drivers/gpu/drm/xe/xe_gpu_scheduler.c
> > > +++ b/drivers/gpu/drm/xe/xe_gpu_scheduler.c
> > > @@ -7,8 +7,7 @@
> > >  
> > >  static void xe_sched_process_msg_queue(struct xe_gpu_scheduler
> > > *sched)
> > >  {
> > > -       if (!READ_ONCE(sched->base.pause_submit))
> > > -               queue_work(sched->base.submit_wq, &sched-
> > > >work_process_msg);
> > > +       queue_work(sched->base.submit_wq, &sched-
> > > >work_process_msg);
> > >  }
> > >  
> > >  static void xe_sched_process_msg_queue_if_ready(struct
> > > xe_gpu_scheduler *sched)
> > > @@ -43,9 +42,6 @@ static void xe_sched_process_msg_work(struct
> > > work_struct *w)
> > >                 container_of(w, struct xe_gpu_scheduler,
> > > work_process_msg);
> > >         struct xe_sched_msg *msg;
> > >  
> > > -       if (READ_ONCE(sched->base.pause_submit))
> > > -               return;
> > > -
> > >         msg = xe_sched_get_msg(sched);
> > >         if (msg) {
> > >                 sched->ops->process_msg(msg);
> > > -- 
> > > 2.34.1
> > > 
>