Message-ID: <20250312112401.5e292612@erd003.prtnl>
Date: Wed, 12 Mar 2025 11:24:01 +0100
From: David Jander <david@...tonic.nl>
To: Kent Gibson <warthog618@...il.com>
Cc: Bartosz Golaszewski <brgl@...ev.pl>, Bartosz Golaszewski
 <bartosz.golaszewski@...aro.org>, Linus Walleij <linus.walleij@...aro.org>,
 linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: regression: gpiolib: switch the line state notifier to atomic
 unexpected impact on performance

On Wed, 12 Mar 2025 17:10:56 +0800
Kent Gibson <warthog618@...il.com> wrote:

> On Wed, Mar 12, 2025 at 09:08:29AM +0100, David Jander wrote:
> > On Wed, 12 Mar 2025 09:32:56 +0800
> > Kent Gibson <warthog618@...il.com> wrote:
> >  
> > > On Tue, Mar 11, 2025 at 12:03:46PM +0100, David Jander wrote:  
> > > >
> > > > Indeed, it does. My application is written in python and uses the python gpiod
> > > > module. Even in such an environment the impact is killing.  
> > >
> > > Interesting - the only reason I could think of for an application
> > > requesting/releasing GPIOs at a high rate was it if was built on top of
> > > the libgpiod tools and so was unable to hold the request fd.  
> >  
> 
> Btw, I'm not suggesting that anyone build an app on top of the libgpiod
> tools - I was just hunting for an explanation as to why anyone might be
> opening and closing chips or requests at a high rate.

Very understandable.

> > I didn't want to bother the list with the details, but this is during the
> > configuration phase of the application.  
> 
> The fact that close() was slow is valid but it left me wondering why you
> were needing to do that so frequently.
> It helps to understand what you are doing and why to see if there are
> other better solutions - or it there should be.

You are right. In this case it was warranted to describe the situation in a
bit more detail.

> > It receives many configuration messages
> > for different IO objects at a fast pace. Most of those objects use one or more
> > GPIO lines identified by their label. So the application calls
> > gpiod.find_line(label) on each of them. Apparently libgiod (version 1.6.3 in
> > our case) isn't very efficient, since it will open and close each of the
> > gpiodev devices in order to query for each of the gpio lines. I wouldn't blame
> > libgpiod (python bindings) for doing it that way, since open()/close() of a
> > chardev are expected to be fast, and caching this information is probably
> > error prone anyway, since AFAIK user space cannot yet be informed of changes
> > to gpio chips from kernel space.
> >  
> 
> 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...

> 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).

> > 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. ;-)

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.

> > > 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.
So you may understand that I was a bit triggered by your suggestion of
inefficiency initially. Sorry ;-)

Best regards,

-- 
David Jander

Powered by blists - more mailing lists