On Fri, Nov 24, 2023 at 01:53:01PM +0100, Jonas Gorski wrote: > > On Mon, 20 Nov 2023 at 02:37, Elliott Mitchell <ehem+open...@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+open...@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+sig...@m5p.com PGP 87145445 | ) / \_CS\ | _____ -O #include <stddisclaimer.h> O- _____ | / _/ 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445 _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
