| Message ID | 20190523131444.36058-1-andriy.shevchenko@linux.intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v1] package/busybox: Avoid "can't create /proc/sys/kernel/hotplug" error | expand |
>>>>> "Andy" == Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes: > At runtime on most of the modern kernels [1] S10mdev script produces an error: > /etc/init.d/S10mdev: line 9: can't create /proc/sys/kernel/hotplug: nonexistent directory > since the commit > caae7fa1d737 ("busybox: register mdev as hotplug helper when selected") > added it unconditionally. > Check the presence of procfs node before writing to it. > [1]: Linux kernels with the commit > 86d56134f1b6 ("kobject: Make support for uevent_helper optional.") > applied. But how can mdev work on such setups?
>>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes: >>>>> "Andy" == Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes: >> At runtime on most of the modern kernels [1] S10mdev script produces an error: >> /etc/init.d/S10mdev: line 9: can't create /proc/sys/kernel/hotplug: nonexistent directory >> since the commit >> caae7fa1d737 ("busybox: register mdev as hotplug helper when selected") >> added it unconditionally. >> Check the presence of procfs node before writing to it. >> [1]: Linux kernels with the commit >> 86d56134f1b6 ("kobject: Make support for uevent_helper optional.") >> applied. > But how can mdev work on such setups? To be clear, if mdev needs CONFIG_UEVENT_HELPER=y for hotplug notifications, then we should rather force that in linux/linux.mk and mention the dependency in the mdev help text rather than silently not having hotplug work. I have marked this patch as changes requested in patchwork.
On Fri, May 24, 2019 at 09:49:30AM +0200, Peter Korsgaard wrote: > >>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes: > > >>>>> "Andy" == Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes: > >> At runtime on most of the modern kernels [1] S10mdev script produces an error: > >> /etc/init.d/S10mdev: line 9: can't create /proc/sys/kernel/hotplug: nonexistent directory > > >> since the commit > > >> caae7fa1d737 ("busybox: register mdev as hotplug helper when selected") > > >> added it unconditionally. > > >> Check the presence of procfs node before writing to it. > > >> [1]: Linux kernels with the commit > >> 86d56134f1b6 ("kobject: Make support for uevent_helper optional.") > >> applied. > > > But how can mdev work on such setups? At least the script is used to initialize modules at the boot time. What happens afterwards I didn't investigate. I have no such issues. > To be clear, if mdev needs CONFIG_UEVENT_HELPER=y for hotplug > notifications, then we should rather force that in linux/linux.mk and > mention the dependency in the mdev help text rather than silently not > having hotplug work. > > I have marked this patch as changes requested in patchwork. Kernel can be built outside of Buildroot. Still this needs to be fixed.
>>>>> "Andy" == Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes: >> > But how can mdev work on such setups? > At least the script is used to initialize modules at the boot time. > What happens afterwards I didn't investigate. I have no such issues. You mean the: # coldplug modules find /sys/ -name modalias -print0 | xargs -0 sort -u | tr '\n' '\0' | \ xargs -0 modprobe -abq That indeed does not depend on mdev working. >> To be clear, if mdev needs CONFIG_UEVENT_HELPER=y for hotplug >> notifications, then we should rather force that in linux/linux.mk and >> mention the dependency in the mdev help text rather than silently not >> having hotplug work. >> >> I have marked this patch as changes requested in patchwork. > Kernel can be built outside of Buildroot. Still this needs to be fixed. And just like all other packages that require specific kernel options enabled, if you are not using Buildroot to build the kernel then you have to ensure the kernel has the right config options enabled. I am not sure what you mean exactly with "Still this needs to be fixed". The only fix I can think of is what I mentioned above, E.G. document the need for CONFIG_UEVENT_HELPER in the help text and automatically enable it in linux.mk. Expecting mdev to work sensible on a system without uevent helper support isn't sensible, it is similar to having radvd (IPv6 router advertising daemon) work on a system without IPv6.
On Fri, May 24, 2019 at 11:12:17PM +0200, Peter Korsgaard wrote: > >>>>> "Andy" == Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes: > > >> > But how can mdev work on such setups? > > > At least the script is used to initialize modules at the boot time. > > What happens afterwards I didn't investigate. I have no such issues. > > You mean the: > > # coldplug modules > find /sys/ -name modalias -print0 | xargs -0 sort -u | tr '\n' '\0' | \ > xargs -0 modprobe -abq > > That indeed does not depend on mdev working. > > > >> To be clear, if mdev needs CONFIG_UEVENT_HELPER=y for hotplug > >> notifications, then we should rather force that in linux/linux.mk and > >> mention the dependency in the mdev help text rather than silently not > >> having hotplug work. > >> > >> I have marked this patch as changes requested in patchwork. > > > Kernel can be built outside of Buildroot. Still this needs to be fixed. > > And just like all other packages that require specific kernel options > enabled, if you are not using Buildroot to build the kernel then you > have to ensure the kernel has the right config options enabled. > > I am not sure what you mean exactly with "Still this needs to be > fixed". The only fix I can think of is what I mentioned above, > E.G. document the need for CONFIG_UEVENT_HELPER in the help text and > automatically enable it in linux.mk. > > Expecting mdev to work sensible on a system without uevent helper > support isn't sensible, it is similar to having radvd (IPv6 router > advertising daemon) work on a system without IPv6. I'm not sure if mdev follows the change in the kernel, but uEvent helper is obsolete mechanism. --- 8< --- 8< --- 8< --- The external binary /sbin/hotplug was used in earlier releases to inform Udev about device state change. That has been replaced and Udev can now directly listen to those events through Netlink. --- 8< --- 8< --- 8< ---
diff --git a/package/busybox/S10mdev b/package/busybox/S10mdev index 7075b77016..ddede9a4a6 100644 --- a/package/busybox/S10mdev +++ b/package/busybox/S10mdev @@ -6,7 +6,7 @@ case "$1" in start) echo "Starting mdev..." - echo /sbin/mdev >/proc/sys/kernel/hotplug + test -f /proc/sys/kernel/hotplug && echo /sbin/mdev > /proc/sys/kernel/hotplug /sbin/mdev -s # coldplug modules find /sys/ -name modalias -print0 | xargs -0 sort -u | tr '\n' '\0' | \
At runtime on most of the modern kernels [1] S10mdev script produces an error: /etc/init.d/S10mdev: line 9: can't create /proc/sys/kernel/hotplug: nonexistent directory since the commit caae7fa1d737 ("busybox: register mdev as hotplug helper when selected") added it unconditionally. Check the presence of procfs node before writing to it. [1]: Linux kernels with the commit 86d56134f1b6 ("kobject: Make support for uevent_helper optional.") applied. Fixes: caae7fa1d737 ("busybox: register mdev as hotplug helper when selected") Cc: Gustavo Zacarias <gustavo@zacarias.com.ar> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- package/busybox/S10mdev | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)