Message ID | 20200418221411.1549783-4-unixmania@gmail.com |
---|---|
State | Rejected, archived |
Headers | show |
Series | Add a kernel module loading mechanism | expand |
CArlos, All, On 2020-04-18 19:14 -0300, unixmania@gmail.com spake thusly: > From: Carlos Santos <unixmania@gmail.com> > > This is compatible with BusyBox/sysvinit (via S02modules-load), OpenRC > and systemd. It also prevents trying to load the module each time the > init script is executed. > > Signed-off-by: Carlos Santos <unixmania@gmail.com> > --- > Tested on QEMU with a kernel config with MD=y and DM_<FOO>=m. Only the > module loading was tested, since there is no RAID device. > --- > package/dmraid/S20dmraid | 3 --- > package/dmraid/dmraid.mk | 12 ++++++++++++ > 2 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/package/dmraid/S20dmraid b/package/dmraid/S20dmraid > index b3bfdcc84b..ff325154c6 100644 > --- a/package/dmraid/S20dmraid > +++ b/package/dmraid/S20dmraid > @@ -2,9 +2,6 @@ > > set -e > > -# try to load module in case that hasn't been done yet > -modprobe dm-mod >/dev/null 2>&1 > - > case "$1" in > start|"") > echo "Setting up DMRAID devices..." > diff --git a/package/dmraid/dmraid.mk b/package/dmraid/dmraid.mk > index 0382cd4d99..c8facbdabd 100644 > --- a/package/dmraid/dmraid.mk > +++ b/package/dmraid/dmraid.mk > @@ -21,4 +21,16 @@ define DMRAID_INSTALL_INIT_SYSV > $(TARGET_DIR)/etc/init.d/S20dmraid > endef > > +ifeq ($(BR2_LINUX_KERNEL),y) > +define DMRAID_GEN_MODULES_CONF > + $(INSTALL) -d -m 755 $(TARGET_DIR)/etc/modules-load.d > + { \ > + find $(TARGET_DIR)/lib/modules/$(LINUX_VERSION_PROBED)/kernel/drivers/md -name 'dm[_-]mod.ko*'; \ > + } 2> /dev/null | { \ > + sed 's:^.*/::; s:\.ko[^ ]*::'; \ > + } > $(TARGET_DIR)/etc/modules-load.d/20-dmraid.conf > +endef > +DMRAID_TARGET_FINALIZE_HOOKS += DMRAID_GEN_MODULES_CONF > +endif Why do we need this complex code? Furthermore, we'll have to repeat that complexity in all packages that want to provide modules; that's not nice... :-( Can't we simply have a modules table, like we have the device amd user tables? Something like: define DMRAID_MODULES dm-stuff dm-blurb dm-blabla endef And then have the package and rootfs infrastructures handle that like the other tables? Regards, Yann E. MORIN. > $(eval $(autotools-package)) > -- > 2.18.2 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
On Sun, 19 Apr 2020 12:42:39 +0200 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > Can't we simply have a modules table, like we have the device amd user > tables? Something like: > > define DMRAID_MODULES > dm-stuff > dm-blurb > dm-blabla > endef > > And then have the package and rootfs infrastructures handle that like > the other tables? If we're going to do that, then we don't need an infra at all, package/dmraid/ can simply contain a file with the list of modules, which gets installed to /usr/lib/modules-load.d/. I think the idea here was to avoid maintaining the list of dmraid modules, and ensure it is generated automatically from the list of installed modules. But even that is not guaranteed to work 100%, for example if one day dmraid adds a module that doesn't match the dm[_-]mod.ko* expression. Thomas
Thomas, Carlos, All, On 2020-04-19 14:49 +0200, Thomas Petazzoni spake thusly: > On Sun, 19 Apr 2020 12:42:39 +0200 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > > Can't we simply have a modules table, like we have the device amd user > > tables? Something like: > > > > define DMRAID_MODULES > > dm-stuff > > dm-blurb > > dm-blabla > > endef > > > > And then have the package and rootfs infrastructures handle that like > > the other tables? > > If we're going to do that, then we don't need an infra at all, > package/dmraid/ can simply contain a file with the list of modules, > which gets installed to /usr/lib/modules-load.d/. Right, adding an infra just to copy a file would over-engineering. > I think the idea here was to avoid maintaining the list of dmraid > modules, and ensure it is generated automatically from the list of > installed modules. But even that is not guaranteed to work 100%, for > example if one day dmraid adds a module that doesn't match the > dm[_-]mod.ko* expression. But suddenly, it strikes me as odd that the dmraid package has to list the modules built by the kernel. Those modules should be auto-loaded on-demand, no? For dmraid, we may load just the main module, dm-mod, as was done so far, and leave to the user the responsibility to provide a module file to load whatever exact module they need? I would be OK for a long list, but this complex code repeated over and over again for each package that needs to load modules, is just ugly to me... Also, this created files during the target-finalize step, and so those files are not accounted to the package that installed them. In the end, I guess such a functionality is mostly useful for those out-of-tree modules that do not behave properly, and need manual loading. In which case, those packages know what modules to list, and they can do so during the installation step (directly or as part of a post-install hook). Using target-finalize is not so nice... Regards, Yann E. MORIN. > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
diff --git a/package/dmraid/S20dmraid b/package/dmraid/S20dmraid index b3bfdcc84b..ff325154c6 100644 --- a/package/dmraid/S20dmraid +++ b/package/dmraid/S20dmraid @@ -2,9 +2,6 @@ set -e -# try to load module in case that hasn't been done yet -modprobe dm-mod >/dev/null 2>&1 - case "$1" in start|"") echo "Setting up DMRAID devices..." diff --git a/package/dmraid/dmraid.mk b/package/dmraid/dmraid.mk index 0382cd4d99..c8facbdabd 100644 --- a/package/dmraid/dmraid.mk +++ b/package/dmraid/dmraid.mk @@ -21,4 +21,16 @@ define DMRAID_INSTALL_INIT_SYSV $(TARGET_DIR)/etc/init.d/S20dmraid endef +ifeq ($(BR2_LINUX_KERNEL),y) +define DMRAID_GEN_MODULES_CONF + $(INSTALL) -d -m 755 $(TARGET_DIR)/etc/modules-load.d + { \ + find $(TARGET_DIR)/lib/modules/$(LINUX_VERSION_PROBED)/kernel/drivers/md -name 'dm[_-]mod.ko*'; \ + } 2> /dev/null | { \ + sed 's:^.*/::; s:\.ko[^ ]*::'; \ + } > $(TARGET_DIR)/etc/modules-load.d/20-dmraid.conf +endef +DMRAID_TARGET_FINALIZE_HOOKS += DMRAID_GEN_MODULES_CONF +endif + $(eval $(autotools-package))