Re: [PATCH 03/20] drm/xe/pci: remove broken driver_release

[Andrzej Hajda <andrzej.hajda@intel.com>]

On 10.05.2024 20:12, Matthew Auld wrote:
> This is quite broken since we are nuking the pdev link to the private
> driver struct, but note here that driver_release is called when the
> drm_device is released (poor mans drmm), which can be long after the
> device has been removed. So here what we are actually doing is nuking
> the pdev link for what is potentially bound to a different drm_device.
> If that happens before our pci remove callback is triggered (for the new
> drm_device) we silently exit and skip some important cleanup steps,
> resulting in hilarity.
> 
> There should be no reason to implement driver_release, when we already
> have nicer stuff like drmm, so just remove completely. The actual pdev
> link is already nuked when removing the device.
> 
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>

Regards
Andrzej
> ---
>   drivers/gpu/drm/xe/xe_device.c | 9 ---------
>   1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index f89c781986a5..d6f9880a15fd 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -188,13 +188,6 @@ static const struct file_operations xe_driver_fops = {
>   #endif
>   };
>   
> -static void xe_driver_release(struct drm_device *dev)
> -{
> -	struct xe_device *xe = to_xe_device(dev);
> -
> -	pci_set_drvdata(to_pci_dev(xe->drm.dev), NULL);
> -}
> -
>   static struct drm_driver driver = {
>   	/* Don't use MTRRs here; the Xserver or userspace app should
>   	 * deal with them for Intel hardware.
> @@ -213,8 +206,6 @@ static struct drm_driver driver = {
>   #ifdef CONFIG_PROC_FS
>   	.show_fdinfo = xe_drm_client_fdinfo,
>   #endif
> -	.release = &xe_driver_release,
> -
>   	.ioctls = xe_ioctls,
>   	.num_ioctls = ARRAY_SIZE(xe_ioctls),
>   	.fops = &xe_driver_fops,