gpiolib: use a single SRCU struct for all GPIO descriptors

We used a per-descriptor SRCU struct in order to not impose a wait with
synchronize_srcu() for descriptor X on read-only operations of
descriptor Y. Now that we no longer call synchronize_srcu() on
descriptor label change but only when releasing descriptor resources, we
can use a single SRCU structure for all GPIO descriptors in a given chip.

Suggested-by: "Paul E. McKenney" <paulmck@kernel.org>
Acked-by: "Paul E. McKenney" <paulmck@kernel.org>
Link: https://lore.kernel.org/r/20240507172414.28513-1-brgl@bgdev.pl
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index d09c7d72836551ab510031179a9b95340cd3fb36..fea149ae77741f730736ad358c30815dabae9d63 100644 (file)
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2351,7 +2351,7 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
 
        dflags = READ_ONCE(desc->flags);
 
-       scoped_guard(srcu, &desc->srcu) {
+       scoped_guard(srcu, &desc->gdev->desc_srcu) {
                label = gpiod_get_label(desc);
                if (label && test_bit(FLAG_REQUESTED, &dflags))
                        strscpy(info->consumer, label,

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 2fa3756c9073a2572722b228c82734b0209ac3d1..fa50db0c3605bff72d23058d8c98be0e96e12865 100644 (file)
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -112,8 +112,8 @@ const char *gpiod_get_label(struct gpio_desc *desc)
        if (!test_bit(FLAG_REQUESTED, &flags))
                return NULL;
 
-       label = srcu_dereference_check(desc->label, &desc->srcu,
-                                      srcu_read_lock_held(&desc->srcu));
+       label = srcu_dereference_check(desc->label, &desc->gdev->desc_srcu,
+                               srcu_read_lock_held(&desc->gdev->desc_srcu));
 
        return label->str;
 }
@@ -138,7 +138,7 @@ static int desc_set_label(struct gpio_desc *desc, const char *label)
 
        old = rcu_replace_pointer(desc->label, new, 1);
        if (old)
-               call_srcu(&desc->srcu, &old->rh, desc_free_label);
+               call_srcu(&desc->gdev->desc_srcu, &old->rh, desc_free_label);
 
        return 0;
 }
@@ -709,13 +709,10 @@ EXPORT_SYMBOL_GPL(gpiochip_line_is_valid);
 static void gpiodev_release(struct device *dev)
 {
        struct gpio_device *gdev = to_gpio_device(dev);
-       unsigned int i;
 
-       for (i = 0; i < gdev->ngpio; i++) {
-               /* Free pending label. */
-               synchronize_srcu(&gdev->descs[i].srcu);
-               cleanup_srcu_struct(&gdev->descs[i].srcu);
-       }
+       /* Call pending kfree()s for descriptor labels. */
+       synchronize_srcu(&gdev->desc_srcu);
+       cleanup_srcu_struct(&gdev->desc_srcu);
 
        ida_free(&gpio_ida, gdev->id);
        kfree_const(gdev->label);
@@ -992,6 +989,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
        if (ret)
                goto err_remove_from_list;
 
+       ret = init_srcu_struct(&gdev->desc_srcu);
+       if (ret)
+               goto err_cleanup_gdev_srcu;
+
 #ifdef CONFIG_PINCTRL
        INIT_LIST_HEAD(&gdev->pin_ranges);
 #endif
@@ -999,23 +1000,19 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
        if (gc->names) {
                ret = gpiochip_set_desc_names(gc);
                if (ret)
-                       goto err_cleanup_gdev_srcu;
+                       goto err_cleanup_desc_srcu;
        }
        ret = gpiochip_set_names(gc);
        if (ret)
-               goto err_cleanup_gdev_srcu;
+               goto err_cleanup_desc_srcu;
 
        ret = gpiochip_init_valid_mask(gc);
        if (ret)
-               goto err_cleanup_gdev_srcu;
+               goto err_cleanup_desc_srcu;
 
        for (desc_index = 0; desc_index < gc->ngpio; desc_index++) {
                struct gpio_desc *desc = &gdev->descs[desc_index];
 
-               ret = init_srcu_struct(&desc->srcu);
-               if (ret)
-                       goto err_cleanup_desc_srcu;
-
                if (gc->get_direction && gpiochip_line_is_valid(gc, desc_index)) {
                        assign_bit(FLAG_IS_OUT,
                                   &desc->flags, !gc->get_direction(gc, desc_index));
@@ -1027,7 +1024,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 
        ret = of_gpiochip_add(gc);
        if (ret)
-               goto err_cleanup_desc_srcu;
+               goto err_free_valid_mask;
 
        ret = gpiochip_add_pin_ranges(gc);
        if (ret)
@@ -1074,10 +1071,10 @@ err_free_hogs:
        gpiochip_remove_pin_ranges(gc);
 err_remove_of_chip:
        of_gpiochip_remove(gc);
-err_cleanup_desc_srcu:
-       while (desc_index--)
-               cleanup_srcu_struct(&gdev->descs[desc_index].srcu);
+err_free_valid_mask:
        gpiochip_free_valid_mask(gc);
+err_cleanup_desc_srcu:
+       cleanup_srcu_struct(&gdev->desc_srcu);
 err_cleanup_gdev_srcu:
        cleanup_srcu_struct(&gdev->srcu);
 err_remove_from_list:
@@ -2407,7 +2404,7 @@ char *gpiochip_dup_line_label(struct gpio_chip *gc, unsigned int offset)
        if (!test_bit(FLAG_REQUESTED, &desc->flags))
                return NULL;
 
-       guard(srcu)(&desc->srcu);
+       guard(srcu)(&desc->gdev->desc_srcu);
 
        label = kstrdup(gpiod_get_label(desc), GFP_KERNEL);
        if (!label)
@@ -4798,7 +4795,7 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev)
        }
 
        for_each_gpio_desc(gc, desc) {
-               guard(srcu)(&desc->srcu);
+               guard(srcu)(&desc->gdev->desc_srcu);
                if (test_bit(FLAG_REQUESTED, &desc->flags)) {
                        gpiod_get_direction(desc);
                        is_out = test_bit(FLAG_IS_OUT, &desc->flags);

diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 69a353c789f08fc6cbf42f26000bcb52917c17a0..8e0e211ebf0836c12cc4bf29a15245af1880b4fb 100644 (file)
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -31,6 +31,7 @@
  * @chip: pointer to the corresponding gpiochip, holding static
  * data for this device
  * @descs: array of ngpio descriptors.
+ * @desc_srcu: ensures consistent state of GPIO descriptors exposed to users
  * @ngpio: the number of GPIO lines on this GPIO device, equal to the size
  * of the @descs array.
  * @can_sleep: indicate whether the GPIO chip driver's callbacks can sleep
@@ -61,6 +62,7 @@ struct gpio_device {
        struct module           *owner;
        struct gpio_chip __rcu  *chip;
        struct gpio_desc        *descs;
+       struct srcu_struct      desc_srcu;
        int                     base;
        u16                     ngpio;
        bool                    can_sleep;
@@ -150,7 +152,6 @@ struct gpio_desc_label {
  * @label:             Name of the consumer
  * @name:              Line name
  * @hog:               Pointer to the device node that hogs this line (if any)
- * @srcu:              SRCU struct protecting the label pointer.
  *
  * These are obtained using gpiod_get() and are preferable to the old
  * integer-based handles.
@@ -188,7 +189,6 @@ struct gpio_desc {
 #ifdef CONFIG_OF_DYNAMIC
        struct device_node      *hog;
 #endif
-       struct srcu_struct      srcu;
 };
 
 #define gpiod_not_found(desc)          (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT)
@@ -256,7 +256,7 @@ static inline int gpio_chip_hwgpio(const struct gpio_desc *desc)
 
 #define gpiod_err(desc, fmt, ...) \
 do { \
-       scoped_guard(srcu, &desc->srcu) { \
+       scoped_guard(srcu, &desc->gdev->desc_srcu) { \
                pr_err("gpio-%d (%s): " fmt, desc_to_gpio(desc), \
                       gpiod_get_label(desc) ? : "?", ##__VA_ARGS__); \
        } \
@@ -264,7 +264,7 @@ do { \
 
 #define gpiod_warn(desc, fmt, ...) \
 do { \
-       scoped_guard(srcu, &desc->srcu) { \
+       scoped_guard(srcu, &desc->gdev->desc_srcu) { \
                pr_warn("gpio-%d (%s): " fmt, desc_to_gpio(desc), \
                        gpiod_get_label(desc) ? : "?", ##__VA_ARGS__); \
        } \
@@ -272,7 +272,7 @@ do { \
 
 #define gpiod_dbg(desc, fmt, ...) \
 do { \
-       scoped_guard(srcu, &desc->srcu) { \
+       scoped_guard(srcu, &desc->gdev->desc_srcu) { \
                pr_debug("gpio-%d (%s): " fmt, desc_to_gpio(desc), \
                         gpiod_get_label(desc) ? : "?", ##__VA_ARGS__); \
        } \