Skip to content

Commit

Permalink
gpio: cdev: use raw notifier for line state events
Browse files Browse the repository at this point in the history
We use a notifier to implement the mechanism of informing the user-space
about changes in GPIO line status. We register with the notifier when
the GPIO character device file is opened and unregister when the last
reference to the associated file descriptor is dropped.

Since commit fcc8b63 ("gpiolib: switch the line state notifier to
atomic") we use the atomic notifier variant. Atomic notifiers call
rcu_synchronize in atomic_notifier_chain_unregister() which caused a
significant performance regression in some circumstances, observed by
user-space when calling close() on the GPIO device file descriptor.

Replace the atomic notifier with the raw variant and provide
synchronization with a read-write spinlock.

Fixes: fcc8b63 ("gpiolib: switch the line state notifier to atomic")
Reported-by: David Jander <david@protonic.nl>
Closes: https://lore.kernel.org/all/20250311110034.53959031@erd003.prtnl/
Tested-by: David Jander <david@protonic.nl>
Tested-by: Kent Gibson <warthog618@gmail.com>
Link: https://lore.kernel.org/r/20250311-gpiolib-line-state-raw-notifier-v2-1-138374581e1e@linaro.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
  • Loading branch information
Bartosz Golaszewski committed Mar 13, 2025
1 parent 0102fbf commit dcb73cb
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 10 deletions.
15 changes: 9 additions & 6 deletions drivers/gpio/gpiolib-cdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -2729,8 +2729,9 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
cdev->gdev = gpio_device_get(gdev);

cdev->lineinfo_changed_nb.notifier_call = lineinfo_changed_notify;
ret = atomic_notifier_chain_register(&gdev->line_state_notifier,
&cdev->lineinfo_changed_nb);
scoped_guard(write_lock_irqsave, &gdev->line_state_lock)
ret = raw_notifier_chain_register(&gdev->line_state_notifier,
&cdev->lineinfo_changed_nb);
if (ret)
goto out_free_bitmap;

Expand All @@ -2754,8 +2755,9 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
blocking_notifier_chain_unregister(&gdev->device_notifier,
&cdev->device_unregistered_nb);
out_unregister_line_notifier:
atomic_notifier_chain_unregister(&gdev->line_state_notifier,
&cdev->lineinfo_changed_nb);
scoped_guard(write_lock_irqsave, &gdev->line_state_lock)
raw_notifier_chain_unregister(&gdev->line_state_notifier,
&cdev->lineinfo_changed_nb);
out_free_bitmap:
gpio_device_put(gdev);
bitmap_free(cdev->watched_lines);
Expand All @@ -2779,8 +2781,9 @@ static int gpio_chrdev_release(struct inode *inode, struct file *file)

blocking_notifier_chain_unregister(&gdev->device_notifier,
&cdev->device_unregistered_nb);
atomic_notifier_chain_unregister(&gdev->line_state_notifier,
&cdev->lineinfo_changed_nb);
scoped_guard(write_lock_irqsave, &gdev->line_state_lock)
raw_notifier_chain_unregister(&gdev->line_state_notifier,
&cdev->lineinfo_changed_nb);
bitmap_free(cdev->watched_lines);
gpio_device_put(gdev);
kfree(cdev);
Expand Down
8 changes: 5 additions & 3 deletions drivers/gpio/gpiolib.c
Original file line number Diff line number Diff line change
Expand Up @@ -1025,7 +1025,8 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
}
}

ATOMIC_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
rwlock_init(&gdev->line_state_lock);
RAW_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier);

ret = init_srcu_struct(&gdev->srcu);
Expand Down Expand Up @@ -4188,8 +4189,9 @@ EXPORT_SYMBOL_GPL(gpiod_set_array_value_cansleep);

void gpiod_line_state_notify(struct gpio_desc *desc, unsigned long action)
{
atomic_notifier_call_chain(&desc->gdev->line_state_notifier,
action, desc);
guard(read_lock_irqsave)(&desc->gdev->line_state_lock);

raw_notifier_call_chain(&desc->gdev->line_state_notifier, action, desc);
}

/**
Expand Down
5 changes: 4 additions & 1 deletion drivers/gpio/gpiolib.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <linux/gpio/driver.h>
#include <linux/module.h>
#include <linux/notifier.h>
#include <linux/spinlock.h>
#include <linux/srcu.h>
#include <linux/workqueue.h>

Expand Down Expand Up @@ -45,6 +46,7 @@
* @list: links gpio_device:s together for traversal
* @line_state_notifier: used to notify subscribers about lines being
* requested, released or reconfigured
* @line_state_lock: RW-spinlock protecting the line state notifier
* @line_state_wq: used to emit line state events from a separate thread in
* process context
* @device_notifier: used to notify character device wait queues about the GPIO
Expand Down Expand Up @@ -72,7 +74,8 @@ struct gpio_device {
const char *label;
void *data;
struct list_head list;
struct atomic_notifier_head line_state_notifier;
struct raw_notifier_head line_state_notifier;
rwlock_t line_state_lock;
struct workqueue_struct *line_state_wq;
struct blocking_notifier_head device_notifier;
struct srcu_struct srcu;
Expand Down

0 comments on commit dcb73cb

Please sign in to comment.