Re: [PATCH 2/8] drm/xe: covert sysfs over to devm

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Apr 29, 2024 at 01:14:38PM +0100, Matthew Auld wrote:
> Hotunplugging the device seems to result in stuff like:
> 
> kobject_add_internal failed for tile0 with -EEXIST, don't try to
> register things with the same name in the same directory.
> 
> We only remove the sysfs as part of drmm, however that is tied to the
> lifetime of the driver instance and not the device underneath. Attempt
> to fix by using devm for all of the remaining sysfs stuff related to the
> device.

hmmm... so basically we should use the drmm only for the global module
stuff and the devm for things that are per device?

> 
> Closes: https://gitlab.freedesktop.org/dt srm/xe/kernel/-/issues/1667
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_gt_ccs_mode.c           |  4 ++--
>  drivers/gpu/drm/xe/xe_gt_freq.c               |  4 ++--
>  drivers/gpu/drm/xe/xe_gt_idle.c               |  4 ++--
>  drivers/gpu/drm/xe/xe_gt_sysfs.c              |  4 ++--
>  drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c     |  4 ++--
>  drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c | 12 ++++++------
>  drivers/gpu/drm/xe/xe_tile_sysfs.c            |  4 ++--
>  drivers/gpu/drm/xe/xe_vram_freq.c             |  4 ++--
>  8 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_ccs_mode.c b/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
> index 396aeb5b9924..890da8870b0d 100644
> --- a/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
> +++ b/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
> @@ -150,7 +150,7 @@ static const struct attribute *gt_ccs_mode_attrs[] = {
>  	NULL,
>  };
>  
> -static void xe_gt_ccs_mode_sysfs_fini(struct drm_device *drm, void *arg)
> +static void xe_gt_ccs_mode_sysfs_fini(void *arg)
>  {
>  	struct xe_gt *gt = arg;
>  
> @@ -182,5 +182,5 @@ int xe_gt_ccs_mode_sysfs_init(struct xe_gt *gt)
>  	if (err)
>  		return err;
>  
> -	return drmm_add_action_or_reset(&xe->drm, xe_gt_ccs_mode_sysfs_fini, gt);
> +	return devm_add_action_or_reset(xe->drm.dev, xe_gt_ccs_mode_sysfs_fini, gt);
>  }
> diff --git a/drivers/gpu/drm/xe/xe_gt_freq.c b/drivers/gpu/drm/xe/xe_gt_freq.c
> index 855de40e40ea..e0305942644c 100644
> --- a/drivers/gpu/drm/xe/xe_gt_freq.c
> +++ b/drivers/gpu/drm/xe/xe_gt_freq.c
> @@ -209,7 +209,7 @@ static const struct attribute *freq_attrs[] = {
>  	NULL
>  };
>  
> -static void freq_fini(struct drm_device *drm, void *arg)
> +static void freq_fini(void *arg)
>  {
>  	struct kobject *kobj = arg;
>  
> @@ -237,7 +237,7 @@ int xe_gt_freq_init(struct xe_gt *gt)
>  	if (!gt->freq)
>  		return -ENOMEM;
>  
> -	err = drmm_add_action_or_reset(&xe->drm, freq_fini, gt->freq);
> +	err = devm_add_action(xe->drm.dev, freq_fini, gt->freq);
>  	if (err)
>  		return err;
>  
> diff --git a/drivers/gpu/drm/xe/xe_gt_idle.c b/drivers/gpu/drm/xe/xe_gt_idle.c
> index 8fc0f3f6ecc5..9e907033f32b 100644
> --- a/drivers/gpu/drm/xe/xe_gt_idle.c
> +++ b/drivers/gpu/drm/xe/xe_gt_idle.c
> @@ -144,7 +144,7 @@ static const struct attribute *gt_idle_attrs[] = {
>  	NULL,
>  };
>  
> -static void gt_idle_sysfs_fini(struct drm_device *drm, void *arg)
> +static void gt_idle_sysfs_fini(void *arg)
>  {
>  	struct kobject *kobj = arg;
>  
> @@ -181,7 +181,7 @@ int xe_gt_idle_sysfs_init(struct xe_gt_idle *gtidle)
>  		return err;
>  	}
>  
> -	return drmm_add_action_or_reset(&xe->drm, gt_idle_sysfs_fini, kobj);
> +	return devm_add_action_or_reset(xe->drm.dev, gt_idle_sysfs_fini, kobj);
>  }
>  
>  void xe_gt_idle_enable_c6(struct xe_gt *gt)
> diff --git a/drivers/gpu/drm/xe/xe_gt_sysfs.c b/drivers/gpu/drm/xe/xe_gt_sysfs.c
> index 1e5971072bc8..a05c3699e8b9 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sysfs.c
> @@ -22,7 +22,7 @@ static const struct kobj_type xe_gt_sysfs_kobj_type = {
>  	.sysfs_ops = &kobj_sysfs_ops,
>  };
>  
> -static void gt_sysfs_fini(struct drm_device *drm, void *arg)
> +static void gt_sysfs_fini(void *arg)
>  {
>  	struct xe_gt *gt = arg;
>  
> @@ -51,5 +51,5 @@ int xe_gt_sysfs_init(struct xe_gt *gt)
>  
>  	gt->sysfs = &kg->base;
>  
> -	return drmm_add_action_or_reset(&xe->drm, gt_sysfs_fini, gt);
> +	return devm_add_action(xe->drm.dev, gt_sysfs_fini, gt);
>  }
> diff --git a/drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c b/drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c
> index fbe21a8599ca..c9e04151286d 100644
> --- a/drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c
> @@ -229,7 +229,7 @@ static const struct attribute_group throttle_group_attrs = {
>  	.attrs = throttle_attrs,
>  };
>  
> -static void gt_throttle_sysfs_fini(struct drm_device *drm, void *arg)
> +static void gt_throttle_sysfs_fini(void *arg)
>  {
>  	struct xe_gt *gt = arg;
>  
> @@ -245,5 +245,5 @@ int xe_gt_throttle_sysfs_init(struct xe_gt *gt)
>  	if (err)
>  		return err;
>  
> -	return drmm_add_action_or_reset(&xe->drm, gt_throttle_sysfs_fini, gt);
> +	return devm_add_action_or_reset(xe->drm.dev, gt_throttle_sysfs_fini, gt);
>  }
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c b/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c
> index 844ec68cbbb8..258078a6b461 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c
> @@ -492,7 +492,7 @@ static const struct attribute * const files[] = {
>  	NULL
>  };
>  
> -static void kobj_xe_hw_engine_class_fini(struct drm_device *drm, void *arg)
> +static void kobj_xe_hw_engine_class_fini(void *arg)
>  {
>  	struct kobject *kobj = arg;
>  
> @@ -517,7 +517,7 @@ kobj_xe_hw_engine_class(struct xe_device *xe, struct kobject *parent, const char
>  	}
>  	keclass->xe = xe;
>  
> -	err = drmm_add_action_or_reset(&xe->drm, kobj_xe_hw_engine_class_fini,
> +	err = devm_add_action_or_reset(xe->drm.dev, kobj_xe_hw_engine_class_fini,
>  				       &keclass->base);
>  	if (err)
>  		return NULL;
> @@ -525,7 +525,7 @@ kobj_xe_hw_engine_class(struct xe_device *xe, struct kobject *parent, const char
>  	return keclass;
>  }
>  
> -static void hw_engine_class_defaults_fini(struct drm_device *drm, void *arg)
> +static void hw_engine_class_defaults_fini(void *arg)
>  {
>  	struct kobject *kobj = arg;
>  
> @@ -552,7 +552,7 @@ static int xe_add_hw_engine_class_defaults(struct xe_device *xe,
>  	if (err)
>  		goto err_object;
>  
> -	return drmm_add_action_or_reset(&xe->drm, hw_engine_class_defaults_fini, kobj);
> +	return devm_add_action_or_reset(xe->drm.dev, hw_engine_class_defaults_fini, kobj);
>  
>  err_object:
>  	kobject_put(kobj);
> @@ -611,7 +611,7 @@ static const struct kobj_type xe_hw_engine_sysfs_kobj_type = {
>  	.sysfs_ops = &xe_hw_engine_class_sysfs_ops,
>  };
>  
> -static void hw_engine_class_sysfs_fini(struct drm_device *drm, void *arg)
> +static void hw_engine_class_sysfs_fini(void *arg)
>  {
>  	struct kobject *kobj = arg;
>  
> @@ -698,7 +698,7 @@ int xe_hw_engine_class_sysfs_init(struct xe_gt *gt)
>  			goto err_object;
>  	}
>  
> -	return drmm_add_action_or_reset(&xe->drm, hw_engine_class_sysfs_fini, kobj);
> +	return devm_add_action_or_reset(xe->drm.dev, hw_engine_class_sysfs_fini, kobj);
>  
>  err_object:
>  	kobject_put(kobj);
> diff --git a/drivers/gpu/drm/xe/xe_tile_sysfs.c b/drivers/gpu/drm/xe/xe_tile_sysfs.c
> index 64661403afcd..b804234a6551 100644
> --- a/drivers/gpu/drm/xe/xe_tile_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_tile_sysfs.c
> @@ -22,7 +22,7 @@ static const struct kobj_type xe_tile_sysfs_kobj_type = {
>  	.sysfs_ops = &kobj_sysfs_ops,
>  };
>  
> -static void tile_sysfs_fini(struct drm_device *drm, void *arg)
> +static void tile_sysfs_fini(void *arg)
>  {
>  	struct xe_tile *tile = arg;
>  
> @@ -55,5 +55,5 @@ int xe_tile_sysfs_init(struct xe_tile *tile)
>  	if (err)
>  		return err;
>  
> -	return drmm_add_action_or_reset(&xe->drm, tile_sysfs_fini, tile);
> +	return devm_add_action_or_reset(xe->drm.dev, tile_sysfs_fini, tile);
>  }
> diff --git a/drivers/gpu/drm/xe/xe_vram_freq.c b/drivers/gpu/drm/xe/xe_vram_freq.c
> index 3e21ddc6e60c..99ff95e408e0 100644
> --- a/drivers/gpu/drm/xe/xe_vram_freq.c
> +++ b/drivers/gpu/drm/xe/xe_vram_freq.c
> @@ -87,7 +87,7 @@ static const struct attribute_group freq_group_attrs = {
>  	.attrs = freq_attrs,
>  };
>  
> -static void vram_freq_sysfs_fini(struct drm_device *drm, void *arg)
> +static void vram_freq_sysfs_fini(void *arg)
>  {
>  	struct kobject *kobj = arg;
>  
> @@ -122,5 +122,5 @@ int xe_vram_freq_sysfs_init(struct xe_tile *tile)
>  		return err;
>  	}
>  
> -	return drmm_add_action_or_reset(&xe->drm, vram_freq_sysfs_fini, kobj);
> +	return devm_add_action_or_reset(xe->drm.dev, vram_freq_sysfs_fini, kobj);
>  }
> -- 
> 2.44.0
>