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

[Elliott Mitchell]

On Sat, Nov 25, 2023 at 07:08:20AM +0100, Felix Fietkau wrote:
> On 25.11.23 03:28, Elliott Mitchell wrote:
> > On Fri, Nov 24, 2023 at 01:53:01PM +0100, Jonas Gorski wrote:
> >> > > 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.

> The duplication you wish to avoid is there for a reason. The way the 
> kernel build system is set up, it makes it easy to build a highly 
> customized build for one target, or make a distribution build with 
> pretty much everything built as module. What it offers no solution for 
> is to maintain and keep in sync kernel configurations for a wide array 
> of different (often storage constrained) targets, while keeping a lot of 
> extra features/drivers optionally installable.
> That's exactly what our packaging around kernel modules + our kernel 
> config handling scripts were made for.
> 
> Since there is no perfect solution, there are always some trade-offs 
> involved. One of these trade-offs was our choice to not support adding 
> kernel modules in the device kernel config by selecting them as =m 
> there. I didn't consider that feature useful enough to justify the cost 
> of dealing with all the corner cases.
> 
> I still don't fully understand your motivation for adding this feature. 
> Is it just because you're bothered by having to write some extra 
> boilerplate code for adding kernel modules? Are there some cases where 
> the existing system cannot work for what you're trying to do?
> Or is there some other reason why you need this?

Mostly I strongly dislike setting =m wasting build time and having near-
zero effect on the resultant output.  In particular I see a
build/configuration targetting VMs to likely want to build/include many
modules, yet many of those not really deserving their own packages (since
many would be unneeded in many cases).

> Just to be clear, I'm not opposed to the feature you're proposing in any 
> way. What I want to avoid is adding something that works for your 
> special case but quickly breaks in weird ways when other people start 
> using it.

That is quite reasonable.  I've pushed this later in my queue since I
concluded the aim of the later patches will make a more acceptable
approach possible.

Hopefully I'll be able to resend the series in a few days, just need
some build testing to ensure I didn't break anything.  The first two
patches are going to remain unchanged as there simply isn't anything
which can be done to them.


-- 
(\___(\___(\______          --=> 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