From: Matthew Brost <matthew.brost@intel.com>
To: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: <intel-xe@lists.freedesktop.org>,
Rodrigo Vivi <rodrigo.vivi@intel.com>,
Matthew Auld <matthew.auld@intel.com>
Subject: Re: [PATCH v4 3/3] drm/xe: Route execlist exec queue teardown off system_dfl_wq
Date: Tue, 29 Sep 2026 09:18:48 -0700 [thread overview]
Message-ID: <arvk6KeiQLEhMvzv@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <20260929142910.47480-4-thomas.hellstrom@linux.intel.com>
On Tue, Sep 29, 2026 at 04:29:10PM +0200, Thomas Hellström wrote:
> execlist_exec_queue_destroy() defers the final teardown of a struct
> xe_exec_queue (which calls drm_sched_fini()) to a work item queued on
> system_dfl_wq, a global, kernel-wide workqueue that xe has no control
> over and never waits on during module unload.
>
> For exec queues without an associated xe_vm (e.g. GT-internal kernel
> queues, the GSC and OA kernel queues, and one PXP kernel queue), there
> is nothing else keeping the xe_device, and hence the module, alive
> while this work is queued or running: rmmod xe could succeed and free
> the module's text while execlist_exec_queue_destroy_async() is still
> queued or running on system_dfl_wq, jumping into freed code from
> drm_sched_fini() or the exec queue's own cleanup.
>
> Fix this the same way GuC exec queue teardown already does it: take an
> explicit drm_device reference across the deferred teardown, and queue
> the work on xe_destroy_wq instead of system_dfl_wq, the module-lifetime
> workqueue that is guaranteed to be drained by
> xe_destroy_wq_module_exit() before the module can unload. Taking the
> reference explicitly, rather than relying on whatever reference an
> associated xe_vm may or may not be holding, keeps this exec queue's
> teardown self-contained and correct regardless of vm lifetime.
>
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Side note - I'm planning on adding a CPU bind backend, once we that we
will have two functional queue backends and I think we can finally nuke
the execlist backend because at this point it just creates noise in
reviews from Sashiko.
Matt
> Assisted-by: LLM
> ---
> drivers/gpu/drm/xe/xe_execlist.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_execlist.c b/drivers/gpu/drm/xe/xe_execlist.c
> index a36db39dcda8..a6984999962d 100644
> --- a/drivers/gpu/drm/xe/xe_execlist.c
> +++ b/drivers/gpu/drm/xe/xe_execlist.c
> @@ -5,6 +5,7 @@
>
> #include "xe_execlist.h"
>
> +#include <drm/drm_drv.h>
> #include <drm/drm_managed.h>
>
> #include "instructions/xe_mi_commands.h"
> @@ -21,6 +22,7 @@
> #include "xe_macros.h"
> #include "xe_mmio.h"
> #include "xe_mocs.h"
> +#include "xe_module.h"
> #include "xe_ring_ops_types.h"
> #include "xe_sched_job.h"
>
> @@ -402,6 +404,7 @@ static void execlist_exec_queue_destroy_async(struct work_struct *w)
> spin_unlock_irqrestore(&exl->port->lock, flags);
>
> xe_exec_queue_fini(q);
> + drm_dev_put(&xe->drm);
> }
>
> static void execlist_exec_queue_kill(struct xe_exec_queue *q)
> @@ -411,8 +414,9 @@ static void execlist_exec_queue_kill(struct xe_exec_queue *q)
>
> static void execlist_exec_queue_destroy(struct xe_exec_queue *q)
> {
> + drm_dev_get(>_to_xe(q->gt)->drm);
> INIT_WORK(&q->execlist->destroy_async, execlist_exec_queue_destroy_async);
> - queue_work(system_dfl_wq, &q->execlist->destroy_async);
> + xe_destroy_wq_queue(&q->execlist->destroy_async);
> }
>
> static int execlist_exec_queue_set_priority(struct xe_exec_queue *q,
> --
> 2.55.0
>
next prev parent reply other threads:[~2026-09-29 16:19 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-29 14:29 [PATCH v4 0/3] drm/xe: Protect against premature module unloads Thomas Hellström
2026-09-29 14:29 ` [PATCH v4 1/3] drm/xe: Don't unload the driver until all drm devices are freed Thomas Hellström
2026-09-29 14:41 ` sashiko-bot
2026-09-29 14:51 ` Thomas Hellström
2026-09-29 14:29 ` [PATCH v4 2/3] drm/xe: Route deferred xe_vma/xe_vm teardown off system_dfl_wq Thomas Hellström
2026-09-29 14:29 ` [PATCH v4 3/3] drm/xe: Route execlist exec queue " Thomas Hellström
2026-09-29 16:18 ` Matthew Brost [this message]
2026-09-29 14:38 ` ✓ CI.KUnit: success for drm/xe: Protect against premature module unloads Patchwork
2026-09-29 16:04 ` ✓ Xe.CI.BAT: " Patchwork
2026-09-29 18:14 ` ✗ Xe.CI.FULL: failure " Patchwork
2026-10-01 10:06 ` Thomas Hellström
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=arvk6KeiQLEhMvzv@gsse-cloud1.jf.intel.com \
--to=matthew.brost@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=matthew.auld@intel.com \
--cc=rodrigo.vivi@intel.com \
--cc=thomas.hellstrom@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.