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