[PATCH RFC] kobject_uevent: notify uevent sysfs file on changes

[PATCH RFC] kobject_uevent: notify uevent sysfs file on changes

[Thomas Weißschuh]

The sysfs file "uevent" that exists for each device
contains the same information that is attached to uevents emitted via
netlink (or the usermode helper).
This is useful for userspace which interacts with sysfs directly,
without using (lib)udev.

However it is not possible to actually get notified when the data in
the "uevent" file changes.

Enable these notifications, so that the "uevent" file can be used
together with inotify and friends.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
My original usecase is to get updates for power supplies.
Current charge status and similar.
All this data is exposed via "uevent", and on updates an KOBJ_CHANGED
event is emitted.

To me, a general solution covering all devices looked the best.

Open questions for the RFC:
* Is this a good idea?
* How to handle other actions?
  _ADD and _REMOVE should not call sysfs_notify() in any case, these
  cases can be detected otherwise.
---
 lib/kobject_uevent.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
index 03b427e2707e..74047a79cb13 100644
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -24,6 +24,7 @@
 #include <linux/netlink.h>
 #include <linux/uidgid.h>
 #include <linux/uuid.h>
+#include <linux/sysfs.h>
 #include <linux/ctype.h>
 #include <net/sock.h>
 #include <net/netlink.h>
@@ -593,6 +594,9 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
 	retval = kobject_uevent_net_broadcast(kobj, env, action_string,
 					      devpath);
 
+	if (action == KOBJ_CHANGE)
+		sysfs_notify(kobj, NULL, "uevent");
+
 #ifdef CONFIG_UEVENT_HELPER
 	/* call uevent_helper, usually only enabled during early boot */
 	if (uevent_helper[0] && !kobj_usermode_filter(kobj)) {

---
base-commit: 3c999d1ae3c75991902a1a7dad0cb62c2a3008b4
change-id: 20240515-uevent-sysfs-notify-4eb9e95397fd

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>

Re: [PATCH RFC] kobject_uevent: notify uevent sysfs file on changes

[Greg Kroah-Hartman]

On Thu, May 16, 2024 at 12:27:58PM +0200, Thomas Weißschuh wrote:
> The sysfs file "uevent" that exists for each device
> contains the same information that is attached to uevents emitted via
> netlink (or the usermode helper).
> This is useful for userspace which interacts with sysfs directly,
> without using (lib)udev.
> 
> However it is not possible to actually get notified when the data in
> the "uevent" file changes.

What is wrong with listening to the uevent that is happening when the
file changes?

> Enable these notifications, so that the "uevent" file can be used
> together with inotify and friends.

uevent files are meant to be listened to by the uevent itself, why not do
that?

thanks,

greg k-h

Re: [PATCH RFC] kobject_uevent: notify uevent sysfs file on changes

[Thomas Weißschuh]

On 2024-05-16 13:17:34+0000, Greg Kroah-Hartman wrote:
> On Thu, May 16, 2024 at 12:27:58PM +0200, Thomas Weißschuh wrote:
> > The sysfs file "uevent" that exists for each device
> > contains the same information that is attached to uevents emitted via
> > netlink (or the usermode helper).
> > This is useful for userspace which interacts with sysfs directly,
> > without using (lib)udev.
> > 
> > However it is not possible to actually get notified when the data in
> > the "uevent" file changes.
> 
> What is wrong with listening to the uevent that is happening when the
> file changes?

It requires netlink or libudev which is not available or ergonimic for
all userspace programs.
Shellscripts, simple C applications, programing languages without
access to netlink/libudev.

I noticed this when using the "Waybar" application[0] and missing power
supply event updates. Both me and the authors of Waybar expected sysfs
notifications to work for uevent files.

Using sysfs notifications looks like an easy quality-of-life
improvement.

> > Enable these notifications, so that the "uevent" file can be used
> > together with inotify and friends.
> 
> uevent files are meant to be listened to by the uevent itself, why not do
> that?

I can't parse this sentence, sorry. Could you elaborate?

Thanks,
Thomas

[0] https://github.com/Alexays/Waybar/pull/2704

Re: [PATCH RFC] kobject_uevent: notify uevent sysfs file on changes

[Greg Kroah-Hartman]

On Thu, May 16, 2024 at 07:41:07PM +0200, Thomas Weißschuh wrote:
> On 2024-05-16 13:17:34+0000, Greg Kroah-Hartman wrote:
> > On Thu, May 16, 2024 at 12:27:58PM +0200, Thomas Weißschuh wrote:
> > > The sysfs file "uevent" that exists for each device
> > > contains the same information that is attached to uevents emitted via
> > > netlink (or the usermode helper).
> > > This is useful for userspace which interacts with sysfs directly,
> > > without using (lib)udev.
> > > 
> > > However it is not possible to actually get notified when the data in
> > > the "uevent" file changes.
> > 
> > What is wrong with listening to the uevent that is happening when the
> > file changes?
> 
> It requires netlink or libudev which is not available or ergonimic for
> all userspace programs.

But that's the api involved for this thing.

> Shellscripts, simple C applications, programing languages without
> access to netlink/libudev.

You can have a shellscript run for every uevent if that's all you have,
why not just do that?

> I noticed this when using the "Waybar" application[0] and missing power
> supply event updates. Both me and the authors of Waybar expected sysfs
> notifications to work for uevent files.
> 
> Using sysfs notifications looks like an easy quality-of-life
> improvement.
> 
> > > Enable these notifications, so that the "uevent" file can be used
> > > together with inotify and friends.
> > 
> > uevent files are meant to be listened to by the uevent itself, why not do
> > that?
> 
> I can't parse this sentence, sorry. Could you elaborate?

The uevent file just mirrors what was sent in the uevent.  It isn't
there to be polled, it's just there to be read if you want the
information later on for some reason (i.e. coldplugging).

I recommend just using libudev to register for the events, don't try to
parse the files yourself, that way is madness :)

Or register for the netlink events you care about, you can filter very
easily that way to only get the ones you want, that's why the netlink
interface is used.

thanks,

greg k-h

Re: [PATCH RFC] kobject_uevent: notify uevent sysfs file on changes

[Greg Kroah-Hartman]

On Thu, May 16, 2024 at 07:41:07PM +0200, Thomas Weißschuh wrote:
> On 2024-05-16 13:17:34+0000, Greg Kroah-Hartman wrote:
> > On Thu, May 16, 2024 at 12:27:58PM +0200, Thomas Weißschuh wrote:
> > > The sysfs file "uevent" that exists for each device
> > > contains the same information that is attached to uevents emitted via
> > > netlink (or the usermode helper).
> > > This is useful for userspace which interacts with sysfs directly,
> > > without using (lib)udev.
> > > 
> > > However it is not possible to actually get notified when the data in
> > > the "uevent" file changes.
> > 
> > What is wrong with listening to the uevent that is happening when the
> > file changes?
> 
> It requires netlink or libudev which is not available or ergonimic for
> all userspace programs.
> Shellscripts, simple C applications, programing languages without
> access to netlink/libudev.
> 
> I noticed this when using the "Waybar" application[0] and missing power
> supply event updates. Both me and the authors of Waybar expected sysfs
> notifications to work for uevent files.

"Waybar" is anything but a "simple C application", the dependancy chain
it has is quite huge, adding libudev should be very easy, and probably
already there.  Oh look, it is there, upower already brings it in, so no
need to worry about adding another dependancy.

So I think maybe just using the library already in the program might be
simplest, even simpler than attempting to use inotify and friends (as
you don't have to figure out the actual file that you care about, gudev
has that information already for you, abstracted away properly.)

thanks,

greg k-h