On Fri, Sep 25, 2026 at 12:56:43PM -0700, Matthew Brost wrote: > On Fri, Sep 25, 2026 at 03:33:35PM +0200, Thomas Hellström wrote: > > xe_vma_destroy() can defer the final teardown of a struct xe_vma to a > > dma_fence completion callback (vma_destroy_cb()), and xe_vm_free() (the > > drm_gpuvm_ops.vm_free callback) always defers struct xe_vm teardown to > > a work item, since destroying a VM needs to sleep. Both used to queue > > their work on system_dfl_wq, a global, kernel-wide workqueue that xe > > has no control over and never waits on during module unload. > > > > drm_gpuvm_free() drops its drm_device reference immediately after > > calling xe_vm_free(), without waiting for the deferred work to run. > > The same applies one level down: whichever xe_vma or xe_vm reference > > happens to be the last one can trigger this chain from a dma_fence > > callback that may fire at an arbitrary time, including after the > > owning file has already been closed and its own module reference > > dropped. Since nothing tracks or waits for work queued on > > system_dfl_wq, `rmmod xe` could succeed and free the module's text > > while vma_destroy_work_func() or vm_destroy_work_func() is still > > queued or running on it, jumping into freed code. > > > > Fix this by queueing this work on xe_destroy_wq instead, the existing > > module-lifetime workqueue already used for GuC exec queue teardown. > > Unlike a per-device workqueue, this requires no dereference of a > > struct xe_device that may already be gone by the time a deferred > > callback fires, and unlike system_dfl_wq it is guaranteed to be > > drained by xe_destroy_wq_module_exit() before the module is unloaded, > > following the drm_pagemap_dev_hold()/unhold_work precedent of using a > > workqueue that is waited on at module unload rather than a bare module > > reference. The previous commit's reordering of xe_destroy_wq_exit() > > to run after xe_device_exit() guarantees that xe_destroy_wq is only > > torn down once the device-count has reached zero, i.e. after any > > xe_vma or xe_vm whose teardown queues work here has already dropped > > its drm_device reference and thus already queued that work. > > > > Signed-off-by: Thomas Hellström <[email protected]> > > Assisted-by: LLM > > Reviewed-by: Matthew Brost <[email protected]> > > --- > > drivers/gpu/drm/xe/xe_module.c | 6 ++++-- > > drivers/gpu/drm/xe/xe_vm.c | 5 +++-- > > 2 files changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c > > index c61bd33546f2..897724cb5cfb 100644 > > --- a/drivers/gpu/drm/xe/xe_module.c > > +++ b/drivers/gpu/drm/xe/xe_module.c > > @@ -114,8 +114,10 @@ static void xe_destroy_wq_module_exit(void) > > * xe_destroy_wq_queue() - Queue work on the destroy workqueue > > * @work: work item to queue > > * > > - * The destroy workqueue has module lifetime and is used for GuC exec queue > > - * teardown that can outlive a single xe_device. SVM pagemap destroy uses > > the > > + * The destroy workqueue has module lifetime, and is guaranteed to outlive > > + * any xe_device, and to be drained before the module is unloaded. It is > > used > > + * for GuC exec queue and xe_vm/xe_vma teardown that can be deferred past > > the > > + * lifetime of the xe_device that triggered it. SVM pagemap destroy uses > > the > > * per-device xe->destroy_wq instead. > > * > > * Return: %true if @work was queued, %false if it was already pending. > > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c > > index 390da884c727..ee369e6c3b28 100644 > > --- a/drivers/gpu/drm/xe/xe_vm.c > > +++ b/drivers/gpu/drm/xe/xe_vm.c > > @@ -29,6 +29,7 @@ > > #include "xe_exec_queue.h" > > #include "xe_gt.h" > > #include "xe_migrate.h" > > +#include "xe_module.h" > > #include "xe_pagefault.h" > > #include "xe_pat.h" > > #include "xe_pm.h" > > @@ -1249,7 +1250,7 @@ static void vma_destroy_cb(struct dma_fence *fence, > > struct xe_vma *vma = container_of(cb, struct xe_vma, destroy_cb); > > > > INIT_WORK(&vma->destroy_work, vma_destroy_work_func); > > - queue_work(system_dfl_wq, &vma->destroy_work); > > + xe_destroy_wq_queue(&vma->destroy_work); > > } > > > > static void xe_vm_assert_write_mode_or_garbage_collector(struct xe_vm *vm) > > @@ -2059,7 +2060,7 @@ static void xe_vm_free(struct drm_gpuvm *gpuvm) > > struct xe_vm *vm = container_of(gpuvm, struct xe_vm, gpuvm); > > > > /* To destroy the VM we need to be able to sleep */ > > - queue_work(system_dfl_wq, &vm->destroy_work); > > + xe_destroy_wq_queue(&vm->destroy_work); > > Actually this is still unsafe, right? > > destroy_work touches vm->xe which could be gone after gpuvm drops > potentially the final drm_dev_put, right? >
Ignore this - we flush this queue before destorying any device. Matt > Matt > > > } > > > > struct xe_vm *xe_vm_lookup(struct xe_file *xef, u32 id) > > -- > > 2.55.0 > >
