* [PATCH 1/2] kbuild: improve cc-option to clean up all temporary files @ 2020-06-14 14:43 Masahiro Yamada 2020-06-14 14:43 ` [PATCH 2/2] kconfig: unify cc-option and as-option Masahiro Yamada 0 siblings, 1 reply; 8+ messages in thread From: Masahiro Yamada @ 2020-06-14 14:43 UTC (permalink / raw) To: linux-kbuild Cc: Nick Desaulniers, Arvind Sankar, Masahiro Yamada, Michal Marek, linux-kernel When cc-option and friends evaluate compiler flags, the temporary file $$TMP is created as an output object, and automatically cleaned up. The actual file path of $$TMP is .<pid>.tmp, here <pid> is the process ID of $(shell ....) invoked from cc-option. (Please note $$$$ is the escape sequence of $$). Such garbage files are cleaned up in most cases, but some compiler flags create additional output files. For example, -gsplit-dwarf creates a .dwo file. When CONFIG_DEBUG_INFO_SPLIT=y, you will see a bunch of .<pid>.dwo files left in the top of build directories. You may not notice them unless you do 'ls -a', but the garbage files will increase every time you run 'make'. This commit changes the temporary object path to .tmp_<pid>/tmp, and removes .tmp_<pid> directory when exiting. The additional files .tmp_<pid>/tmp.dwo, will be cleaned away altogether. When the compiler creates separate build artifacts, their file paths are usually determined based on the base name of the object. Another example is -ftest-coverage, which outputs the coverage data into <base-name-of-object>.gcno Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/Kbuild.include | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index 0c3dc983439b..5dfd224599b7 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -86,20 +86,21 @@ cc-cross-prefix = $(firstword $(foreach c, $(1), \ $(if $(shell command -v -- $(c)gcc 2>/dev/null), $(c)))) # output directory for tests below -TMPOUT := $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/) +TMPOUT = $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/).tmp_$$$$ # try-run # Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise) -# Exit code chooses option. "$$TMP" serves as a temporary file and is +# Exit code chooses option. "$$TMP" serves as a temporary directory and is # automatically cleaned up. try-run = $(shell set -e; \ - TMP="$(TMPOUT).$$$$.tmp"; \ - TMPO="$(TMPOUT).$$$$.o"; \ + TMP=$(TMPOUT)/tmp; \ + TMPO=$(TMPOUT)/tmp.o; \ + mkdir -p $(TMPOUT); \ + trap "rm -rf $(TMPOUT)" EXIT; \ if ($(1)) >/dev/null 2>&1; \ then echo "$(2)"; \ else echo "$(3)"; \ - fi; \ - rm -f "$$TMP" "$$TMPO") + fi) # as-option # Usage: cflags-y += $(call as-option,-Wa$(comma)-isa=foo,) -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] kconfig: unify cc-option and as-option 2020-06-14 14:43 [PATCH 1/2] kbuild: improve cc-option to clean up all temporary files Masahiro Yamada @ 2020-06-14 14:43 ` Masahiro Yamada 2020-06-15 10:00 ` Will Deacon 2021-05-04 20:17 ` Johannes Berg 0 siblings, 2 replies; 8+ messages in thread From: Masahiro Yamada @ 2020-06-14 14:43 UTC (permalink / raw) To: linux-kbuild Cc: Nick Desaulniers, Arvind Sankar, Masahiro Yamada, Andrew Morton, Brendan Higgins, Catalin Marinas, Changbin Du, Krzysztof Kozlowski, Mauro Carvalho Chehab, Randy Dunlap, Will Deacon, linux-arm-kernel, linux-kernel cc-option and as-option are almost the same; both pass the flag to $(CC). The main difference is the cc-option stops before the assemble stage (-S option) whereas as-option stops after it (-c option). I chose -S because it is slightly faster, but $(cc-option,-gz=zlib) returns a wrong result (https://lkml.org/lkml/2020/6/9/1529). It has been fixed by a separate patch, but using -c is more robust. However, you cannot simply replace -S with -c because the following code would break: depends on $(cc-option,-gsplit-dwarf) The combination of -c and -gsplit-dwarf does not accept /dev/null as output. $ cat /dev/null | gcc -gsplit-dwarf -S -x c - -o /dev/null $ echo $? 0 $ cat /dev/null | gcc -gsplit-dwarf -c -x c - -o /dev/null objcopy: Warning: '/dev/null' is not an ordinary file $ echo $? 1 $ cat /dev/null | gcc -gsplit-dwarf -c -x c - -o tmp.o $ echo $? 0 There is another flag that creates an separate file based on the object file path: $ cat /dev/null | gcc -ftest-coverage -c -x c - -o /dev/null <stdin>:1: error: cannot open /dev/null.gcno So, we cannot use /dev/null to sink the output. Align the cc-option implementation with scripts/Kbuild.include. With -c option used in cc-option, as-option is unneeded. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- arch/arm64/Kconfig | 2 +- lib/Kconfig.debug | 1 - scripts/Kconfig.include | 8 +------- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 31380da53689..6eb18f45258e 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1564,7 +1564,7 @@ config CC_HAS_SIGN_RETURN_ADDRESS def_bool $(cc-option,-msign-return-address=all) config AS_HAS_PAC - def_bool $(as-option,-Wa$(comma)-march=armv8.3-a) + def_bool $(cc-option,-Wa$(comma)-march=armv8.3-a) config AS_HAS_CFI_NEGATE_RA_STATE def_bool $(as-instr,.cfi_startproc\n.cfi_negate_ra_state\n.cfi_endproc\n) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 96999d4d2dda..9ad9210d70a1 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -229,7 +229,6 @@ config DEBUG_INFO_COMPRESSED bool "Compressed debugging information" depends on DEBUG_INFO depends on $(cc-option,-gz=zlib) - depends on $(as-option,-gz=zlib) depends on $(ld-option,--compress-debug-sections=zlib) help Compress the debug information using zlib. Requires GCC 5.0+ or Clang diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include index c264da2b9b30..a5fe72c504ff 100644 --- a/scripts/Kconfig.include +++ b/scripts/Kconfig.include @@ -25,18 +25,12 @@ failure = $(if-success,$(1),n,y) # $(cc-option,<flag>) # Return y if the compiler supports <flag>, n otherwise -cc-option = $(success,$(CC) -Werror $(CLANG_FLAGS) $(1) -S -x c /dev/null -o /dev/null) +cc-option = $(success,mkdir .tmp_$$$$; trap "rm -rf .tmp_$$$$" EXIT; $(CC) -Werror $(CLANG_FLAGS) $(1) -c -x c /dev/null -o .tmp_$$$$/tmp.o) # $(ld-option,<flag>) # Return y if the linker supports <flag>, n otherwise ld-option = $(success,$(LD) -v $(1)) -# $(as-option,<flag>) -# /dev/zero is used as output instead of /dev/null as some assembler cribs when -# both input and output are same. Also both of them have same write behaviour so -# can be easily substituted. -as-option = $(success, $(CC) $(CLANG_FLAGS) $(1) -c -x assembler /dev/null -o /dev/zero) - # $(as-instr,<instr>) # Return y if the assembler supports <instr>, n otherwise as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) -c -x assembler -o /dev/null -) -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] kconfig: unify cc-option and as-option 2020-06-14 14:43 ` [PATCH 2/2] kconfig: unify cc-option and as-option Masahiro Yamada @ 2020-06-15 10:00 ` Will Deacon 2021-05-04 20:17 ` Johannes Berg 1 sibling, 0 replies; 8+ messages in thread From: Will Deacon @ 2020-06-15 10:00 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, Nick Desaulniers, Arvind Sankar, Andrew Morton, Brendan Higgins, Catalin Marinas, Changbin Du, Krzysztof Kozlowski, Mauro Carvalho Chehab, Randy Dunlap, linux-arm-kernel, linux-kernel On Sun, Jun 14, 2020 at 11:43:41PM +0900, Masahiro Yamada wrote: > cc-option and as-option are almost the same; both pass the flag to > $(CC). The main difference is the cc-option stops before the assemble > stage (-S option) whereas as-option stops after it (-c option). > > I chose -S because it is slightly faster, but $(cc-option,-gz=zlib) > returns a wrong result (https://lkml.org/lkml/2020/6/9/1529). > It has been fixed by a separate patch, but using -c is more robust. > > However, you cannot simply replace -S with -c because the following > code would break: > > depends on $(cc-option,-gsplit-dwarf) > > The combination of -c and -gsplit-dwarf does not accept /dev/null as > output. > > $ cat /dev/null | gcc -gsplit-dwarf -S -x c - -o /dev/null > $ echo $? > 0 > > $ cat /dev/null | gcc -gsplit-dwarf -c -x c - -o /dev/null > objcopy: Warning: '/dev/null' is not an ordinary file > $ echo $? > 1 > > $ cat /dev/null | gcc -gsplit-dwarf -c -x c - -o tmp.o > $ echo $? > 0 > > There is another flag that creates an separate file based on the > object file path: > > $ cat /dev/null | gcc -ftest-coverage -c -x c - -o /dev/null > <stdin>:1: error: cannot open /dev/null.gcno > > So, we cannot use /dev/null to sink the output. > > Align the cc-option implementation with scripts/Kbuild.include. > > With -c option used in cc-option, as-option is unneeded. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > arch/arm64/Kconfig | 2 +- > lib/Kconfig.debug | 1 - > scripts/Kconfig.include | 8 +------- > 3 files changed, 2 insertions(+), 9 deletions(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 31380da53689..6eb18f45258e 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -1564,7 +1564,7 @@ config CC_HAS_SIGN_RETURN_ADDRESS > def_bool $(cc-option,-msign-return-address=all) > > config AS_HAS_PAC > - def_bool $(as-option,-Wa$(comma)-march=armv8.3-a) > + def_bool $(cc-option,-Wa$(comma)-march=armv8.3-a) For this arm64 part: Acked-by: Will Deacon <will@kernel.org> Will ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] kconfig: unify cc-option and as-option 2020-06-14 14:43 ` [PATCH 2/2] kconfig: unify cc-option and as-option Masahiro Yamada 2020-06-15 10:00 ` Will Deacon @ 2021-05-04 20:17 ` Johannes Berg 2021-05-04 20:46 ` Masahiro Yamada 1 sibling, 1 reply; 8+ messages in thread From: Johannes Berg @ 2021-05-04 20:17 UTC (permalink / raw) To: Masahiro Yamada, linux-kbuild Cc: Nick Desaulniers, Arvind Sankar, Andrew Morton, Brendan Higgins, Catalin Marinas, Changbin Du, Krzysztof Kozlowski, Mauro Carvalho Chehab, Randy Dunlap, Will Deacon, linux-arm-kernel, linux-kernel Hi, So... I realized it's been a while: On Sun, 2020-06-14 at 23:43 +0900, Masahiro Yamada wrote: > cc-option and as-option are almost the same; both pass the flag to > $(CC). The main difference is the cc-option stops before the assemble > stage (-S option) whereas as-option stops after it (-c option). > But, I had noticed for a while now that M= build for an out-of-tree driver were causing some trouble. Not really completely "out-of-tree" but rather backported (https://backports.wiki.kernel.org/). And then I finally narrowed it down to this commit, specifically this: > # Return y if the compiler supports <flag>, n otherwise > -cc-option = $(success,$(CC) -Werror $(CLANG_FLAGS) $(1) -S -x c /dev/null -o /dev/null) > +cc-option = $(success,mkdir .tmp_$$$$; trap "rm -rf .tmp_$$$$" EXIT; $(CC) -Werror $(CLANG_FLAGS) $(1) -c -x c /dev/null -o .tmp_$$$$/tmp.o) What happens is that we're doing make -C /path/to/kernel M=/path/to/driver But /path/to/kernel may be the installed distro kernel headers, and thus not be writable to the user doing the driver compile. Obviously, the user may need to 'sudo' anyway to install the result, but if just test- compiling, or even as better practice to not run everything as root, this ".tmp_$$" dir cannot be created. IOW, this broke compiler option detection when KBUILD_EXTMOD=/M= is used. It seems this is still supported (documented in kbuild docs), so I'm kind of hoping it could be fixed? But OTOH, I really don't know how, perhaps just using "mktemp -d" here instead of the hardcoded temp dir? Thanks, johannes ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] kconfig: unify cc-option and as-option 2021-05-04 20:17 ` Johannes Berg @ 2021-05-04 20:46 ` Masahiro Yamada 2021-05-04 20:52 ` Johannes Berg 0 siblings, 1 reply; 8+ messages in thread From: Masahiro Yamada @ 2021-05-04 20:46 UTC (permalink / raw) To: Johannes Berg Cc: Linux Kbuild mailing list, Nick Desaulniers, Arvind Sankar, Andrew Morton, Brendan Higgins, Catalin Marinas, Changbin Du, Krzysztof Kozlowski, Mauro Carvalho Chehab, Randy Dunlap, Will Deacon, linux-arm-kernel, Linux Kernel Mailing List On Wed, May 5, 2021 at 5:17 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > Hi, > > So... I realized it's been a while: > > On Sun, 2020-06-14 at 23:43 +0900, Masahiro Yamada wrote: > > cc-option and as-option are almost the same; both pass the flag to > > $(CC). The main difference is the cc-option stops before the assemble > > stage (-S option) whereas as-option stops after it (-c option). > > > > But, I had noticed for a while now that M= build for an out-of-tree > driver were causing some trouble. Not really completely "out-of-tree" > but rather backported (https://backports.wiki.kernel.org/). > > And then I finally narrowed it down to this commit, specifically this: > > > # Return y if the compiler supports <flag>, n otherwise > > -cc-option = $(success,$(CC) -Werror $(CLANG_FLAGS) $(1) -S -x c /dev/null -o /dev/null) > > +cc-option = $(success,mkdir .tmp_$$$$; trap "rm -rf .tmp_$$$$" EXIT; $(CC) -Werror $(CLANG_FLAGS) $(1) -c -x c /dev/null -o .tmp_$$$$/tmp.o) > > What happens is that we're doing > > make -C /path/to/kernel M=/path/to/driver > > But /path/to/kernel may be the installed distro kernel headers, and thus > not be writable to the user doing the driver compile. Obviously, the > user may need to 'sudo' anyway to install the result, but if just test- > compiling, or even as better practice to not run everything as root, > this ".tmp_$$" dir cannot be created. > > IOW, this broke compiler option detection when KBUILD_EXTMOD=/M= is > used. It seems this is still supported (documented in kbuild docs), so > I'm kind of hoping it could be fixed? But OTOH, I really don't know how, > perhaps just using "mktemp -d" here instead of the hardcoded temp dir? > > Thanks, > johannes > - This commit touches scripts/Kconfig.include. - External module builds (M= builds) never invoke Kconfig Putting these two together, your claim is really odd. If external module builds invoke Kconfig, your kernel is already broken. If you claim this is an issue, please describe how to reproduce it in *upstream* kernel. I do not know (or care about) your backport kernel. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] kconfig: unify cc-option and as-option 2021-05-04 20:46 ` Masahiro Yamada @ 2021-05-04 20:52 ` Johannes Berg 2021-05-04 21:05 ` Masahiro Yamada 0 siblings, 1 reply; 8+ messages in thread From: Johannes Berg @ 2021-05-04 20:52 UTC (permalink / raw) To: Masahiro Yamada Cc: Linux Kbuild mailing list, Nick Desaulniers, Arvind Sankar, Andrew Morton, Brendan Higgins, Catalin Marinas, Changbin Du, Krzysztof Kozlowski, Mauro Carvalho Chehab, Randy Dunlap, Will Deacon, linux-arm-kernel, Linux Kernel Mailing List On Wed, 2021-05-05 at 05:46 +0900, Masahiro Yamada wrote: > > - This commit touches scripts/Kconfig.include. > - External module builds (M= builds) never invoke Kconfig > > Putting these two together, your claim is really odd. Hmm. > If external module builds invoke Kconfig, > your kernel is already broken. Well, it's not about the kernel, that's just the normal upstream (or perhaps distribution) kernel. Anyway, you're right, it's much simpler. The problem isn't invoking M= or something like that, that happens much later and sent me on the completely wrong track. The problem is simply doing make kernelversion to determine the version of a tree that's not writable to the user, e.g. make -C /lib/modules/$(uname -r)/build/ kernelversion Which basically also means that it's harmless, since the version is of course not affected by cc-option. johannes ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] kconfig: unify cc-option and as-option 2021-05-04 20:52 ` Johannes Berg @ 2021-05-04 21:05 ` Masahiro Yamada 2021-05-04 21:08 ` Johannes Berg 0 siblings, 1 reply; 8+ messages in thread From: Masahiro Yamada @ 2021-05-04 21:05 UTC (permalink / raw) To: Johannes Berg Cc: Linux Kbuild mailing list, Nick Desaulniers, Arvind Sankar, Andrew Morton, Brendan Higgins, Catalin Marinas, Changbin Du, Krzysztof Kozlowski, Mauro Carvalho Chehab, Randy Dunlap, Will Deacon, linux-arm-kernel, Linux Kernel Mailing List On Wed, May 5, 2021 at 5:52 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > On Wed, 2021-05-05 at 05:46 +0900, Masahiro Yamada wrote: > > > > - This commit touches scripts/Kconfig.include. > > - External module builds (M= builds) never invoke Kconfig > > > > Putting these two together, your claim is really odd. > > Hmm. > > > If external module builds invoke Kconfig, > > your kernel is already broken. > > Well, it's not about the kernel, that's just the normal upstream (or > perhaps distribution) kernel. > > Anyway, you're right, it's much simpler. The problem isn't invoking M= > or something like that, that happens much later and sent me on the > completely wrong track. > > The problem is simply doing > > make kernelversion > > to determine the version of a tree that's not writable to the user, e.g. > > make -C /lib/modules/$(uname -r)/build/ kernelversion > > Which basically also means that it's harmless, since the version is of > course not affected by cc-option. It was fixed. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=805b2e1d427aab4bb27fa7c51ebb9db7547551b1 If you want to make it work without that commit, make -C /lib/modules/$(uname -r)/build/ kernelversion M=/tmp will work. Pass a writable directory to M=. M= build never touches the kernel source tree. > johannes > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] kconfig: unify cc-option and as-option 2021-05-04 21:05 ` Masahiro Yamada @ 2021-05-04 21:08 ` Johannes Berg 0 siblings, 0 replies; 8+ messages in thread From: Johannes Berg @ 2021-05-04 21:08 UTC (permalink / raw) To: Masahiro Yamada Cc: Linux Kbuild mailing list, Nick Desaulniers, Arvind Sankar, Andrew Morton, Brendan Higgins, Catalin Marinas, Changbin Du, Krzysztof Kozlowski, Mauro Carvalho Chehab, Randy Dunlap, Will Deacon, linux-arm-kernel, Linux Kernel Mailing List On Wed, 2021-05-05 at 06:05 +0900, Masahiro Yamada wrote: > > It was fixed. > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=805b2e1d427aab4bb27fa7c51ebb9db7547551b1 > Ah, ok, great. > > > If you want to make it work without that commit, > > make -C /lib/modules/$(uname -r)/build/ kernelversion M=/tmp > > will work. I guess that works. I was just going to ignore the errors now that I realized what it was :) Thanks! johannes ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-05-04 21:08 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-14 14:43 [PATCH 1/2] kbuild: improve cc-option to clean up all temporary files Masahiro Yamada 2020-06-14 14:43 ` [PATCH 2/2] kconfig: unify cc-option and as-option Masahiro Yamada 2020-06-15 10:00 ` Will Deacon 2021-05-04 20:17 ` Johannes Berg 2021-05-04 20:46 ` Masahiro Yamada 2021-05-04 20:52 ` Johannes Berg 2021-05-04 21:05 ` Masahiro Yamada 2021-05-04 21:08 ` Johannes Berg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).