[PATCH 07/20] kernel: copy all built kernel modules to root filesystem image

[Elliott Mitchell]

On Fri, Nov 24, 2023 at 01:53:01PM +0100, Jonas Gorski wrote:
> 
> On Mon, 20 Nov 2023 at 02:37, Elliott Mitchell <ehem+openwrt at m5p.com> wrote:
> >
> > On Sat, Nov 18, 2023 at 07:57:35AM +0100, Felix Fietkau wrote:
> > > On 17.11.23 22:31, Elliott Mitchell wrote:
> > > > On Fri, Nov 17, 2023 at 05:20:33PM +0100, Felix Fietkau wrote:
> > > >> On 11.11.23 01:21, Elliott Mitchell wrote:
> > > >> > This removes the requirement for to create a package for all modules.
> > > >> > Now devices can simply specify in-tree drivers/other to be built as
> > > >> > modules and they will be present in the resultant image.
> > > >> >
> > > >> > Signed-off-by: Elliott Mitchell <ehem+openwrt at m5p.com>
> > > >>
> > > >> It seems to me that this completely ignores the use case of having
> > > >> release builds that ship a lot of kernel modules as installable
> > > >> packages. Did I misread something?
> > > >> If not, this gets a strong NACK from me.
> > > >
> > > > Should be completely orthogonal, though it could have bugs.
> > > >
> > > > Using `cp -l` has two valuable effects:  First, it reduces storage space
> > > > usage.  Second, it serves to mark module files as belonging to a package.
> > > >
> > > > My goal is previously setting a kernel option to "m" in a configuration
> > > > file, but not having a package causes it to be built, then ignored.  I
> > > > want this to do something sensible, not simply waste electricity
> > > > building a module and then tossing it in the garbage.
> > > >
> > > > Hmm, come to think of it, that should be $(XARGS) (fix on commit?).
> > >
> > > Thanks for the explanation, it makes more sense to me now.
> > > That said, I see a few pitfalls here:
> > >
> > > 1. If you select kernel modules that depend on other modules selected
> > > via kmod packages, you end up with non-functional modules with missing
> > > dependencies in the rootfs.
> >
> > Is this actually that much of a problem?  From what I've seen most kmod
> > packages handled during image build get preinstalled onto the root fs
> > image.  As such these would nominally function as long as the packages
> > weren't removed.
> 
> If you do a build with ALL_KMODS=y because you don't know which kmods
> you may need later on, and want to avoid having to reflash in this
> case, there will be plenty of kmod packages build but not installed
> into the rootfs.
> 
> >
> > Wouldn't this also indicate breakage in the module package anyway?
> 
> No, not necessarily. Let's say there is a kmod-foo that packages FOO
> (foo.ko). This is selected as =m.
> 
> Then you run kernel_*config and select BAR=m (bar.ko), since there is
> no kmod defined for it. bar.ko has a dependency on foo.ko
> 
> With your patch (AFAIU) the bar.ko would be then installed in the
> rootfs, but since foo.ko is packaged separately as a m-package, it
> won't be in the rootfs => bar.ko is missing its dependencies in the
> rootfs.

Yet isn't this breakage in the module package?  It has stolen a module
away from the kernel configuration.

Certainly this is a problem and I've got a rough idea how to solve it.
(simply take advantage of what the patch series is aiming for)

> > > 2. If the kmod package selection accidentally ends up selecting extra
> > > modules that aren't stored in the package, you end up with rootfs bloat.
> >
> > Eww, that would be gross.  As with the above, wouldn't this be indicating
> > breakage in the module packaging anyway?
> 
> Maybe, but sometimes modules/drivers in the kernel default to =m for
> silly reasons. And again as the above example, if the triggering kmod
> package is selected as =m, then the additional modules land in the
> rootfs without their dependencies.

The inverse is also a problem.  If the kernel selected CONFIG_USB=m,
then kmod-chaoskey was built, it would be impossible to load the module
since usbcore.ko would be unavailable.

Both directions currently have breakage.

> > > I'm fine with the cp -l change, but I think adding all remaining modules
> > > to the rootfs is not something we should do by default (maybe opt-in?)
> >
> > Perhaps.  This could also be handled by the approach the series as a
> > whole is aiming for.  If kernel module building used a separate object
> > directory from the kernel build, then modules selected in the device
> > configuration could be isolated.
> 
> Maybe instead of putting them into the rootfs, we could wrap them in a
> special package? Then you can select it if you want to include them in
> your image or not. No idea what to name it, kmod-remaining?
> kmod-unaccounted-for? kmod-not-appearing-in-the-definitions?
> 
> We could also try to wrap any unexpected .kos into autogenerated kmod
> packages based in their names (e.g. if we find a foo.ko, we
> autogenerate a kmod-foo.ipk for it), but these wouldn't be selectable
> then in menuconfig. Also not sure how well the build system would
> handle dynamic package generation.
> 
> Also going the other way around, maybe the build system should
> warn/complain about any .ko found that isn't wrapped in a kmod-*
> package.

All of these are plausible.  I think modules selected in device
configurations should get built and installed into the root filesystem.
Otherwise setting a device kernel configuration option to =m is broken.

Whereas forcing the explicit creation of packages for each and every
kernel module forces duplication of Kconfig functionality.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg at m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445