Re: [PATCH v3 1/2] drm/xe/xe_device_sysfs: Add cleanup action in xe_device_sysfs_init

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

Hi,

I guess you need to modify the subject to something like:

"drm/xe/sysfs: ..."

On 9/15/2025 11:47 PM, Zongyao Bai wrote:
> xe_device_sysfs_init(): on partial failure, any sysfs files created
> before the failure will be cleaned up via xe_device_sysfs_fini,
> ensuring no leaked sysfs entries.

IMO this should be rephrased into something like:

"On partial failure, some sysfs files created before the failure might
not be removed. Add common cleanup step to remove them all immediately,
as is should be harmless to attempt to remove non-existing files."

see https://docs.kernel.org/process/submitting-patches.html#describe-your-changes

> 
> v2: Using "goto cleanup" to reduced code duplication.(Stuart)
> 
> v3: Split patch & remove unnecessary notes (Lucas)
>     Adjust the comments accordingly.
> 
> Fixes: 0e414bf7ad01 ("drm/xe: Expose PCIe link downgrade attributes")
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Stuart Summers <stuart.summers@intel.com>
> Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Zongyao Bai <zongyao.bai@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device_sysfs.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/xe/xe_device_sysfs.c
> index 6ee422594b56..e5e3c56ea53c 100644
> --- a/drivers/gpu/drm/xe/xe_device_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
> @@ -311,12 +311,17 @@ int xe_device_sysfs_init(struct xe_device *xe)
>  	if (xe->info.platform == XE_BATTLEMAGE) {
>  		ret = sysfs_create_files(&dev->kobj, auto_link_downgrade_attrs);
>  		if (ret)
> -			return ret;
> +			goto cleanup;
>  
>  		ret = late_bind_create_files(dev);
>  		if (ret)
> -			return ret;
> +			goto cleanup;
>  	}
>  
>  	return devm_add_action_or_reset(dev, xe_device_sysfs_fini, xe);
> +
> +cleanup:
> +	xe_device_sysfs_fini(xe);
> +	return ret;
> +
>  }