From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0C34EC021BE for ; Thu, 27 Feb 2025 10:59:48 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 859E910E26D; Thu, 27 Feb 2025 10:59:47 +0000 (UTC) Received: from smtp.dudau.co.uk (dliviu.plus.com [80.229.23.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 32CCB10E26D for ; Thu, 27 Feb 2025 10:59:44 +0000 (UTC) Received: from mail.dudau.co.uk (bart.dudau.co.uk [192.168.14.2]) by smtp.dudau.co.uk (Postfix) with SMTP id F2A5651E5202; Thu, 27 Feb 2025 10:59:41 +0000 (GMT) Received: by mail.dudau.co.uk (sSMTP sendmail emulation); Thu, 27 Feb 2025 10:59:41 +0000 Date: Thu, 27 Feb 2025 10:59:41 +0000 From: Liviu Dudau To: Shixiong Ou Cc: Liviu Dudau , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Shixiong Ou Subject: Re: [PATCH] drm/arm/komeda: Add a condition check before removing sysfs attribute Message-ID: References: <20250220085358.232883-1-oushixiong1025@163.com> <9ec1ac6c-903e-9605-e8ad-3e555db4625c@163.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <9ec1ac6c-903e-9605-e8ad-3e555db4625c@163.com> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" 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 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@163.com wrote: > > > From: Shixiong Ou > > > > > > [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 > > > --- > > > 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!