Re: [PATCH v2 08/11] kbuild: create *.mod with full directory path and remove MODVERDIR

From: Masahiro Yamada <yamada.masahiro@socionext.com>
To: Joe Lawrence <joe.lawrence@redhat.com>
Cc: Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	Sam Ravnborg <sam@ravnborg.org>, Nicolas Pitre <nico@fluxnic.net>,
	"open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Michal Marek <michal.lkml@markovi.net>
Subject: Re: [PATCH v2 08/11] kbuild: create *.mod with full directory path and remove MODVERDIR
Date: Wed, 17 Jul 2019 14:21:45 +0900	[thread overview]
Message-ID: <CAK7LNAQ41NhPPO6xoVObgFctTO6WewSXPfZkE7_bZXsdAtKSpA@mail.gmail.com> (raw)
In-Reply-To: <20190716214023.GA15159@redhat.com>

Hi Joe

On Wed, Jul 17, 2019 at 6:40 AM Joe Lawrence <joe.lawrence@redhat.com> wrote:
>
> On Thu, Jul 11, 2019 at 02:44:31PM +0900, Masahiro Yamada wrote:
> > While descending directories, Kbuild produces objects for modules,
> > but do not link final *.ko files; it is done in the modpost.
> >
> > To keep track of modules, Kbuild creates a *.mod file in $(MODVERDIR)
> > for every module it is building. Some post-processing steps read the
> > necessary information from *.mod files. This avoids descending into
> > directories again. This mechanism was introduced in 2003 or so.
> >
> > Later, commit 551559e13af1 ("kbuild: implement modules.order") added
> > modules.order. So, we can simply read it out to know all the modules
> > with directory paths. This is easier than parsing the first line of
> > *.mod files.
> >
> > $(MODVERDIR) has a flat directory structure, that is, *.mod files
> > are named only with base names. This is based on the assumption that
> > the module name is unique across the tree. This assumption is really
> > fragile.
> >
> > Stephen Rothwell reported a race condition caused by a module name
> > conflict:
> >
> >   https://lkml.org/lkml/2019/5/13/991
> >
> > In parallel building, two different threads could write to the same
> > $(MODVERDIR)/*.mod simultaneously.
> >
> > Non-unique module names are the source of all kind of troubles, hence
> > commit 3a48a91901c5 ("kbuild: check uniqueness of module names")
> > introduced a new checker script.
> >
> > However, it is still fragile in the build system point of view because
> > this race happens before scripts/modules-check.sh is invoked. If it
> > happens again, the modpost will emit unclear error messages.
> >
> > To fix this issue completely, create *.mod in the same directory as
> > *.ko so that two threads never attempt to write to the same file.
> > $(MODVERDIR) is no longer needed.
> >
> > Since modules with directory paths are listed in modules.order, Kbuild
> > is still able to find *.mod files without additional descending.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > Acked-by: Nicolas Pitre <nico@fluxnic.net>

> >
>
> Hi Masahiro,
>
> I'm following this patchset changes as they will affect the klp-convert
> series [1] that the livepatching folks have been working on...

Empty files .tmp_versions/*.livepatch are touched
to keep track of 'LIVEPATCH_* := y', right?

Perhaps, adding a new field
to *.mod files might be cleaner.

> Just wondering if these other files should be checked for more MODVERDIR
> fallout:
>
>   % grep -R 'tmp_versions'
>   tools/power/cpupower/debug/kernel/Makefile:     - rm -rf .tmp_versions* Module.symvers modules.order
>   scripts/export_report.pl:    while (<.tmp_versions/*.mod>) {
>   scripts/adjust_autoksyms.sh:# .tmp_versions/*.mod files.
>
> export_report.pl is probably the only interesting one on this list.

Good catch. I will fix it.

> Also, can you cc me on subsequent patchset versions?

Yes, will do.

--
Best Regards
Masahiro Yamada