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

[Shixiong Ou]

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,oushixiong1...@163.com  wrote:
From: Shixiong Ou<oushixi...@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<oushixi...@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