Re: [PATCH] drm/xe/guc: skip CLEANUP message when device is hot-unplugged
Matthew Brost <[email protected]>
| Newsgroups | org.freedesktop.lists.intel-xe |
|---|---|
| Message-ID | <amkPHS9Oj4Eqmbc/@gsse-cloud1.jf.intel.com> |
On Tue, Jul 28, 2026 at 08:54:53PM +0530, Nitin Gote wrote: > When a device is hot-unplugged while a DRM fd with active exec queues is > still open, the fd close path runs during process exit and calls > guc_exec_queue_destroy(). This queues a CLEANUP message via > guc_exec_queue_add_msg(), which internally calls > xe_pm_runtime_get_noresume(). > > After hot-unplug, pm_runtime_disable() has already been called, so > xe_pm_runtime_get_noresume() warns with "Missing outer runtime PM > protection". > > GuC communication is no longer valid after unplug, so skip the CLEANUP > message and destroy the exec queue locally via __guc_exec_queue_destroy(). > > Observed with new IGT core_hotunplug subtests: > > igt@core_hotunplug@hotreplug-with-load > igt@core_hotunplug@hotunplug-rescan-with-load > > Link: https://patchwork.freedesktop.org/series/166744/ > Cc: Matthew Auld <[email protected]> > Assisted-by: GitHub-Copilot:claude-sonnet-4.6 > Signed-off-by: Nitin Gote <[email protected]> > --- > drivers/gpu/drm/xe/xe_guc_submit.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c > index 8aaed4fd13ea..b5035c19d7b0 100644 > --- a/drivers/gpu/drm/xe/xe_guc_submit.c > +++ b/drivers/gpu/drm/xe/xe_guc_submit.c > @@ -2131,12 +2131,15 @@ static bool guc_exec_queue_try_add_msg(struct xe_exec_queue *q, > #define STATIC_MSG_RESUME 2 > static void guc_exec_queue_destroy(struct xe_exec_queue *q) > { > + struct xe_guc *guc = exec_queue_to_guc(q); > struct xe_sched_msg *msg = q->guc->static_msgs + STATIC_MSG_CLEANUP; > > - if (!(q->flags & EXEC_QUEUE_FLAG_PERMANENT) && !exec_queue_wedged(q)) > + /* If the device has been hot-unplugged the GuC is already gone. */ > + if (!(q->flags & EXEC_QUEUE_FLAG_PERMANENT) && !exec_queue_wedged(q) && > + !drm_dev_is_unplugged(&guc_to_xe(guc)->drm)) Isn't there a TOCTOU here - immedaitely the value of drm_dev_is_unplugged can change after the CLEANUP message is added? > guc_exec_queue_add_msg(q, msg, CLEANUP); > else > - __guc_exec_queue_destroy(exec_queue_to_guc(q), q); > + __guc_exec_queue_destroy(guc, q); I think after merging [1] we should also aim to get rid of this bypass teardown path and always add a CLEANUP message and within message processing either send the H2G to initiatate teardown of the queue or kick the worker which tears down the queue (i.e., the async worker is the only place where final queue down can occur). Matt [1] https://patchwork.freedesktop.org/patch/740542/?series=168424&rev=10 > } > > static int guc_exec_queue_set_priority(struct xe_exec_queue *q, > -- > 2.50.1 >