[PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled

Bartosz Golaszewski posted 2 patches 1 month, 1 week ago
drivers/base/class.c         |  6 ++++++
drivers/gpio/Kconfig         | 18 ++++++++++++++++++
drivers/gpio/gpiolib.c       |  6 ++++++
include/linux/device/class.h |  6 ++++++
4 files changed, 36 insertions(+)
[PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
Posted by Bartosz Golaszewski 1 month, 1 week ago
Greg, Rafael,

The first patch in this series is small but may be seen as controversial
so here's a little backstory.

We have two sets of GPIO APIs currently in the kernel: the legacy one
based on numbers and the modern one using descriptors. Our goal is to
remove the old one from the kernel to which there are two obstacles: the
first one is easier and consists of converting all remaining in-kernel
users to the preferred API. This is tedious but it's all within our
control, just demands a lot of effort. The second obstacle is much harder
as it involves removing an existing kernel uABI that is the GPIO sysfs
interface at /sys/class/gpio.

Despite providing a number of user-space tools making using the GPIO
character device easier, it's become clear that some users just prefer
how the sysfs interface works and want to keep using it. Unless we can
provide a drop-in replacement, they will protest any attempts at
removing it from the kernel. As the GPIO sysfs module is the main user
of the global GPIO numberspace, we will not be able to remove it from
the kernel either.

I am working on a FUSE-based libgpiod-to-sysfs compatibility layer that
could replace the in-kernel sysfs and keep all the user-space programs
running but in order to keep it fully compatible, we need to be able to
mount it at /sys/class/gpio. We can't create directories in sysfs from
user-space and with GPIO_SYSFS disabled, the directory is simply not
there.

I would like to do what we already do for /sys/kernel/debug,
/sys/kernel/config, etc. and create an always-empty mount point at
/sys/class/gpio. To that end, I need the address of the /sys/class
kobject and the first patch in this series exports it.

The second adds a Kconfig switch in GPIOLIB and code that creates the
mount point.

I proposed creating an empty class stub in v1 but figured that a proper
mount point will work better.

If this is ok with you, please consider leaving your Ack or taking
patch 1/2 through your tree and providing me with an immutable tag.

To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Rafael J. Wysocki <rafael@kernel.org>
To: Linus Walleij <linus.walleij@linaro.org>
To: Bartosz Golaszewski <brgl@bgdev.pl>
To: Kent Gibson <warthog618@gmail.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-gpio@vger.kernel.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Changes in v2:
- Don't create an empty class but use sysfs_create_mount_point() instead
- Link to v1: https://lore.kernel.org/all/20241014121047.103179-1-brgl@bgdev.pl/

---
Bartosz Golaszewski (2):
      driver core: class: expose the class kobject
      gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled

 drivers/base/class.c         |  6 ++++++
 drivers/gpio/Kconfig         | 18 ++++++++++++++++++
 drivers/gpio/gpiolib.c       |  6 ++++++
 include/linux/device/class.h |  6 ++++++
 4 files changed, 36 insertions(+)
---
base-commit: b852e1e7a0389ed6168ef1d38eb0bad71a6b11e8
change-id: 20241014-gpio-class-mountpoint-2da8c54fafbe

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
Posted by Greg Kroah-Hartman 1 month, 1 week ago
On Tue, Oct 15, 2024 at 10:00:22AM +0200, Bartosz Golaszewski wrote:
> Greg, Rafael,
> 
> The first patch in this series is small but may be seen as controversial
> so here's a little backstory.

Ah, now I see the 0/2 email...

> We have two sets of GPIO APIs currently in the kernel: the legacy one
> based on numbers and the modern one using descriptors. Our goal is to
> remove the old one from the kernel to which there are two obstacles: the
> first one is easier and consists of converting all remaining in-kernel
> users to the preferred API. This is tedious but it's all within our
> control, just demands a lot of effort. The second obstacle is much harder
> as it involves removing an existing kernel uABI that is the GPIO sysfs
> interface at /sys/class/gpio.
> 
> Despite providing a number of user-space tools making using the GPIO
> character device easier, it's become clear that some users just prefer
> how the sysfs interface works and want to keep using it. Unless we can
> provide a drop-in replacement, they will protest any attempts at
> removing it from the kernel. As the GPIO sysfs module is the main user
> of the global GPIO numberspace, we will not be able to remove it from
> the kernel either.

They should protest it's removal, and you should support it for forever,
as that's the api that they rely on and you need to handle.  That's the
joy of kernel development, you can't drop support for stuff people rely
on, sorry.

> I am working on a FUSE-based libgpiod-to-sysfs compatibility layer that
> could replace the in-kernel sysfs and keep all the user-space programs
> running but in order to keep it fully compatible, we need to be able to
> mount it at /sys/class/gpio. We can't create directories in sysfs from
> user-space and with GPIO_SYSFS disabled, the directory is simply not
> there.

Ick, no, just keep the kernel stuff please.

> I would like to do what we already do for /sys/kernel/debug,
> /sys/kernel/config, etc. and create an always-empty mount point at
> /sys/class/gpio. To that end, I need the address of the /sys/class
> kobject and the first patch in this series exports it.

No, debug and config are different, they are not "fake" subsystems, they
are totally different interfaces and as such, can use those locations as
mount points for their in-kernel filesystem interfaces.  You are wanting
a random userspace mount point, that's totally different.

Sorry, just live with the kernel code please.  Work to get all userspace
moved off of it if you feel it is so bad, and only then can you remove
it.

greg k-h
Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
Posted by Bartosz Golaszewski 1 month, 1 week ago
On Tue, Oct 15, 2024 at 11:30 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> >
> > Despite providing a number of user-space tools making using the GPIO
> > character device easier, it's become clear that some users just prefer
> > how the sysfs interface works and want to keep using it. Unless we can
> > provide a drop-in replacement, they will protest any attempts at
> > removing it from the kernel. As the GPIO sysfs module is the main user
> > of the global GPIO numberspace, we will not be able to remove it from
> > the kernel either.
>
> They should protest it's removal, and you should support it for forever,
> as that's the api that they rely on and you need to handle.  That's the
> joy of kernel development, you can't drop support for stuff people rely
> on, sorry.
>

Yet every now and then some clearly bad decisions from the past are
amended by removing support for older interfaces. I'm not trying to
deprive people of something they rely on, I'm trying to provide a
drop-in replacement in user-space using an existing, better kernel
interface, so that we can get rid of the old one and - in the process
- improve the entire in-kernel GPIO support.

> > I am working on a FUSE-based libgpiod-to-sysfs compatibility layer that
> > could replace the in-kernel sysfs and keep all the user-space programs
> > running but in order to keep it fully compatible, we need to be able to
> > mount it at /sys/class/gpio. We can't create directories in sysfs from
> > user-space and with GPIO_SYSFS disabled, the directory is simply not
> > there.
>
> Ick, no, just keep the kernel stuff please.
>

Ick? I'm not sure how to take it but are you criticising the idea
itself of using the better kernel interface to provide a thin
compatibility layer in user-space to the bad one that people are just
too used to to spend time converting? Or just the mounting at the old
mount-point part?

> > I would like to do what we already do for /sys/kernel/debug,
> > /sys/kernel/config, etc. and create an always-empty mount point at
> > /sys/class/gpio. To that end, I need the address of the /sys/class
> > kobject and the first patch in this series exports it.
>
> No, debug and config are different, they are not "fake" subsystems, they
> are totally different interfaces and as such, can use those locations as
> mount points for their in-kernel filesystem interfaces.  You are wanting
> a random userspace mount point, that's totally different.
>
> Sorry, just live with the kernel code please.  Work to get all userspace
> moved off of it if you feel it is so bad, and only then can you remove
> it.
>

What if we just add a Kconfig option allowing to disable the sysfs
attributes inside /sys/class/gpio but keep the directory? It's not
like it's a new one, it's already there as a baked in interface.

Bart
Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
Posted by Greg Kroah-Hartman 1 month, 1 week ago
On Tue, Oct 15, 2024 at 02:11:45PM +0200, Bartosz Golaszewski wrote:
> On Tue, Oct 15, 2024 at 11:30 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > >
> > > Despite providing a number of user-space tools making using the GPIO
> > > character device easier, it's become clear that some users just prefer
> > > how the sysfs interface works and want to keep using it. Unless we can
> > > provide a drop-in replacement, they will protest any attempts at
> > > removing it from the kernel. As the GPIO sysfs module is the main user
> > > of the global GPIO numberspace, we will not be able to remove it from
> > > the kernel either.
> >
> > They should protest it's removal, and you should support it for forever,
> > as that's the api that they rely on and you need to handle.  That's the
> > joy of kernel development, you can't drop support for stuff people rely
> > on, sorry.
> >
> 
> Yet every now and then some clearly bad decisions from the past are
> amended by removing support for older interfaces. I'm not trying to
> deprive people of something they rely on, I'm trying to provide a
> drop-in replacement in user-space using an existing, better kernel
> interface, so that we can get rid of the old one and - in the process
> - improve the entire in-kernel GPIO support.

How is emulating the existing sysfs api anything "better"?  It's still
the same api.

> > > I am working on a FUSE-based libgpiod-to-sysfs compatibility layer that
> > > could replace the in-kernel sysfs and keep all the user-space programs
> > > running but in order to keep it fully compatible, we need to be able to
> > > mount it at /sys/class/gpio. We can't create directories in sysfs from
> > > user-space and with GPIO_SYSFS disabled, the directory is simply not
> > > there.
> >
> > Ick, no, just keep the kernel stuff please.
> >
> 
> Ick? I'm not sure how to take it but are you criticising the idea
> itself of using the better kernel interface to provide a thin
> compatibility layer in user-space to the bad one that people are just
> too used to to spend time converting? Or just the mounting at the old
> mount-point part?

I'm saying "Ick" for a userspace mounted filesystem on top of sysfs to
emulate a sysfs api that the kernel provides today.

> > > I would like to do what we already do for /sys/kernel/debug,
> > > /sys/kernel/config, etc. and create an always-empty mount point at
> > > /sys/class/gpio. To that end, I need the address of the /sys/class
> > > kobject and the first patch in this series exports it.
> >
> > No, debug and config are different, they are not "fake" subsystems, they
> > are totally different interfaces and as such, can use those locations as
> > mount points for their in-kernel filesystem interfaces.  You are wanting
> > a random userspace mount point, that's totally different.
> >
> > Sorry, just live with the kernel code please.  Work to get all userspace
> > moved off of it if you feel it is so bad, and only then can you remove
> > it.
> >
> 
> What if we just add a Kconfig option allowing to disable the sysfs
> attributes inside /sys/class/gpio but keep the directory? It's not
> like it's a new one, it's already there as a baked in interface.

That's up to you, but again, please do not mount a filesystem there,
that's going to cause nothing but problems in the end (like debugfs and
tracefs and configfs do all the time when people get confused and start
poking around in sysfs code looking for the logic involved in other
places.)

thanks,

greg k-h
Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
Posted by Bartosz Golaszewski 2 weeks ago
On Tue, Oct 15, 2024 at 2:46 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> >
> > What if we just add a Kconfig option allowing to disable the sysfs
> > attributes inside /sys/class/gpio but keep the directory? It's not
> > like it's a new one, it's already there as a baked in interface.
>
> That's up to you, but again, please do not mount a filesystem there,
> that's going to cause nothing but problems in the end (like debugfs and
> tracefs and configfs do all the time when people get confused and start
> poking around in sysfs code looking for the logic involved in other
> places.)
>

Fortunately no kernel change is required after all. The user-space can
work around it ekhm... "elegantly" by mounting a read-only overlay on
top of /sys/class that contains the gpio directory and mounting the
user-space compatibility layer onto it. I know you don't like it
either but at least I won't be submitting any such proposal to the
kernel anymore. :)

Bartosz
Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
Posted by Bartosz Golaszewski 1 month, 1 week ago
On Tue, Oct 15, 2024 at 2:46 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Oct 15, 2024 at 02:11:45PM +0200, Bartosz Golaszewski wrote:
> > On Tue, Oct 15, 2024 at 11:30 AM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > >
> > > > Despite providing a number of user-space tools making using the GPIO
> > > > character device easier, it's become clear that some users just prefer
> > > > how the sysfs interface works and want to keep using it. Unless we can
> > > > provide a drop-in replacement, they will protest any attempts at
> > > > removing it from the kernel. As the GPIO sysfs module is the main user
> > > > of the global GPIO numberspace, we will not be able to remove it from
> > > > the kernel either.
> > >
> > > They should protest it's removal, and you should support it for forever,
> > > as that's the api that they rely on and you need to handle.  That's the
> > > joy of kernel development, you can't drop support for stuff people rely
> > > on, sorry.
> > >
> >
> > Yet every now and then some clearly bad decisions from the past are
> > amended by removing support for older interfaces. I'm not trying to
> > deprive people of something they rely on, I'm trying to provide a
> > drop-in replacement in user-space using an existing, better kernel
> > interface, so that we can get rid of the old one and - in the process
> > - improve the entire in-kernel GPIO support.
>
> How is emulating the existing sysfs api anything "better"?  It's still
> the same api.
>

The existence of the sysfs API in the kernel makes us stick to some
sub-optimal design decisions (like the global GPIO numberspace) that
we'll be able to entirely remove once sysfs is gone. We want people to
use the GPIO character device and if it takes a layer emulating the
old API on top of it to make them switch then it's still better than
keeping the API in the kernel.

> > > Sorry, just live with the kernel code please.  Work to get all userspace
> > > moved off of it if you feel it is so bad, and only then can you remove
> > > it.
> > >
> >
> > What if we just add a Kconfig option allowing to disable the sysfs
> > attributes inside /sys/class/gpio but keep the directory? It's not
> > like it's a new one, it's already there as a baked in interface.
>
> That's up to you, but again, please do not mount a filesystem there,
> that's going to cause nothing but problems in the end (like debugfs and
> tracefs and configfs do all the time when people get confused and start
> poking around in sysfs code looking for the logic involved in other
> places.)
>

Oh, I would never do it! I hope no user-space program ever comes up
with a crazy idea like that! :)

Bart
Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
Posted by Greg Kroah-Hartman 1 month, 1 week ago
On Tue, Oct 15, 2024 at 07:52:53PM +0200, Bartosz Golaszewski wrote:
> On Tue, Oct 15, 2024 at 2:46 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Oct 15, 2024 at 02:11:45PM +0200, Bartosz Golaszewski wrote:
> > > On Tue, Oct 15, 2024 at 11:30 AM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > >
> > > > > Despite providing a number of user-space tools making using the GPIO
> > > > > character device easier, it's become clear that some users just prefer
> > > > > how the sysfs interface works and want to keep using it. Unless we can
> > > > > provide a drop-in replacement, they will protest any attempts at
> > > > > removing it from the kernel. As the GPIO sysfs module is the main user
> > > > > of the global GPIO numberspace, we will not be able to remove it from
> > > > > the kernel either.
> > > >
> > > > They should protest it's removal, and you should support it for forever,
> > > > as that's the api that they rely on and you need to handle.  That's the
> > > > joy of kernel development, you can't drop support for stuff people rely
> > > > on, sorry.
> > > >
> > >
> > > Yet every now and then some clearly bad decisions from the past are
> > > amended by removing support for older interfaces. I'm not trying to
> > > deprive people of something they rely on, I'm trying to provide a
> > > drop-in replacement in user-space using an existing, better kernel
> > > interface, so that we can get rid of the old one and - in the process
> > > - improve the entire in-kernel GPIO support.
> >
> > How is emulating the existing sysfs api anything "better"?  It's still
> > the same api.
> >
> 
> The existence of the sysfs API in the kernel makes us stick to some
> sub-optimal design decisions (like the global GPIO numberspace) that
> we'll be able to entirely remove once sysfs is gone. We want people to
> use the GPIO character device and if it takes a layer emulating the
> old API on top of it to make them switch then it's still better than
> keeping the API in the kernel.

How are you going to emulate the "global numberspace" in usersapce if
the kernel isn't exposing that?  Why can't you just do it in the same
way that you would in userspace here?

Again, the issue is "do not remove apis that userspace relies on".
That's all.  I'm going to add another one called "do not mount any
filesystem at /sys/devices/class/ as that is insane" as well :)

thanks,

greg k-h
Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
Posted by Bartosz Golaszewski 1 month, 1 week ago
On Wed, Oct 16, 2024 at 9:02 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> >
> > The existence of the sysfs API in the kernel makes us stick to some
> > sub-optimal design decisions (like the global GPIO numberspace) that
> > we'll be able to entirely remove once sysfs is gone. We want people to
> > use the GPIO character device and if it takes a layer emulating the
> > old API on top of it to make them switch then it's still better than
> > keeping the API in the kernel.
>
> How are you going to emulate the "global numberspace" in usersapce if
> the kernel isn't exposing that?  Why can't you just do it in the same
> way that you would in userspace here?
>

In the new kernel API, users don't care about the GPIO numbers. Even
the GPIO drivers only care about hardware offsets within the chip they
control. But we still use the numbers for the sake of the sysfs
interface, users of the legacy in-kernel API and some really old
drivers that used to set a hard-coded base GPIO number. For most part
the dynamic base assignment algorithm can be simply moved to
user-space. For those few instances where the base needs to be a
specific value, the FUSE program will take an argument allowing to
specify it.

> Again, the issue is "do not remove apis that userspace relies on".
> That's all.  I'm going to add another one called "do not mount any
> filesystem at /sys/devices/class/ as that is insane" as well :)
>

I know you're not being 100% serious but I think it's worth mentioning
that the mounting is not done from the kernel. You can't really impose
the second one on user-space.

If user-space decides to go "mount -t configfs none
/sys/bus/platform/devices/" or "mount -t devtmpfs none /proc", then
AFAIK there's nothing you can do to stop it.

Bart
Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
Posted by Greg Kroah-Hartman 1 month, 1 week ago
On Wed, Oct 16, 2024 at 10:49:57AM +0200, Bartosz Golaszewski wrote:
> > Again, the issue is "do not remove apis that userspace relies on".
> > That's all.  I'm going to add another one called "do not mount any
> > filesystem at /sys/devices/class/ as that is insane" as well :)
> >
> 
> I know you're not being 100% serious but I think it's worth mentioning
> that the mounting is not done from the kernel. You can't really impose
> the second one on user-space.

Understood, but I can reject a patch that does "create a mount point for
userspace to use in /sys/class/", which is what I meant here.

thanks,

greg k-h