The underlying chip can be removed asynchronously. `gdev->srcu` is used
to ensure the synchronization before accessing `gdev->chip`.
Revocable encapsulates the details. Add revocable provider handle for
the corresponding struct gpio_chip in struct gpio_device so that it can
start to hide the synchronization details.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v2:
- Change usages accordingly after applying
https://lore.kernel.org/all/20260129143733.45618-2-tzungbi@kernel.org.
- Add __rcu for `chip_rp`.
- Pass pointer of pointer to revocable_provider_revoke().
- Rebase accordingly after applying
https://lore.kernel.org/all/20260203060210.972243-1-tzungbi@kernel.org.
v1: https://lore.kernel.org/all/20260116081036.352286-13-tzungbi@kernel.org
drivers/gpio/gpiolib.c | 20 ++++++++++++++++++--
drivers/gpio/gpiolib.h | 2 ++
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 7885dcd1e49d..fdae10ec3a17 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -22,6 +22,7 @@
#include <linux/nospec.h>
#include <linux/of.h>
#include <linux/pinctrl/consumer.h>
+#include <linux/revocable.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
#include <linux/srcu.h>
@@ -1110,6 +1111,12 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
goto err_put_device;
}
+ gdev->chip_rp = revocable_provider_alloc(gc);
+ if (!gdev->chip_rp) {
+ ret = -ENOMEM;
+ goto err_put_device;
+ }
+
gdev->can_sleep = gc->can_sleep;
rwlock_init(&gdev->line_state_lock);
RAW_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
@@ -1139,7 +1146,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
if (base < 0) {
ret = base;
base = 0;
- goto err_put_device;
+ goto err_free_rp;
}
/*
@@ -1159,7 +1166,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
ret = gpiodev_add_to_list_unlocked(gdev);
if (ret) {
gpiochip_err(gc, "GPIO integer space overlap, cannot add chip\n");
- goto err_put_device;
+ goto err_free_rp;
}
}
@@ -1256,6 +1263,14 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
scoped_guard(mutex, &gpio_devices_lock)
list_del_rcu(&gdev->list);
synchronize_srcu(&gpio_devices_srcu);
+err_free_rp:
+ /*
+ * Unlike other allocated resources for `gdev` can be freed
+ * in gpiodev_release(). Call revocable_provider_revoke()
+ * here as it's designed to be called when the chip is gone
+ * (i.e., gpiochip_remove()).
+ */
+ revocable_provider_revoke(&gdev->chip_rp);
err_put_device:
gpio_device_put(gdev);
goto err_print_message;
@@ -1302,6 +1317,7 @@ void gpiochip_remove(struct gpio_chip *gc)
/* Numb the device, cancelling all outstanding operations */
rcu_assign_pointer(gdev->chip, NULL);
synchronize_srcu(&gdev->srcu);
+ revocable_provider_revoke(&gdev->chip_rp);
gpio_device_teardown_shared(gdev);
gpiochip_irqchip_remove(gc);
acpi_gpiochip_remove(gc);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 3abb90385829..cd136d5b52e9 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -52,6 +52,7 @@
* @device_notifier: used to notify character device wait queues about the GPIO
* device being unregistered
* @srcu: protects the pointer to the underlying GPIO chip
+ * @chip_rp: revocable provider handle for the corresponding struct gpio_chip.
* @pin_ranges: range of pins served by the GPIO driver
*
* This state container holds most of the runtime variable data
@@ -79,6 +80,7 @@ struct gpio_device {
struct workqueue_struct *line_state_wq;
struct blocking_notifier_head device_notifier;
struct srcu_struct srcu;
+ struct revocable_provider __rcu *chip_rp;
#ifdef CONFIG_PINCTRL
/*
--
2.53.0.rc2.204.g2597b5adb4-googOn Tue, 3 Feb 2026 07:10:54 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> The underlying chip can be removed asynchronously. `gdev->srcu` is used
> to ensure the synchronization before accessing `gdev->chip`.
>
> Revocable encapsulates the details. Add revocable provider handle for
> the corresponding struct gpio_chip in struct gpio_device so that it can
> start to hide the synchronization details.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
[snip]
> diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
> index 3abb90385829..cd136d5b52e9 100644
> --- a/drivers/gpio/gpiolib.h
> +++ b/drivers/gpio/gpiolib.h
> @@ -52,6 +52,7 @@
> * @device_notifier: used to notify character device wait queues about the GPIO
> * device being unregistered
> * @srcu: protects the pointer to the underlying GPIO chip
> + * @chip_rp: revocable provider handle for the corresponding struct gpio_chip.
> * @pin_ranges: range of pins served by the GPIO driver
> *
> * This state container holds most of the runtime variable data
> @@ -79,6 +80,7 @@ struct gpio_device {
> struct workqueue_struct *line_state_wq;
> struct blocking_notifier_head device_notifier;
> struct srcu_struct srcu;
> + struct revocable_provider __rcu *chip_rp;
>
Why __rcu? This doesn't live in a different address space, only the internal
resource it protects does. If anything - this could be __attribute__((noderef))
but even that is questionable as this is an opaque structure.
Bart
> #ifdef CONFIG_PINCTRL
> /*
> --
> 2.53.0.rc2.204.g2597b5adb4-goog
>
>
On Wed, Feb 04, 2026 at 07:58:44AM -0500, Bartosz Golaszewski wrote:
> On Tue, 3 Feb 2026 07:10:54 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
> > index 3abb90385829..cd136d5b52e9 100644
> > --- a/drivers/gpio/gpiolib.h
> > +++ b/drivers/gpio/gpiolib.h
> > @@ -52,6 +52,7 @@
> > * @device_notifier: used to notify character device wait queues about the GPIO
> > * device being unregistered
> > * @srcu: protects the pointer to the underlying GPIO chip
> > + * @chip_rp: revocable provider handle for the corresponding struct gpio_chip.
> > * @pin_ranges: range of pins served by the GPIO driver
> > *
> > * This state container holds most of the runtime variable data
> > @@ -79,6 +80,7 @@ struct gpio_device {
> > struct workqueue_struct *line_state_wq;
> > struct blocking_notifier_head device_notifier;
> > struct srcu_struct srcu;
> > + struct revocable_provider __rcu *chip_rp;
> >
>
> Why __rcu? This doesn't live in a different address space, only the internal
> resource it protects does. If anything - this could be __attribute__((noderef))
> but even that is questionable as this is an opaque structure.
For fixing a race on the pointer itself. See also [1].
[1] https://lore.kernel.org/all/20260129143733.45618-2-tzungbi@kernel.org
On Thu, Feb 5, 2026 at 9:52 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Wed, Feb 04, 2026 at 07:58:44AM -0500, Bartosz Golaszewski wrote:
> > On Tue, 3 Feb 2026 07:10:54 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > > diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
> > > index 3abb90385829..cd136d5b52e9 100644
> > > --- a/drivers/gpio/gpiolib.h
> > > +++ b/drivers/gpio/gpiolib.h
> > > @@ -52,6 +52,7 @@
> > > * @device_notifier: used to notify character device wait queues about the GPIO
> > > * device being unregistered
> > > * @srcu: protects the pointer to the underlying GPIO chip
> > > + * @chip_rp: revocable provider handle for the corresponding struct gpio_chip.
> > > * @pin_ranges: range of pins served by the GPIO driver
> > > *
> > > * This state container holds most of the runtime variable data
> > > @@ -79,6 +80,7 @@ struct gpio_device {
> > > struct workqueue_struct *line_state_wq;
> > > struct blocking_notifier_head device_notifier;
> > > struct srcu_struct srcu;
> > > + struct revocable_provider __rcu *chip_rp;
> > >
> >
> > Why __rcu? This doesn't live in a different address space, only the internal
> > resource it protects does. If anything - this could be __attribute__((noderef))
> > but even that is questionable as this is an opaque structure.
>
> For fixing a race on the pointer itself. See also [1].
>
> [1] https://lore.kernel.org/all/20260129143733.45618-2-tzungbi@kernel.org
So we're just using a double RCU here? One to protect the resource and
another to protect the protector of the resource? I can't say I'm a
fan of this. I really want to like this interface but is there really
no way to hide the implementation details from the caller? Isn't this
the whole point? As it is: the user still has to care about an
RCU-protected pointer.
Bartosz
On Thu, Feb 05, 2026 at 05:57:15PM +0100, Bartosz Golaszewski wrote:
> On Thu, Feb 5, 2026 at 9:52 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > On Wed, Feb 04, 2026 at 07:58:44AM -0500, Bartosz Golaszewski wrote:
> > > On Tue, 3 Feb 2026 07:10:54 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > > > diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
> > > > index 3abb90385829..cd136d5b52e9 100644
> > > > --- a/drivers/gpio/gpiolib.h
> > > > +++ b/drivers/gpio/gpiolib.h
> > > > @@ -52,6 +52,7 @@
> > > > * @device_notifier: used to notify character device wait queues about the GPIO
> > > > * device being unregistered
> > > > * @srcu: protects the pointer to the underlying GPIO chip
> > > > + * @chip_rp: revocable provider handle for the corresponding struct gpio_chip.
> > > > * @pin_ranges: range of pins served by the GPIO driver
> > > > *
> > > > * This state container holds most of the runtime variable data
> > > > @@ -79,6 +80,7 @@ struct gpio_device {
> > > > struct workqueue_struct *line_state_wq;
> > > > struct blocking_notifier_head device_notifier;
> > > > struct srcu_struct srcu;
> > > > + struct revocable_provider __rcu *chip_rp;
> > > >
> > >
> > > Why __rcu? This doesn't live in a different address space, only the internal
> > > resource it protects does. If anything - this could be __attribute__((noderef))
> > > but even that is questionable as this is an opaque structure.
> >
> > For fixing a race on the pointer itself. See also [1].
> >
> > [1] https://lore.kernel.org/all/20260129143733.45618-2-tzungbi@kernel.org
>
> So we're just using a double RCU here? One to protect the resource and
> another to protect the protector of the resource? I can't say I'm a
> fan of this. I really want to like this interface but is there really
> no way to hide the implementation details from the caller? Isn't this
> the whole point? As it is: the user still has to care about an
> RCU-protected pointer.
Will think about it but I have no better idea for now.
Ideally, I think the user doesn't need to interact with the RCU (even if it's
annotated with __rcu) but revocable APIs should handle it correctly.
© 2016 - 2026 Red Hat, Inc.