public inbox for linux-gpio@vger.kernel.org
 help / color / mirror / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Thierry Reding <thierry.reding@gmail.com>
Subject: Re: GPIOLIB locking is broken and how to fix it
Date: Tue, 12 Dec 2023 08:47:07 +0800	[thread overview]
Message-ID: <ZXetiwHbDGBWBDDF@rigel> (raw)
In-Reply-To: <CAMRc=MfG5jSrmiAxqnDTBd6L=RbD+ZZ2w5KqvH5zK70He6hG5g@mail.gmail.com>

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@gmail.com> 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.

  reply	other threads:[~2023-12-12  0:47 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-24 16:00 GPIOLIB locking is broken and how to fix it Bartosz Golaszewski
2023-11-24 17:27 ` Andy Shevchenko
2023-11-24 17:33   ` Andy Shevchenko
2023-11-24 20:55   ` Bartosz Golaszewski
2023-11-24 23:20 ` Linus Walleij
2023-11-25  1:29 ` Kent Gibson
2023-11-25 20:13   ` Bartosz Golaszewski
2023-11-26  0:05     ` Kent Gibson
2023-11-28 10:47       ` Bartosz Golaszewski
2023-12-07 18:37         ` Bartosz Golaszewski
2023-12-08  1:01           ` Kent Gibson
2023-12-08  8:13             ` Bartosz Golaszewski
2023-12-08  8:38               ` Kent Gibson
2023-12-08  9:52                 ` Bartosz Golaszewski
2023-12-08 10:27                   ` Kent Gibson
2023-12-08 18:54                     ` Bartosz Golaszewski
2023-12-09  1:56                       ` Kent Gibson
2023-12-09 19:24                         ` Bartosz Golaszewski
2023-12-10  2:34                           ` Kent Gibson
2023-12-10 13:28                             ` Kent Gibson
2023-12-11 15:10                               ` Bartosz Golaszewski
2023-12-12  0:47                                 ` Kent Gibson [this message]
2023-12-08 13:12           ` Linus Walleij
2023-12-08 13:56             ` Thierry Reding
2023-12-08 14:47               ` Bartosz Golaszewski
2023-12-08 16:40                 ` Thierry Reding
2023-12-08 18:30                   ` Bartosz Golaszewski
2023-12-11 10:55                     ` Thierry Reding
2023-12-11 15:49                       ` Bartosz Golaszewski
2023-12-12 10:12                         ` Aaro Koskinen
2023-12-12 11:00                           ` Bartosz Golaszewski
2023-12-12 14:32                             ` Aaro Koskinen
2023-12-12 15:15                               ` Bartosz Golaszewski
2023-12-08 13:53           ` Thierry Reding
2023-11-28 11:05       ` Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZXetiwHbDGBWBDDF@rigel \
    --to=warthog618@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=brgl@bgdev.pl \
    --cc=geert@linux-m68k.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=thierry.reding@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox