Re: Patch "gpio: protect the list of GPIO devices with SRCU" has been added to the 6.8-stable tree

Re: Patch "gpio: protect the list of GPIO devices with SRCU" has been added to the 6.8-stable tree

[Bartosz Golaszewski]

On Wed, 3 Apr 2024 at 18:02, Sasha Levin <sashal@kernel.org> wrote:
>
> This is a note to let you know that I've just added the patch titled
>
>     gpio: protect the list of GPIO devices with SRCU
>
> to the 6.8-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>
> The filename of the patch is:
>      gpio-protect-the-list-of-gpio-devices-with-srcu.patch
> and it can be found in the queue-6.8 subdirectory.
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.
>
>
>
> commit 077106f97c7d113ebacb00725d83b817d0e89288
> Author: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Date:   Fri Jan 19 16:43:13 2024 +0100
>
>     gpio: protect the list of GPIO devices with SRCU
>
>     [ 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>
>

I'm not sure what the reason for picking this up into stable was but I
believe it's not a good idea. This is just the first patch in a big
series[1] of 24 commits total on top of which we had several bug fixes
during the stabilization phase in next. Without the rest of the
rework, it doesn't really improve the situation a lot.

I suggest dropping this and not trying to backport any of the GPIOLIB
locking rework to stable branches.

Best Regards,
Bartosz

[1] https://lore.kernel.org/lkml/20240208095920.8035-22-brgl@bgdev.pl/T/

Re: Patch "gpio: protect the list of GPIO devices with SRCU" has been added to the 6.8-stable tree

[Greg KH]

On Wed, Apr 03, 2024 at 11:05:42PM +0200, Bartosz Golaszewski wrote:
> On Wed, 3 Apr 2024 at 18:02, Sasha Levin <sashal@kernel.org> wrote:
> >
> > This is a note to let you know that I've just added the patch titled
> >
> >     gpio: protect the list of GPIO devices with SRCU
> >
> > to the 6.8-stable tree which can be found at:
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> >
> > The filename of the patch is:
> >      gpio-protect-the-list-of-gpio-devices-with-srcu.patch
> > and it can be found in the queue-6.8 subdirectory.
> >
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@vger.kernel.org> know about it.
> >
> >
> >
> > commit 077106f97c7d113ebacb00725d83b817d0e89288
> > Author: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > Date:   Fri Jan 19 16:43:13 2024 +0100
> >
> >     gpio: protect the list of GPIO devices with SRCU
> >
> >     [ 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>
> >
> 
> I'm not sure what the reason for picking this up into stable was but I
> believe it's not a good idea. This is just the first patch in a big
> series[1] of 24 commits total on top of which we had several bug fixes
> during the stabilization phase in next. Without the rest of the
> rework, it doesn't really improve the situation a lot.
> 
> I suggest dropping this and not trying to backport any of the GPIOLIB
> locking rework to stable branches.

I've dropped this and fixed up the commit this was a dependency for now,
thanks for the review!

greg k-h