| Message ID | 20240130124828.14678-2-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: rework locking and object life-time control | expand |
Hi Bartosz,
kernel test robot noticed the following build warnings:
[auto build test WARNING on brgl/gpio/for-next]
[also build test WARNING on linus/master v6.8-rc2 next-20240131]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/gpio-protect-the-list-of-GPIO-devices-with-SRCU/20240130-205537
base: https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link: https://lore.kernel.org/r/20240130124828.14678-2-brgl%40bgdev.pl
patch subject: [PATCH 01/22] gpio: protect the list of GPIO devices with SRCU
config: i386-randconfig-141-20240131 (https://download.01.org/0day-ci/archive/20240131/202401311746.be3dlVTg-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401311746.be3dlVTg-lkp@intel.com/
New smatch warnings:
drivers/gpio/gpiolib.c:4167 gpiod_find_and_request() error: uninitialized symbol 'ret'.
drivers/gpio/gpiolib.c:4181 gpiod_find_and_request() error: uninitialized symbol 'desc'.
Old smatch warnings:
drivers/gpio/gpiolib.c:4184 gpiod_find_and_request() error: uninitialized symbol 'desc'.
vim +/ret +4167 drivers/gpio/gpiolib.c
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4128
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4129 static struct gpio_desc *gpiod_find_and_request(struct device *consumer,
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4130 struct fwnode_handle *fwnode,
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4131 const char *con_id,
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4132 unsigned int idx,
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4133 enum gpiod_flags flags,
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4134 const char *label,
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4135 bool platform_lookup_allowed)
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4136 {
ba2dc1cb5491712 Hans de Goede 2022-12-29 4137 unsigned long lookupflags = GPIO_LOOKUP_FLAGS_DEFAULT;
c122f461ccac0e7 Andy Shevchenko 2023-03-09 4138 struct gpio_desc *desc;
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4139 int ret;
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4140
1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4141 scoped_guard(srcu, &gpio_devices_srcu) {
1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4142 desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx,
1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4143 &flags, &lookupflags);
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4144 if (gpiod_not_found(desc) && platform_lookup_allowed) {
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4145 /*
1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4146 * Either we are not using DT or ACPI, or their lookup
1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4147 * did not return a result. In that case, use platform
1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4148 * lookup as a fallback.
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4149 */
1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4150 dev_dbg(consumer,
1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4151 "using lookup tables for GPIO lookup\n");
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4152 desc = gpiod_find(consumer, con_id, idx, &lookupflags);
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4153 }
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4154
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4155 if (IS_ERR(desc)) {
1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4156 dev_dbg(consumer, "No GPIO consumer %s found\n",
1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4157 con_id);
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4158 return desc;
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4159 }
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4160
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4161 /*
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4162 * If a connection label was passed use that, else attempt to use
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4163 * the device name as label
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4164 */
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4165 ret = gpiod_request(desc, label);
1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4166 }
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 @4167 if (ret) {
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4168 if (!(ret == -EBUSY && flags & GPIOD_FLAGS_BIT_NONEXCLUSIVE))
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4169 return ERR_PTR(ret);
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4170
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4171 /*
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4172 * This happens when there are several consumers for
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4173 * the same GPIO line: we just return here without
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4174 * further initialization. It is a bit of a hack.
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4175 * This is necessary to support fixed regulators.
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4176 *
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4177 * FIXME: Make this more sane and safe.
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4178 */
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4179 dev_info(consumer,
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4180 "nonexclusive access to GPIO for %s\n", con_id);
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 @4181 return desc;
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4182 }
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4183
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4184 ret = gpiod_configure_flags(desc, con_id, lookupflags, flags);
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4185 if (ret < 0) {
8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4186 dev_dbg(consumer, "setup of GPIO %s failed\n", con_id);
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4187 gpiod_put(desc);
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4188 return ERR_PTR(ret);
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4189 }
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4190
9ce4ed5b4db1363 Bartosz Golaszewski 2023-08-21 4191 gpiod_line_state_notify(desc, GPIOLINE_CHANGED_REQUESTED);
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4192
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4193 return desc;
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4194 }
0eadd36d9123745 Dmitry Torokhov 2022-09-03 4195
On Wed, Jan 31, 2024 at 10:37 AM kernel test robot <lkp@intel.com> wrote: > > Hi Bartosz, > > kernel test robot noticed the following build warnings: > > [auto build test WARNING on brgl/gpio/for-next] > [also build test WARNING on linus/master v6.8-rc2 next-20240131] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/gpio-protect-the-list-of-GPIO-devices-with-SRCU/20240130-205537 > base: https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next > patch link: https://lore.kernel.org/r/20240130124828.14678-2-brgl%40bgdev.pl > patch subject: [PATCH 01/22] gpio: protect the list of GPIO devices with SRCU > config: i386-randconfig-141-20240131 (https://download.01.org/0day-ci/archive/20240131/202401311746.be3dlVTg-lkp@intel.com/config) > compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202401311746.be3dlVTg-lkp@intel.com/ > > New smatch warnings: > drivers/gpio/gpiolib.c:4167 gpiod_find_and_request() error: uninitialized symbol 'ret'. > drivers/gpio/gpiolib.c:4181 gpiod_find_and_request() error: uninitialized symbol 'desc'. > > Old smatch warnings: > drivers/gpio/gpiolib.c:4184 gpiod_find_and_request() error: uninitialized symbol 'desc'. > > vim +/ret +4167 drivers/gpio/gpiolib.c > > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4128 > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4129 static struct gpio_desc *gpiod_find_and_request(struct device *consumer, > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4130 struct fwnode_handle *fwnode, > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4131 const char *con_id, > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4132 unsigned int idx, > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4133 enum gpiod_flags flags, > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4134 const char *label, > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4135 bool platform_lookup_allowed) > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4136 { > ba2dc1cb5491712 Hans de Goede 2022-12-29 4137 unsigned long lookupflags = GPIO_LOOKUP_FLAGS_DEFAULT; > c122f461ccac0e7 Andy Shevchenko 2023-03-09 4138 struct gpio_desc *desc; > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4139 int ret; > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4140 > 1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4141 scoped_guard(srcu, &gpio_devices_srcu) { > 1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4142 desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx, > 1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4143 &flags, &lookupflags); > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4144 if (gpiod_not_found(desc) && platform_lookup_allowed) { > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4145 /* > 1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4146 * Either we are not using DT or ACPI, or their lookup > 1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4147 * did not return a result. In that case, use platform > 1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4148 * lookup as a fallback. > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4149 */ > 1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4150 dev_dbg(consumer, > 1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4151 "using lookup tables for GPIO lookup\n"); > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4152 desc = gpiod_find(consumer, con_id, idx, &lookupflags); > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4153 } > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4154 > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4155 if (IS_ERR(desc)) { > 1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4156 dev_dbg(consumer, "No GPIO consumer %s found\n", > 1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4157 con_id); > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4158 return desc; > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4159 } > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4160 > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4161 /* > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4162 * If a connection label was passed use that, else attempt to use > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4163 * the device name as label > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4164 */ > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4165 ret = gpiod_request(desc, label); > 1fe5210a1bfba00 Bartosz Golaszewski 2024-01-30 4166 } > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 @4167 if (ret) { > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4168 if (!(ret == -EBUSY && flags & GPIOD_FLAGS_BIT_NONEXCLUSIVE)) > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4169 return ERR_PTR(ret); > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4170 > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4171 /* > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4172 * This happens when there are several consumers for > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4173 * the same GPIO line: we just return here without > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4174 * further initialization. It is a bit of a hack. > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4175 * This is necessary to support fixed regulators. > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4176 * > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4177 * FIXME: Make this more sane and safe. > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4178 */ > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4179 dev_info(consumer, > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4180 "nonexclusive access to GPIO for %s\n", con_id); > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 @4181 return desc; > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4182 } > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4183 > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4184 ret = gpiod_configure_flags(desc, con_id, lookupflags, flags); > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4185 if (ret < 0) { > 8eb1f71e7acca4f Dmitry Torokhov 2022-11-11 4186 dev_dbg(consumer, "setup of GPIO %s failed\n", con_id); > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4187 gpiod_put(desc); > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4188 return ERR_PTR(ret); > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4189 } > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4190 > 9ce4ed5b4db1363 Bartosz Golaszewski 2023-08-21 4191 gpiod_line_state_notify(desc, GPIOLINE_CHANGED_REQUESTED); > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4192 > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4193 return desc; > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4194 } > 0eadd36d9123745 Dmitry Torokhov 2022-09-03 4195 > > -- > 0-DAY CI Kernel Test Service > https://github.com/intel/lkp-tests/wiki This is a false-positive coming from the fact the scoped_guard() is implemented as a for loop. I will initialize the variables anyway to make smatch happy. Bart
On Tue, Jan 30, 2024 at 1:48 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > 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> This looks to be doing the right thing to my RCU-untrained eye, so: Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index d50a786f8176..69979de76485 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; } @@ -813,7 +817,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 +881,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); + scoped_guard(mutex, &gpio_devices_lock) { + /* + * 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) { - spin_unlock_irqrestore(&gpio_lock, flags); - ret = base; - base = 0; + base = gpiochip_find_base_unlocked(gc->ngpio); + if (base < 0) { + ret = base; + base = 0; + 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; } - /* - * 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) { - spin_unlock_irqrestore(&gpio_lock, flags); - 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); @@ -1011,9 +1012,9 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, goto err_print_message; } 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); err_free_label: kfree_const(gdev->label); err_free_descs: @@ -1076,8 +1077,9 @@ 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); + scoped_guard(mutex, &gpio_devices_lock) + list_del_rcu(&gdev->list); + synchronize_srcu(&gpio_devices_srcu); /* * The gpiochip side puts its use of the device to rest here: @@ -1125,7 +1127,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)) @@ -4136,27 +4138,32 @@ static struct gpio_desc *gpiod_find_and_request(struct device *consumer, struct gpio_desc *desc; int ret; - desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx, &flags, &lookupflags); - if (gpiod_not_found(desc) && platform_lookup_allowed) { + 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; + } + /* - * 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); + ret = gpiod_request(desc, label); } - - if (IS_ERR(desc)) { - dev_dbg(consumer, "No GPIO consumer %s found\n", con_id); - return desc; - } - - /* - * 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); @@ -4727,35 +4734,33 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev) static void *gpiolib_seq_start(struct seq_file *s, loff_t *pos) { - unsigned long flags; struct gpio_device *gdev = NULL; loff_t index = *pos; s->private = ""; - spin_lock_irqsave(&gpio_lock, flags); - list_for_each_entry(gdev, &gpio_devices, list) - if (index-- == 0) { - spin_unlock_irqrestore(&gpio_lock, flags); + guard(srcu)(&gpio_devices_srcu); + + list_for_each_entry(gdev, &gpio_devices, list) { + 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; - 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); + scoped_guard(srcu, &gpio_devices_srcu) { + if (list_is_last(&gdev->list, &gpio_devices)) + ret = NULL; + else + ret = list_first_entry(&gdev->list, struct gpio_device, + list); + } s->private = "\n"; ++*pos;