diff mbox series

[v2,1/2] kbuild: move scripts/mod/ build to modules_prepare

Message ID 20220514040207.282910-1-masahiroy@kernel.org (mailing list archive)
State New
Headers show
Series [v2,1/2] kbuild: move scripts/mod/ build to modules_prepare | expand

Commit Message

Masahiro Yamada May 14, 2022, 4:02 a.m. UTC
Do $(build)=scripts/mod after $(build)=. so that
scripts/mod/devicetable-offsets.c can use headers generated by ./Kbuild.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

Changes for v2:
  - New patch to fix errors reported by 0-day bot


 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

kernel test robot May 16, 2022, 6:02 a.m. UTC | #1
Hi Masahiro,

I love your patch! Yet something to improve:

[auto build test ERROR on masahiroy-kbuild/for-next]
[also build test ERROR on linux/master linus/master v5.18-rc7 next-20220513]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Masahiro-Yamada/kbuild-move-scripts-mod-build-to-modules_prepare/20220514-120519
base:   https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
config: s390-randconfig-m031-20220514 (https://download.01.org/0day-ci/archive/20220516/202205161340.DscM2Rht-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/e538a965b373cb35813fd55f75b7646c4fb92a8d
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Masahiro-Yamada/kbuild-move-scripts-mod-build-to-modules_prepare/20220514-120519
        git checkout e538a965b373cb35813fd55f75b7646c4fb92a8d
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> /bin/bash: line 1: scripts/mod/modpost: No such file or directory
Masahiro Yamada May 16, 2022, 9:30 a.m. UTC | #2
On Mon, May 16, 2022 at 5:43 PM Mark Rutland <mark.rutland@arm.com> wrote:
>
> [adding the atomics maintainers to Cc]
>
> On Sat, May 14, 2022 at 01:02:07PM +0900, Masahiro Yamada wrote:
> > include/linux/atomic/*.h are generated by scripts/atomic/gen-atomics.sh.
> >
> > To ensure they are not edited, scripts/atomic/check-atomics.sh checks
> > sha1sum every time.
> >
> > This commit moves include/linux/atomic/*.h to scripts/atomic/*.h_shipped,
> > which are copied to include/generated/ at build time:
> >
> >   COPY    include/generated/atomic-instrumented.h
> >   COPY    include/generated/atomic-long.h
> >   COPY    include/generated/atomic-arch-fallback.h
>
> FWIW, moving these and copying them at build time is fine by me, given that
> better indicates that these are generated.
>
> > I dropped the sha1sum checks. I hope nobody would try to directly modify
> > *_shipped files.
>
> I'd prefer to keep the sha1sum check, because we're worried that people *will*
> modify them, and that won't be noticed until the next time they're regenerated.

OK, but is there any reason to embed the checksum to the headers?

Generally, you can have *.sha1 file to collect the checksums,
and use the "sha1sum --check" command.

Maybe, scripts/atomic/check-atomics.sh was unneeded.


$ sha1sum include/linux/atomic/*.h   >  scripts/atomic/atomics.sha1
$ cat scripts/atomic/atomics.sha1
c0f1a9e951f38ccfa146ca2431f9e1611191a402
include/linux/atomic/atomic-arch-fallback.h
97ce73d2c176725d199a810eb81c574022ffa899
include/linux/atomic/atomic-instrumented.h
b0a5ee2e9497a41795644fa115df184d6331b9c2  include/linux/atomic/atomic-long.h
$ sha1sum --check   scripts/atomic/atomics.sha1
include/linux/atomic/atomic-arch-fallback.h: OK
include/linux/atomic/atomic-instrumented.h: OK
include/linux/atomic/atomic-long.h: OK


It is possible to do "sha1sum --check && cp".



> > Kbuild runs more and more tools at build time these days because they
> > are fast enough on modern systems.
> >
> > For example,
> >
> >  - 29c833061c1d8c2d1d23a62e7061561eadd76cdb
> >    ("kconfig: generate lexer and parser during build instead of shipping")
> >
> >  - 7c0303ff7e67b637c47d8afee533ca9e2a02359b
> >    ("crypto: arm - generate *.S by Perl at build time instead of shipping them")
> >
> > Yet, gen-atomics.sh is too slow.
>
> Yes; we'd originally wanted to run them at build time, but that was too slow,
> and as a compromise we moved to regenerating them whenever we changed the
> scripting.

I remember it.

https://lore.kernel.org/lkml/20181123153321.8561-1-mark.rutland@arm.com/


>
> > I guess it can be improved because it is just simple text processing.
> > Then, Kbuild can execute it at build time.
>
> It is in theory possible to make them much faster, yes. The actual slowness of
> the scripting is largely due to using sub-shells resulting in a load of
> fork+exec, which could be avoided if using a real scripting language.
>
> Practically speaking though, we're rather limited in what we can rely upon
> being available. We chose to use POSIX shell as a lowest-common-demoninator,
> and anything that'd be *nice* to use isn't going to always be available,
> meaning that even if we make it faster we can't necessarily build it all the
> time anyway.


Kernel builds already rely on Perl.

The base building of the kernel does not depend on Python,
but some targets such as "make clang-tidy", "make clang-analyzer"
need Python3.


>
> Thanks,
> Mark.


BTW, full-quoting to this thread is not a good idea.
I cut down the code diff.
Mark Rutland May 19, 2022, 9:16 a.m. UTC | #3
On Mon, May 16, 2022 at 06:30:41PM +0900, Masahiro Yamada wrote:
> On Mon, May 16, 2022 at 5:43 PM Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > [adding the atomics maintainers to Cc]
> >
> > On Sat, May 14, 2022 at 01:02:07PM +0900, Masahiro Yamada wrote:
> > > include/linux/atomic/*.h are generated by scripts/atomic/gen-atomics.sh.
> > >
> > > To ensure they are not edited, scripts/atomic/check-atomics.sh checks
> > > sha1sum every time.
> > >
> > > This commit moves include/linux/atomic/*.h to scripts/atomic/*.h_shipped,
> > > which are copied to include/generated/ at build time:
> > >
> > >   COPY    include/generated/atomic-instrumented.h
> > >   COPY    include/generated/atomic-long.h
> > >   COPY    include/generated/atomic-arch-fallback.h
> >
> > FWIW, moving these and copying them at build time is fine by me, given that
> > better indicates that these are generated.
> >
> > > I dropped the sha1sum checks. I hope nobody would try to directly modify
> > > *_shipped files.
> >
> > I'd prefer to keep the sha1sum check, because we're worried that people *will*
> > modify them, and that won't be noticed until the next time they're regenerated.
> 
> OK, but is there any reason to embed the checksum to the headers?
> 
> Generally, you can have *.sha1 file to collect the checksums,
> and use the "sha1sum --check" command.

TBH, I have no preference either way; splitting it out is fine by me.

The only thing that needs to be ensured is that if the sha1sum tool doesn't
exist we don't complain about that.

[...]

> > > I guess it can be improved because it is just simple text processing.
> > > Then, Kbuild can execute it at build time.
> >
> > It is in theory possible to make them much faster, yes. The actual slowness of
> > the scripting is largely due to using sub-shells resulting in a load of
> > fork+exec, which could be avoided if using a real scripting language.
> >
> > Practically speaking though, we're rather limited in what we can rely upon
> > being available. We chose to use POSIX shell as a lowest-common-demoninator,
> > and anything that'd be *nice* to use isn't going to always be available,
> > meaning that even if we make it faster we can't necessarily build it all the
> > time anyway.
> 
> Kernel builds already rely on Perl.

I didn't realise that we always relied upon perl -- I know that there's
recordmcount.pl, but I didn't think all targets used that, and I thought we
couldn't rely upon perl when we originally wrote the atomci scripting.

If we can rely upom perl being available then I'm not averse to writing the
scripting in perl; I'm just not immediately sure about how to handle the
fallback templates.

Thanks,
Mark.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index f90fcc1a7ff4..baa3440aea7f 100644
--- a/Makefile
+++ b/Makefile
@@ -1190,7 +1190,6 @@  archprepare: outputmakefile archheaders archscripts scripts include/config/kerne
 	include/generated/autoconf.h remove-stale-files
 
 prepare0: archprepare
-	$(Q)$(MAKE) $(build)=scripts/mod
 	$(Q)$(MAKE) $(build)=.
 
 # All the preparing..
@@ -1441,6 +1440,7 @@  targets += modules.order
 PHONY += modules_prepare
 modules_prepare: prepare
 	$(Q)$(MAKE) $(build)=scripts scripts/module.lds
+	$(Q)$(MAKE) $(build)=scripts/mod
 
 export modules_sign_only :=