Re: GPIOLIB locking is broken and how to fix it

[Kent Gibson <warthog618@xxxxxxxxx>]

On Mon, Dec 11, 2023 at 04:10:27PM +0100, Bartosz Golaszewski wrote:
> On Sun, Dec 10, 2023 at 2:28 PM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> >
> > On Sun, Dec 10, 2023 at 10:34:47AM +0800, Kent Gibson wrote:
> > > > >
> > > >
> >
> > Bah, just ignore me wrt the supplemental info per chip.
> > That solution only works for the chip fd used to request the lines.
> > If you close the chip and re-open it there will be no connection between
> > the new gpio_chardev_data and the existing line requests or the
> > supplemental info.
> >
> > So it would have to be a global indexed by desc as you suggested.
> > Well that sucks.
> >
>
> Yeah, I don't see any other way if we want to contain this within
> gpiolib-cdev.c. I tried fiddling with notifiers but it went nowhere.
> :(
>

I've got a working patch that uses a global rbtree to contain any
struct line that contains supplemental information required by
the chip to fill out the info, i.e. a non-zero debounce period.
The rbtree keeps the memory overhead minimal, as compared to a radix_tree
or xarray, and only the lines containing supplemental info go in the tree,
so the tree size, and the lookup overhead, is kept to a minimum.
There is no performance impact on general use, though there is a minor
overhead in adding the line to the tree when necessary or doing the lookup
to construct lineinfo.

As well as reducing the visibility of that cdev specific field, it reduces
the size of the gpio_desc at the expense of larger struct line, which is
a net win (as there are fewer struct lines than descs), albeit a small one.

I'll tidy it up and submit it so you can see if that works for you.

Cheers,
Kent.