On Wed, Feb 26, 2025 at 10:52:56AM +0800, Shixiong Ou wrote:
>
> 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.
You are right, I stand corrected.
Acked-by: Liviu Dudau <liviu.dudau@xxxxxxx>
Best regards,
Liviu
>
>
> 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@xxxxxxx wrote:
> > > From: Shixiong Ou<oushixiong@xxxxxxxxxx>
> > >
> > > [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@xxxxxxxxxx>
> > > ---
> > > 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
> > >
--
Everyone who uses computers frequently has had, from time to time,
a mad desire to attack the precocious abacus with an axe.
-- John D. Clark, Ignition!
