linux-kernel - Re: [PATCH v2 03/11] gpio: sysfs: Remove redundant check for struct gpio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMRc=Mf=gUMEaeExH6mVrhMSNWbCchRfkrugr492Ccp-j3V8Ag@mail.gmail.com>
Date: Wed, 4 Feb 2026 04:33:50 -0600
From: Bartosz Golaszewski <brgl@...nel.org>
To: Tzung-Bi Shih <tzungbi@...nel.org>
Cc: Jonathan Corbet <corbet@....net>, Shuah Khan <shuah@...nel.org>, 
	Laurent Pinchart <laurent.pinchart@...asonboard.com>, 
	Wolfram Sang <wsa+renesas@...g-engineering.com>, Jason Gunthorpe <jgg@...dia.com>, 
	Johan Hovold <johan@...nel.org>, linux-kernel@...r.kernel.org, 
	linux-kselftest@...r.kernel.org, chrome-platform@...ts.linux.dev, 
	Dan Williams <dan.j.williams@...el.com>, linux-gpio@...r.kernel.org, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Danilo Krummrich <dakr@...nel.org>, Bartosz Golaszewski <brgl@...ev.pl>, Linus Walleij <linusw@...nel.org>
Subject: Re: [PATCH v2 03/11] gpio: sysfs: Remove redundant check for struct gpio_chip

On Tue, 3 Feb 2026 07:10:50 +0100, Tzung-Bi Shih <tzungbi@...nel.org> said:
> gpiochip_sysfs_unregister() is only called by gpiochip_remove() where
> the struct gpio_chip is ensured.
>
> Remove the redundant check.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@...nel.org>
> ---
> v2:
> - No changes.
>
> v1: https://lore.kernel.org/all/20260116081036.352286-9-tzungbi@kernel.org
>
>  drivers/gpio/gpiolib-sysfs.c | 9 +--------
>  drivers/gpio/gpiolib-sysfs.h | 6 ++++--
>  drivers/gpio/gpiolib.c       | 2 +-
>  3 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
> index cd553acf3055..8e6b09d8b559 100644
> --- a/drivers/gpio/gpiolib-sysfs.c
> +++ b/drivers/gpio/gpiolib-sysfs.c
> @@ -1048,11 +1048,10 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
>  	return 0;
>  }
>
> -void gpiochip_sysfs_unregister(struct gpio_device *gdev)
> +void gpiochip_sysfs_unregister(struct gpio_device *gdev, struct gpio_chip *chip)
>  {
>  	struct gpiodev_data *data;
>  	struct gpio_desc *desc;
> -	struct gpio_chip *chip;
>
>  	scoped_guard(mutex, &sysfs_lock) {
>  		data = gdev_get_data(gdev);
> @@ -1066,12 +1065,6 @@ void gpiochip_sysfs_unregister(struct gpio_device *gdev)
>  		kfree(data);
>  	}
>
> -	guard(srcu)(&gdev->srcu);
> -
> -	chip = srcu_dereference(gdev->chip, &gdev->srcu);
> -	if (!chip)
> -		return;
> -
>  	/* unregister gpiod class devices owned by sysfs */
>  	for_each_gpio_desc_with_flag(chip, desc, GPIOD_FLAG_SYSFS) {
>  		gpiod_unexport(desc);
> diff --git a/drivers/gpio/gpiolib-sysfs.h b/drivers/gpio/gpiolib-sysfs.h
> index b794b396d6a5..93debe8e118c 100644
> --- a/drivers/gpio/gpiolib-sysfs.h
> +++ b/drivers/gpio/gpiolib-sysfs.h
> @@ -8,7 +8,8 @@ struct gpio_device;
>  #ifdef CONFIG_GPIO_SYSFS
>
>  int gpiochip_sysfs_register(struct gpio_device *gdev);
> -void gpiochip_sysfs_unregister(struct gpio_device *gdev);
> +void gpiochip_sysfs_unregister(struct gpio_device *gdev,
> +			       struct gpio_chip *chip);
>
>  #else
>
> @@ -17,7 +18,8 @@ static inline int gpiochip_sysfs_register(struct gpio_device *gdev)
>  	return 0;
>  }
>
> -static inline void gpiochip_sysfs_unregister(struct gpio_device *gdev)
> +static inline void gpiochip_sysfs_unregister(struct gpio_device *gdev,
> +					     struct gpio_chip *chip)
>  {
>  }
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index a6dd07be126c..3137e6f1108a 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1281,7 +1281,7 @@ void gpiochip_remove(struct gpio_chip *gc)
>  	struct gpio_device *gdev = gc->gpiodev;
>
>  	/* FIXME: should the legacy sysfs handling be moved to gpio_device? */
> -	gpiochip_sysfs_unregister(gdev);
> +	gpiochip_sysfs_unregister(gdev, gc);

I understand the intention here but I really don't like passing both gc and
gdev here. We can get the address of the gpio_device from gpio_chip so why not
do this and pass only variable?

Bartosz

>  	gpiochip_free_hogs(gc);
>  	gpiochip_free_remaining_irqs(gc);
>
> --
> 2.53.0.rc2.204.g2597b5adb4-goog
>
>