linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
@ 2024-10-15  8:00 Bartosz Golaszewski
  2024-10-15  8:00 ` [PATCH v2 1/2] driver core: class: expose the class kobject Bartosz Golaszewski
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2024-10-15  8:00 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J. Wysocki, Linus Walleij,
	Bartosz Golaszewski, Kent Gibson
  Cc: linux-kernel, linux-gpio, Bartosz Golaszewski

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>


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

* [PATCH v2 1/2] driver core: class: expose the class kobject
  2024-10-15  8:00 [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled Bartosz Golaszewski
@ 2024-10-15  8:00 ` Bartosz Golaszewski
  2024-10-15  9:24   ` Greg Kroah-Hartman
  2024-10-15  8:00 ` [PATCH v2 2/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled Bartosz Golaszewski
  2024-10-15  9:29 ` [PATCH v2 0/2] " Greg Kroah-Hartman
  2 siblings, 1 reply; 13+ messages in thread
From: Bartosz Golaszewski @ 2024-10-15  8:00 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J. Wysocki, Linus Walleij,
	Bartosz Golaszewski, Kent Gibson
  Cc: linux-kernel, linux-gpio, Bartosz Golaszewski

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Export the address of the /sys/class kobject to users that need to
chain off of it by means other than calling register_class(). This will
be used by the GPIO subsystem to provide a backward compatibility
mount-point for the GPIO sysfs class once it's disabled.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/base/class.c         | 6 ++++++
 include/linux/device/class.h | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/base/class.c b/drivers/base/class.c
index cb5359235c70..68474aff53df 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -23,6 +23,9 @@
 /* /sys/class */
 static struct kset *class_kset;
 
+struct kobject *class_kobj;
+EXPORT_SYMBOL_GPL(class_kobj);
+
 #define to_class_attr(_attr) container_of(_attr, struct class_attribute, attr)
 
 /**
@@ -668,5 +671,8 @@ int __init classes_init(void)
 	class_kset = kset_create_and_add("class", NULL, NULL);
 	if (!class_kset)
 		return -ENOMEM;
+
+	class_kobj = &class_kset->kobj;
+
 	return 0;
 }
diff --git a/include/linux/device/class.h b/include/linux/device/class.h
index 518c9c83d64b..898f7948293d 100644
--- a/include/linux/device/class.h
+++ b/include/linux/device/class.h
@@ -69,6 +69,12 @@ struct class {
 	const struct dev_pm_ops *pm;
 };
 
+/*
+ * Global /sys/class kobject for creating sub-groups by means other than
+ * class_register().
+ */
+extern struct kobject *class_kobj;
+
 struct class_dev_iter {
 	struct klist_iter		ki;
 	const struct device_type	*type;

-- 
2.43.0


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

* [PATCH v2 2/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
  2024-10-15  8:00 [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled Bartosz Golaszewski
  2024-10-15  8:00 ` [PATCH v2 1/2] driver core: class: expose the class kobject Bartosz Golaszewski
@ 2024-10-15  8:00 ` Bartosz Golaszewski
  2024-10-15  9:27   ` Greg Kroah-Hartman
  2024-10-15  9:29 ` [PATCH v2 0/2] " Greg Kroah-Hartman
  2 siblings, 1 reply; 13+ messages in thread
From: Bartosz Golaszewski @ 2024-10-15  8:00 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J. Wysocki, Linus Walleij,
	Bartosz Golaszewski, Kent Gibson
  Cc: linux-kernel, linux-gpio, Bartosz Golaszewski

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

User-space may want to use some kind of a compatibility layer for the
deprecated GPIO sysfs ABI. This would typically involve mounting
a fuse-based filesystem using the GPIO character device to emulate the
sysfs behavior and layout.

With GPIO_SYSFS disabled, the /sys/class/gpio directory doesn't exist
and user-space cannot create it. In order to facilitate moving away from
the sysfs, add a new Kconfig option that indicates to GPIOLIB that is
should create an always-empty directory where the GPIO class interface
would exist and enable this option by default if GPIO_SYSFS is not
selected.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/Kconfig   | 18 ++++++++++++++++++
 drivers/gpio/gpiolib.c |  6 ++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index efddc6455315..1a3535bda779 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -69,6 +69,24 @@ config GPIO_SYSFS
 	  use the character device /dev/gpiochipN with the appropriate
 	  ioctl() operations instead.
 
+config GPIO_SYSFS_CLASS_MOUNT_POINT
+	bool "Create empty /sys/class/gpio directory" if EXPERT
+	depends on !GPIO_SYSFS
+	default y
+	help
+	  Say Y here to create an empty /sys/class/gpio directory.
+
+	  User-space may want to use some kind of a compatibility layer for the
+	  deprecated GPIO sysfs ABI. This would typically involve mounting
+	  a fuse-based filesystem using the GPIO character device to emulate
+	  the sysfs behavior and layout.
+
+	  This option makes GPIOLIB create an empty directory at /sys/class/gpio
+	  where user-space can mount the sysfs replacement and avoid having to
+	  change existing programs to adjust to different filesystem paths.
+
+	  If unsure, say Y.
+
 config GPIO_CDEV
 	bool
 	prompt "Character device (/dev/gpiochipN) support" if EXPERT
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 97346b746ef5..1c8bd765d8e1 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4899,6 +4899,12 @@ static int __init gpiolib_dev_init(void)
 		return ret;
 	}
 
+#if IS_ENABLED(CONFIG_GPIO_SYSFS_CLASS_MOUNT_POINT)
+	ret = sysfs_create_mount_point(class_kobj, "gpio");
+	if (ret)
+		pr_err("gpiolib: failed to create the GPIO class mountpoint\n");
+#endif /* CONFIG_GPIO_SYSFS_CLASS_MOUNT_POINT */
+
 	gpiolib_initialized = true;
 	gpiochip_setup_devs();
 

-- 
2.43.0


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

* Re: [PATCH v2 1/2] driver core: class: expose the class kobject
  2024-10-15  8:00 ` [PATCH v2 1/2] driver core: class: expose the class kobject Bartosz Golaszewski
@ 2024-10-15  9:24   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2024-10-15  9:24 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Rafael J. Wysocki, Linus Walleij, Kent Gibson, linux-kernel,
	linux-gpio, Bartosz Golaszewski

On Tue, Oct 15, 2024 at 10:00:23AM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Export the address of the /sys/class kobject to users that need to
> chain off of it by means other than calling register_class().

Ick, no.

> This will
> be used by the GPIO subsystem to provide a backward compatibility
> mount-point for the GPIO sysfs class once it's disabled.

Again, ick, no.

No "mount point" should be messing with sysfs, don't do that.

sysfs is a simple "if the file or directory is not there, don't worry
about it" type of interface (i.e. fixing the issues we had with /proc).
If a userspace tool can't find something there, then it should just
error out or move on.

Let's not allow anything to mount anything at /sys/class/ please, that
way lies madness for the next 40+ years.  If you want to drop a
userspace api, drop it, don't paper over it with something that you
can't drop either.

thanks,

greg k-h

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

* Re: [PATCH v2 2/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
  2024-10-15  8:00 ` [PATCH v2 2/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled Bartosz Golaszewski
@ 2024-10-15  9:27   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2024-10-15  9:27 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Rafael J. Wysocki, Linus Walleij, Kent Gibson, linux-kernel,
	linux-gpio, Bartosz Golaszewski

On Tue, Oct 15, 2024 at 10:00:24AM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> User-space may want to use some kind of a compatibility layer for the
> deprecated GPIO sysfs ABI. This would typically involve mounting
> a fuse-based filesystem using the GPIO character device to emulate the
> sysfs behavior and layout.

Ick, no, don't mount a filesystem in /sys/class/ that's crazy, and
wrong.  See my other response for why.

> With GPIO_SYSFS disabled, the /sys/class/gpio directory doesn't exist
> and user-space cannot create it.

userspace should NOT be creating it.

> In order to facilitate moving away from
> the sysfs, add a new Kconfig option that indicates to GPIOLIB that is
> should create an always-empty directory where the GPIO class interface
> would exist and enable this option by default if GPIO_SYSFS is not
> selected.

No, either support a real sysfs api here, or don't.  Don't paper over
the api change by allowing this type of fake interface to live here,
that is just going to be a maintance nightmare.  Attempting to push the
"we don't support this user/kernel api anymore" off to a userspace
developer is not how to do kernel development.

> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  drivers/gpio/Kconfig   | 18 ++++++++++++++++++
>  drivers/gpio/gpiolib.c |  6 ++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index efddc6455315..1a3535bda779 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -69,6 +69,24 @@ config GPIO_SYSFS
>  	  use the character device /dev/gpiochipN with the appropriate
>  	  ioctl() operations instead.
>  
> +config GPIO_SYSFS_CLASS_MOUNT_POINT
> +	bool "Create empty /sys/class/gpio directory" if EXPERT
> +	depends on !GPIO_SYSFS
> +	default y

Only "default y" if you can not boot without this enabled.  I doubt
that's the case here.  If it is the case here, then don't remove the
sysfs api in the first place.

And why is it being removed?  Who relies on it that can't live with it
being gone?


> +	help
> +	  Say Y here to create an empty /sys/class/gpio directory.
> +
> +	  User-space may want to use some kind of a compatibility layer for the
> +	  deprecated GPIO sysfs ABI. This would typically involve mounting
> +	  a fuse-based filesystem using the GPIO character device to emulate
> +	  the sysfs behavior and layout.
> +
> +	  This option makes GPIOLIB create an empty directory at /sys/class/gpio
> +	  where user-space can mount the sysfs replacement and avoid having to
> +	  change existing programs to adjust to different filesystem paths.
> +
> +	  If unsure, say Y.
> +
>  config GPIO_CDEV
>  	bool
>  	prompt "Character device (/dev/gpiochipN) support" if EXPERT
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 97346b746ef5..1c8bd765d8e1 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -4899,6 +4899,12 @@ static int __init gpiolib_dev_init(void)
>  		return ret;
>  	}
>  
> +#if IS_ENABLED(CONFIG_GPIO_SYSFS_CLASS_MOUNT_POINT)
> +	ret = sysfs_create_mount_point(class_kobj, "gpio");
> +	if (ret)
> +		pr_err("gpiolib: failed to create the GPIO class mountpoint\n");
> +#endif /* CONFIG_GPIO_SYSFS_CLASS_MOUNT_POINT */

Nit, I think we know what the #endif is for, it was only 3 lines above :)

thanks,

greg k-h

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

* Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
  2024-10-15  8:00 [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled Bartosz Golaszewski
  2024-10-15  8:00 ` [PATCH v2 1/2] driver core: class: expose the class kobject Bartosz Golaszewski
  2024-10-15  8:00 ` [PATCH v2 2/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled Bartosz Golaszewski
@ 2024-10-15  9:29 ` Greg Kroah-Hartman
  2024-10-15 12:11   ` Bartosz Golaszewski
  2 siblings, 1 reply; 13+ messages in thread
From: Greg Kroah-Hartman @ 2024-10-15  9:29 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Rafael J. Wysocki, Linus Walleij, Kent Gibson, linux-kernel,
	linux-gpio, Bartosz Golaszewski

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

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

* Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
  2024-10-15  9:29 ` [PATCH v2 0/2] " Greg Kroah-Hartman
@ 2024-10-15 12:11   ` Bartosz Golaszewski
  2024-10-15 12:46     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 13+ messages in thread
From: Bartosz Golaszewski @ 2024-10-15 12:11 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Linus Walleij, Kent Gibson, linux-kernel,
	linux-gpio, Bartosz Golaszewski

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

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

* Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
  2024-10-15 12:11   ` Bartosz Golaszewski
@ 2024-10-15 12:46     ` Greg Kroah-Hartman
  2024-10-15 17:52       ` Bartosz Golaszewski
  2024-11-12 10:04       ` Bartosz Golaszewski
  0 siblings, 2 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2024-10-15 12:46 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Rafael J. Wysocki, Linus Walleij, Kent Gibson, linux-kernel,
	linux-gpio, Bartosz Golaszewski

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

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

* Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
  2024-10-15 12:46     ` Greg Kroah-Hartman
@ 2024-10-15 17:52       ` Bartosz Golaszewski
  2024-10-16  7:02         ` Greg Kroah-Hartman
  2024-11-12 10:04       ` Bartosz Golaszewski
  1 sibling, 1 reply; 13+ messages in thread
From: Bartosz Golaszewski @ 2024-10-15 17:52 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Linus Walleij, Kent Gibson, linux-kernel,
	linux-gpio, Bartosz Golaszewski

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

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

* Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
  2024-10-15 17:52       ` Bartosz Golaszewski
@ 2024-10-16  7:02         ` Greg Kroah-Hartman
  2024-10-16  8:49           ` Bartosz Golaszewski
  0 siblings, 1 reply; 13+ messages in thread
From: Greg Kroah-Hartman @ 2024-10-16  7:02 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Rafael J. Wysocki, Linus Walleij, Kent Gibson, linux-kernel,
	linux-gpio, Bartosz Golaszewski

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

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

* Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
  2024-10-16  7:02         ` Greg Kroah-Hartman
@ 2024-10-16  8:49           ` Bartosz Golaszewski
  2024-10-16  9:12             ` Greg Kroah-Hartman
  0 siblings, 1 reply; 13+ messages in thread
From: Bartosz Golaszewski @ 2024-10-16  8:49 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Linus Walleij, Kent Gibson, linux-kernel,
	linux-gpio, Bartosz Golaszewski

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

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

* Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
  2024-10-16  8:49           ` Bartosz Golaszewski
@ 2024-10-16  9:12             ` Greg Kroah-Hartman
  0 siblings, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2024-10-16  9:12 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Rafael J. Wysocki, Linus Walleij, Kent Gibson, linux-kernel,
	linux-gpio, Bartosz Golaszewski

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

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

* Re: [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled
  2024-10-15 12:46     ` Greg Kroah-Hartman
  2024-10-15 17:52       ` Bartosz Golaszewski
@ 2024-11-12 10:04       ` Bartosz Golaszewski
  1 sibling, 0 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2024-11-12 10:04 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Linus Walleij, Kent Gibson, linux-kernel,
	linux-gpio, Bartosz Golaszewski

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

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

end of thread, other threads:[~2024-11-12 10:04 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-15  8:00 [PATCH v2 0/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled Bartosz Golaszewski
2024-10-15  8:00 ` [PATCH v2 1/2] driver core: class: expose the class kobject Bartosz Golaszewski
2024-10-15  9:24   ` Greg Kroah-Hartman
2024-10-15  8:00 ` [PATCH v2 2/2] gpio: create the /sys/class/gpio mount point with GPIO_SYSFS disabled Bartosz Golaszewski
2024-10-15  9:27   ` Greg Kroah-Hartman
2024-10-15  9:29 ` [PATCH v2 0/2] " Greg Kroah-Hartman
2024-10-15 12:11   ` Bartosz Golaszewski
2024-10-15 12:46     ` Greg Kroah-Hartman
2024-10-15 17:52       ` Bartosz Golaszewski
2024-10-16  7:02         ` Greg Kroah-Hartman
2024-10-16  8:49           ` Bartosz Golaszewski
2024-10-16  9:12             ` Greg Kroah-Hartman
2024-11-12 10:04       ` Bartosz Golaszewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).