From: Bartosz Golaszewski <[email protected]>
When discussing the recent user-space changes with Kent and while working
on dbus API for libgpiod I noticed that we really don't have any way of
keeping the line info synchronized between the kernel and user-space
processes. We can of course periodically re-read the line information or
even do it every time we want to read a property but this isn't optimal.
This series adds a new ioctl() that allows user-space to set up a watch on
the GPIO chardev file-descriptor which can then be polled for events
emitted by the kernel when the line is requested, released or its status
changed. This of course doesn't require the line to be requested. Multiple
user-space processes can watch the same lines.
This series also includes a variety of minor tweaks & fixes for problems
discovered during development. For instance it addresses a race-condition
in current line event fifo.
v1: https://lkml.org/lkml/2019/11/27/327
v1 -> v2:
- rework the main patch of the series: re-use the existing file-descriptor
associated with an open character device
- add a patch adding a debug message when the line event kfifo is full and
we're discarding another event
- rework the locking mechanism for lineevent kfifo: reuse the spinlock
from the waitqueue structure
- other minor changes
v2 -> v3:
- added patches providing new implementation for some kfifo macros
- fixed a regression in the patch reworking the line event fifo: reading
multiple events is now still possible
- reworked the structure for new ioctl: it's now padded such that there
be no alignment issues if running a 64-bit kernel on 32-bit userspace
- fixed a bug where one process could disable the status watch of another
- use kstrtoul() instead of atoi() in gpio-watch for string validation
v3 -> v4:
- removed a binary file checked in by mistake
- drop __func__ from debug messages
- restructure the code in the notifier call
- add comments about the alignment of the new uAPI structure
- remove a stray new line that doesn't belong in this series
- tested the series on 32-bit user-space with 64-bit kernel
Bartosz Golaszewski (13):
gpiolib: use 'unsigned int' instead of 'unsigned' in gpio_set_config()
gpiolib: have a single place of calling set_config()
gpiolib: convert the type of hwnum to unsigned int in
gpiochip_get_desc()
gpiolib: use gpiochip_get_desc() in linehandle_create()
gpiolib: use gpiochip_get_desc() in lineevent_create()
gpiolib: use gpiochip_get_desc() in gpio_ioctl()
kfifo: provide noirqsave variants of spinlocked in and out helpers
kfifo: provide kfifo_is_empty_spinlocked()
gpiolib: rework the locking mechanism for lineevent kfifo
gpiolib: emit a debug message when adding events to a full kfifo
gpiolib: provide a dedicated function for setting lineinfo
gpiolib: add new ioctl() for monitoring changes in line info
tools: gpio: implement gpio-watch
drivers/gpio/gpiolib.c | 397 +++++++++++++++++++++++++++---------
drivers/gpio/gpiolib.h | 4 +-
include/linux/gpio/driver.h | 3 +-
include/linux/kfifo.h | 73 +++++++
include/uapi/linux/gpio.h | 30 +++
tools/gpio/.gitignore | 1 +
tools/gpio/Build | 1 +
tools/gpio/Makefile | 11 +-
tools/gpio/gpio-watch.c | 99 +++++++++
9 files changed, 515 insertions(+), 104 deletions(-)
create mode 100644 tools/gpio/gpio-watch.c
--
2.23.0
On Tue, Dec 24, 2019 at 1:07 PM Bartosz Golaszewski <[email protected]> wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> When discussing the recent user-space changes with Kent and while working
> on dbus API for libgpiod I noticed that we really don't have any way of
> keeping the line info synchronized between the kernel and user-space
> processes. We can of course periodically re-read the line information or
> even do it every time we want to read a property but this isn't optimal.
>
> This series adds a new ioctl() that allows user-space to set up a watch on
> the GPIO chardev file-descriptor which can then be polled for events
> emitted by the kernel when the line is requested, released or its status
> changed. This of course doesn't require the line to be requested. Multiple
> user-space processes can watch the same lines.
>
> This series also includes a variety of minor tweaks & fixes for problems
> discovered during development. For instance it addresses a race-condition
> in current line event fifo.
The patch set overall looks good to me, I don't understand the kfifo
parts but I trust you on this, though we need review from a FIFO
maintainer.
Could you send me a pull request of the first patches before the
FIFO changes start, they are good cleanups on their own, also
it brings down the size of your patch stack.
Yours,
Linus Walleij
wt., 7 sty 2020 o 11:07 Linus Walleij <[email protected]> napisał(a):
>
> On Tue, Dec 24, 2019 at 1:07 PM Bartosz Golaszewski <[email protected]> wrote:
>
> > From: Bartosz Golaszewski <[email protected]>
> >
> > When discussing the recent user-space changes with Kent and while working
> > on dbus API for libgpiod I noticed that we really don't have any way of
> > keeping the line info synchronized between the kernel and user-space
> > processes. We can of course periodically re-read the line information or
> > even do it every time we want to read a property but this isn't optimal.
> >
> > This series adds a new ioctl() that allows user-space to set up a watch on
> > the GPIO chardev file-descriptor which can then be polled for events
> > emitted by the kernel when the line is requested, released or its status
> > changed. This of course doesn't require the line to be requested. Multiple
> > user-space processes can watch the same lines.
> >
> > This series also includes a variety of minor tweaks & fixes for problems
> > discovered during development. For instance it addresses a race-condition
> > in current line event fifo.
>
> The patch set overall looks good to me, I don't understand the kfifo
> parts but I trust you on this, though we need review from a FIFO
> maintainer.
Ha! This may be a problem - there doesn't seem to be one. This is why
I Cc'd Greg.
>
> Could you send me a pull request of the first patches before the
> FIFO changes start, they are good cleanups on their own, also
> it brings down the size of your patch stack.
Sure, will do.
>
> Yours,
> Linus Walleij
Bart
On Tue, Jan 7, 2020 at 11:38 AM Bartosz Golaszewski
<[email protected]> wrote:
> wt., 7 sty 2020 o 11:07 Linus Walleij <[email protected]> napisał(a):
> > The patch set overall looks good to me, I don't understand the kfifo
> > parts but I trust you on this, though we need review from a FIFO
> > maintainer.
>
> Ha! This may be a problem - there doesn't seem to be one. This is why
> I Cc'd Greg.
I was under the impression that KFIFO was part of the driver core.
Let's try to CC the actual author (Stefani Seibold) and see if the mail
address works and if he can look at it. Or did you already talk to
Stefani?
(git blame is always my best friend in cases like this, hehe)
Yours,
Linus Walleij
Am Dienstag, den 07.01.2020, 13:50 +0100 schrieb Linus Walleij:
> On Tue, Jan 7, 2020 at 11:38 AM Bartosz Golaszewski
> <[email protected]> wrote:
> > wt., 7 sty 2020 o 11:07 Linus Walleij <[email protected]>
> > napisał(a):
> > > The patch set overall looks good to me, I don't understand the
> > > kfifo
> > > parts but I trust you on this, though we need review from a FIFO
> > > maintainer.
> >
> > Ha! This may be a problem - there doesn't seem to be one. This is
> > why
> > I Cc'd Greg.
>
> I was under the impression that KFIFO was part of the driver core.
> Let's try to CC the actual author (Stefani Seibold) and see if the
> mail
> address works and if he can look at it. Or did you already talk to
> Stefani?
>
> (git blame is always my best friend in cases like this, hehe)
>
> Yours,
> Linus Walleij
I have looked around for the patches and I found the following patches
[PATCH v4 07/13] kfifo: provide noirqsave variants of spinlocked in and out helpers
[PATCH v4 08/13] kfifo: provide kfifo_is_empty_spinlocked()
dated on 24 Dec 2019.
Both seems to be okay. The patch is non intrusive to KFIFO adding only
spinlock wrapper functions for the contemporary kfifo functions.
So...
Acked by Stefani Seibold <[email protected]>
On Tue, Jan 07, 2020 at 01:50:28PM +0100, Linus Walleij wrote:
...
> Let's try to CC the actual author (Stefani Seibold) and see if the mail
> address works and if he can look at it. Or did you already talk to
> Stefani?
>
> (git blame is always my best friend in cases like this, hehe)
Recently I started to be smarted in such cases, i.e. I run also
`git log --author='$AUTHOR'` to see if they are still active and
what address had been used lately.
--
With Best Regards,
Andy Shevchenko
On Tue, Jan 07, 2020 at 04:44:55PM +0200, Andy Shevchenko wrote:
> On Tue, Jan 07, 2020 at 01:50:28PM +0100, Linus Walleij wrote:
>
> ...
>
> > Let's try to CC the actual author (Stefani Seibold) and see if the mail
> > address works and if he can look at it. Or did you already talk to
> > Stefani?
> >
> > (git blame is always my best friend in cases like this, hehe)
>
> Recently I started to be smarted in such cases, i.e. I run also
> `git log --author='$AUTHOR'` to see if they are still active and
> what address had been used lately.
...and another possibility to `git log --grep '$AUTHOR'`.
--
With Best Regards,
Andy Shevchenko
wt., 7 sty 2020 o 15:45 Andy Shevchenko
<[email protected]> napisał(a):
>
> On Tue, Jan 07, 2020 at 04:44:55PM +0200, Andy Shevchenko wrote:
> > On Tue, Jan 07, 2020 at 01:50:28PM +0100, Linus Walleij wrote:
> >
> > ...
> >
> > > Let's try to CC the actual author (Stefani Seibold) and see if the mail
> > > address works and if he can look at it. Or did you already talk to
> > > Stefani?
> > >
> > > (git blame is always my best friend in cases like this, hehe)
> >
> > Recently I started to be smarted in such cases, i.e. I run also
> > `git log --author='$AUTHOR'` to see if they are still active and
> > what address had been used lately.
>
> ...and another possibility to `git log --grep '$AUTHOR'`.
>
> --
> With Best Regards,
> Andy Shevchenko
>
So if some module doesn't have an official maintainer listed in
MAINTAINERS, we should still get a review from the original author?
KFIFO lives in lib/ - is there even an official maintainer for all
library helpers?
Bart
On Tue, Jan 07, 2020 at 04:19:59PM +0100, Bartosz Golaszewski wrote:
> wt., 7 sty 2020 o 15:45 Andy Shevchenko
> <[email protected]> napisał(a):
> >
> > On Tue, Jan 07, 2020 at 04:44:55PM +0200, Andy Shevchenko wrote:
> > > On Tue, Jan 07, 2020 at 01:50:28PM +0100, Linus Walleij wrote:
> > >
> > > ...
> > >
> > > > Let's try to CC the actual author (Stefani Seibold) and see if the mail
> > > > address works and if he can look at it. Or did you already talk to
> > > > Stefani?
> > > >
> > > > (git blame is always my best friend in cases like this, hehe)
> > >
> > > Recently I started to be smarted in such cases, i.e. I run also
> > > `git log --author='$AUTHOR'` to see if they are still active and
> > > what address had been used lately.
> >
> > ...and another possibility to `git log --grep '$AUTHOR'`.
> So if some module doesn't have an official maintainer listed in
> MAINTAINERS, we should still get a review from the original author?
If you asking me, I do it in a way of playing good citizen. It's not required,
but may give a good feedback.
> KFIFO lives in lib/ - is there even an official maintainer for all
> library helpers?
lib/ is (in most cases) under akpm@ realm.
--
With Best Regards,
Andy Shevchenko
wt., 7 sty 2020 o 16:58 Andy Shevchenko
<[email protected]> napisał(a):
>
> On Tue, Jan 07, 2020 at 04:19:59PM +0100, Bartosz Golaszewski wrote:
> > wt., 7 sty 2020 o 15:45 Andy Shevchenko
> > <[email protected]> napisał(a):
> > >
> > > On Tue, Jan 07, 2020 at 04:44:55PM +0200, Andy Shevchenko wrote:
> > > > On Tue, Jan 07, 2020 at 01:50:28PM +0100, Linus Walleij wrote:
> > > >
> > > > ...
> > > >
> > > > > Let's try to CC the actual author (Stefani Seibold) and see if the mail
> > > > > address works and if he can look at it. Or did you already talk to
> > > > > Stefani?
> > > > >
> > > > > (git blame is always my best friend in cases like this, hehe)
> > > >
> > > > Recently I started to be smarted in such cases, i.e. I run also
> > > > `git log --author='$AUTHOR'` to see if they are still active and
> > > > what address had been used lately.
> > >
> > > ...and another possibility to `git log --grep '$AUTHOR'`.
>
> > So if some module doesn't have an official maintainer listed in
> > MAINTAINERS, we should still get a review from the original author?
>
> If you asking me, I do it in a way of playing good citizen. It's not required,
> but may give a good feedback.
>
> > KFIFO lives in lib/ - is there even an official maintainer for all
> > library helpers?
>
> lib/ is (in most cases) under akpm@ realm.
>
Once the first part of the series is in Linus' branch, I'll resend the
remaining patches with akpm in Cc.
Bart