Re: [PATCH 1/5] drm/xe: Hold a device reference across deferred VM destruction

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

On Wed, Sep 16, 2026 at 03:23:33PM +0530, Arvind Yadav wrote:
> xe_vm_free() is the drm_gpuvm vm_free callback. It hands the final
> teardown to vm_destroy_work_func() on a workqueue and returns.
> 
> drm_gpuvm_free() drops its device reference immediately after the
> callback returns:
> 
> 	gpuvm->ops->vm_free(gpuvm);
> 	drm_dev_put(drm);
> 
> vm_destroy_work_func() then keeps using device state: xe_pm_runtime_put()
> for an LR mode VM, ttm_lru_bulk_move_fini() on xe->ttm, and the tile
> iteration. If the freed VM held the last device reference, the work runs
> against a released xe_device.
> 
> Take a device reference in xe_vm_free() and drop it once
> vm_destroy_work_func() has finished using the device.
> 
> Cc: Matthew Brost <matthew.brost@intel.com>

This is a fix, IMO. Ideally, we should probably push the delayed-destroy
semantics into gpuvm if they are really needed. I'm also questioning
whether the VM destroy worker is actually required. This dates back to
the very early days of Xe, and I doubt we've ever revisited whether it
is necessary.

Let's follow up with one of the following:
- Introduce async destroy in gpuvm and have it own the drm_dev_get/put.
- Drop delayed destroy entirely in Xe.

As a temporary fix that can be backported, this looks good to me, so
with a Fixes tag:

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

> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_vm.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index efa5ff6cc823..264bdab75de2 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -2054,12 +2054,21 @@ static void vm_destroy_work_func(struct work_struct *w)
>  		xe_file_put(vm->xef);
>  
>  	kfree(vm);
> +
> +	drm_dev_put(&xe->drm);
>  }
>  
>  static void xe_vm_free(struct drm_gpuvm *gpuvm)
>  {
>  	struct xe_vm *vm = container_of(gpuvm, struct xe_vm, gpuvm);
>  
> +	/*
> +	 * drm_gpuvm drops its device reference as soon as this callback
> +	 * returns, but vm_destroy_work_func() still uses device state. Hold a
> +	 * reference across the deferred work.
> +	 */
> +	drm_dev_get(&vm->xe->drm);
> +
>  	/* To destroy the VM we need to be able to sleep */
>  	queue_work(system_dfl_wq, &vm->destroy_work);
>  }
> -- 
> 2.43.0
>