+++ /dev/null
-From 077106f97c7d113ebacb00725d83b817d0e89288 Mon Sep 17 00:00:00 2001
-From: Sasha Levin <sashal@kernel.org>
-Date: Fri, 19 Jan 2024 16:43:13 +0100
-Subject: gpio: protect the list of GPIO devices with SRCU
-
-From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
-
-[ Upstream commit e348544f7994d252427ed3ae637c7081cbb90f66 ]
-
-We're working towards removing the "multi-function" GPIO spinlock that's
-implemented terribly wrong. We tried using an RW-semaphore to protect
-the list of GPIO devices but it turned out that we still have old code
-using legacy GPIO calls that need to translate the global GPIO number to
-the address of the associated descriptor and - to that end - traverse
-the list while holding the lock. If we change the spinlock to a sleeping
-lock then we'll end up with "scheduling while atomic" bugs.
-
-Let's allow lockless traversal of the list using SRCU and only use the
-mutex when modyfing the list.
-
-While at it: let's protect the period between when we start the lookup
-and when we finally request the descriptor (increasing the reference
-count of the GPIO device) with the SRCU read lock.
-
-Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
-Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
-Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-Stable-dep-of: 5c887b65bbd1 ("gpiolib: Fix debug messaging in gpiod_find_and_request()")
-Signed-off-by: Sasha Levin <sashal@kernel.org>
----
- drivers/gpio/gpiolib.c | 247 ++++++++++++++++++++++-------------------
- 1 file changed, 135 insertions(+), 112 deletions(-)
-
-diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
-index 3ad09d2193330..a6f1a4e2e3a4e 100644
---- a/drivers/gpio/gpiolib.c
-+++ b/drivers/gpio/gpiolib.c
-@@ -2,6 +2,7 @@
-
- #include <linux/acpi.h>
- #include <linux/bitmap.h>
-+#include <linux/cleanup.h>
- #include <linux/compat.h>
- #include <linux/debugfs.h>
- #include <linux/device.h>
-@@ -14,12 +15,14 @@
- #include <linux/irq.h>
- #include <linux/kernel.h>
- #include <linux/list.h>
-+#include <linux/lockdep.h>
- #include <linux/module.h>
- #include <linux/of.h>
- #include <linux/pinctrl/consumer.h>
- #include <linux/seq_file.h>
- #include <linux/slab.h>
- #include <linux/spinlock.h>
-+#include <linux/srcu.h>
- #include <linux/string.h>
-
- #include <linux/gpio.h>
-@@ -81,7 +84,12 @@ DEFINE_SPINLOCK(gpio_lock);
-
- static DEFINE_MUTEX(gpio_lookup_lock);
- static LIST_HEAD(gpio_lookup_list);
-+
- LIST_HEAD(gpio_devices);
-+/* Protects the GPIO device list against concurrent modifications. */
-+static DEFINE_MUTEX(gpio_devices_lock);
-+/* Ensures coherence during read-only accesses to the list of GPIO devices. */
-+DEFINE_STATIC_SRCU(gpio_devices_srcu);
-
- static DEFINE_MUTEX(gpio_machine_hogs_mutex);
- static LIST_HEAD(gpio_machine_hogs);
-@@ -113,20 +121,16 @@ static inline void desc_set_label(struct gpio_desc *d, const char *label)
- struct gpio_desc *gpio_to_desc(unsigned gpio)
- {
- struct gpio_device *gdev;
-- unsigned long flags;
--
-- spin_lock_irqsave(&gpio_lock, flags);
-
-- list_for_each_entry(gdev, &gpio_devices, list) {
-- if (gdev->base <= gpio &&
-- gdev->base + gdev->ngpio > gpio) {
-- spin_unlock_irqrestore(&gpio_lock, flags);
-- return &gdev->descs[gpio - gdev->base];
-+ scoped_guard(srcu, &gpio_devices_srcu) {
-+ list_for_each_entry_srcu(gdev, &gpio_devices, list,
-+ srcu_read_lock_held(&gpio_devices_srcu)) {
-+ if (gdev->base <= gpio &&
-+ gdev->base + gdev->ngpio > gpio)
-+ return &gdev->descs[gpio - gdev->base];
- }
- }
-
-- spin_unlock_irqrestore(&gpio_lock, flags);
--
- if (!gpio_is_valid(gpio))
- pr_warn("invalid GPIO %d\n", gpio);
-
-@@ -282,7 +286,8 @@ static int gpiochip_find_base_unlocked(int ngpio)
- struct gpio_device *gdev;
- int base = GPIO_DYNAMIC_BASE;
-
-- list_for_each_entry(gdev, &gpio_devices, list) {
-+ list_for_each_entry_srcu(gdev, &gpio_devices, list,
-+ lockdep_is_held(&gpio_devices_lock)) {
- /* found a free space? */
- if (gdev->base >= base + ngpio)
- break;
-@@ -354,23 +359,25 @@ static int gpiodev_add_to_list_unlocked(struct gpio_device *gdev)
- {
- struct gpio_device *prev, *next;
-
-+ lockdep_assert_held(&gpio_devices_lock);
-+
- if (list_empty(&gpio_devices)) {
- /* initial entry in list */
-- list_add_tail(&gdev->list, &gpio_devices);
-+ list_add_tail_rcu(&gdev->list, &gpio_devices);
- return 0;
- }
-
- next = list_first_entry(&gpio_devices, struct gpio_device, list);
- if (gdev->base + gdev->ngpio <= next->base) {
- /* add before first entry */
-- list_add(&gdev->list, &gpio_devices);
-+ list_add_rcu(&gdev->list, &gpio_devices);
- return 0;
- }
-
- prev = list_last_entry(&gpio_devices, struct gpio_device, list);
- if (prev->base + prev->ngpio <= gdev->base) {
- /* add behind last entry */
-- list_add_tail(&gdev->list, &gpio_devices);
-+ list_add_tail_rcu(&gdev->list, &gpio_devices);
- return 0;
- }
-
-@@ -382,11 +389,13 @@ static int gpiodev_add_to_list_unlocked(struct gpio_device *gdev)
- /* add between prev and next */
- if (prev->base + prev->ngpio <= gdev->base
- && gdev->base + gdev->ngpio <= next->base) {
-- list_add(&gdev->list, &prev->list);
-+ list_add_rcu(&gdev->list, &prev->list);
- return 0;
- }
- }
-
-+ synchronize_srcu(&gpio_devices_srcu);
-+
- return -EBUSY;
- }
-
-@@ -399,26 +408,21 @@ static int gpiodev_add_to_list_unlocked(struct gpio_device *gdev)
- static struct gpio_desc *gpio_name_to_desc(const char * const name)
- {
- struct gpio_device *gdev;
-- unsigned long flags;
-+ struct gpio_desc *desc;
-
- if (!name)
- return NULL;
-
-- spin_lock_irqsave(&gpio_lock, flags);
--
-- list_for_each_entry(gdev, &gpio_devices, list) {
-- struct gpio_desc *desc;
-+ guard(srcu)(&gpio_devices_srcu);
-
-+ list_for_each_entry_srcu(gdev, &gpio_devices, list,
-+ srcu_read_lock_held(&gpio_devices_srcu)) {
- for_each_gpio_desc(gdev->chip, desc) {
-- if (desc->name && !strcmp(desc->name, name)) {
-- spin_unlock_irqrestore(&gpio_lock, flags);
-+ if (desc->name && !strcmp(desc->name, name))
- return desc;
-- }
- }
- }
-
-- spin_unlock_irqrestore(&gpio_lock, flags);
--
- return NULL;
- }
-
-@@ -748,7 +752,10 @@ static void gpiochip_setup_devs(void)
- struct gpio_device *gdev;
- int ret;
-
-- list_for_each_entry(gdev, &gpio_devices, list) {
-+ guard(srcu)(&gpio_devices_srcu);
-+
-+ list_for_each_entry_srcu(gdev, &gpio_devices, list,
-+ srcu_read_lock_held(&gpio_devices_srcu)) {
- ret = gpiochip_setup_dev(gdev);
- if (ret)
- dev_err(&gdev->dev,
-@@ -813,7 +820,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
- struct lock_class_key *request_key)
- {
- struct gpio_device *gdev;
-- unsigned long flags;
- unsigned int i;
- int base = 0;
- int ret = 0;
-@@ -878,49 +884,47 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
-
- gdev->ngpio = gc->ngpio;
-
-- spin_lock_irqsave(&gpio_lock, flags);
--
-- /*
-- * TODO: this allocates a Linux GPIO number base in the global
-- * GPIO numberspace for this chip. In the long run we want to
-- * get *rid* of this numberspace and use only descriptors, but
-- * it may be a pipe dream. It will not happen before we get rid
-- * of the sysfs interface anyways.
-- */
-- base = gc->base;
-- if (base < 0) {
-- base = gpiochip_find_base_unlocked(gc->ngpio);
-- if (base < 0) {
-- spin_unlock_irqrestore(&gpio_lock, flags);
-- ret = base;
-- base = 0;
-- goto err_free_label;
-- }
-+ scoped_guard(mutex, &gpio_devices_lock) {
- /*
-- * TODO: it should not be necessary to reflect the assigned
-- * base outside of the GPIO subsystem. Go over drivers and
-- * see if anyone makes use of this, else drop this and assign
-- * a poison instead.
-+ * TODO: this allocates a Linux GPIO number base in the global
-+ * GPIO numberspace for this chip. In the long run we want to
-+ * get *rid* of this numberspace and use only descriptors, but
-+ * it may be a pipe dream. It will not happen before we get rid
-+ * of the sysfs interface anyways.
- */
-- gc->base = base;
-- } else {
-- dev_warn(&gdev->dev,
-- "Static allocation of GPIO base is deprecated, use dynamic allocation.\n");
-- }
-- gdev->base = base;
-+ base = gc->base;
-+ if (base < 0) {
-+ base = gpiochip_find_base_unlocked(gc->ngpio);
-+ if (base < 0) {
-+ ret = base;
-+ base = 0;
-+ goto err_free_label;
-+ }
-
-- ret = gpiodev_add_to_list_unlocked(gdev);
-- if (ret) {
-- spin_unlock_irqrestore(&gpio_lock, flags);
-- chip_err(gc, "GPIO integer space overlap, cannot add chip\n");
-- goto err_free_label;
-+ /*
-+ * TODO: it should not be necessary to reflect the
-+ * assigned base outside of the GPIO subsystem. Go over
-+ * drivers and see if anyone makes use of this, else
-+ * drop this and assign a poison instead.
-+ */
-+ gc->base = base;
-+ } else {
-+ dev_warn(&gdev->dev,
-+ "Static allocation of GPIO base is deprecated, use dynamic allocation.\n");
-+ }
-+
-+ gdev->base = base;
-+
-+ ret = gpiodev_add_to_list_unlocked(gdev);
-+ if (ret) {
-+ chip_err(gc, "GPIO integer space overlap, cannot add chip\n");
-+ goto err_free_label;
-+ }
- }
-
- for (i = 0; i < gc->ngpio; i++)
- gdev->descs[i].gdev = gdev;
-
-- spin_unlock_irqrestore(&gpio_lock, flags);
--
- BLOCKING_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
- BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier);
- init_rwsem(&gdev->sem);
-@@ -1006,9 +1010,9 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
- err_free_gpiochip_mask:
- gpiochip_free_valid_mask(gc);
- err_remove_from_list:
-- spin_lock_irqsave(&gpio_lock, flags);
-- list_del(&gdev->list);
-- spin_unlock_irqrestore(&gpio_lock, flags);
-+ scoped_guard(mutex, &gpio_devices_lock)
-+ list_del_rcu(&gdev->list);
-+ synchronize_srcu(&gpio_devices_srcu);
- if (gdev->dev.release) {
- /* release() has been registered by gpiochip_setup_dev() */
- gpio_device_put(gdev);
-@@ -1052,6 +1056,11 @@ void gpiochip_remove(struct gpio_chip *gc)
- /* FIXME: should the legacy sysfs handling be moved to gpio_device? */
- gpiochip_sysfs_unregister(gdev);
- gpiochip_free_hogs(gc);
-+
-+ scoped_guard(mutex, &gpio_devices_lock)
-+ list_del_rcu(&gdev->list);
-+ synchronize_srcu(&gpio_devices_srcu);
-+
- /* Numb the device, cancelling all outstanding operations */
- gdev->chip = NULL;
- gpiochip_irqchip_remove(gc);
-@@ -1076,9 +1085,6 @@ void gpiochip_remove(struct gpio_chip *gc)
- dev_crit(&gdev->dev,
- "REMOVING GPIOCHIP WITH GPIOS STILL REQUESTED\n");
-
-- scoped_guard(spinlock_irqsave, &gpio_lock)
-- list_del(&gdev->list);
--
- /*
- * The gpiochip side puts its use of the device to rest here:
- * if there are no userspace clients, the chardev and device will
-@@ -1125,7 +1131,7 @@ struct gpio_device *gpio_device_find(void *data,
- */
- might_sleep();
-
-- guard(spinlock_irqsave)(&gpio_lock);
-+ guard(srcu)(&gpio_devices_srcu);
-
- list_for_each_entry(gdev, &gpio_devices, list) {
- if (gdev->chip && match(gdev->chip, data))
-@@ -4147,30 +4153,39 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
- bool platform_lookup_allowed)
- {
- unsigned long lookupflags = GPIO_LOOKUP_FLAGS_DEFAULT;
-- struct gpio_desc *desc;
-- int ret;
-+ /*
-+ * scoped_guard() is implemented as a for loop, meaning static
-+ * analyzers will complain about these two not being initialized.
-+ */
-+ struct gpio_desc *desc = NULL;
-+ int ret = 0;
-+
-+ scoped_guard(srcu, &gpio_devices_srcu) {
-+ desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx,
-+ &flags, &lookupflags);
-+ if (gpiod_not_found(desc) && platform_lookup_allowed) {
-+ /*
-+ * Either we are not using DT or ACPI, or their lookup
-+ * did not return a result. In that case, use platform
-+ * lookup as a fallback.
-+ */
-+ dev_dbg(consumer,
-+ "using lookup tables for GPIO lookup\n");
-+ desc = gpiod_find(consumer, con_id, idx, &lookupflags);
-+ }
-+
-+ if (IS_ERR(desc)) {
-+ dev_dbg(consumer, "No GPIO consumer %s found\n",
-+ con_id);
-+ return desc;
-+ }
-
-- desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx, &flags, &lookupflags);
-- if (gpiod_not_found(desc) && platform_lookup_allowed) {
- /*
-- * Either we are not using DT or ACPI, or their lookup did not
-- * return a result. In that case, use platform lookup as a
-- * fallback.
-+ * If a connection label was passed use that, else attempt to use
-+ * the device name as label
- */
-- dev_dbg(consumer, "using lookup tables for GPIO lookup\n");
-- desc = gpiod_find(consumer, con_id, idx, &lookupflags);
-- }
--
-- if (IS_ERR(desc)) {
-- dev_dbg(consumer, "No GPIO consumer %s found\n", con_id);
-- return desc;
-+ ret = gpiod_request(desc, label);
- }
--
-- /*
-- * If a connection label was passed use that, else attempt to use
-- * the device name as label
-- */
-- ret = gpiod_request(desc, label);
- if (ret) {
- if (!(ret == -EBUSY && flags & GPIOD_FLAGS_BIT_NONEXCLUSIVE))
- return ERR_PTR(ret);
-@@ -4739,61 +4754,69 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev)
- }
- }
-
-+struct gpiolib_seq_priv {
-+ bool newline;
-+ int idx;
-+};
-+
- static void *gpiolib_seq_start(struct seq_file *s, loff_t *pos)
- {
-- unsigned long flags;
-- struct gpio_device *gdev = NULL;
-+ struct gpiolib_seq_priv *priv;
-+ struct gpio_device *gdev;
- loff_t index = *pos;
-
-- s->private = "";
-+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-+ if (!priv)
-+ return NULL;
-+
-+ s->private = priv;
-+ priv->idx = srcu_read_lock(&gpio_devices_srcu);
-
-- spin_lock_irqsave(&gpio_lock, flags);
-- list_for_each_entry(gdev, &gpio_devices, list)
-- if (index-- == 0) {
-- spin_unlock_irqrestore(&gpio_lock, flags);
-+ list_for_each_entry_srcu(gdev, &gpio_devices, list,
-+ srcu_read_lock_held(&gpio_devices_srcu)) {
-+ if (index-- == 0)
- return gdev;
-- }
-- spin_unlock_irqrestore(&gpio_lock, flags);
-+ }
-
- return NULL;
- }
-
- static void *gpiolib_seq_next(struct seq_file *s, void *v, loff_t *pos)
- {
-- unsigned long flags;
-- struct gpio_device *gdev = v;
-- void *ret = NULL;
-+ struct gpiolib_seq_priv *priv = s->private;
-+ struct gpio_device *gdev = v, *next;
-
-- spin_lock_irqsave(&gpio_lock, flags);
-- if (list_is_last(&gdev->list, &gpio_devices))
-- ret = NULL;
-- else
-- ret = list_first_entry(&gdev->list, struct gpio_device, list);
-- spin_unlock_irqrestore(&gpio_lock, flags);
--
-- s->private = "\n";
-+ next = list_entry_rcu(gdev->list.next, struct gpio_device, list);
-+ gdev = &next->list == &gpio_devices ? NULL : next;
-+ priv->newline = true;
- ++*pos;
-
-- return ret;
-+ return gdev;
- }
-
- static void gpiolib_seq_stop(struct seq_file *s, void *v)
- {
-+ struct gpiolib_seq_priv *priv = s->private;
-+
-+ srcu_read_unlock(&gpio_devices_srcu, priv->idx);
-+ kfree(priv);
- }
-
- static int gpiolib_seq_show(struct seq_file *s, void *v)
- {
-+ struct gpiolib_seq_priv *priv = s->private;
- struct gpio_device *gdev = v;
- struct gpio_chip *gc = gdev->chip;
- struct device *parent;
-
- if (!gc) {
-- seq_printf(s, "%s%s: (dangling chip)", (char *)s->private,
-+ seq_printf(s, "%s%s: (dangling chip)",
-+ priv->newline ? "\n" : "",
- dev_name(&gdev->dev));
- return 0;
- }
-
-- seq_printf(s, "%s%s: GPIOs %d-%d", (char *)s->private,
-+ seq_printf(s, "%s%s: GPIOs %d-%d", priv->newline ? "\n" : "",
- dev_name(&gdev->dev),
- gdev->base, gdev->base + gdev->ngpio - 1);
- parent = gc->parent;
---
-2.43.0
-
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
- drivers/gpio/gpiolib.c | 32 ++++++++++++++++++--------------
- 1 file changed, 18 insertions(+), 14 deletions(-)
+ drivers/gpio/gpiolib.c | 31 ++++++++++++++++++-------------
+ 1 file changed, 18 insertions(+), 13 deletions(-)
-diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
-index a6f1a4e2e3a4e..3bd2b3a986be5 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
-@@ -2408,6 +2408,11 @@ char *gpiochip_dup_line_label(struct gpio_chip *gc, unsigned int offset)
+@@ -2402,6 +2402,11 @@ char *gpiochip_dup_line_label(struct gpi
}
EXPORT_SYMBOL_GPL(gpiochip_dup_line_label);
/**
* gpiochip_request_own_desc - Allow GPIO chip to request its own descriptor
* @gc: GPIO chip
-@@ -2436,10 +2441,11 @@ struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc,
+@@ -2430,10 +2435,11 @@ struct gpio_desc *gpiochip_request_own_d
enum gpiod_flags dflags)
{
struct gpio_desc *desc = gpiochip_get_desc(gc, hwnum);
return desc;
}
-@@ -2449,8 +2455,8 @@ struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc,
+@@ -2443,8 +2449,8 @@ struct gpio_desc *gpiochip_request_own_d
ret = gpiod_configure_flags(desc, label, lflags, dflags);
if (ret) {
return ERR_PTR(ret);
}
-@@ -4125,19 +4131,17 @@ static struct gpio_desc *gpiod_find_by_fwnode(struct fwnode_handle *fwnode,
+@@ -4119,19 +4125,17 @@ static struct gpio_desc *gpiod_find_by_f
enum gpiod_flags *flags,
unsigned long *lookupflags)
{
desc = swnode_find_gpio(fwnode, con_id, idx, lookupflags);
}
-@@ -4153,6 +4157,7 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
+@@ -4147,6 +4151,7 @@ struct gpio_desc *gpiod_find_and_request
bool platform_lookup_allowed)
{
unsigned long lookupflags = GPIO_LOOKUP_FLAGS_DEFAULT;
+ const char *name = function_name_or_default(con_id);
- /*
- * scoped_guard() is implemented as a for loop, meaning static
- * analyzers will complain about these two not being initialized.
-@@ -4175,8 +4180,7 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
- }
+ struct gpio_desc *desc;
+ int ret;
- if (IS_ERR(desc)) {
-- dev_dbg(consumer, "No GPIO consumer %s found\n",
-- con_id);
-+ dev_dbg(consumer, "No GPIO consumer %s found\n", name);
- return desc;
- }
+@@ -4162,7 +4167,7 @@ struct gpio_desc *gpiod_find_and_request
+ }
-@@ -4198,15 +4202,14 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
+ if (IS_ERR(desc)) {
+- dev_dbg(consumer, "No GPIO consumer %s found\n", con_id);
++ dev_dbg(consumer, "No GPIO consumer %s found\n", name);
+ return desc;
+ }
+
+@@ -4183,15 +4188,14 @@ struct gpio_desc *gpiod_find_and_request
*
* FIXME: Make this more sane and safe.
*/
return ERR_PTR(ret);
}
-@@ -4322,6 +4325,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_optional);
+@@ -4307,6 +4311,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_optional);
int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
unsigned long lflags, enum gpiod_flags dflags)
{
int ret;
if (lflags & GPIO_ACTIVE_LOW)
-@@ -4365,7 +4369,7 @@ int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
+@@ -4350,7 +4355,7 @@ int gpiod_configure_flags(struct gpio_de
/* No particular flag request, return here... */
if (!(dflags & GPIOD_FLAGS_BIT_DIR_SET)) {
return 0;
}
---
-2.43.0
-
projects / thirdparty / kernel / stable-queue.git / commitdiff
