All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] Automatic module loading with mdev is broken?
@ 2018-05-06  9:46 Daniel Palmer
  2018-05-07  6:45 ` Peter Korsgaard
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Palmer @ 2018-05-06  9:46 UTC (permalink / raw)
  To: buildroot

Hi all,

I'm trying to get automatic module loading with mdev and it looks like
it was broken with this commit
b4fc5a180c81689a982d5c595844331684c14f51
(https://github.com/buildroot/buildroot/commit/b4fc5a180c81689a982d5c595844331684c14f51)
or has maybe regressed?

Anyhow the previous version of the line that the commit changed works
(the modules I expect to get loaded get loaded).
The changed version results in nothing being loaded.

I think it's to do with this part "xargs -0 sort -u -z". Sort is
taking in the big list of files correctly and sorting them but the
result
seems to still have new lines in it. So when that is passed to xargs
(which is expecting the items to be null terminated) again
to run modprobe the module aliases don't match and thus nothing gets loaded.

This *works* for me but I'm not sure if it's right:

diff --git a/package/busybox/S10mdev b/package/busybox/S10mdev
index 63ca955b1c..71693e7a9d 100644
--- a/package/busybox/S10mdev
+++ b/package/busybox/S10mdev
@@ -9,7 +9,7 @@ case "$1" in
       echo /sbin/mdev >/proc/sys/kernel/hotplug
       /sbin/mdev -s
       # coldplug modules
-       find /sys/ -name modalias -print0 | xargs -0 sort -u -z |
xargs -0 modprobe -abq
+       find /sys/ -name modalias -print0 | xargs -0 sort -u -z | tr
-d '\n' | xargs -0 modprobe -abq
       ;;
  stop)
       ;;

Cheers,

Daniel

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

* [Buildroot] Automatic module loading with mdev is broken?
  2018-05-06  9:46 [Buildroot] Automatic module loading with mdev is broken? Daniel Palmer
@ 2018-05-07  6:45 ` Peter Korsgaard
  2018-05-07  8:30   ` Daniel Palmer
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Korsgaard @ 2018-05-07  6:45 UTC (permalink / raw)
  To: buildroot

>>>>> "Daniel" == Daniel Palmer <daniel@0x0f.com> writes:

 > Hi all,
 > I'm trying to get automatic module loading with mdev and it looks like
 > it was broken with this commit
 > b4fc5a180c81689a982d5c595844331684c14f51
 > (https://github.com/buildroot/buildroot/commit/b4fc5a180c81689a982d5c595844331684c14f51)
 > or has maybe regressed?

 > Anyhow the previous version of the line that the commit changed works
 > (the modules I expect to get loaded get loaded).
 > The changed version results in nothing being loaded.

 > I think it's to do with this part "xargs -0 sort -u -z". Sort is
 > taking in the big list of files correctly and sorting them but the
 > result
 > seems to still have new lines in it. So when that is passed to xargs
 > (which is expecting the items to be null terminated) again
 > to run modprobe the module aliases don't match and thus nothing gets loaded.

 > This *works* for me but I'm not sure if it's right:

Hmm, you are right. sort -z means that the lines in the input files
should be zero terminated and not newline terminated in addition to
outputting zero terminated strings.

The lines in modalias files are newline terminated, and not zero, E.G.:

hexdump -C /sys/devices/LNXSYSTM:00/LNXCPU:00/modalias
00000000  61 63 70 69 3a 4c 4e 58  43 50 55 3a 0a           |acpi:LNXCPU:.|

The modalias entries themselves afaik never contains spaces, those can
only happen in the path name to the modalias file (as b4fc5a180c81
showed):

find /sys -name modalias | xargs cat | egrep '[[:space:]]'

So I guess we can just do:

find /sys/ -name modalias -print0 | xargs -0 sort u | xargs modprobe -abq

Can you give that a try? Thanks.


 > diff --git a/package/busybox/S10mdev b/package/busybox/S10mdev
 > index 63ca955b1c..71693e7a9d 100644
 > --- a/package/busybox/S10mdev
 > +++ b/package/busybox/S10mdev
 > @@ -9,7 +9,7 @@ case "$1" in
 >        echo /sbin/mdev >/proc/sys/kernel/hotplug
 >        /sbin/mdev -s
 >        # coldplug modules
 > -       find /sys/ -name modalias -print0 | xargs -0 sort -u -z |
 > xargs -0 modprobe -abq
 > +       find /sys/ -name modalias -print0 | xargs -0 sort -u -z | tr
 > -d '\n' | xargs -0 modprobe -abq
 >        ;;
 >   stop)
 >        ;;

 > Cheers,

 > Daniel
 > _______________________________________________
 > buildroot mailing list
 > buildroot at busybox.net
 > http://lists.busybox.net/mailman/listinfo/buildroot

-- 
Bye, Peter Korsgaard

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

* [Buildroot] Automatic module loading with mdev is broken?
  2018-05-07  6:45 ` Peter Korsgaard
@ 2018-05-07  8:30   ` Daniel Palmer
  2018-05-07  8:53     ` Peter Korsgaard
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Palmer @ 2018-05-07  8:30 UTC (permalink / raw)
  To: buildroot

Hi Peter,

Your change works but looking at the text of the original change that
caused the regression 'First alias in question is "platform:Fixed MDIO
bus".' it seems it would fail with that? The driver that creates that
alias still exists ->
https://github.com/torvalds/linux/blob/master/drivers/net/phy/fixed_phy.c#L261

Cheers,

Daniel

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

* [Buildroot] Automatic module loading with mdev is broken?
  2018-05-07  8:30   ` Daniel Palmer
@ 2018-05-07  8:53     ` Peter Korsgaard
  2018-05-07  9:40       ` Daniel Palmer
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Korsgaard @ 2018-05-07  8:53 UTC (permalink / raw)
  To: buildroot

>>>>> "Daniel" == Daniel Palmer <daniel@0x0f.com> writes:

 > Hi Peter,
 > Your change works but looking at the text of the original change that
 > caused the regression 'First alias in question is "platform:Fixed MDIO
 > bus".' it seems it would fail with that? The driver that creates that
 > alias still exists ->
 > https://github.com/torvalds/linux/blob/master/drivers/net/phy/fixed_phy.c#L261

Ok. Looking at the error message it was really about the sysfs entry:

sort: /sys/devices/platform/Fixed

So that was presumably '/sys/devices/platform/Fixed MDIO bus'. This is
fixed by the find .. -print0 | xargs -0 sort

There may or may not be situations where the modalias info contains
spaces. I have so far not seen such situation though. fixed_phy does not
provide it:

/sbin/modinfo build/linux-4.14.34/drivers/net/phy/fixed_phy.ko
icense:        GPL
author:         Vitaly Bordug
description:    Fixed MDIO bus (MDIO bus emulation with fixed PHYs)
depends:        libphy
intree:         Y
name:           fixed_phy
vermagic:       4.14.34 SMP

There unfortunately doesn't seem to be an obvious/easy way to handle
modalias entries containing spaces. The easy solution would be to use
xargs -d '\n' but that is not supported by the busybox applet, so I
guess we need to use tr to replace \n with \0 and then xargs -0.

But I would prefer to wait with that until we know it is needed.

-- 
Bye, Peter Korsgaard

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

* [Buildroot] Automatic module loading with mdev is broken?
  2018-05-07  8:53     ` Peter Korsgaard
@ 2018-05-07  9:40       ` Daniel Palmer
  2018-05-07 18:29         ` Peter Korsgaard
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Palmer @ 2018-05-07  9:40 UTC (permalink / raw)
  To: buildroot

Hi Peter,

I found one of my systems that actually has this module loaded:

daniel at espressobin2:/sys/devices/platform/Fixed MDIO bus.0$ cat modalias
platform:Fixed MDIO bus

So I think we do need the tr fix.

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

* [Buildroot] Automatic module loading with mdev is broken?
  2018-05-07  9:40       ` Daniel Palmer
@ 2018-05-07 18:29         ` Peter Korsgaard
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Korsgaard @ 2018-05-07 18:29 UTC (permalink / raw)
  To: buildroot

>>>>> "Daniel" == Daniel Palmer <daniel@0x0f.com> writes:

 > Hi Peter,
 > I found one of my systems that actually has this module loaded:

 > daniel at espressobin2:/sys/devices/platform/Fixed MDIO bus.0$ cat modalias
 > platform:Fixed MDIO bus

 > So I think we do need the tr fix.

Ok :/ - Lets then add a tr '\n' '\0' between sort and xargs -0.

-- 
Bye, Peter Korsgaard

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

end of thread, other threads:[~2018-05-07 18:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-06  9:46 [Buildroot] Automatic module loading with mdev is broken? Daniel Palmer
2018-05-07  6:45 ` Peter Korsgaard
2018-05-07  8:30   ` Daniel Palmer
2018-05-07  8:53     ` Peter Korsgaard
2018-05-07  9:40       ` Daniel Palmer
2018-05-07 18:29         ` Peter Korsgaard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.