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

[Jonas Gorski]

Hi,

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.

> > 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.

> > 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.

WARNING: unpackaged kernel module found: foo/bar.ko

etc.

Regards,
Jonas