Re: [PATCH] libkmod: Always search modules.builtin if no alias has been found

[Lucas De Marchi <lucas.de.marchi@gmail.com>]

On Tue, May 11, 2021 at 9:58 AM Lucas De Marchi
<lucas.de.marchi@gmail.com> wrote:
>
> On Mon, May 10, 2021 at 6:55 AM Peter Kjellerstedt
> <peter.kjellerstedt@axis.com> wrote:
> >
> > > -----Original Message-----
> > > From: Lucas De Marchi <lucas.de.marchi@gmail.com>
> > > Sent: den 9 maj 2021 07:55
> > > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > > Cc: linux-modules <linux-modules@vger.kernel.org>; Peter Kjellerstedt
> > > <peter.kjellerstedt@axis.com>
> > > Subject: Re: [PATCH] libkmod: Always search modules.builtin if no alias
> > > has been found
> > >
> > > On Tue, May 4, 2021 at 5:17 PM Peter Kjellerstedt <pkj@axis.com> wrote:
> > > >
> > > > Commit 89443220e broke the lookup for builtin modules. modules.builtin
> > > > was no longer searched if kmod_lookup_alias_from_kernel_builtin_file()
> > > > returned 0.
> > > >
> > > > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > > > ---
> > > >
> > > > I do not know if this is the correct thing to do, or if the commit
> > > > message makes any sense. However, it solves the problem we were seeing.
> > > > We use fuse, which installs /etc/modules-load.d/fuse.conf to load the
> > > > fuse kernel module. However, we have fuse built-in. Normally, the
> > > > following can be seen in the log:
> > > >
> > > >   systemd-modules-load[192]: Module 'fuse' is built in
> > > >
> > > > but after commit 89443220e, we instead got:
> > > >
> > > >   systemd-modules-load[193]: Failed to find module 'fuse'
> > > >
> > > > //Peter
> > > >
> > > >  libkmod/libkmod-module.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
> > > > index 76a6dc3..6720930 100644
> > > > --- a/libkmod/libkmod-module.c
> > > > +++ b/libkmod/libkmod-module.c
> > > > @@ -577,7 +577,7 @@ KMOD_EXPORT int kmod_module_new_from_lookup(struct kmod_ctx *ctx,
> > > >
> > > >         DBG(ctx, "lookup modules.builtin.modinfo %s\n", alias);
> > > >         err = kmod_lookup_alias_from_kernel_builtin_file(ctx, alias, list);
> > > > -       if (err == -ENOSYS) {
> > > > +       if (err == 0 || err == -ENOSYS) {
> > >
> > > So in your case you do have modules.builtin.modinfo, but fuse doesn't
> > > show up there. On the other hand it is listed in modules.builtin.
> > > Does modules.builtin.info contain anything or is it an empty file?
> >
> > We have neither modules.builtin.modinfo nor modules.builtin.info.
> > A little googling turned out that modules.builtin.modinfo seems to have
> > been introduced in 5.2, but this product uses a 4.19 based kernel.
>
> ok, now I understood the entire context. So it seems the problem is
> not that we are
> missing the handling for return 0, but rather that
> kmod_lookup_alias_from_kernel_builtin_file()
> is not returning -ENOSYS when it should (index doesn't exist).  I
> thought this was covered, but
> obviously I was wrong. I will take a look what's going on.... we
> should not handle err == 0 the same
> way we handle err == -ENOSYS. If the index is missing we want to
> fallback to the old one, but if
> the index is there and we didn't find the module, we should just
> return an error.

Ok, I think I see what's happening. If you have a recent kmod with an
old kernel,
you have this scenario:

1) modules.builtin.modinfo doesn't exist since kernel didn't create it
2) depmod will write and empty index to modules.builtin.alias.bin when
modules.builtin.modinfo
is not present

I have a fix to depmod to stop writing an empty index. I will submit
that for review.

thanks
Lucas De Marchi


>
> Lucas De Marchi
>
> >
> > > It seems to me something else is broken:  all modules in
> > > modules.builtin should be in modules.builtin.modinfo as well. What is
> > > the result of the following commands?
> > >
> > > grep fuse /lib/modules/$(uname -r)/modules.builtin
> >
> > kernel/fs/fuse/fuse.ko
> >
> > > grep fuse /lib/modules/$(uname -r)/modules.builtin.modinfo
> >
> > grep: /lib/modules/4.19.110-axis8/modules.builtin.modinfo: No such file or directory
> >
> > This is an embedded product built with our own distribution based on
> > Poky Gatesgarth from the Yocto Project. The rootfs is read-only, including
> > /lib/modules, so any contest there is created when the firmware image is
> > built.
> >
> > > thanks
> > > Lucas De Marchi
> > >
> > > >                 /* Optional index missing, try the old one */
> > > >                 DBG(ctx, "lookup modules.builtin %s\n", alias);
> > > >                 err = kmod_lookup_alias_from_builtin_file(ctx, alias, list);
> >
> > //Peter
> >