[PATCH] kobject_uevent: add uevent_helper exist check

[PATCH] kobject_uevent: add uevent_helper exist check

[zhoumin]

The kernel creates uevent_helper process for every uevent sent,
even if the uevent_helper does not exist. Before the rootfs is
mounted, a large number of events are generated. This change
introduces uevent_helper existence check to prevent unnecessary
operations.

Logs a debug messase before call_usermodehelper_setup.
e.g.: pr_emerg("action:%s devpath:%s\n", action_string, devpath);
You will see a large number of uevent_helper processes
are attempted to be created before the rootfs is mounted.

Signed-off-by: zhoumin <teczm@foxmail.com>
---
 lib/kobject_uevent.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
index b7f2fa08d9c8..f3d34ded141a 100644
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -28,6 +28,7 @@
 #include <net/sock.h>
 #include <net/netlink.h>
 #include <net/net_namespace.h>
+#include <linux/namei.h>
 
 
 atomic64_t uevent_seqnum;
@@ -58,6 +59,23 @@ static const char *kobject_actions[] = {
 	[KOBJ_UNBIND] =		"unbind",
 };
 
+#ifdef CONFIG_UEVENT_HELPER
+static int uevent_helper_lookup(void)
+{
+	static int ret = -ENOENT;
+	struct path path;
+
+	if (!ret)
+		return ret;
+
+	ret = kern_path(uevent_helper, LOOKUP_FOLLOW, &path);
+	if (!ret)
+		path_put(&path);
+
+	return ret;
+}
+#endif
+
 static int kobject_action_type(const char *buf, size_t count,
 			       enum kobject_action *type,
 			       const char **args)
@@ -610,7 +628,7 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
 
 #ifdef CONFIG_UEVENT_HELPER
 	/* call uevent_helper, usually only enabled during early boot */
-	if (uevent_helper[0] && !kobj_usermode_filter(kobj)) {
+	if (uevent_helper[0] && !uevent_helper_lookup() && !kobj_usermode_filter(kobj)) {
 		struct subprocess_info *info;
 
 		retval = add_uevent_var(env, "HOME=/");
-- 
2.43.0

Re: [PATCH] kobject_uevent: add uevent_helper exist check

[Andrew Morton]

On Sat,  5 Apr 2025 01:31:02 +0800 zhoumin <teczm@foxmail.com> wrote:

> The kernel creates uevent_helper process for every uevent sent,
> even if the uevent_helper does not exist. Before the rootfs is
> mounted, a large number of events are generated. This change
> introduces uevent_helper existence check to prevent unnecessary
> operations.
> 
> Logs a debug messase before call_usermodehelper_setup.
> e.g.: pr_emerg("action:%s devpath:%s\n", action_string, devpath);
> You will see a large number of uevent_helper processes
> are attempted to be created before the rootfs is mounted.

Is there any measurable reduction in boot time?

> ...
>
> --- a/lib/kobject_uevent.c
> +++ b/lib/kobject_uevent.c
> @@ -28,6 +28,7 @@
>  #include <net/sock.h>
>  #include <net/netlink.h>
>  #include <net/net_namespace.h>
> +#include <linux/namei.h>
>  
>  
>  atomic64_t uevent_seqnum;
> @@ -58,6 +59,23 @@ static const char *kobject_actions[] = {
>  	[KOBJ_UNBIND] =		"unbind",
>  };
>  
> +#ifdef CONFIG_UEVENT_HELPER
> +static int uevent_helper_lookup(void)
> +{
> +	static int ret = -ENOENT;
> +	struct path path;
> +
> +	if (!ret)
> +		return ret;

Cached in a static variable.  So if a uevent helper later becomes
available, we won't know that and a reboot will be needed?

> +
> +	ret = kern_path(uevent_helper, LOOKUP_FOLLOW, &path);

I wonder if this is the official/preferred way of detecting the
presence of a file.

> +	if (!ret)
> +		path_put(&path);
> +
> +	return ret;
> +}
> +#endif
> +
>  static int kobject_action_type(const char *buf, size_t count,
>  			       enum kobject_action *type,
>  			       const char **args)
> @@ -610,7 +628,7 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
>  
>  #ifdef CONFIG_UEVENT_HELPER
>  	/* call uevent_helper, usually only enabled during early boot */
> -	if (uevent_helper[0] && !kobj_usermode_filter(kobj)) {
> +	if (uevent_helper[0] && !uevent_helper_lookup() && !kobj_usermode_filter(kobj)) {
>  		struct subprocess_info *info;
>  
>  		retval = add_uevent_var(env, "HOME=/");
> -- 
> 2.43.0

Re: [PATCH] kobject_uevent: add uevent_helper exist check

[zhoumin]

Hi Andrew,

Thank you for your feedback.

> Is there any measurable reduction in boot time?

This depends on the number of uevent and the performance of the device. 
In fact, I found this issue during a project to optomize boot time,
and on my embedded device, it can be optimized by at least 2 seconds.

> Cached in a static variable.  So if a uevent helper later becomes
> available, we won't know that and a reboot will be needed?

The static variable is used to avoid repeatedly checking for uevent_helper 
after the rootfs is mounted and uevent_helper is detected.

As for the uevent before the rootfs is mounted, that is another issue.
Whether I submit this patch or not, system always miss these uevent, because
queue is not used to handle this currently. In fact, I have also noticed
this problem, but based on experience, this seems to cause no issue.

> I wonder if this is the official/preferred way of detecting the
presence of a file.

For only checking file existence, kern_path is a sufficiently simple
approach, you can find it in many kernel lookup functions.

Let me know if you have further suggestions.

Best regards,  
zhoumin

Re: [PATCH] kobject_uevent: add uevent_helper exist check

[Greg KH]

On Sun, Apr 06, 2025 at 02:19:15PM +0800, zhoumin wrote:
> Hi Andrew,
> 
> Thank you for your feedback.
> 
> > Is there any measurable reduction in boot time?
> 
> This depends on the number of uevent and the performance of the device. 
> In fact, I found this issue during a project to optomize boot time,
> and on my embedded device, it can be optimized by at least 2 seconds.

So you are spending 2 seconds in failing to lookup the uevent helper
path?  That feels really wrong, how long does your boot take overall?
What % is 2 seconds?

thanks,

greg k-h

Re: [PATCH] kobject_uevent: add uevent_helper exist check

[Greg KH]

On Sat, Apr 05, 2025 at 01:31:02AM +0800, zhoumin wrote:
> The kernel creates uevent_helper process for every uevent sent,
> even if the uevent_helper does not exist. Before the rootfs is
> mounted, a large number of events are generated. This change
> introduces uevent_helper existence check to prevent unnecessary
> operations.

What problem is this causing?  What changed to make this actually be a
problem?

> Logs a debug messase before call_usermodehelper_setup.

I can not parse this sentence, sorry.

> e.g.: pr_emerg("action:%s devpath:%s\n", action_string, devpath);
> You will see a large number of uevent_helper processes
> are attempted to be created before the rootfs is mounted.

Again, why is that a problem?

> Signed-off-by: zhoumin <teczm@foxmail.com>

Please use your name here, not an alias.

> ---
>  lib/kobject_uevent.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
> index b7f2fa08d9c8..f3d34ded141a 100644
> --- a/lib/kobject_uevent.c
> +++ b/lib/kobject_uevent.c
> @@ -28,6 +28,7 @@
>  #include <net/sock.h>
>  #include <net/netlink.h>
>  #include <net/net_namespace.h>
> +#include <linux/namei.h>
>  
>  
>  atomic64_t uevent_seqnum;
> @@ -58,6 +59,23 @@ static const char *kobject_actions[] = {
>  	[KOBJ_UNBIND] =		"unbind",
>  };
>  
> +#ifdef CONFIG_UEVENT_HELPER
> +static int uevent_helper_lookup(void)
> +{
> +	static int ret = -ENOENT;
> +	struct path path;
> +
> +	if (!ret)
> +		return ret;
> +
> +	ret = kern_path(uevent_helper, LOOKUP_FOLLOW, &path);
> +	if (!ret)
> +		path_put(&path);

What happens when the root filesystem changes to the new one?  This
feels wrong as Andrew said.

thanks,

greg k-h

Re: [PATCH] kobject_uevent: add uevent_helper exist check

[zhoumin]

Hi Greg

I appreciate your patience in addressing this.

> > The kernel creates uevent_helper process for every uevent sent,
> > even if the uevent_helper does not exist. Before the rootfs is
> > mounted, a large number of events are generated. This change
> > introduces uevent_helper existence check to prevent unnecessary
> > operations.

> What problem is this causing? What changed to make this actually be a
problem?

I think calling uevent_helper before rootfs mount is pointless and waste
time, because uevent_helper does not exist at that stage. For example,
fs_initcall(chr_dev_init),subsys_initcall(usb_init) and etc, these module
run before rootfs_initcall and will trigger kobject_uevent when
uevent_helper isn't ready.

However, I've discovered this issue was already addressed by commit:
<b234ed6d629420827e2839c8c8935be85a0867fd> ("init: move
usermodehelper_enable() to populate_rootfs()")

Due to my device running an outdated kernel version, this change wasn't
immediately apparent to me.

But I propose we should make call_usermodehelper_exec fail earlier, link
this:
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -610,7 +610,7 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,

 #ifdef CONFIG_UEVENT_HELPER
        /* call uevent_helper, usually only enabled during early boot */
-       if (uevent_helper[0] && !kobj_usermode_filter(kobj)) {
+       if (uevent_helper[0] && !usermodehelper_disabled && !kobj_usermode_filter(kobj)) {
                struct subprocess_info *info;

                retval = add_uevent_var(env, "HOME=/");


If this is helpful, I can submit a follow-up patch.

> > Logs a debug messase before call_usermodehelper_setup.

> I can not parse this sentence, sorry.

What I'd say is to add such a line of debug logs.

> > e.g.: pr_emerg("action:%s devpath:%s\n", action_string, devpath);
> > You will see a large number of uevent_helper processes
> > are attempted to be created before the rootfs is mounted.

> Again, why is that a problem?

Like I said at the beginning.


> Signed-off-by: zhoumin

> Please use your name here, not an alias.

This is the Pinyin spelling of my Chinese name.

> ---
> lib/kobject_uevent.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
> index b7f2fa08d9c8..f3d34ded141a 100644
> --- a/lib/kobject_uevent.c
> +++ b/lib/kobject_uevent.c
> @@ -28,6 +28,7 @@
> #include
> #include
> #include
> +#include
>
>
> atomic64_t uevent_seqnum;
> @@ -58,6 +59,23 @@ static const char *kobject_actions[] = {
> [KOBJ_UNBIND] = "unbind",
> };
>
> +#ifdef CONFIG_UEVENT_HELPER
> +static int uevent_helper_lookup(void)
> +{
> + static int ret = -ENOENT;
> + struct path path;
> +
> + if (!ret)
> + return ret;
> +
> + ret = kern_path(uevent_helper, LOOKUP_FOLLOW, &path);
> + if (!ret)
> + path_put(&path);

> What happens when the root filesystem changes to the new one? This
feels wrong as Andrew said.

If a user performs actions like chroot into a new rootfs that lacks
uevent_helper, this should be considered a rootfs configuration error
rather than a kernel issue. The kernel shouldn't handle missing helpers
at this stage.

Apply to the question of another mail.
Link: https://lore.kernel.org/lkml/2025040731-eternity-statutory-4a80@gregkh/


> > > Is there any measurable reduction in boot time?
> >
> > This depends on the number of uevent and the performance of the device.
> > In fact, I found this issue during a project to optomize boot time,
> > and on my embedded device, it can be optimized by at least 2 seconds.
 
> So you are spending 2 seconds in failing to lookup the uevent helper
> path? That feels really wrong, how long does your boot take overall?
> What % is 2 seconds?

The measurements were taken on a device running kernel 4.4.65 using
/proc/uptime. The 129s average boot time served as baseline for comparing
the time difference before and after the changes, though this metric is no
longer the primary concern.

Would you like me to submit a formal patch with above changes.


Best regards,

zhoumin

Re: [PATCH] kobject_uevent: add uevent_helper exist check

[Greg KH]

On Mon, Apr 07, 2025 at 11:18:12PM +0800, zhoumin wrote:
> Hi Greg
> 
> I appreciate your patience in addressing this.
> 
> > > The kernel creates uevent_helper process for every uevent sent,
> > > even if the uevent_helper does not exist. Before the rootfs is
> > > mounted, a large number of events are generated. This change
> > > introduces uevent_helper existence check to prevent unnecessary
> > > operations.
> 
> > What problem is this causing? What changed to make this actually be a
> problem?
> 
> I think calling uevent_helper before rootfs mount is pointless and waste
> time, because uevent_helper does not exist at that stage. For example,
> fs_initcall(chr_dev_init),subsys_initcall(usb_init) and etc, these module
> run before rootfs_initcall and will trigger kobject_uevent when
> uevent_helper isn't ready.
> 
> However, I've discovered this issue was already addressed by commit:
> <b234ed6d629420827e2839c8c8935be85a0867fd> ("init: move
> usermodehelper_enable() to populate_rootfs()")
> 
> Due to my device running an outdated kernel version, this change wasn't
> immediately apparent to me.

Ah, great, let's just stick with the change that we already have.

> But I propose we should make call_usermodehelper_exec fail earlier, link
> this:
> --- a/lib/kobject_uevent.c
> +++ b/lib/kobject_uevent.c
> @@ -610,7 +610,7 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
> 
>  #ifdef CONFIG_UEVENT_HELPER
>         /* call uevent_helper, usually only enabled during early boot */
> -       if (uevent_helper[0] && !kobj_usermode_filter(kobj)) {
> +       if (uevent_helper[0] && !usermodehelper_disabled && !kobj_usermode_filter(kobj)) {
>                 struct subprocess_info *info;
> 
>                 retval = add_uevent_var(env, "HOME=/");

Why?  Will this actually change the speed of anything?

> > Signed-off-by: zhoumin
> 
> > Please use your name here, not an alias.
> 
> This is the Pinyin spelling of my Chinese name.

No reason you can not just use the Chinese spelling of your name either :)

thanks,

greg k-h

Re: [PATCH] kobject_uevent: add uevent_helper exist check

[zhoumin]

> > But I propose we should make call_usermodehelper_exec fail earlier, link
> > this:
> > --- a/lib/kobject_uevent.c
> > +++ b/lib/kobject_uevent.c
> > @@ -610,7 +610,7 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
> >
> > #ifdef CONFIG_UEVENT_HELPER
> > /* call uevent_helper, usually only enabled during early boot */
> > - if (uevent_helper[0] && !kobj_usermode_filter(kobj)) {
> > + if (uevent_helper[0] && !usermodehelper_disabled && !kobj_usermode_filter(kobj)) {
> > struct subprocess_info *info;
> >
> > retval = add_uevent_var(env, "HOME=/");
 
> Why? Will this actually change the speed of anything?

While the actual speed improvement may be minimal(may vary depending on
device performance and how many uevents are triggered before rootfs mount),
I just think executing add_uevent_var(), call_usermodehelper_setup(), and
call_usermodehelper_exec() is unnecessary in these cases. We can simply
return early. If you consider this change unnecessary, I'll respectfully
close this discussion and focus on other areas where more significant
improvements might be achieved. 

---
Appendix: QEMU Test Logs
In my current QEMU test environment, this code path executes
2,127 times before rootfs mount. Sample events collected via debug logs:
	pr_emerg("action:%s devpath:%s\n", action_string, devpath);

[    0.263272] action:add devpath:/bus/faux
[    0.264278] action:add devpath:/bus/faux/drivers/faux_driver
[    0.267476] action:add devpath:/bus/platform
[    0.267559] action:add devpath:/bus/auxiliary
[    0.267669] action:add devpath:/bus/memory
[    0.268789] action:add devpath:/devices/system/memory/memory8
[    0.269501] action:add devpath:/devices/system/memory/memory9
[    0.269664] action:add devpath:/devices/system/memory/memory10
[    0.269793] action:add devpath:/devices/system/memory/memory11
[    0.269889] action:add devpath:/devices/system/memory/memory12
[    0.270008] action:add devpath:/devices/system/memory/memory13
[    0.270108] action:add devpath:/devices/system/memory/memory14
[    0.270208] action:add devpath:/devices/system/memory/memory15
[    0.270297] action:add devpath:/devices/system/memory/memory16
[    0.270409] action:add devpath:/devices/system/memory/memory17
...
[    1.068185] action:add devpath:/devices/virtual/tty/tty58
[    1.069597] action:add devpath:/devices/virtual/tty/tty59
[    1.070887] action:add devpath:/devices/virtual/tty/tty60
[    1.071801] action:add devpath:/devices/virtual/tty/tty61
[    1.073554] action:add devpath:/devices/virtual/tty/tty62
[    1.075086] action:add devpath:/devices/virtual/tty/tty63
...
[    3.519928] action:add devpath:/devices/platform/gpio-keys/input/input0/event0
[    3.521347] action:remove devpath:/devices/virtual/devlink/amba:9030000.pl061--platform:gpio-keys
[    3.523161] action:bind devpath:/devices/platform/gpio-keys
[    3.523831] action:add devpath:/bus/platform/drivers/gpio-keys
[    3.529957] action:add devpath:/bus/amba/drivers/amba-proxy

Best regards,

zhoumin