From: Kent Gibson <warthog618@gmail.com>
To: David Jander <david@protonic.nl>
Cc: Bartosz Golaszewski <brgl@bgdev.pl>,
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>,
Linus Walleij <linus.walleij@linaro.org>,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: regression: gpiolib: switch the line state notifier to atomic unexpected impact on performance
Date: Wed, 12 Mar 2025 20:10:53 +0800 [thread overview]
Message-ID: <20250312121053.GA132624@rigel> (raw)
In-Reply-To: <20250312112401.5e292612@erd003.prtnl>
On Wed, Mar 12, 2025 at 11:24:01AM +0100, David Jander wrote:
> >
> > Ok, if the issue is purely the name -> (chip,offset) mapping it is pretty
> > safe to assume that line names are immutable - though not unique, so
> > caching the mapping should be fine.
>
> On our board that would be fine, but what about hot-pluggable GPIO
> chips/devices, or modules that are loaded at a later time? I was thinking
> about libgpiod in general...
>
Indeed. A general solution suitable for libgpiod is a hard problem.
It is probably something better left to a higher layer.
> > The kernel can already tell userspace about a number of changes.
> > What changes are you concerned about - adding/removing chips?
>
> Yes, since the patches from Bartosz I understand that is indeed possible now
> ;-)
> No special concern, just thinking about general applicability of caching such
> information in libgpiod (especially considering the old version I am using
> presumably from long before the kernel could do this).
>
The change notifiers you are referring to have been there for some time
- they were first introduced late in uAPI v1.
I was also thinking of udev events for when devices are added or removed.
Though again, not something core libgpiod should be getting involved with.
> > > If this had been this slow always (even before 6.13), I would probably have
> > > done things a bit differently and cached the config requests to then "find"
> > > the lines in batches directly working on the character devices instead of
> > > using gpiod, so I could open/close each one just once for finding many
> > > different lines each time.
> >
> > The libgpiod v2 tools do just that - they scan the chips once rather
> > than once per line. But that functionality is not exposed in the
> > libgpiod v2 API as the C interface is hideous and it is difficult to
> > provide well defined behaviour (e.g. in what order are the chips scanned?).
> > So it is left to the application to determine how they want to do it.
> > There isn't even a find_line() equivalent in v2, IIRC.
>
> I think I should move to v2 as soon as I find the time to do it. ;-)
>
You certainly should - the v2 Python bindings are much more pythonic and
easier to use.
> In any case, I also could reproduce the issue with the gpiodetect tool from
> v2. You can visually see each found chips being printed individually on the
> terminal with kernel v6.13, while with 6.12 all chip names would appear
> "instantly". Hard to describe with words, but you could in fact tell which
> kernel was running just by looking at the terminal output of "gpiodetect"
> while it was being executed... my board has 16 gpio chips, so you can really
> see it "scrolling" up as it prints them with kernel 6.13.
>
For sure - the close() issue is a real issue.
> > > > Generally an application should request the lines it requires once and hold
> > > > them for the duration. Similarly functions such as find_line() should be
> > > > performed once per line.
> > >
> > > Of course it does that ;-)
> > > This board has a large amount of GPIO lines, and like I said, it is during the
> > > initial configuration phase of the application that I am seeing this problem.
> > >
> >
> > Good to hear - from your earlier description I was concerned that
> > you might be doing it continuously.
>
> Thanks. Tbh, I am quite proud of the efficiency and speed of the application
> itself. Being written in pure python and running on a rather slow Cortex-A7,
> it is surprisingly fast, controlling 16 stepper motors, reacting to 26 sensor
> inputs changing at a blazing high pace and continuously sampling several IIO
> adcs at 16kHz sample rate, all with rather low CPU usage (in python!). It makes
> heavy use of asyncio an thus of course the epoll() event mechanisms the kernel
> provides for GPIOs, IIO, LMC and other interfaces.
You can do some surprising things with Python.
> So you may understand that I was a bit triggered by your suggestion of
> inefficiency initially. Sorry ;-)
>
No problems - I've seen people do some silly things and just wanted
to be sure you weren't one of them ;-).
Glad everything is working for you.
Cheers,
Kent.
next prev parent reply other threads:[~2025-03-12 12:10 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-11 10:00 regression: gpiolib: switch the line state notifier to atomic unexpected impact on performance David Jander
2025-03-11 10:21 ` Bartosz Golaszewski
2025-03-11 11:03 ` David Jander
2025-03-12 1:32 ` Kent Gibson
2025-03-12 8:08 ` David Jander
2025-03-12 9:10 ` Kent Gibson
2025-03-12 10:24 ` David Jander
2025-03-12 12:10 ` Kent Gibson [this message]
2025-03-11 11:45 ` Bartosz Golaszewski
2025-03-11 12:30 ` David Jander
2025-03-11 13:21 ` Bartosz Golaszewski
2025-03-11 14:09 ` David Jander
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=20250312121053.GA132624@rigel \
--to=warthog618@gmail.com \
--cc=bartosz.golaszewski@linaro.org \
--cc=brgl@bgdev.pl \
--cc=david@protonic.nl \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/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