linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Kbuild: lto: add make version checking for MODVERSIONS
@ 2021-07-02  3:29 Lecopzer Chen
  2021-07-02  3:29 ` [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION Lecopzer Chen
  2021-07-02  3:29 ` [PATCH v3 2/2] Kbuild: lto: add make version checking Lecopzer Chen
  0 siblings, 2 replies; 11+ messages in thread
From: Lecopzer Chen @ 2021-07-02  3:29 UTC (permalink / raw)
  To: ndesaulniers, keescook, samitolvanen, linux-kbuild, nathan
  Cc: clang-built-linux, linux-kernel, yj.chiang, masahiroy,
	michal.lkml, Lecopzer Chen

LTO with MODVERSIONS will fail in generating correct CRC because
the makefile rule doesn't work for make with version 3.8X.

Refer to [1]:
> When building modules(CONFIG_...=m), I found some of module versions
> are incorrect and set to 0.
> This can be found in build log for first clean build which shows

> WARNING: EXPORT symbol "XXXX" [drivers/XXX/XXX.ko] version generation failed,
> symbol will not be versioned.

> But in second build(incremental build), the WARNING disappeared and the
> module version becomes valid CRC and make someone who want to change
> modules without updating kernel image can't insert their modules.

> The problematic code is
> + $(foreach n, $(filter-out FORCE,$^),        \
> +   $(if $(wildcard $(n).symversions),      \
> +     ; cat $(n).symversions >> $@.symversions))

The issue is fixed when make version upgrading to 4.2.

Thus we need to check make version during selecting on LTO Kconfig.
Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits
for arithmetic comparisons.

[1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>


----
v3:
- fix comment in make-version.sh

v2:
https://lore.kernel.org/lkml/20210701092841.1419-1-lecopzer.chen@mediatek.com/
- change MAKE_VERSION_INT to MAKE_VERSION
- remove $(make-version)
- tweak commit message

v1:
https://lore.kernel.org/lkml/20210630121436.19581-1-lecopzer.chen@mediatek.com/
----


Lecopzer Chen (2):
  Kbuild: lto: add CONFIG_MAKE_VERSION
  Kbuild: lto: add make version checking

 Makefile                |  2 +-
 arch/Kconfig            |  1 +
 init/Kconfig            |  4 ++++
 scripts/make-version.sh | 13 +++++++++++++
 4 files changed, 19 insertions(+), 1 deletion(-)
 create mode 100755 scripts/make-version.sh

-- 
2.18.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION
  2021-07-02  3:29 [PATCH v3 0/2] Kbuild: lto: add make version checking for MODVERSIONS Lecopzer Chen
@ 2021-07-02  3:29 ` Lecopzer Chen
  2021-07-04  0:15   ` Nathan Chancellor
  2021-07-05  2:02   ` Masahiro Yamada
  2021-07-02  3:29 ` [PATCH v3 2/2] Kbuild: lto: add make version checking Lecopzer Chen
  1 sibling, 2 replies; 11+ messages in thread
From: Lecopzer Chen @ 2021-07-02  3:29 UTC (permalink / raw)
  To: ndesaulniers, keescook, samitolvanen, linux-kbuild, nathan
  Cc: clang-built-linux, linux-kernel, yj.chiang, masahiroy,
	michal.lkml, Lecopzer Chen

To check the GNU make version. Used by the LTO Kconfig.

LTO with MODVERSIONS will fail in generating correct CRC because
the makefile rule doesn't work for make with version 3.8X.[1]

Thus we need to check make version during selecting on LTO Kconfig.
Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits
for arithmetic comparisons.

[1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
---
 Makefile                |  2 +-
 init/Kconfig            |  4 ++++
 scripts/make-version.sh | 13 +++++++++++++
 3 files changed, 18 insertions(+), 1 deletion(-)
 create mode 100755 scripts/make-version.sh

diff --git a/Makefile b/Makefile
index 88888fff4c62..2402745b2ba9 100644
--- a/Makefile
+++ b/Makefile
@@ -516,7 +516,7 @@ CLANG_FLAGS :=
 
 export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC
 export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL
-export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
+export PERL PYTHON3 CHECK CHECKFLAGS MAKE MAKE_VERSION UTS_MACHINE HOSTCXX
 export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD
 export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE
 
diff --git a/init/Kconfig b/init/Kconfig
index 55f9f7738ebb..ecc110504f87 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -86,6 +86,10 @@ config CC_HAS_ASM_INLINE
 config CC_HAS_NO_PROFILE_FN_ATTR
 	def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror)
 
+config MAKE_VERSION
+	int
+	default $(shell,$(srctree)/scripts/make-version.sh $(MAKE_VERSION))
+
 config CONSTRUCTORS
 	bool
 
diff --git a/scripts/make-version.sh b/scripts/make-version.sh
new file mode 100755
index 000000000000..3a451db3c067
--- /dev/null
+++ b/scripts/make-version.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# Print the GNU make version in a 5 or 6-digit form.
+
+set -e
+
+# Convert the version string x.y.z to a canonical 5 or 6-digit form.
+IFS=.
+set -- $1
+
+# If the 2nd or 3rd field is missing, fill it with a zero.
+echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0}))
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/2] Kbuild: lto: add make version checking
  2021-07-02  3:29 [PATCH v3 0/2] Kbuild: lto: add make version checking for MODVERSIONS Lecopzer Chen
  2021-07-02  3:29 ` [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION Lecopzer Chen
@ 2021-07-02  3:29 ` Lecopzer Chen
  2021-07-04  0:16   ` Nathan Chancellor
  2021-07-05  2:04   ` Masahiro Yamada
  1 sibling, 2 replies; 11+ messages in thread
From: Lecopzer Chen @ 2021-07-02  3:29 UTC (permalink / raw)
  To: ndesaulniers, keescook, samitolvanen, linux-kbuild, nathan
  Cc: clang-built-linux, linux-kernel, yj.chiang, masahiroy,
	michal.lkml, Lecopzer Chen

LTO with MODVERSIONS will fail in generating correct CRC because
the makefile rule doesn't work for make with version 3.8X.[1]

Thus we need to check make version during selecting on LTO Kconfig.
and the suitable version should be 4.2(40200) which released in 2016[2].

[1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/
[2] https://ftp.gnu.org/gnu/make/
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
---
 arch/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index 129df498a8e1..beee68149da8 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -639,6 +639,7 @@ config HAS_LTO_CLANG
 	def_bool y
 	# Clang >= 11: https://github.com/ClangBuiltLinux/linux/issues/510
 	depends on CC_IS_CLANG && CLANG_VERSION >= 110000 && LD_IS_LLD && AS_IS_LLVM
+	depends on !MODVERSIONS || MAKE_VERSION >= 40200
 	depends on $(success,$(NM) --help | head -n 1 | grep -qi llvm)
 	depends on $(success,$(AR) --help | head -n 1 | grep -qi llvm)
 	depends on ARCH_SUPPORTS_LTO_CLANG
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION
  2021-07-02  3:29 ` [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION Lecopzer Chen
@ 2021-07-04  0:15   ` Nathan Chancellor
  2021-07-05  2:02   ` Masahiro Yamada
  1 sibling, 0 replies; 11+ messages in thread
From: Nathan Chancellor @ 2021-07-04  0:15 UTC (permalink / raw)
  To: Lecopzer Chen
  Cc: ndesaulniers, keescook, samitolvanen, linux-kbuild,
	clang-built-linux, linux-kernel, yj.chiang, masahiroy,
	michal.lkml

On Fri, Jul 02, 2021 at 11:29:42AM +0800, Lecopzer Chen wrote:
> To check the GNU make version. Used by the LTO Kconfig.
> 
> LTO with MODVERSIONS will fail in generating correct CRC because
> the makefile rule doesn't work for make with version 3.8X.[1]
> 
> Thus we need to check make version during selecting on LTO Kconfig.
> Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits
> for arithmetic comparisons.
> 
> [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/
> Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  Makefile                |  2 +-
>  init/Kconfig            |  4 ++++
>  scripts/make-version.sh | 13 +++++++++++++
>  3 files changed, 18 insertions(+), 1 deletion(-)
>  create mode 100755 scripts/make-version.sh
> 
> diff --git a/Makefile b/Makefile
> index 88888fff4c62..2402745b2ba9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -516,7 +516,7 @@ CLANG_FLAGS :=
>  
>  export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC
>  export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL
> -export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
> +export PERL PYTHON3 CHECK CHECKFLAGS MAKE MAKE_VERSION UTS_MACHINE HOSTCXX
>  export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD
>  export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE
>  
> diff --git a/init/Kconfig b/init/Kconfig
> index 55f9f7738ebb..ecc110504f87 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -86,6 +86,10 @@ config CC_HAS_ASM_INLINE
>  config CC_HAS_NO_PROFILE_FN_ATTR
>  	def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror)
>  
> +config MAKE_VERSION
> +	int
> +	default $(shell,$(srctree)/scripts/make-version.sh $(MAKE_VERSION))
> +
>  config CONSTRUCTORS
>  	bool
>  
> diff --git a/scripts/make-version.sh b/scripts/make-version.sh
> new file mode 100755
> index 000000000000..3a451db3c067
> --- /dev/null
> +++ b/scripts/make-version.sh
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Print the GNU make version in a 5 or 6-digit form.
> +
> +set -e
> +
> +# Convert the version string x.y.z to a canonical 5 or 6-digit form.
> +IFS=.
> +set -- $1
> +
> +# If the 2nd or 3rd field is missing, fill it with a zero.
> +echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0}))
> -- 
> 2.18.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] Kbuild: lto: add make version checking
  2021-07-02  3:29 ` [PATCH v3 2/2] Kbuild: lto: add make version checking Lecopzer Chen
@ 2021-07-04  0:16   ` Nathan Chancellor
  2021-07-05  2:04   ` Masahiro Yamada
  1 sibling, 0 replies; 11+ messages in thread
From: Nathan Chancellor @ 2021-07-04  0:16 UTC (permalink / raw)
  To: Lecopzer Chen
  Cc: ndesaulniers, keescook, samitolvanen, linux-kbuild,
	clang-built-linux, linux-kernel, yj.chiang, masahiroy,
	michal.lkml

On Fri, Jul 02, 2021 at 11:29:43AM +0800, Lecopzer Chen wrote:
> LTO with MODVERSIONS will fail in generating correct CRC because
> the makefile rule doesn't work for make with version 3.8X.[1]
> 
> Thus we need to check make version during selecting on LTO Kconfig.
> and the suitable version should be 4.2(40200) which released in 2016[2].
> 
> [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/
> [2] https://ftp.gnu.org/gnu/make/
> Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  arch/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 129df498a8e1..beee68149da8 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -639,6 +639,7 @@ config HAS_LTO_CLANG
>  	def_bool y
>  	# Clang >= 11: https://github.com/ClangBuiltLinux/linux/issues/510
>  	depends on CC_IS_CLANG && CLANG_VERSION >= 110000 && LD_IS_LLD && AS_IS_LLVM
> +	depends on !MODVERSIONS || MAKE_VERSION >= 40200
>  	depends on $(success,$(NM) --help | head -n 1 | grep -qi llvm)
>  	depends on $(success,$(AR) --help | head -n 1 | grep -qi llvm)
>  	depends on ARCH_SUPPORTS_LTO_CLANG
> -- 
> 2.18.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION
  2021-07-02  3:29 ` [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION Lecopzer Chen
  2021-07-04  0:15   ` Nathan Chancellor
@ 2021-07-05  2:02   ` Masahiro Yamada
  2021-07-05 17:59     ` Sami Tolvanen
  1 sibling, 1 reply; 11+ messages in thread
From: Masahiro Yamada @ 2021-07-05  2:02 UTC (permalink / raw)
  To: Lecopzer Chen
  Cc: Nick Desaulniers, Kees Cook, Sami Tolvanen,
	Linux Kbuild mailing list, Nathan Chancellor, clang-built-linux,
	Linux Kernel Mailing List, yj.chiang, Michal Marek

On Fri, Jul 2, 2021 at 12:29 PM Lecopzer Chen
<lecopzer.chen@mediatek.com> wrote:
>
> To check the GNU make version. Used by the LTO Kconfig.
>
> LTO with MODVERSIONS will fail in generating correct CRC because
> the makefile rule doesn't work for make with version 3.8X.[1]
>
> Thus we need to check make version during selecting on LTO Kconfig.
> Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits
> for arithmetic comparisons.
>
> [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/
> Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
> ---


NACK.

"Let's add MAKE_VERSION >= 40200 restriction
just because I cannot write correct code that
works for older Make" is a horrible idea.

Also, Kconfig is supposed to check the compiler
(or toolchains) capability, not host tool versions.







>  Makefile                |  2 +-
>  init/Kconfig            |  4 ++++
>  scripts/make-version.sh | 13 +++++++++++++
>  3 files changed, 18 insertions(+), 1 deletion(-)
>  create mode 100755 scripts/make-version.sh
>
> diff --git a/Makefile b/Makefile
> index 88888fff4c62..2402745b2ba9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -516,7 +516,7 @@ CLANG_FLAGS :=
>
>  export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC
>  export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL
> -export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
> +export PERL PYTHON3 CHECK CHECKFLAGS MAKE MAKE_VERSION UTS_MACHINE HOSTCXX
>  export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD
>  export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 55f9f7738ebb..ecc110504f87 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -86,6 +86,10 @@ config CC_HAS_ASM_INLINE
>  config CC_HAS_NO_PROFILE_FN_ATTR
>         def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror)
>
> +config MAKE_VERSION
> +       int
> +       default $(shell,$(srctree)/scripts/make-version.sh $(MAKE_VERSION))
> +
>  config CONSTRUCTORS
>         bool
>
> diff --git a/scripts/make-version.sh b/scripts/make-version.sh
> new file mode 100755
> index 000000000000..3a451db3c067
> --- /dev/null
> +++ b/scripts/make-version.sh
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Print the GNU make version in a 5 or 6-digit form.
> +
> +set -e
> +
> +# Convert the version string x.y.z to a canonical 5 or 6-digit form.
> +IFS=.
> +set -- $1
> +
> +# If the 2nd or 3rd field is missing, fill it with a zero.
> +echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0}))
> --
> 2.18.0
>


--
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] Kbuild: lto: add make version checking
  2021-07-02  3:29 ` [PATCH v3 2/2] Kbuild: lto: add make version checking Lecopzer Chen
  2021-07-04  0:16   ` Nathan Chancellor
@ 2021-07-05  2:04   ` Masahiro Yamada
  1 sibling, 0 replies; 11+ messages in thread
From: Masahiro Yamada @ 2021-07-05  2:04 UTC (permalink / raw)
  To: Lecopzer Chen
  Cc: Nick Desaulniers, Kees Cook, Sami Tolvanen,
	Linux Kbuild mailing list, Nathan Chancellor, clang-built-linux,
	Linux Kernel Mailing List, yj.chiang, Michal Marek

On Fri, Jul 2, 2021 at 12:29 PM Lecopzer Chen
<lecopzer.chen@mediatek.com> wrote:
>
> LTO with MODVERSIONS will fail in generating correct CRC because
> the makefile rule doesn't work for make with version 3.8X.[1]
>
> Thus we need to check make version during selecting on LTO Kconfig.
> and the suitable version should be 4.2(40200) which released in 2016[2].
>
> [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/
> [2] https://ftp.gnu.org/gnu/make/
> Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
> ---

NACK for the same reason as 1/2.

We need to write correct code.
If we cannot, we need to add 'depends on BROKEN'.




>  arch/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 129df498a8e1..beee68149da8 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -639,6 +639,7 @@ config HAS_LTO_CLANG
>         def_bool y
>         # Clang >= 11: https://github.com/ClangBuiltLinux/linux/issues/510
>         depends on CC_IS_CLANG && CLANG_VERSION >= 110000 && LD_IS_LLD && AS_IS_LLVM
> +       depends on !MODVERSIONS || MAKE_VERSION >= 40200
>         depends on $(success,$(NM) --help | head -n 1 | grep -qi llvm)
>         depends on $(success,$(AR) --help | head -n 1 | grep -qi llvm)
>         depends on ARCH_SUPPORTS_LTO_CLANG
> --
> 2.18.0
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION
  2021-07-05  2:02   ` Masahiro Yamada
@ 2021-07-05 17:59     ` Sami Tolvanen
  2021-07-06  9:06       ` Lecopzer Chen
  0 siblings, 1 reply; 11+ messages in thread
From: Sami Tolvanen @ 2021-07-05 17:59 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Lecopzer Chen, Nick Desaulniers, Kees Cook,
	Linux Kbuild mailing list, Nathan Chancellor, clang-built-linux,
	Linux Kernel Mailing List, yj.chiang, Michal Marek

On Sun, Jul 4, 2021 at 7:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Fri, Jul 2, 2021 at 12:29 PM Lecopzer Chen
> <lecopzer.chen@mediatek.com> wrote:
> >
> > To check the GNU make version. Used by the LTO Kconfig.
> >
> > LTO with MODVERSIONS will fail in generating correct CRC because
> > the makefile rule doesn't work for make with version 3.8X.[1]
> >
> > Thus we need to check make version during selecting on LTO Kconfig.
> > Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits
> > for arithmetic comparisons.
> >
> > [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/
> > Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
> > ---
>
>
> NACK.
>
> "Let's add MAKE_VERSION >= 40200 restriction
> just because I cannot write correct code that
> works for older Make" is a horrible idea.
>
> Also, Kconfig is supposed to check the compiler
> (or toolchains) capability, not host tool versions.

I feel like requiring a Make that's half a decade old for a feature
that also requires a toolchain released last October ago isn't
entirely unreasonable.

That being said, if Masahiro prefers not to rely on the wildcard
function's behavior here, which is a reasonable request, we could
simply use the shell to test for the file's existence:

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 34d257653fb4..c6bd62f518ff 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -388,7 +388,7 @@ ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
       cmd_update_lto_symversions =                                     \
        rm -f $@.symversions                                            \
        $(foreach n, $(filter-out FORCE,$^),                            \
-               $(if $(wildcard $(n).symversions),                      \
+               $(if $(shell test -s $(n).symversions && echo y),       \
                        ; cat $(n).symversions >> $@.symversions))
 else
       cmd_update_lto_symversions = echo >/dev/null

This is not quite as efficient as using wildcard, but should work with
older Make versions too. Thoughts?

Sami

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION
  2021-07-05 17:59     ` Sami Tolvanen
@ 2021-07-06  9:06       ` Lecopzer Chen
  2021-07-07 17:02         ` Sami Tolvanen
  0 siblings, 1 reply; 11+ messages in thread
From: Lecopzer Chen @ 2021-07-06  9:06 UTC (permalink / raw)
  To: samitolvanen
  Cc: clang-built-linux, keescook, lecopzer.chen, linux-kbuild,
	linux-kernel, masahiroy, michal.lkml, nathan, ndesaulniers,
	yj.chiang

> On Sun, Jul 4, 2021 at 7:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Fri, Jul 2, 2021 at 12:29 PM Lecopzer Chen
> > <lecopzer.chen@mediatek.com> wrote:
> > >
> > > To check the GNU make version. Used by the LTO Kconfig.
> > >
> > > LTO with MODVERSIONS will fail in generating correct CRC because
> > > the makefile rule doesn't work for make with version 3.8X.[1]
> > >
> > > Thus we need to check make version during selecting on LTO Kconfig.
> > > Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits
> > > for arithmetic comparisons.
> > >
> > > [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/
> > > Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
> > > ---
> >
> >
> > NACK.
> >
> > "Let's add MAKE_VERSION >= 40200 restriction
> > just because I cannot write correct code that
> > works for older Make" is a horrible idea.
> >
> > Also, Kconfig is supposed to check the compiler
> > (or toolchains) capability, not host tool versions.
> 
> I feel like requiring a Make that's half a decade old for a feature
> that also requires a toolchain released last October ago isn't
> entirely unreasonable.
> 
> That being said, if Masahiro prefers not to rely on the wildcard
> function's behavior here, which is a reasonable request, we could
> simply use the shell to test for the file's existence:
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 34d257653fb4..c6bd62f518ff 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -388,7 +388,7 @@ ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
>        cmd_update_lto_symversions =                                     \
>         rm -f $@.symversions                                            \
>         $(foreach n, $(filter-out FORCE,$^),                            \
> -               $(if $(wildcard $(n).symversions),                      \
> +               $(if $(shell test -s $(n).symversions && echo y),       \
>                         ; cat $(n).symversions >> $@.symversions))
>  else
>        cmd_update_lto_symversions = echo >/dev/null
> 
> This is not quite as efficient as using wildcard, but should work with
> older Make versions too. Thoughts?
> 


I've tested this in both make-4.3 and 3.81, and the CRC is correct.
But I'm not sure if anyone would have the "arg list too long" issue.

Tested-by: Lecopzer Chen <lecopzer.chen@mediatek.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION
  2021-07-06  9:06       ` Lecopzer Chen
@ 2021-07-07 17:02         ` Sami Tolvanen
  2021-07-09  8:25           ` Lecopzer Chen
  0 siblings, 1 reply; 11+ messages in thread
From: Sami Tolvanen @ 2021-07-07 17:02 UTC (permalink / raw)
  To: Lecopzer Chen
  Cc: clang-built-linux, keescook, linux-kbuild, linux-kernel,
	masahiroy, michal.lkml, nathan, ndesaulniers, yj.chiang

On Tue, Jul 6, 2021 at 2:06 AM Lecopzer Chen <lecopzer.chen@mediatek.com> wrote:
>
> > On Sun, Jul 4, 2021 at 7:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > On Fri, Jul 2, 2021 at 12:29 PM Lecopzer Chen
> > > <lecopzer.chen@mediatek.com> wrote:
> > > >
> > > > To check the GNU make version. Used by the LTO Kconfig.
> > > >
> > > > LTO with MODVERSIONS will fail in generating correct CRC because
> > > > the makefile rule doesn't work for make with version 3.8X.[1]
> > > >
> > > > Thus we need to check make version during selecting on LTO Kconfig.
> > > > Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits
> > > > for arithmetic comparisons.
> > > >
> > > > [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/
> > > > Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
> > > > ---
> > >
> > >
> > > NACK.
> > >
> > > "Let's add MAKE_VERSION >= 40200 restriction
> > > just because I cannot write correct code that
> > > works for older Make" is a horrible idea.
> > >
> > > Also, Kconfig is supposed to check the compiler
> > > (or toolchains) capability, not host tool versions.
> >
> > I feel like requiring a Make that's half a decade old for a feature
> > that also requires a toolchain released last October ago isn't
> > entirely unreasonable.
> >
> > That being said, if Masahiro prefers not to rely on the wildcard
> > function's behavior here, which is a reasonable request, we could
> > simply use the shell to test for the file's existence:
> >
> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > index 34d257653fb4..c6bd62f518ff 100644
> > --- a/scripts/Makefile.build
> > +++ b/scripts/Makefile.build
> > @@ -388,7 +388,7 @@ ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
> >        cmd_update_lto_symversions =                                     \
> >         rm -f $@.symversions                                            \
> >         $(foreach n, $(filter-out FORCE,$^),                            \
> > -               $(if $(wildcard $(n).symversions),                      \
> > +               $(if $(shell test -s $(n).symversions && echo y),       \
> >                         ; cat $(n).symversions >> $@.symversions))
> >  else
> >        cmd_update_lto_symversions = echo >/dev/null
> >
> > This is not quite as efficient as using wildcard, but should work with
> > older Make versions too. Thoughts?
> >
>
>
> I've tested this in both make-4.3 and 3.81, and the CRC is correct.
> But I'm not sure if anyone would have the "arg list too long" issue.
>
> Tested-by: Lecopzer Chen <lecopzer.chen@mediatek.com>

Thank you for testing. This should produce a command identical to the
wildcard version (with newer Make versions), so that shouldn't be an
issue. If nobody objects to this approach, would you mind putting this
into a proper patch and sending it as v4?

Sami

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION
  2021-07-07 17:02         ` Sami Tolvanen
@ 2021-07-09  8:25           ` Lecopzer Chen
  0 siblings, 0 replies; 11+ messages in thread
From: Lecopzer Chen @ 2021-07-09  8:25 UTC (permalink / raw)
  To: samitolvanen
  Cc: clang-built-linux, keescook, lecopzer.chen, linux-kbuild,
	linux-kernel, masahiroy, michal.lkml, nathan, ndesaulniers,
	yj.chiang

> On Tue, Jul 6, 2021 at 2:06 AM Lecopzer Chen <lecopzer.chen@mediatek.com> wrote:
> >
> > > On Sun, Jul 4, 2021 at 7:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > >
> > > > On Fri, Jul 2, 2021 at 12:29 PM Lecopzer Chen
> > > > <lecopzer.chen@mediatek.com> wrote:
> > > > >
> > > > > To check the GNU make version. Used by the LTO Kconfig.
> > > > >
> > > > > LTO with MODVERSIONS will fail in generating correct CRC because
> > > > > the makefile rule doesn't work for make with version 3.8X.[1]
> > > > >
> > > > > Thus we need to check make version during selecting on LTO Kconfig.
> > > > > Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits
> > > > > for arithmetic comparisons.
> > > > >
> > > > > [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/
> > > > > Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
> > > > > ---
> > > >
> > > >
> > > > NACK.
> > > >
> > > > "Let's add MAKE_VERSION >= 40200 restriction
> > > > just because I cannot write correct code that
> > > > works for older Make" is a horrible idea.
> > > >
> > > > Also, Kconfig is supposed to check the compiler
> > > > (or toolchains) capability, not host tool versions.
> > >
> > > I feel like requiring a Make that's half a decade old for a feature
> > > that also requires a toolchain released last October ago isn't
> > > entirely unreasonable.
> > >
> > > That being said, if Masahiro prefers not to rely on the wildcard
> > > function's behavior here, which is a reasonable request, we could
> > > simply use the shell to test for the file's existence:
> > >
> > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > > index 34d257653fb4..c6bd62f518ff 100644
> > > --- a/scripts/Makefile.build
> > > +++ b/scripts/Makefile.build
> > > @@ -388,7 +388,7 @@ ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
> > >        cmd_update_lto_symversions =                                     \
> > >         rm -f $@.symversions                                            \
> > >         $(foreach n, $(filter-out FORCE,$^),                            \
> > > -               $(if $(wildcard $(n).symversions),                      \
> > > +               $(if $(shell test -s $(n).symversions && echo y),       \
> > >                         ; cat $(n).symversions >> $@.symversions))
> > >  else
> > >        cmd_update_lto_symversions = echo >/dev/null
> > >
> > > This is not quite as efficient as using wildcard, but should work with
> > > older Make versions too. Thoughts?
> > >
> >
> >
> > I've tested this in both make-4.3 and 3.81, and the CRC is correct.
> > But I'm not sure if anyone would have the "arg list too long" issue.
> >
> > Tested-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
> 
> Thank you for testing. This should produce a command identical to the
> wildcard version (with newer Make versions), so that shouldn't be an
> issue. If nobody objects to this approach, would you mind putting this
> into a proper patch and sending it as v4?

Sure, I'll rebase the whole commit and send as v4 soon.



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-07-09  8:25 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-02  3:29 [PATCH v3 0/2] Kbuild: lto: add make version checking for MODVERSIONS Lecopzer Chen
2021-07-02  3:29 ` [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION Lecopzer Chen
2021-07-04  0:15   ` Nathan Chancellor
2021-07-05  2:02   ` Masahiro Yamada
2021-07-05 17:59     ` Sami Tolvanen
2021-07-06  9:06       ` Lecopzer Chen
2021-07-07 17:02         ` Sami Tolvanen
2021-07-09  8:25           ` Lecopzer Chen
2021-07-02  3:29 ` [PATCH v3 2/2] Kbuild: lto: add make version checking Lecopzer Chen
2021-07-04  0:16   ` Nathan Chancellor
2021-07-05  2:04   ` Masahiro Yamada

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).