From: Kent Gibson <warthog618@gmail.com>
To: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Cc: Linus Walleij <linusw@kernel.org>,
Bartosz Golaszewski <brgl@kernel.org>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Adrian Ambrozewicz <aambrozewicz@nvidia.com>,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
Subject: Re: [PATCH 4/5] gpiolib: don't overwrite the entire flags field in gpiod_free_commit()
Date: Thu, 5 Nov 2026 12:29:05 +0800 [thread overview]
Message-ID: <20261105042905.GB8436@rigel> (raw)
In-Reply-To: <20261104-gpio-desc-flags-atomic-v1-4-e23cedf619ba@oss.qualcomm.com>
On Wed, Nov 04, 2026 at 04:36:59PM +0100, Bartosz Golaszewski wrote:
> Use gpiod_update_flags() to clear all request-related flags in one go
> instead of risking racing with concurrent users and overwriting any
> changes they could have done between READ_ONCE() and WRITE_ONCE().
>
> Cc: stable@vger.kernel.org
> Fixes: 35b545332b80 ("gpio: remove gpio_lock")
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
> drivers/gpio/gpiolib.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 63b65b646fbcfa9ea0f724d1d3e1ac22ba28f638..28d4cf0cef70f437dc4271f82975d09dc55125ea 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -2693,23 +2693,23 @@ void gpiod_free_commit(struct gpio_desc *desc)
> if (guard.gc->free)
> guard.gc->free(guard.gc, gpiod_hwgpio(desc));
>
> - clear_bit(GPIOD_FLAG_ACTIVE_LOW, &flags);
> - clear_bit(GPIOD_FLAG_REQUESTED, &flags);
> - clear_bit(GPIOD_FLAG_OPEN_DRAIN, &flags);
> - clear_bit(GPIOD_FLAG_OPEN_SOURCE, &flags);
> - clear_bit(GPIOD_FLAG_PULL_UP, &flags);
> - clear_bit(GPIOD_FLAG_PULL_DOWN, &flags);
> - clear_bit(GPIOD_FLAG_BIAS_DISABLE, &flags);
> - clear_bit(GPIOD_FLAG_EDGE_RISING, &flags);
> - clear_bit(GPIOD_FLAG_EDGE_FALLING, &flags);
> - clear_bit(GPIOD_FLAG_EVENT_CLOCK_REALTIME, &flags);
> - clear_bit(GPIOD_FLAG_EVENT_CLOCK_HTE, &flags);
> - clear_bit(GPIOD_FLAG_IS_HOGGED, &flags);
> #ifdef CONFIG_OF_DYNAMIC
> WRITE_ONCE(desc->hog, NULL);
> #endif
> desc_set_label(desc, NULL);
> - WRITE_ONCE(desc->flags, flags);
> + gpiod_update_flags(desc,
> + BIT(GPIOD_FLAG_ACTIVE_LOW) |
> + BIT(GPIOD_FLAG_REQUESTED) |
> + BIT(GPIOD_FLAG_OPEN_DRAIN) |
> + BIT(GPIOD_FLAG_OPEN_SOURCE) |
> + BIT(GPIOD_FLAG_PULL_UP) |
> + BIT(GPIOD_FLAG_PULL_DOWN) |
> + BIT(GPIOD_FLAG_BIAS_DISABLE) |
> + BIT(GPIOD_FLAG_EDGE_RISING) |
> + BIT(GPIOD_FLAG_EDGE_FALLING) |
> + BIT(GPIOD_FLAG_EVENT_CLOCK_REALTIME) |
> + BIT(GPIOD_FLAG_EVENT_CLOCK_HTE) |
> + BIT(GPIOD_FLAG_IS_HOGGED), 0);
> #ifdef CONFIG_GPIO_CDEV
> WRITE_ONCE(desc->debounce_period_us, 0);
> #endif
>
Not directly related to your change, but the debounce_period_us is
protected by the GPIOD_FLAG_REQUESTED flag, so it should be cleared
before the flag, else it could potentially race with a new request.
Not sure if you want to fix it here or in a separate patch.
Cheers,
Kent.
next prev parent reply other threads:[~2026-11-05 4:29 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-04 15:36 [PATCH 0/5] gpiolib: improve concurrent access to the flags field of struct gpio_desc Bartosz Golaszewski
2026-11-04 15:36 ` [PATCH 1/5] gpiolib: don't overwrite the entire flags field in gpiod_get_direction() Bartosz Golaszewski
2026-11-04 15:36 ` [PATCH 2/5] gpio: shared: use atomic set_bit() for descriptor flags Bartosz Golaszewski
2026-11-04 15:36 ` [PATCH 3/5] gpiolib: provide gpiod_update_flags() Bartosz Golaszewski
2026-11-04 15:52 ` sashiko-bot
2026-11-05 12:59 ` Linus Walleij
2026-11-04 15:36 ` [PATCH 4/5] gpiolib: don't overwrite the entire flags field in gpiod_free_commit() Bartosz Golaszewski
2026-11-05 4:29 ` Kent Gibson [this message]
2026-11-04 15:37 ` [PATCH 5/5] gpio: cdev: don't overwrite the whole flags field when configuring lines Bartosz Golaszewski
2026-11-04 21:55 ` Andy Shevchenko
2026-11-05 4:23 ` Kent Gibson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261105042905.GB8436@rigel \
--to=warthog618@gmail.com \
--cc=aambrozewicz@nvidia.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=bartosz.golaszewski@oss.qualcomm.com \
--cc=brgl@kernel.org \
--cc=linusw@kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.