Re: [PATCH 1/3] CI_ONLY: Hold a device reference across deferred VM

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] drm/xe: self-deadlock in vm_destroy_work_func() on
  xe->destroy_wq
--

commit a82e5f7cac31a25d1e51cdc2f9e9c8bed2db07a4
Author: Arvind Yadav <arvind.yadav@intel.com>

CI_ONLY: Hold a device reference across deferred VM

This commit acquires a drm_device reference in xe_vm_free() and releases
it at the end of vm_destroy_work_func() so the device remains valid across
deferred VM destruction.

> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index dd855876f571c..d94a27db2cf40 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -2109,6 +2109,8 @@ 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)
> @@ -2116,10 +2118,13 @@ 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.
> -	 * drm_gpuvm keeps at least one device reference at this
> -	 * point so xe->destroy_wq must still be alive.
> +	 * 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(vm->xe->destroy_wq, &vm->destroy_work);
>  }

[Severity: High]
Can dropping the last drm_device reference in vm_destroy_work_func()
self-deadlock on xe->destroy_wq?

When the freed VM holds the final device reference (for example, after PCI
device unbind), drm_gpuvm_free() drops its reference as soon as
xe_vm_free() returns, leaving the reference taken here as the last one.
When vm_destroy_work_func() runs on xe->destroy_wq and calls
drm_dev_put(&xe->drm), it synchronously triggers drm_dev_release() ->
drm_managed_release() -> xe_device_destroy():

drivers/gpu/drm/xe/xe_device.c:xe_device_destroy() {
	...
	if (xe->destroy_wq)
		destroy_workqueue(xe->destroy_wq);
	...
}

destroy_workqueue(xe->destroy_wq) then tries to drain xe->destroy_wq from
inside vm_destroy_work_func() running on that same workqueue, triggering a
lockdep recursive-locking warning and hanging forever waiting for the
current work item to finish.

As done in vma_destroy_cb(), vm->destroy_work should be queued on the
module-wide destroy workqueue via xe_destroy_wq_queue(&vm->destroy_work);
a later patch in this series, commit "CI-ONLY:Defer VM teardown until exec
queue cleanup", makes that change, which is needed here to avoid breaking
bisectability.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112150302.3837896-1-arvind.yadav@intel.com?part=1