Re: [PATCH] drm/arm/komeda: Add a condition check before removing sysfs attribute

[Shixiong Ou <oushixiong1025@163.com>]

[-- Attachment #1: Type: text/plain, Size: 2785 bytes --]


Hello,

In my opinion, the corresponding error handling has already been 
implemented in
sysfs_create_group(), so we do not need to call sysfs_remove_group() if 
sysfs_create_group() fails.


Thanks and Regards,
Shixiong Ou.


在 2025/2/25 18:28, Liviu Dudau 写道:
> Hello,
>
> Replying from my personal email as the corporate system seems to have blackholed your emails
> while I was on holiday.
>
> Can you tell me more why you think that if sysfs_create_group() fails we should not call
> sysfs_remove_group()? After all, we don't know how far sysfs_create_group() has progressed before
> it encountered an error, so we still need to do some cleanup.
>
> Best regards,
> Liviu
>
> On Thu, Feb 20, 2025 at 04:53:58PM +0800,oushixiong1025@163.com  wrote:
>> From: Shixiong Ou<oushixiong@kylinos.cn>
>>
>> [WHY] If the call to sysfs_create_group() fails, there is
>>        no need to call function sysfs_remove_group().
>>
>> [HOW] Add a condition check before removing sysfs attribute.
>>
>> Signed-off-by: Shixiong Ou<oushixiong@kylinos.cn>
>> ---
>>   drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 7 ++++++-
>>   drivers/gpu/drm/arm/display/komeda/komeda_dev.h | 2 ++
>>   2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
>> index 5ba62e637a61..7d646f978640 100644
>> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
>> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
>> @@ -259,6 +259,8 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
>>   		goto err_cleanup;
>>   	}
>>   
>> +	mdev->sysfs_attr_enabled = true;
>> +
>>   	mdev->err_verbosity = KOMEDA_DEV_PRINT_ERR_EVENTS;
>>   
>>   	komeda_debugfs_init(mdev);
>> @@ -278,7 +280,10 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
>>   	const struct komeda_dev_funcs *funcs = mdev->funcs;
>>   	int i;
>>   
>> -	sysfs_remove_group(&dev->kobj, &komeda_sysfs_attr_group);
>> +	if (mdev->sysfs_attr_enabled) {
>> +		sysfs_remove_group(&dev->kobj, &komeda_sysfs_attr_group);
>> +		mdev->sysfs_attr_enabled = false;
>> +	}
>>   
>>   	debugfs_remove_recursive(mdev->debugfs_root);
>>   
>> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
>> index 5b536f0cb548..af087540325c 100644
>> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
>> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
>> @@ -216,6 +216,8 @@ struct komeda_dev {
>>   #define KOMEDA_DEV_PRINT_DUMP_STATE_ON_EVENT BIT(8)
>>   	/* Disable rate limiting of event prints (normally one per commit) */
>>   #define KOMEDA_DEV_PRINT_DISABLE_RATELIMIT BIT(12)
>> +
>> +	bool sysfs_attr_enabled;
>>   };
>>   
>>   static inline bool
>> -- 
>> 2.17.1
>>

[-- Attachment #2: Type: text/html, Size: 3692 bytes --]