| Message ID | 20241015-gpio-class-mountpoint-v2-0-7709301876ef@linaro.org |
|---|---|
| Headers | show |
| Series | gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled | expand |
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
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
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
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
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
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
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
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,