Re: [PATCH v4 1/3] drm/xe: Don't unload the driver until all drm devices are freed

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] xe: module unload race causing instruction page fault in xe_device_destroy()
--

commit 4a4203d8cd4165e1a99345416f131e7eafe530d1
Author: Thomas Hellström <thomas.hellstrom@linux.intel.com>

drm/xe: Don't unload the driver until all drm devices are freed

This commit introduces a device count and a module exit hook to wait
until all bare drm_device references are freed before unloading the xe
module. It uses a 20-second timeout with wait_var_event_timeout() to
block rmmod from unloading while devices remain active.

> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c

[ ... ]

> @@ -391,6 +394,9 @@ static void xe_device_destroy(struct drm_device *dev, void *dummy)
>  		destroy_workqueue(xe->destroy_wq);
>  
>  	ttm_device_fini(&xe->ttm);
> +
> +	if (atomic_dec_and_test(&xe_device_count))
> +		wake_up_var(&xe_device_count);

[Severity: High]
Does this wake up the rmmod thread while still executing inside module text?
If the thread executing xe_device_destroy() is preempted immediately after
wake_up_var(), rmmod can complete the module exit sequence and unmap the
xe module text. When the preempted thread resumes, it might attempt to
execute the function epilogue in unmapped memory, potentially causing an
instruction page fault.

>  }
>  
>  /**

[ ... ]

> @@ -1501,3 +1508,25 @@ struct xe_vm *xe_device_asid_to_vm(struct xe_device *xe, u32 asid)
>  
>  	return vm;
>  }
> +
> +/**
> + * xe_device_exit() - Device subsystem exit function.
> + *
> + * Exit function to be called at module unload time.
> + */
> +void xe_device_exit(void)
> +{
> +	/*
> +	 * Wait for all devices to be freed. 20s is well above the typical
> +	 * maximum dma_fence signalling time, so warn and keep waiting if
> +	 * we're still not done by then, since it may indicate a leaked
> +	 * xe_device reference is stalling module unload.
> +	 */
> +	if (!wait_var_event_timeout(&xe_device_count,
> +				    !atomic_read(&xe_device_count),
> +				    HZ * 20)) {

[Severity: High]
Is there a risk that the rmmod thread waiting here proceeds to exit and unmap
the module before the waker has safely exited xe_device_destroy()? Once
awoken, this thread is unaware that the waker is still executing in module
text.

> +		pr_warn("%s: Waiting for %d xe device(s) to be freed before unloading.\n",
> +			DRIVER_NAME, atomic_read(&xe_device_count));
> +		wait_var_event(&xe_device_count, !atomic_read(&xe_device_count));
> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929142910.47480-1-thomas.hellstrom@linux.intel.com?part=1