Re: [PATCH v2 3/3] drm/xe: Route deferred xe_vma/xe_vm teardown off system_dfl_wq

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

On Thu, Sep 24, 2026 at 10:04:55AM +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 <thomas.hellstrom@linux.intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> Assisted-by: LLM
> ---
>  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);
>  }
>  
>  struct xe_vm *xe_vm_lookup(struct xe_file *xef, u32 id)
> -- 
> 2.55.0
>