Re: GPIOLIB locking is broken and how to fix it

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Fri, Dec 8, 2023 at 11:27 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
>
> On Fri, Dec 08, 2023 at 10:52:09AM +0100, Bartosz Golaszewski wrote:
> > On Fri, Dec 8, 2023 at 9:38 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> > >
> > > On Fri, Dec 08, 2023 at 09:13:17AM +0100, Bartosz Golaszewski wrote:
> > > > On Fri, Dec 8, 2023 at 2:01 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> > > > >
> > > > > On Thu, Dec 07, 2023 at 07:37:54PM +0100, Bartosz Golaszewski wrote:
> > > > > > On Tue, Nov 28, 2023 at 11:47 AM Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
> > > > > > >
> > > > > >
> > > > > > [snip]
> > > >
> > >
> > > Yeah, no need to risk other GPIO users messing with it if it is only there
> > > for cdev.
> > > Want me to take a look at it or are you happy to take care of it?
> > >
> >
> > If you'll find the time to do it in the following days then sure, go
> > ahead, otherwise, I'll have some pare cycles today and next week to
> > spend on it.
> >
>
> It would probably take me longer than that to context switch, so go for
> it.
>

Well I went for it and it turns out to be quite tricky. We have
linereq and gpio_chardev_data that have independent lifetimes and the
only resource they share is the gpio_device and - by extension - gpio
descriptors . If we want to store some additional data locally within
the context of gpiolib-cdev.c then I see the following options:

1. Global radix_tree containing additional configuration
(debounce_period_us for now) looked up by the descriptor's address.
Lookup can be done locklessly using RCU and from atomic context
(interrupt handlers in cdev code).

2. Reference counted wrapper around descriptors. It would look something like:

struct gpio_cdev_desc {
    struct kref ref;
    struct gpio_desc *desc;
    unsigned int debounce_period_us;
};

And then struct gpio_chardev_data would store an array of pointers to
this wrapper while struct line would store a pointer to it instead of
directly referencing struct gpio_desc.

Any other ideas?

Bart

> Cheers,
> Kent.
>