On Mon, Aug 3, 2015 at 9:07 AM, Denys Vlasenko <[email protected]> wrote: > On Wed, Jul 15, 2015 at 11:10 PM, Gregory Fong <[email protected]> wrote: >> From: Simon Edlund <[email protected]> >> >> When mdev -s traverses the /sys directory looking for "dev" files, it >> starts with the block devices under /sys/block, and will find the "dev" >> file through the symlink, and create a block device node. In the next >> stage it will scan the /sys/class looking for char devices, and will >> find mtdX/dev again, but this time the mknod will fail because there is >> already a device node with that name. > > By swapping recursive_action("/sys/class") and > recursive_action("/sys/block"), now the same thing will happen > when the second scan is done. > > Why this is better?
This is better because if you're using a newer kernel with both CONFIG_SYSFS_DEPRECATED=y (and CONFIG_SYSFS_DEPRECATED_V2=y to enable it), then where traversing /sys/block with the deprecated layout: - /sys/block/mtdblock0 is not a symlink, but an actual directory - /sys/block/mtdblock0/device is a symlink to mtd0 IOW, on deprecated sysfs: # ls -l /sys/block/mtdblock0 -r--r--r-- 1 root root 4096 Jan 1 00:00 alignment_offset lrwxrwxrwx 1 root root 0 Jan 1 00:00 bdi -> ../../devices/virtual/bdi/31:0 -r--r--r-- 1 root root 4096 Jan 1 00:00 capability -r--r--r-- 1 root root 4096 Jan 1 00:00 dev lrwxrwxrwx 1 root root 0 Jan 1 00:00 device -> ../../devices/platform/brcmnand.0/mtd/mtd0 -r--r--r-- 1 root root 4096 Jan 1 00:00 discard_alignment -r--r--r-- 1 root root 4096 Jan 1 00:00 ext_range drwxr-xr-x 2 root root 0 Jan 1 00:00 holders -r--r--r-- 1 root root 4096 Jan 1 00:00 inflight drwxr-xr-x 2 root root 0 Jan 1 00:00 power drwxr-xr-x 3 root root 0 Jan 1 00:00 queue -r--r--r-- 1 root root 4096 Jan 1 00:00 range -r--r--r-- 1 root root 4096 Jan 1 00:00 removable -r--r--r-- 1 root root 4096 Jan 1 00:00 ro -r--r--r-- 1 root root 4096 Jan 1 00:00 size drwxr-xr-x 2 root root 0 Jan 1 00:00 slaves -r--r--r-- 1 root root 4096 Jan 1 00:00 stat lrwxrwxrwx 1 root root 0 Jan 1 00:00 subsystem -> ../../block -rw-r--r-- 1 root root 4096 Jan 1 00:00 uevent On non-deprecated sysfs: # ls -l /sys/block/mtdblock0 lrwxrwxrwx 1 root root 0 Jan 3 18:57 /sys/block/mtdblock0 -> ../devices/platform/rdb/f03e2800.nand/mtd/mtd0/mtdblock0 Now, I hadn't really investigated the problem carefully enough until now. This patch simply solved a real problem for us when using CONFIG_SYSFS_DEPRECATED, and I figured that submitting it would be at least a good starting point for someone who actually understands this better to figure out a better fix. But let's see... since recursive_action is called with ACTION_FOLLOWLINKS, it will end up checking both /sys/block/mtdblock0/dev and /sys/block/mtdblock0/device/dev and creating block devices for both of those. That's not correct. So I think I now have a real fix here, which is to change the recursive_action call that will only be invoked if CONFIG_SYSFS_DEPRECATED=y. In that case, we _shouldn't_ need to follow symlinks because everything in the top level of /sys/block/ will be a real directory, and following them will result in the aforementioned problem. It was a bit surprising to me that this problem isn't also seen with /sys/class/block, since that has the same hierarchy where it could follow the link and get to /sys/class/block/mtdblock0/device/dev, but looking at recursive_action(), it appears that links are only followed where depth == 0, which would explain that. I'm stuck using the gmail interface right now and so can't copy and paste the new patch inline... will send to the list shortly. Best regards, Gregory > >> [gregory: added commit message to patch from BZ #6806] >> Signed-off-by: Gregory Fong <[email protected]> >> --- >> util-linux/mdev.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/util-linux/mdev.c b/util-linux/mdev.c >> index ca4b915..4495b0a 100644 >> --- a/util-linux/mdev.c >> +++ b/util-linux/mdev.c >> @@ -1063,6 +1063,9 @@ int mdev_main(int argc UNUSED_PARAM, char **argv) >> >> putenv((char*)"ACTION=add"); >> >> + recursive_action("/sys/class", >> + ACTION_RECURSE | ACTION_FOLLOWLINKS, >> + fileAction, dirAction, temp, 0); >> /* ACTION_FOLLOWLINKS is needed since in newer kernels >> * /sys/block/loop* (for example) are symlinks to dirs, >> * not real directories. >> @@ -1079,9 +1082,6 @@ int mdev_main(int argc UNUSED_PARAM, char **argv) >> ACTION_RECURSE | ACTION_FOLLOWLINKS | >> ACTION_QUIET, >> fileAction, dirAction, temp, 0); >> } >> - recursive_action("/sys/class", >> - ACTION_RECURSE | ACTION_FOLLOWLINKS, >> - fileAction, dirAction, temp, 0); >> } else { >> char *fw; >> char *seq; >> -- >> 1.9.1 >> >> _______________________________________________ >> busybox mailing list >> [email protected] >> http://lists.busybox.net/mailman/listinfo/busybox _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
