Linux-GPIO Archive on lore.kernel.org
 help / color / Atom feed
* gpiolib gpio_chrdev_release duration is about 30 ms
@ 2020-07-17 12:56 Maxim Kochetkov
  2020-07-17 13:37 ` Linus Walleij
  0 siblings, 1 reply; 11+ messages in thread
From: Maxim Kochetkov @ 2020-07-17 12:56 UTC (permalink / raw)
  To: linux-gpio; +Cc: bgolaszewski, linus.walleij

Hi.

I'm using libgpiod in userspace.
I have 6 gpiochip's on my board.
gpiod_line_find takes about 300ms to find GPIO line.

gpiod_line_find calls gpiod_foreach_chip
then gpiod_chip_iter_next
then gpiod_chip_close then close(chip->fd)
then we are going to kernel gpiolib gpio_chrdev_release
then atomic_notifier_chain_unregister
then synchronize_rcu()

synchronize_rcu takes about 30 ms (6*30ms=280ms)

I tried to remove synchronize_rcu from atomic_notifier_chain_unregister 
and gpiod_line_find takes about 2ms now.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: gpiolib gpio_chrdev_release duration is about 30 ms
  2020-07-17 12:56 gpiolib gpio_chrdev_release duration is about 30 ms Maxim Kochetkov
@ 2020-07-17 13:37 ` Linus Walleij
  2020-07-17 13:44   ` Maxim Kochetkov
  2020-07-17 14:17   ` Maxim Kochetkov
  0 siblings, 2 replies; 11+ messages in thread
From: Linus Walleij @ 2020-07-17 13:37 UTC (permalink / raw)
  To: Maxim Kochetkov; +Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski

Hi Maxim,

On Fri, Jul 17, 2020 at 2:56 PM Maxim Kochetkov <fido_max@inbox.ru> wrote:

> I'm using libgpiod in userspace.
> I have 6 gpiochip's on my board.
> gpiod_line_find takes about 300ms to find GPIO line.
>
> gpiod_line_find calls gpiod_foreach_chip
> then gpiod_chip_iter_next
> then gpiod_chip_close then close(chip->fd)
> then we are going to kernel gpiolib gpio_chrdev_release
> then atomic_notifier_chain_unregister
> then synchronize_rcu()
>
> synchronize_rcu takes about 30 ms (6*30ms=280ms)
>
> I tried to remove synchronize_rcu from atomic_notifier_chain_unregister
> and gpiod_line_find takes about 2ms now.

Interesting! Can you provide some context? Are you just testing because
curious or do you need to meet a design objective?

Did you use ftrace or similar instrumentation to drill down and find
where time is spent?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: gpiolib gpio_chrdev_release duration is about 30 ms
  2020-07-17 13:37 ` Linus Walleij
@ 2020-07-17 13:44   ` Maxim Kochetkov
  2020-07-17 14:17   ` Maxim Kochetkov
  1 sibling, 0 replies; 11+ messages in thread
From: Maxim Kochetkov @ 2020-07-17 13:44 UTC (permalink / raw)
  To: Linus Walleij; +Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski

------------------------------------------
#define GPIOS_NUM 8
static char* gpio_names[GPIOS_NUM] = {"IO0", "IO1", "IO2", "IO3", "WR", 
"CS", "OE", "ADD_EN"};
................
gpio_init() {

     int i;
     for (i = 0; i < GPIOS_NUM; ++i) {
         syslog(LOG_INFO,"%s %d",__FUNCTION__,__LINE__);
         gpio_lines[i] = gpiod_line_find(gpio_names[i]);
         syslog(LOG_INFO,"%s %d",__FUNCTION__,__LINE__);
     }
}
-------------------------------------------
I used syslog/printk to measure time.
syslog in example shows about 300ms on each gpiod_line_find call.
17.07.2020 16:37, Linus Walleij пишет:
> Hi Maxim,
> 
> On Fri, Jul 17, 2020 at 2:56 PM Maxim Kochetkov <fido_max@inbox.ru> wrote:
> 
>> I'm using libgpiod in userspace.
>> I have 6 gpiochip's on my board.
>> gpiod_line_find takes about 300ms to find GPIO line.
>>
>> gpiod_line_find calls gpiod_foreach_chip
>> then gpiod_chip_iter_next
>> then gpiod_chip_close then close(chip->fd)
>> then we are going to kernel gpiolib gpio_chrdev_release
>> then atomic_notifier_chain_unregister
>> then synchronize_rcu()
>>
>> synchronize_rcu takes about 30 ms (6*30ms=280ms)
>>
>> I tried to remove synchronize_rcu from atomic_notifier_chain_unregister
>> and gpiod_line_find takes about 2ms now.
> 
> Interesting! Can you provide some context? Are you just testing because
> curious or do you need to meet a design objective?
> 
> Did you use ftrace or similar instrumentation to drill down and find
> where time is spent?
> 
> Yours,
> Linus Walleij
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: gpiolib gpio_chrdev_release duration is about 30 ms
  2020-07-17 13:37 ` Linus Walleij
  2020-07-17 13:44   ` Maxim Kochetkov
@ 2020-07-17 14:17   ` Maxim Kochetkov
  2020-07-17 15:07     ` Andy Shevchenko
  1 sibling, 1 reply; 11+ messages in thread
From: Maxim Kochetkov @ 2020-07-17 14:17 UTC (permalink / raw)
  To: Linus Walleij; +Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski

I need a small userspace program to do some GPIO magic to communicate 
other hardware like devmem. This program takes about 2,5 seconds just to 
find GPIO lines by name.

replacing synchronize_rcu to synchronize_rcu_expedited in 
atomic_notifier_chain_unregister gives the same boost as removing 
synchronize_rcu

17.07.2020 16:37, Linus Walleij пишет:
> Are you just testing because
> curious or do you need to meet a design objective?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: gpiolib gpio_chrdev_release duration is about 30 ms
  2020-07-17 14:17   ` Maxim Kochetkov
@ 2020-07-17 15:07     ` Andy Shevchenko
  2020-07-18  4:25       ` Kent Gibson
  0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2020-07-17 15:07 UTC (permalink / raw)
  To: Maxim Kochetkov
  Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski

On Fri, Jul 17, 2020 at 5:17 PM Maxim Kochetkov <fido_max@inbox.ru> wrote:
>
> I need a small userspace program to do some GPIO magic to communicate
> other hardware like devmem. This program takes about 2,5 seconds just to
> find GPIO lines by name.
>
> replacing synchronize_rcu to synchronize_rcu_expedited in
> atomic_notifier_chain_unregister gives the same boost as removing
> synchronize_rcu

Have you tried to replace an atomic notifier call with a regular one?
IIRC it's still not clear why atomic is used there.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: gpiolib gpio_chrdev_release duration is about 30 ms
  2020-07-17 15:07     ` Andy Shevchenko
@ 2020-07-18  4:25       ` Kent Gibson
  2020-07-20  8:14         ` Maxim Kochetkov
  0 siblings, 1 reply; 11+ messages in thread
From: Kent Gibson @ 2020-07-18  4:25 UTC (permalink / raw)
  To: Maxim Kochetkov
  Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
	Andy Shevchenko

On Fri, Jul 17, 2020 at 06:07:04PM +0300, Andy Shevchenko wrote:
> On Fri, Jul 17, 2020 at 5:17 PM Maxim Kochetkov <fido_max@inbox.ru> wrote:
> >
> > I need a small userspace program to do some GPIO magic to communicate
> > other hardware like devmem. This program takes about 2,5 seconds just to
> > find GPIO lines by name.
> >
> > replacing synchronize_rcu to synchronize_rcu_expedited in
> > atomic_notifier_chain_unregister gives the same boost as removing
> > synchronize_rcu
> 
> Have you tried to replace an atomic notifier call with a regular one?
> IIRC it's still not clear why atomic is used there.
> 

Indeed, I recently submitted a patch to switch the
atomic_notifier_call_chain to blocking_notifier_call_chain, as some of
the chained calls can sleep.
Not sure if that is related, or if the change would make this case better
or worse, but it would be interesting to find out.
The patch is in the current gpio/devel, btw.

Cheers,
Kent.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: gpiolib gpio_chrdev_release duration is about 30 ms
  2020-07-18  4:25       ` Kent Gibson
@ 2020-07-20  8:14         ` Maxim Kochetkov
  2020-07-24 19:36           ` Bartosz Golaszewski
  0 siblings, 1 reply; 11+ messages in thread
From: Maxim Kochetkov @ 2020-07-20  8:14 UTC (permalink / raw)
  To: Kent Gibson
  Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
	Andy Shevchenko

It works fine with this patch. Thank you so much!

18.07.2020 07:25, Kent Gibson пишет:
> On Fri, Jul 17, 2020 at 06:07:04PM +0300, Andy Shevchenko wrote:
>> On Fri, Jul 17, 2020 at 5:17 PM Maxim Kochetkov <fido_max@inbox.ru> wrote:
>>>
>>> I need a small userspace program to do some GPIO magic to communicate
>>> other hardware like devmem. This program takes about 2,5 seconds just to
>>> find GPIO lines by name.
>>>
>>> replacing synchronize_rcu to synchronize_rcu_expedited in
>>> atomic_notifier_chain_unregister gives the same boost as removing
>>> synchronize_rcu
>>
>> Have you tried to replace an atomic notifier call with a regular one?
>> IIRC it's still not clear why atomic is used there.
>>
> 
> Indeed, I recently submitted a patch to switch the
> atomic_notifier_call_chain to blocking_notifier_call_chain, as some of
> the chained calls can sleep.
> Not sure if that is related, or if the change would make this case better
> or worse, but it would be interesting to find out.
> The patch is in the current gpio/devel, btw.
> 
> Cheers,
> Kent.
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: gpiolib gpio_chrdev_release duration is about 30 ms
  2020-07-20  8:14         ` Maxim Kochetkov
@ 2020-07-24 19:36           ` Bartosz Golaszewski
  2020-07-25  3:52             ` Kent Gibson
  0 siblings, 1 reply; 11+ messages in thread
From: Bartosz Golaszewski @ 2020-07-24 19:36 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Kent Gibson, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
	Andy Shevchenko, Maxim Kochetkov

On Mon, Jul 20, 2020 at 10:14 AM Maxim Kochetkov <fido_max@inbox.ru> wrote:
>
> It works fine with this patch. Thank you so much!
>
> 18.07.2020 07:25, Kent Gibson пишет:
> > On Fri, Jul 17, 2020 at 06:07:04PM +0300, Andy Shevchenko wrote:
> >> On Fri, Jul 17, 2020 at 5:17 PM Maxim Kochetkov <fido_max@inbox.ru> wrote:
> >>>
> >>> I need a small userspace program to do some GPIO magic to communicate
> >>> other hardware like devmem. This program takes about 2,5 seconds just to
> >>> find GPIO lines by name.
> >>>
> >>> replacing synchronize_rcu to synchronize_rcu_expedited in
> >>> atomic_notifier_chain_unregister gives the same boost as removing
> >>> synchronize_rcu
> >>
> >> Have you tried to replace an atomic notifier call with a regular one?
> >> IIRC it's still not clear why atomic is used there.
> >>
> >
> > Indeed, I recently submitted a patch to switch the
> > atomic_notifier_call_chain to blocking_notifier_call_chain, as some of
> > the chained calls can sleep.
> > Not sure if that is related, or if the change would make this case better
> > or worse, but it would be interesting to find out.
> > The patch is in the current gpio/devel, btw.
> >
> > Cheers,
> > Kent.
> >

Linus,

I think we should consider submitting this patch for stable then
because this slow-down was affects previously existing use-cases.

Bartosz

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: gpiolib gpio_chrdev_release duration is about 30 ms
  2020-07-24 19:36           ` Bartosz Golaszewski
@ 2020-07-25  3:52             ` Kent Gibson
  2020-07-26 11:04               ` Bartosz Golaszewski
  2020-07-26 22:32               ` Linus Walleij
  0 siblings, 2 replies; 11+ messages in thread
From: Kent Gibson @ 2020-07-25  3:52 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
	Andy Shevchenko, Maxim Kochetkov

On Fri, Jul 24, 2020 at 09:36:03PM +0200, Bartosz Golaszewski wrote:
> On Mon, Jul 20, 2020 at 10:14 AM Maxim Kochetkov <fido_max@inbox.ru> wrote:
> >
> > It works fine with this patch. Thank you so much!
> >
> > 18.07.2020 07:25, Kent Gibson пишет:
> > > On Fri, Jul 17, 2020 at 06:07:04PM +0300, Andy Shevchenko wrote:
> > >> On Fri, Jul 17, 2020 at 5:17 PM Maxim Kochetkov <fido_max@inbox.ru> wrote:
> > >>>
> > >>> I need a small userspace program to do some GPIO magic to communicate
> > >>> other hardware like devmem. This program takes about 2,5 seconds just to
> > >>> find GPIO lines by name.
> > >>>
> > >>> replacing synchronize_rcu to synchronize_rcu_expedited in
> > >>> atomic_notifier_chain_unregister gives the same boost as removing
> > >>> synchronize_rcu
> > >>
> > >> Have you tried to replace an atomic notifier call with a regular one?
> > >> IIRC it's still not clear why atomic is used there.
> > >>
> > >
> > > Indeed, I recently submitted a patch to switch the
> > > atomic_notifier_call_chain to blocking_notifier_call_chain, as some of
> > > the chained calls can sleep.
> > > Not sure if that is related, or if the change would make this case better
> > > or worse, but it would be interesting to find out.
> > > The patch is in the current gpio/devel, btw.
> > >
> > > Cheers,
> > > Kent.
> > >
> 
> Linus,
> 
> I think we should consider submitting this patch for stable then
> because this slow-down was affects previously existing use-cases.
> 

That patch is post the cdev split, so it certainly wont apply to an
older kernel.  I'm happy to backport it if you need it - just
nominate the branch you want it for.

Cheers,
Kent.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: gpiolib gpio_chrdev_release duration is about 30 ms
  2020-07-25  3:52             ` Kent Gibson
@ 2020-07-26 11:04               ` Bartosz Golaszewski
  2020-07-26 22:32               ` Linus Walleij
  1 sibling, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2020-07-26 11:04 UTC (permalink / raw)
  To: Kent Gibson
  Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
	Andy Shevchenko, Maxim Kochetkov

On Sat, Jul 25, 2020 at 5:52 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Fri, Jul 24, 2020 at 09:36:03PM +0200, Bartosz Golaszewski wrote:
> > On Mon, Jul 20, 2020 at 10:14 AM Maxim Kochetkov <fido_max@inbox.ru> wrote:
> > >
> > > It works fine with this patch. Thank you so much!
> > >
> > > 18.07.2020 07:25, Kent Gibson пишет:
> > > > On Fri, Jul 17, 2020 at 06:07:04PM +0300, Andy Shevchenko wrote:
> > > >> On Fri, Jul 17, 2020 at 5:17 PM Maxim Kochetkov <fido_max@inbox.ru> wrote:
> > > >>>
> > > >>> I need a small userspace program to do some GPIO magic to communicate
> > > >>> other hardware like devmem. This program takes about 2,5 seconds just to
> > > >>> find GPIO lines by name.
> > > >>>
> > > >>> replacing synchronize_rcu to synchronize_rcu_expedited in
> > > >>> atomic_notifier_chain_unregister gives the same boost as removing
> > > >>> synchronize_rcu
> > > >>
> > > >> Have you tried to replace an atomic notifier call with a regular one?
> > > >> IIRC it's still not clear why atomic is used there.
> > > >>
> > > >
> > > > Indeed, I recently submitted a patch to switch the
> > > > atomic_notifier_call_chain to blocking_notifier_call_chain, as some of
> > > > the chained calls can sleep.
> > > > Not sure if that is related, or if the change would make this case better
> > > > or worse, but it would be interesting to find out.
> > > > The patch is in the current gpio/devel, btw.
> > > >
> > > > Cheers,
> > > > Kent.
> > > >
> >
> > Linus,
> >
> > I think we should consider submitting this patch for stable then
> > because this slow-down was affects previously existing use-cases.
> >
>
> That patch is post the cdev split, so it certainly wont apply to an
> older kernel.  I'm happy to backport it if you need it - just
> nominate the branch you want it for.
>
> Cheers,
> Kent.

Linus' devel branch is fine - I haven't picked up new patches in past weeks.

Bartosz

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: gpiolib gpio_chrdev_release duration is about 30 ms
  2020-07-25  3:52             ` Kent Gibson
  2020-07-26 11:04               ` Bartosz Golaszewski
@ 2020-07-26 22:32               ` Linus Walleij
  1 sibling, 0 replies; 11+ messages in thread
From: Linus Walleij @ 2020-07-26 22:32 UTC (permalink / raw)
  To: Kent Gibson
  Cc: Bartosz Golaszewski, open list:GPIO SUBSYSTEM,
	Bartosz Golaszewski, Andy Shevchenko, Maxim Kochetkov

On Sat, Jul 25, 2020 at 5:52 AM Kent Gibson <warthog618@gmail.com> wrote:
> On Fri, Jul 24, 2020 at 09:36:03PM +0200, Bartosz Golaszewski wrote:
> > On Mon, Jul 20, 2020 at 10:14 AM Maxim Kochetkov <fido_max@inbox.ru> wrote:
> > >
> > > It works fine with this patch. Thank you so much!
> > >
> > > 18.07.2020 07:25, Kent Gibson пишет:
> > > > On Fri, Jul 17, 2020 at 06:07:04PM +0300, Andy Shevchenko wrote:
> > > >> On Fri, Jul 17, 2020 at 5:17 PM Maxim Kochetkov <fido_max@inbox.ru> wrote:
> > > >>>
> > > >>> I need a small userspace program to do some GPIO magic to communicate
> > > >>> other hardware like devmem. This program takes about 2,5 seconds just to
> > > >>> find GPIO lines by name.
> > > >>>
> > > >>> replacing synchronize_rcu to synchronize_rcu_expedited in
> > > >>> atomic_notifier_chain_unregister gives the same boost as removing
> > > >>> synchronize_rcu
> > > >>
> > > >> Have you tried to replace an atomic notifier call with a regular one?
> > > >> IIRC it's still not clear why atomic is used there.
> > > >>
> > > >
> > > > Indeed, I recently submitted a patch to switch the
> > > > atomic_notifier_call_chain to blocking_notifier_call_chain, as some of
> > > > the chained calls can sleep.
> > > > Not sure if that is related, or if the change would make this case better
> > > > or worse, but it would be interesting to find out.
> > > > The patch is in the current gpio/devel, btw.
> > > >
> > > > Cheers,
> > > > Kent.
> > > >
> >
> > Linus,
> >
> > I think we should consider submitting this patch for stable then
> > because this slow-down was affects previously existing use-cases.
> >
>
> That patch is post the cdev split, so it certainly wont apply to an
> older kernel.  I'm happy to backport it if you need it - just
> nominate the branch you want it for.

Let's let this land upstream in the merge window and then we can consider
backports for performance regressions.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-17 12:56 gpiolib gpio_chrdev_release duration is about 30 ms Maxim Kochetkov
2020-07-17 13:37 ` Linus Walleij
2020-07-17 13:44   ` Maxim Kochetkov
2020-07-17 14:17   ` Maxim Kochetkov
2020-07-17 15:07     ` Andy Shevchenko
2020-07-18  4:25       ` Kent Gibson
2020-07-20  8:14         ` Maxim Kochetkov
2020-07-24 19:36           ` Bartosz Golaszewski
2020-07-25  3:52             ` Kent Gibson
2020-07-26 11:04               ` Bartosz Golaszewski
2020-07-26 22:32               ` Linus Walleij

Linux-GPIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-gpio/0 linux-gpio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-gpio linux-gpio/ https://lore.kernel.org/linux-gpio \
		linux-gpio@vger.kernel.org
	public-inbox-index linux-gpio

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-gpio


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git