Re: [PATCH 1/8] drm/xe/device_sysfs: switch over to devm

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

On 29.04.2024 14:14, Matthew Auld wrote:
> When hotunplugging the device we currently hit:
> 
> kernfs: can not remove 'vram_d3cold_threshold', no directory
> 
> Due to directory already being removed. Using drmm for sysfs device
> stuff like vram_d3cold_threshold doesn't really make sense when you
> consider hot unplugging of the pci device. In such a case the device is
> removed however the drmm release action will only fire once the driver
> instance can be released, which can be long after the device has already
> been hotunplugged. Rather use devm which is directly tied to the device
> and should be called when it is removed.
> 
> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1432
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

I am not sure why this patch is separate from the next one, IMO they 
could be merged.

Apparently I have started working on similar issue [1], anyway I was 
later and took care only of sysfs stuff, thx Lucas for response.

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

[1]: https://patchwork.freedesktop.org/series/133080/

Regards
Andrzej

> ---
>   drivers/gpu/drm/xe/xe_device_sysfs.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/xe/xe_device_sysfs.c
> index 21677b8cd977..7375937934fa 100644
> --- a/drivers/gpu/drm/xe/xe_device_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
> @@ -69,7 +69,7 @@ vram_d3cold_threshold_store(struct device *dev, struct device_attribute *attr,
>   
>   static DEVICE_ATTR_RW(vram_d3cold_threshold);
>   
> -static void xe_device_sysfs_fini(struct drm_device *drm, void *arg)
> +static void xe_device_sysfs_fini(void *arg)
>   {
>   	struct xe_device *xe = arg;
>   
> @@ -85,5 +85,5 @@ int xe_device_sysfs_init(struct xe_device *xe)
>   	if (ret)
>   		return ret;
>   
> -	return drmm_add_action_or_reset(&xe->drm, xe_device_sysfs_fini, xe);
> +	return devm_add_action_or_reset(dev, xe_device_sysfs_fini, xe);
>   }