All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] compat vdso cleanups
@ 2021-10-12 23:46 ` Nick Desaulniers
  0 siblings, 0 replies; 22+ messages in thread
From: Nick Desaulniers @ 2021-10-12 23:46 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon
  Cc: llvm, linux-kernel, linux-arm-kernel, Vincenzo Frascino,
	Nick Desaulniers

Three fixes for compat vdso, the first two are related, the third is
standalone.

The first two fix a warning observed for `mrproper` targets when
$(CROSS_COMPILE_COMPAT)gcc is not in the $PATH.

The third makes is so that CROSS_COMPILE_COMPAT is not necessary to
select COMPAT_VDSO when using clang+lld.

Based on arm64/linux.git/for-next/misc.

Nick Desaulniers (3):
  arm64: vdso32: drop the test for dmb ishld
  arm64: vdso32: lazily invoke COMPAT_CC
  arm64: vdso32: require CROSS_COMPILE_COMPAT for gcc+bfd

 arch/arm64/Kconfig                           |  3 +-
 arch/arm64/include/asm/vdso/compat_barrier.h |  2 +-
 arch/arm64/kernel/vdso32/Makefile            | 33 ++++++--------------
 3 files changed, 12 insertions(+), 26 deletions(-)


base-commit: de56379f21c70196ff18c48790e8e43865893869
-- 
2.33.0.882.g93a45727a2-goog


^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 0/3] compat vdso cleanups
@ 2021-10-12 23:46 ` Nick Desaulniers
  0 siblings, 0 replies; 22+ messages in thread
From: Nick Desaulniers @ 2021-10-12 23:46 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon
  Cc: llvm, linux-kernel, linux-arm-kernel, Vincenzo Frascino,
	Nick Desaulniers

Three fixes for compat vdso, the first two are related, the third is
standalone.

The first two fix a warning observed for `mrproper` targets when
$(CROSS_COMPILE_COMPAT)gcc is not in the $PATH.

The third makes is so that CROSS_COMPILE_COMPAT is not necessary to
select COMPAT_VDSO when using clang+lld.

Based on arm64/linux.git/for-next/misc.

Nick Desaulniers (3):
  arm64: vdso32: drop the test for dmb ishld
  arm64: vdso32: lazily invoke COMPAT_CC
  arm64: vdso32: require CROSS_COMPILE_COMPAT for gcc+bfd

 arch/arm64/Kconfig                           |  3 +-
 arch/arm64/include/asm/vdso/compat_barrier.h |  2 +-
 arch/arm64/kernel/vdso32/Makefile            | 33 ++++++--------------
 3 files changed, 12 insertions(+), 26 deletions(-)


base-commit: de56379f21c70196ff18c48790e8e43865893869
-- 
2.33.0.882.g93a45727a2-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 1/3] arm64: vdso32: drop the test for dmb ishld
  2021-10-12 23:46 ` Nick Desaulniers
@ 2021-10-12 23:46   ` Nick Desaulniers
  -1 siblings, 0 replies; 22+ messages in thread
From: Nick Desaulniers @ 2021-10-12 23:46 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon
  Cc: llvm, linux-kernel, linux-arm-kernel, Vincenzo Frascino,
	Nick Desaulniers, Christian Biesinger, Simon Marchi

Binutils added support for this instruction in commit
e797f7e0b2bedc9328d4a9a0ebc63ca7a2dbbebc which shipped in 2.34 (just
missing the 2.33 release) but was cherry-picked into 2.33 in commit
27a50d6755bae906bc73b4ec1a8b448467f0bea1. Thanks to Christian and Simon
for helping me with the patch archaeology.

According to Documentation/process/changes.rst, the minimum supported
version of binutils is 2.33. Since all supported versions of GAS support
this instruction, drop the assembler invocation, preprocessor
flags/guards, and the cross assembler macro that's now unused.

This also avoids a recursive self reference in a follow up cleanup
patch.

Cc: Christian Biesinger <cbiesinger@google.com>
Cc: Simon Marchi <simon.marchi@polymtl.ca>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/arm64/include/asm/vdso/compat_barrier.h | 2 +-
 arch/arm64/kernel/vdso32/Makefile            | 8 --------
 2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/arch/arm64/include/asm/vdso/compat_barrier.h b/arch/arm64/include/asm/vdso/compat_barrier.h
index 3fd8fd6d8fc2..fb60a88b5ed4 100644
--- a/arch/arm64/include/asm/vdso/compat_barrier.h
+++ b/arch/arm64/include/asm/vdso/compat_barrier.h
@@ -20,7 +20,7 @@
 
 #define dmb(option) __asm__ __volatile__ ("dmb " #option : : : "memory")
 
-#if __LINUX_ARM_ARCH__ >= 8 && defined(CONFIG_AS_DMB_ISHLD)
+#if __LINUX_ARM_ARCH__ >= 8
 #define aarch32_smp_mb()	dmb(ish)
 #define aarch32_smp_rmb()	dmb(ishld)
 #define aarch32_smp_wmb()	dmb(ishst)
diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index 3dba0c4f8f42..89299a26638b 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -29,8 +29,6 @@ cc32-option = $(call try-run,\
         $(CC_COMPAT) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
 cc32-disable-warning = $(call try-run,\
 	$(CC_COMPAT) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
-cc32-as-instr = $(call try-run,\
-	printf "%b\n" "$(1)" | $(CC_COMPAT) $(VDSO_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3))
 
 # We cannot use the global flags to compile the vDSO files, the main reason
 # being that the 32-bit compiler may be older than the main (64-bit) compiler
@@ -113,12 +111,6 @@ endif
 VDSO_AFLAGS := $(VDSO_CAFLAGS)
 VDSO_AFLAGS += -D__ASSEMBLY__
 
-# Check for binutils support for dmb ishld
-dmbinstr := $(call cc32-as-instr,dmb ishld,-DCONFIG_AS_DMB_ISHLD=1)
-
-VDSO_CFLAGS += $(dmbinstr)
-VDSO_AFLAGS += $(dmbinstr)
-
 # From arm vDSO Makefile
 VDSO_LDFLAGS += -Bsymbolic --no-undefined -soname=linux-vdso.so.1
 VDSO_LDFLAGS += -z max-page-size=4096 -z common-page-size=4096
-- 
2.33.0.882.g93a45727a2-goog


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 1/3] arm64: vdso32: drop the test for dmb ishld
@ 2021-10-12 23:46   ` Nick Desaulniers
  0 siblings, 0 replies; 22+ messages in thread
From: Nick Desaulniers @ 2021-10-12 23:46 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon
  Cc: llvm, linux-kernel, linux-arm-kernel, Vincenzo Frascino,
	Nick Desaulniers, Christian Biesinger, Simon Marchi

Binutils added support for this instruction in commit
e797f7e0b2bedc9328d4a9a0ebc63ca7a2dbbebc which shipped in 2.34 (just
missing the 2.33 release) but was cherry-picked into 2.33 in commit
27a50d6755bae906bc73b4ec1a8b448467f0bea1. Thanks to Christian and Simon
for helping me with the patch archaeology.

According to Documentation/process/changes.rst, the minimum supported
version of binutils is 2.33. Since all supported versions of GAS support
this instruction, drop the assembler invocation, preprocessor
flags/guards, and the cross assembler macro that's now unused.

This also avoids a recursive self reference in a follow up cleanup
patch.

Cc: Christian Biesinger <cbiesinger@google.com>
Cc: Simon Marchi <simon.marchi@polymtl.ca>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/arm64/include/asm/vdso/compat_barrier.h | 2 +-
 arch/arm64/kernel/vdso32/Makefile            | 8 --------
 2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/arch/arm64/include/asm/vdso/compat_barrier.h b/arch/arm64/include/asm/vdso/compat_barrier.h
index 3fd8fd6d8fc2..fb60a88b5ed4 100644
--- a/arch/arm64/include/asm/vdso/compat_barrier.h
+++ b/arch/arm64/include/asm/vdso/compat_barrier.h
@@ -20,7 +20,7 @@
 
 #define dmb(option) __asm__ __volatile__ ("dmb " #option : : : "memory")
 
-#if __LINUX_ARM_ARCH__ >= 8 && defined(CONFIG_AS_DMB_ISHLD)
+#if __LINUX_ARM_ARCH__ >= 8
 #define aarch32_smp_mb()	dmb(ish)
 #define aarch32_smp_rmb()	dmb(ishld)
 #define aarch32_smp_wmb()	dmb(ishst)
diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index 3dba0c4f8f42..89299a26638b 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -29,8 +29,6 @@ cc32-option = $(call try-run,\
         $(CC_COMPAT) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
 cc32-disable-warning = $(call try-run,\
 	$(CC_COMPAT) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
-cc32-as-instr = $(call try-run,\
-	printf "%b\n" "$(1)" | $(CC_COMPAT) $(VDSO_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3))
 
 # We cannot use the global flags to compile the vDSO files, the main reason
 # being that the 32-bit compiler may be older than the main (64-bit) compiler
@@ -113,12 +111,6 @@ endif
 VDSO_AFLAGS := $(VDSO_CAFLAGS)
 VDSO_AFLAGS += -D__ASSEMBLY__
 
-# Check for binutils support for dmb ishld
-dmbinstr := $(call cc32-as-instr,dmb ishld,-DCONFIG_AS_DMB_ISHLD=1)
-
-VDSO_CFLAGS += $(dmbinstr)
-VDSO_AFLAGS += $(dmbinstr)
-
 # From arm vDSO Makefile
 VDSO_LDFLAGS += -Bsymbolic --no-undefined -soname=linux-vdso.so.1
 VDSO_LDFLAGS += -z max-page-size=4096 -z common-page-size=4096
-- 
2.33.0.882.g93a45727a2-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 2/3] arm64: vdso32: lazily invoke COMPAT_CC
  2021-10-12 23:46 ` Nick Desaulniers
@ 2021-10-12 23:46   ` Nick Desaulniers
  -1 siblings, 0 replies; 22+ messages in thread
From: Nick Desaulniers @ 2021-10-12 23:46 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon
  Cc: llvm, linux-kernel, linux-arm-kernel, Vincenzo Frascino,
	Nick Desaulniers, Masahiro Yamada, Lucas Henneman

When running the following command without arm-linux-gnueabi-gcc in
one's $PATH, the following warning is observed:

$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 mrproper
make[1]: arm-linux-gnueabi-gcc: No such file or directory

This is because KCONFIG is not run for mrproper, so CONFIG_CC_IS_CLANG
is not set, and we end up eagerly evaluating various variables that try
to invoke CC_COMPAT.

This is a similar problem to what was observed in
commit 3ec8a5b33dea ("kbuild: do not export LDFLAGS_vmlinux")

Cc: Masahiro Yamada <masahiroy@kernel.org>
Reported-by: Lucas Henneman <henneman@google.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/arm64/kernel/vdso32/Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index 89299a26638b..d24b12318f4c 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -38,12 +38,12 @@ cc32-disable-warning = $(call try-run,\
 # As a result we set our own flags here.
 
 # KBUILD_CPPFLAGS and NOSTDINC_FLAGS from top-level Makefile
-VDSO_CPPFLAGS := -DBUILD_VDSO -D__KERNEL__ -nostdinc -isystem $(shell $(CC_COMPAT) -print-file-name=include)
+VDSO_CPPFLAGS = -DBUILD_VDSO -D__KERNEL__ -nostdinc -isystem $(shell $(CC_COMPAT) -print-file-name=include)
 VDSO_CPPFLAGS += $(LINUXINCLUDE)
 
 # Common C and assembly flags
 # From top-level Makefile
-VDSO_CAFLAGS := $(VDSO_CPPFLAGS)
+VDSO_CAFLAGS = $(VDSO_CPPFLAGS)
 ifneq ($(shell $(CC_COMPAT) --version 2>&1 | head -n 1 | grep clang),)
 VDSO_CAFLAGS += --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%))
 endif
@@ -73,7 +73,7 @@ VDSO_CAFLAGS += -DDISABLE_BRANCH_PROFILING
 VDSO_CAFLAGS += $(call cc32-option,-march=armv8-a -D__LINUX_ARM_ARCH__=8,\
                                    -march=armv7-a -D__LINUX_ARM_ARCH__=7)
 
-VDSO_CFLAGS := $(VDSO_CAFLAGS)
+VDSO_CFLAGS = $(VDSO_CAFLAGS)
 VDSO_CFLAGS += -DENABLE_COMPAT_VDSO=1
 # KBUILD_CFLAGS from top-level Makefile
 VDSO_CFLAGS += -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
@@ -108,7 +108,7 @@ else
 VDSO_CFLAGS += -marm
 endif
 
-VDSO_AFLAGS := $(VDSO_CAFLAGS)
+VDSO_AFLAGS = $(VDSO_CAFLAGS)
 VDSO_AFLAGS += -D__ASSEMBLY__
 
 # From arm vDSO Makefile
-- 
2.33.0.882.g93a45727a2-goog


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 2/3] arm64: vdso32: lazily invoke COMPAT_CC
@ 2021-10-12 23:46   ` Nick Desaulniers
  0 siblings, 0 replies; 22+ messages in thread
From: Nick Desaulniers @ 2021-10-12 23:46 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon
  Cc: llvm, linux-kernel, linux-arm-kernel, Vincenzo Frascino,
	Nick Desaulniers, Masahiro Yamada, Lucas Henneman

When running the following command without arm-linux-gnueabi-gcc in
one's $PATH, the following warning is observed:

$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 mrproper
make[1]: arm-linux-gnueabi-gcc: No such file or directory

This is because KCONFIG is not run for mrproper, so CONFIG_CC_IS_CLANG
is not set, and we end up eagerly evaluating various variables that try
to invoke CC_COMPAT.

This is a similar problem to what was observed in
commit 3ec8a5b33dea ("kbuild: do not export LDFLAGS_vmlinux")

Cc: Masahiro Yamada <masahiroy@kernel.org>
Reported-by: Lucas Henneman <henneman@google.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/arm64/kernel/vdso32/Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index 89299a26638b..d24b12318f4c 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -38,12 +38,12 @@ cc32-disable-warning = $(call try-run,\
 # As a result we set our own flags here.
 
 # KBUILD_CPPFLAGS and NOSTDINC_FLAGS from top-level Makefile
-VDSO_CPPFLAGS := -DBUILD_VDSO -D__KERNEL__ -nostdinc -isystem $(shell $(CC_COMPAT) -print-file-name=include)
+VDSO_CPPFLAGS = -DBUILD_VDSO -D__KERNEL__ -nostdinc -isystem $(shell $(CC_COMPAT) -print-file-name=include)
 VDSO_CPPFLAGS += $(LINUXINCLUDE)
 
 # Common C and assembly flags
 # From top-level Makefile
-VDSO_CAFLAGS := $(VDSO_CPPFLAGS)
+VDSO_CAFLAGS = $(VDSO_CPPFLAGS)
 ifneq ($(shell $(CC_COMPAT) --version 2>&1 | head -n 1 | grep clang),)
 VDSO_CAFLAGS += --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%))
 endif
@@ -73,7 +73,7 @@ VDSO_CAFLAGS += -DDISABLE_BRANCH_PROFILING
 VDSO_CAFLAGS += $(call cc32-option,-march=armv8-a -D__LINUX_ARM_ARCH__=8,\
                                    -march=armv7-a -D__LINUX_ARM_ARCH__=7)
 
-VDSO_CFLAGS := $(VDSO_CAFLAGS)
+VDSO_CFLAGS = $(VDSO_CAFLAGS)
 VDSO_CFLAGS += -DENABLE_COMPAT_VDSO=1
 # KBUILD_CFLAGS from top-level Makefile
 VDSO_CFLAGS += -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
@@ -108,7 +108,7 @@ else
 VDSO_CFLAGS += -marm
 endif
 
-VDSO_AFLAGS := $(VDSO_CAFLAGS)
+VDSO_AFLAGS = $(VDSO_CAFLAGS)
 VDSO_AFLAGS += -D__ASSEMBLY__
 
 # From arm vDSO Makefile
-- 
2.33.0.882.g93a45727a2-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 3/3] arm64: vdso32: require CROSS_COMPILE_COMPAT for gcc+bfd
  2021-10-12 23:46 ` Nick Desaulniers
@ 2021-10-12 23:46   ` Nick Desaulniers
  -1 siblings, 0 replies; 22+ messages in thread
From: Nick Desaulniers @ 2021-10-12 23:46 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon
  Cc: llvm, linux-kernel, linux-arm-kernel, Vincenzo Frascino,
	Nick Desaulniers

Similar to
commit 231ad7f409f1 ("Makefile: infer --target from ARCH for CC=clang")
There really is no point in setting --target based on
$CROSS_COMPILE_COMPAT for clang when the integrated assembler is being
used.

Allows COMPAT_VDSO to be selected without setting $CROSS_COMPILE_COMPAT
when using clang and lld together.

Before:
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 defconfig
$ grep CONFIG_COMPAT_VDSO .config
CONFIG_COMPAT_VDSO=y
$ ARCH=arm64 make -j72 LLVM=1 defconfig
$ grep CONFIG_COMPAT_VDSO .config
$

After:
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 defconfig
$ grep CONFIG_COMPAT_VDSO .config
CONFIG_COMPAT_VDSO=y
$ ARCH=arm64 make -j72 LLVM=1 defconfig
$ grep CONFIG_COMPAT_VDSO .config
CONFIG_COMPAT_VDSO=y

Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/arm64/Kconfig                |  3 ++-
 arch/arm64/kernel/vdso32/Makefile | 17 +++++------------
 2 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5c7ae4c3954b..7b28dad2fb80 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1264,7 +1264,8 @@ config KUSER_HELPERS
 
 config COMPAT_VDSO
 	bool "Enable vDSO for 32-bit applications"
-	depends on !CPU_BIG_ENDIAN && "$(CROSS_COMPILE_COMPAT)" != ""
+	depends on !CPU_BIG_ENDIAN
+	depends on CC_IS_CLANG && LD_IS_LLD || "$(CROSS_COMPILE_COMPAT)" != ""
 	select GENERIC_COMPAT_VDSO
 	default y
 	help
diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index d24b12318f4c..376261d3791f 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -10,18 +10,15 @@ include $(srctree)/lib/vdso/Makefile
 
 # Same as cc-*option, but using CC_COMPAT instead of CC
 ifeq ($(CONFIG_CC_IS_CLANG), y)
-CC_COMPAT_CLANG_FLAGS := --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%))
-
 CC_COMPAT ?= $(CC)
-CC_COMPAT += $(CC_COMPAT_CLANG_FLAGS)
-
-ifneq ($(LLVM),)
-LD_COMPAT ?= $(LD)
+CC_COMPAT += --target=arm-linux-gnueabi
 else
-LD_COMPAT ?= $(CROSS_COMPILE_COMPAT)ld
+CC_COMPAT ?= $(CROSS_COMPILE_COMPAT)gcc
 endif
+
+ifeq ($(CONFIG_LD_IS_LLD), y)
+LD_COMPAT ?= $(LD)
 else
-CC_COMPAT ?= $(CROSS_COMPILE_COMPAT)gcc
 LD_COMPAT ?= $(CROSS_COMPILE_COMPAT)ld
 endif
 
@@ -44,10 +41,6 @@ VDSO_CPPFLAGS += $(LINUXINCLUDE)
 # Common C and assembly flags
 # From top-level Makefile
 VDSO_CAFLAGS = $(VDSO_CPPFLAGS)
-ifneq ($(shell $(CC_COMPAT) --version 2>&1 | head -n 1 | grep clang),)
-VDSO_CAFLAGS += --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%))
-endif
-
 VDSO_CAFLAGS += $(call cc32-option,-fno-PIE)
 ifdef CONFIG_DEBUG_INFO
 VDSO_CAFLAGS += -g
-- 
2.33.0.882.g93a45727a2-goog


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 3/3] arm64: vdso32: require CROSS_COMPILE_COMPAT for gcc+bfd
@ 2021-10-12 23:46   ` Nick Desaulniers
  0 siblings, 0 replies; 22+ messages in thread
From: Nick Desaulniers @ 2021-10-12 23:46 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon
  Cc: llvm, linux-kernel, linux-arm-kernel, Vincenzo Frascino,
	Nick Desaulniers

Similar to
commit 231ad7f409f1 ("Makefile: infer --target from ARCH for CC=clang")
There really is no point in setting --target based on
$CROSS_COMPILE_COMPAT for clang when the integrated assembler is being
used.

Allows COMPAT_VDSO to be selected without setting $CROSS_COMPILE_COMPAT
when using clang and lld together.

Before:
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 defconfig
$ grep CONFIG_COMPAT_VDSO .config
CONFIG_COMPAT_VDSO=y
$ ARCH=arm64 make -j72 LLVM=1 defconfig
$ grep CONFIG_COMPAT_VDSO .config
$

After:
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 defconfig
$ grep CONFIG_COMPAT_VDSO .config
CONFIG_COMPAT_VDSO=y
$ ARCH=arm64 make -j72 LLVM=1 defconfig
$ grep CONFIG_COMPAT_VDSO .config
CONFIG_COMPAT_VDSO=y

Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/arm64/Kconfig                |  3 ++-
 arch/arm64/kernel/vdso32/Makefile | 17 +++++------------
 2 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5c7ae4c3954b..7b28dad2fb80 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1264,7 +1264,8 @@ config KUSER_HELPERS
 
 config COMPAT_VDSO
 	bool "Enable vDSO for 32-bit applications"
-	depends on !CPU_BIG_ENDIAN && "$(CROSS_COMPILE_COMPAT)" != ""
+	depends on !CPU_BIG_ENDIAN
+	depends on CC_IS_CLANG && LD_IS_LLD || "$(CROSS_COMPILE_COMPAT)" != ""
 	select GENERIC_COMPAT_VDSO
 	default y
 	help
diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index d24b12318f4c..376261d3791f 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -10,18 +10,15 @@ include $(srctree)/lib/vdso/Makefile
 
 # Same as cc-*option, but using CC_COMPAT instead of CC
 ifeq ($(CONFIG_CC_IS_CLANG), y)
-CC_COMPAT_CLANG_FLAGS := --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%))
-
 CC_COMPAT ?= $(CC)
-CC_COMPAT += $(CC_COMPAT_CLANG_FLAGS)
-
-ifneq ($(LLVM),)
-LD_COMPAT ?= $(LD)
+CC_COMPAT += --target=arm-linux-gnueabi
 else
-LD_COMPAT ?= $(CROSS_COMPILE_COMPAT)ld
+CC_COMPAT ?= $(CROSS_COMPILE_COMPAT)gcc
 endif
+
+ifeq ($(CONFIG_LD_IS_LLD), y)
+LD_COMPAT ?= $(LD)
 else
-CC_COMPAT ?= $(CROSS_COMPILE_COMPAT)gcc
 LD_COMPAT ?= $(CROSS_COMPILE_COMPAT)ld
 endif
 
@@ -44,10 +41,6 @@ VDSO_CPPFLAGS += $(LINUXINCLUDE)
 # Common C and assembly flags
 # From top-level Makefile
 VDSO_CAFLAGS = $(VDSO_CPPFLAGS)
-ifneq ($(shell $(CC_COMPAT) --version 2>&1 | head -n 1 | grep clang),)
-VDSO_CAFLAGS += --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%))
-endif
-
 VDSO_CAFLAGS += $(call cc32-option,-fno-PIE)
 ifdef CONFIG_DEBUG_INFO
 VDSO_CAFLAGS += -g
-- 
2.33.0.882.g93a45727a2-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] arm64: vdso32: lazily invoke COMPAT_CC
  2021-10-12 23:46   ` Nick Desaulniers
@ 2021-10-13  3:02     ` Masahiro Yamada
  -1 siblings, 0 replies; 22+ messages in thread
From: Masahiro Yamada @ 2021-10-13  3:02 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Catalin Marinas, Will Deacon, llvm, Linux Kernel Mailing List,
	linux-arm-kernel, Vincenzo Frascino, Lucas Henneman

On Wed, Oct 13, 2021 at 8:46 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> When running the following command without arm-linux-gnueabi-gcc in
> one's $PATH, the following warning is observed:
>
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 mrproper
> make[1]: arm-linux-gnueabi-gcc: No such file or directory
>
> This is because KCONFIG is not run for mrproper, so CONFIG_CC_IS_CLANG
> is not set, and we end up eagerly evaluating various variables that try
> to invoke CC_COMPAT.
>
> This is a similar problem to what was observed in
> commit 3ec8a5b33dea ("kbuild: do not export LDFLAGS_vmlinux")
>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Reported-by: Lucas Henneman <henneman@google.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>


There are two ways to fix it:

  [1]: sink the error message to /dev/null
        (as in commit dc960bfeedb01cf832c5632ed1f3daed4416b142)
  [2]: use a recursively-expanded variable as you did.


"Simple variable (:=) vs recursive variable (=)" is a trade-off.

Please be careful about the cost when you try the [2] approach.



Simple variables are immediately expanded while parsing Makefile.
There are 7 call-sites for cc32-option, hence
the compiler is invoked 7 times for building vdso32,
0 times for cleaning.
(Since 57fd251c789647552d32d2fc51bedd4f90d70f9f,
try-run is no-op for 'make clean').




Recursive variables are expanded every time they are used.

IIUC, if_changed expands the command line 3 times.
There are 2 objects (note.o and vgettimeofday.o)
There are 7 call-sites for cc32-option.

So, the compiler is invoked 42 (3 * 2 * 7) times
for building vdso32.







> ---
>  arch/arm64/kernel/vdso32/Makefile | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
> index 89299a26638b..d24b12318f4c 100644
> --- a/arch/arm64/kernel/vdso32/Makefile
> +++ b/arch/arm64/kernel/vdso32/Makefile
> @@ -38,12 +38,12 @@ cc32-disable-warning = $(call try-run,\
>  # As a result we set our own flags here.
>
>  # KBUILD_CPPFLAGS and NOSTDINC_FLAGS from top-level Makefile
> -VDSO_CPPFLAGS := -DBUILD_VDSO -D__KERNEL__ -nostdinc -isystem $(shell $(CC_COMPAT) -print-file-name=include)
> +VDSO_CPPFLAGS = -DBUILD_VDSO -D__KERNEL__ -nostdinc -isystem $(shell $(CC_COMPAT) -print-file-name=include)
>  VDSO_CPPFLAGS += $(LINUXINCLUDE)
>
>  # Common C and assembly flags
>  # From top-level Makefile
> -VDSO_CAFLAGS := $(VDSO_CPPFLAGS)
> +VDSO_CAFLAGS = $(VDSO_CPPFLAGS)
>  ifneq ($(shell $(CC_COMPAT) --version 2>&1 | head -n 1 | grep clang),)
>  VDSO_CAFLAGS += --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%))
>  endif
> @@ -73,7 +73,7 @@ VDSO_CAFLAGS += -DDISABLE_BRANCH_PROFILING
>  VDSO_CAFLAGS += $(call cc32-option,-march=armv8-a -D__LINUX_ARM_ARCH__=8,\
>                                     -march=armv7-a -D__LINUX_ARM_ARCH__=7)
>
> -VDSO_CFLAGS := $(VDSO_CAFLAGS)
> +VDSO_CFLAGS = $(VDSO_CAFLAGS)
>  VDSO_CFLAGS += -DENABLE_COMPAT_VDSO=1
>  # KBUILD_CFLAGS from top-level Makefile
>  VDSO_CFLAGS += -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
> @@ -108,7 +108,7 @@ else
>  VDSO_CFLAGS += -marm
>  endif
>
> -VDSO_AFLAGS := $(VDSO_CAFLAGS)
> +VDSO_AFLAGS = $(VDSO_CAFLAGS)
>  VDSO_AFLAGS += -D__ASSEMBLY__
>
>  # From arm vDSO Makefile
> --
> 2.33.0.882.g93a45727a2-goog
>


--
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] arm64: vdso32: lazily invoke COMPAT_CC
@ 2021-10-13  3:02     ` Masahiro Yamada
  0 siblings, 0 replies; 22+ messages in thread
From: Masahiro Yamada @ 2021-10-13  3:02 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Catalin Marinas, Will Deacon, llvm, Linux Kernel Mailing List,
	linux-arm-kernel, Vincenzo Frascino, Lucas Henneman

On Wed, Oct 13, 2021 at 8:46 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> When running the following command without arm-linux-gnueabi-gcc in
> one's $PATH, the following warning is observed:
>
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 mrproper
> make[1]: arm-linux-gnueabi-gcc: No such file or directory
>
> This is because KCONFIG is not run for mrproper, so CONFIG_CC_IS_CLANG
> is not set, and we end up eagerly evaluating various variables that try
> to invoke CC_COMPAT.
>
> This is a similar problem to what was observed in
> commit 3ec8a5b33dea ("kbuild: do not export LDFLAGS_vmlinux")
>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Reported-by: Lucas Henneman <henneman@google.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>


There are two ways to fix it:

  [1]: sink the error message to /dev/null
        (as in commit dc960bfeedb01cf832c5632ed1f3daed4416b142)
  [2]: use a recursively-expanded variable as you did.


"Simple variable (:=) vs recursive variable (=)" is a trade-off.

Please be careful about the cost when you try the [2] approach.



Simple variables are immediately expanded while parsing Makefile.
There are 7 call-sites for cc32-option, hence
the compiler is invoked 7 times for building vdso32,
0 times for cleaning.
(Since 57fd251c789647552d32d2fc51bedd4f90d70f9f,
try-run is no-op for 'make clean').




Recursive variables are expanded every time they are used.

IIUC, if_changed expands the command line 3 times.
There are 2 objects (note.o and vgettimeofday.o)
There are 7 call-sites for cc32-option.

So, the compiler is invoked 42 (3 * 2 * 7) times
for building vdso32.







> ---
>  arch/arm64/kernel/vdso32/Makefile | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
> index 89299a26638b..d24b12318f4c 100644
> --- a/arch/arm64/kernel/vdso32/Makefile
> +++ b/arch/arm64/kernel/vdso32/Makefile
> @@ -38,12 +38,12 @@ cc32-disable-warning = $(call try-run,\
>  # As a result we set our own flags here.
>
>  # KBUILD_CPPFLAGS and NOSTDINC_FLAGS from top-level Makefile
> -VDSO_CPPFLAGS := -DBUILD_VDSO -D__KERNEL__ -nostdinc -isystem $(shell $(CC_COMPAT) -print-file-name=include)
> +VDSO_CPPFLAGS = -DBUILD_VDSO -D__KERNEL__ -nostdinc -isystem $(shell $(CC_COMPAT) -print-file-name=include)
>  VDSO_CPPFLAGS += $(LINUXINCLUDE)
>
>  # Common C and assembly flags
>  # From top-level Makefile
> -VDSO_CAFLAGS := $(VDSO_CPPFLAGS)
> +VDSO_CAFLAGS = $(VDSO_CPPFLAGS)
>  ifneq ($(shell $(CC_COMPAT) --version 2>&1 | head -n 1 | grep clang),)
>  VDSO_CAFLAGS += --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%))
>  endif
> @@ -73,7 +73,7 @@ VDSO_CAFLAGS += -DDISABLE_BRANCH_PROFILING
>  VDSO_CAFLAGS += $(call cc32-option,-march=armv8-a -D__LINUX_ARM_ARCH__=8,\
>                                     -march=armv7-a -D__LINUX_ARM_ARCH__=7)
>
> -VDSO_CFLAGS := $(VDSO_CAFLAGS)
> +VDSO_CFLAGS = $(VDSO_CAFLAGS)
>  VDSO_CFLAGS += -DENABLE_COMPAT_VDSO=1
>  # KBUILD_CFLAGS from top-level Makefile
>  VDSO_CFLAGS += -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
> @@ -108,7 +108,7 @@ else
>  VDSO_CFLAGS += -marm
>  endif
>
> -VDSO_AFLAGS := $(VDSO_CAFLAGS)
> +VDSO_AFLAGS = $(VDSO_CAFLAGS)
>  VDSO_AFLAGS += -D__ASSEMBLY__
>
>  # From arm vDSO Makefile
> --
> 2.33.0.882.g93a45727a2-goog
>


--
Best Regards
Masahiro Yamada

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 1/3] arm64: vdso32: drop the test for dmb ishld
  2021-10-12 23:46   ` Nick Desaulniers
@ 2021-10-13  7:55     ` Arnd Bergmann
  -1 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2021-10-13  7:55 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Catalin Marinas, Will Deacon, llvm, Linux Kernel Mailing List,
	Linux ARM, Vincenzo Frascino, Christian Biesinger, Simon Marchi

On Wed, Oct 13, 2021 at 1:46 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Binutils added support for this instruction in commit
> e797f7e0b2bedc9328d4a9a0ebc63ca7a2dbbebc which shipped in 2.34 (just
> missing the 2.33 release) but was cherry-picked into 2.33 in commit
> 27a50d6755bae906bc73b4ec1a8b448467f0bea1. Thanks to Christian and Simon
> for helping me with the patch archaeology.
>
> According to Documentation/process/changes.rst, the minimum supported
> version of binutils is 2.33. Since all supported versions of GAS support
> this instruction, drop the assembler invocation, preprocessor
> flags/guards, and the cross assembler macro that's now unused.
>
> This also avoids a recursive self reference in a follow up cleanup
> patch.
>
> Cc: Christian Biesinger <cbiesinger@google.com>
> Cc: Simon Marchi <simon.marchi@polymtl.ca>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

This change looks good, but I think we should do the same for the gcc
version check:

> -#if __LINUX_ARM_ARCH__ >= 8 && defined(CONFIG_AS_DMB_ISHLD)
> +#if __LINUX_ARM_ARCH__ >= 8
>  #define aarch32_smp_mb()       dmb(ish)
>  #define aarch32_smp_rmb()      dmb(ishld)
>  #define aarch32_smp_wmb()      dmb(ishst)

gcc-4.8 already supported -march=armv8, and we require gcc-5.1 now, so both
this #if/#else construct and the corresponding "cc32-option,-march=armv8-a"
check should be obsolete now.

       Arnd

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 1/3] arm64: vdso32: drop the test for dmb ishld
@ 2021-10-13  7:55     ` Arnd Bergmann
  0 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2021-10-13  7:55 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Catalin Marinas, Will Deacon, llvm, Linux Kernel Mailing List,
	Linux ARM, Vincenzo Frascino, Christian Biesinger, Simon Marchi

On Wed, Oct 13, 2021 at 1:46 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Binutils added support for this instruction in commit
> e797f7e0b2bedc9328d4a9a0ebc63ca7a2dbbebc which shipped in 2.34 (just
> missing the 2.33 release) but was cherry-picked into 2.33 in commit
> 27a50d6755bae906bc73b4ec1a8b448467f0bea1. Thanks to Christian and Simon
> for helping me with the patch archaeology.
>
> According to Documentation/process/changes.rst, the minimum supported
> version of binutils is 2.33. Since all supported versions of GAS support
> this instruction, drop the assembler invocation, preprocessor
> flags/guards, and the cross assembler macro that's now unused.
>
> This also avoids a recursive self reference in a follow up cleanup
> patch.
>
> Cc: Christian Biesinger <cbiesinger@google.com>
> Cc: Simon Marchi <simon.marchi@polymtl.ca>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

This change looks good, but I think we should do the same for the gcc
version check:

> -#if __LINUX_ARM_ARCH__ >= 8 && defined(CONFIG_AS_DMB_ISHLD)
> +#if __LINUX_ARM_ARCH__ >= 8
>  #define aarch32_smp_mb()       dmb(ish)
>  #define aarch32_smp_rmb()      dmb(ishld)
>  #define aarch32_smp_wmb()      dmb(ishst)

gcc-4.8 already supported -march=armv8, and we require gcc-5.1 now, so both
this #if/#else construct and the corresponding "cc32-option,-march=armv8-a"
check should be obsolete now.

       Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 1/3] arm64: vdso32: drop the test for dmb ishld
  2021-10-12 23:46   ` Nick Desaulniers
@ 2021-10-13 17:24     ` Christian Biesinger
  -1 siblings, 0 replies; 22+ messages in thread
From: Christian Biesinger @ 2021-10-13 17:24 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Catalin Marinas, Will Deacon, llvm, linux-kernel,
	linux-arm-kernel, Vincenzo Frascino, Simon Marchi

Hi Nick,

On Tue, Oct 12, 2021 at 7:46 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Binutils added support for this instruction in commit
> e797f7e0b2bedc9328d4a9a0ebc63ca7a2dbbebc which shipped in 2.34 (just
> missing the 2.33 release) but was cherry-picked into 2.33 in commit

Shouldn't that be 2.24 and 2.23, respectively?

> 27a50d6755bae906bc73b4ec1a8b448467f0bea1. Thanks to Christian and Simon
> for helping me with the patch archaeology.
>
> According to Documentation/process/changes.rst, the minimum supported
> version of binutils is 2.33. Since all supported versions of GAS support

(I have not checked whether this version is correct or not.)

Christian

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 1/3] arm64: vdso32: drop the test for dmb ishld
@ 2021-10-13 17:24     ` Christian Biesinger
  0 siblings, 0 replies; 22+ messages in thread
From: Christian Biesinger @ 2021-10-13 17:24 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Catalin Marinas, Will Deacon, llvm, linux-kernel,
	linux-arm-kernel, Vincenzo Frascino, Simon Marchi

Hi Nick,

On Tue, Oct 12, 2021 at 7:46 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Binutils added support for this instruction in commit
> e797f7e0b2bedc9328d4a9a0ebc63ca7a2dbbebc which shipped in 2.34 (just
> missing the 2.33 release) but was cherry-picked into 2.33 in commit

Shouldn't that be 2.24 and 2.23, respectively?

> 27a50d6755bae906bc73b4ec1a8b448467f0bea1. Thanks to Christian and Simon
> for helping me with the patch archaeology.
>
> According to Documentation/process/changes.rst, the minimum supported
> version of binutils is 2.33. Since all supported versions of GAS support

(I have not checked whether this version is correct or not.)

Christian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] arm64: vdso32: lazily invoke COMPAT_CC
  2021-10-13  3:02     ` Masahiro Yamada
@ 2021-10-14 20:59       ` Nick Desaulniers
  -1 siblings, 0 replies; 22+ messages in thread
From: Nick Desaulniers @ 2021-10-14 20:59 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Catalin Marinas, Will Deacon, llvm, Linux Kernel Mailing List,
	linux-arm-kernel, Vincenzo Frascino, Lucas Henneman

On Tue, Oct 12, 2021 at 8:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Oct 13, 2021 at 8:46 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > When running the following command without arm-linux-gnueabi-gcc in
> > one's $PATH, the following warning is observed:
> >
> > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 mrproper
> > make[1]: arm-linux-gnueabi-gcc: No such file or directory
> >
> > This is because KCONFIG is not run for mrproper, so CONFIG_CC_IS_CLANG
> > is not set, and we end up eagerly evaluating various variables that try
> > to invoke CC_COMPAT.
> >
> > This is a similar problem to what was observed in
> > commit 3ec8a5b33dea ("kbuild: do not export LDFLAGS_vmlinux")
> >
> > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > Reported-by: Lucas Henneman <henneman@google.com>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>
>
> There are two ways to fix it:
>
>   [1]: sink the error message to /dev/null
>         (as in commit dc960bfeedb01cf832c5632ed1f3daed4416b142)
>   [2]: use a recursively-expanded variable as you did.
>
>
> "Simple variable (:=) vs recursive variable (=)" is a trade-off.
>
> Please be careful about the cost when you try the [2] approach.
>
>
>
> Simple variables are immediately expanded while parsing Makefile.
> There are 7 call-sites for cc32-option, hence
> the compiler is invoked 7 times for building vdso32,
> 0 times for cleaning.
> (Since 57fd251c789647552d32d2fc51bedd4f90d70f9f,
> try-run is no-op for 'make clean').
>
>
>
>
> Recursive variables are expanded every time they are used.
>
> IIUC, if_changed expands the command line 3 times.
> There are 2 objects (note.o and vgettimeofday.o)
> There are 7 call-sites for cc32-option.
>
> So, the compiler is invoked 42 (3 * 2 * 7) times
> for building vdso32.

With this patch applied:
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
clean defconfig
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
clang | wc -l
55
$ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
clang | wc -l
2

Prior to this series:
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
clean defconfig
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
clang | wc -l
55
$ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
clang | wc -l
2

With patch 3 applied, we can drop CROSS_COMPILE_COMPAT, and we now get:
$ ARCH=arm64 make LLVM=1 -j72 clean defconfig
ARCH=arm64 make LLVM=1 -j72 arch/arm64/kernel/vdso32/ V=1 | tr -s ' '
| cut -d ' ' -f 2 | grep clang | wc -l
44
$ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
$ ARCH=arm64 make LLVM=1 -j72 arch/arm64/kernel/vdso32/ V=1 | tr -s '
' | cut -d ' ' -f 2 | grep clang | wc -l
2

Please confirm; perhaps my pipeline missed some invocations? Or was
there a different target you were referring to?

--
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] arm64: vdso32: lazily invoke COMPAT_CC
@ 2021-10-14 20:59       ` Nick Desaulniers
  0 siblings, 0 replies; 22+ messages in thread
From: Nick Desaulniers @ 2021-10-14 20:59 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Catalin Marinas, Will Deacon, llvm, Linux Kernel Mailing List,
	linux-arm-kernel, Vincenzo Frascino, Lucas Henneman

On Tue, Oct 12, 2021 at 8:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Oct 13, 2021 at 8:46 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > When running the following command without arm-linux-gnueabi-gcc in
> > one's $PATH, the following warning is observed:
> >
> > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 mrproper
> > make[1]: arm-linux-gnueabi-gcc: No such file or directory
> >
> > This is because KCONFIG is not run for mrproper, so CONFIG_CC_IS_CLANG
> > is not set, and we end up eagerly evaluating various variables that try
> > to invoke CC_COMPAT.
> >
> > This is a similar problem to what was observed in
> > commit 3ec8a5b33dea ("kbuild: do not export LDFLAGS_vmlinux")
> >
> > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > Reported-by: Lucas Henneman <henneman@google.com>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>
>
> There are two ways to fix it:
>
>   [1]: sink the error message to /dev/null
>         (as in commit dc960bfeedb01cf832c5632ed1f3daed4416b142)
>   [2]: use a recursively-expanded variable as you did.
>
>
> "Simple variable (:=) vs recursive variable (=)" is a trade-off.
>
> Please be careful about the cost when you try the [2] approach.
>
>
>
> Simple variables are immediately expanded while parsing Makefile.
> There are 7 call-sites for cc32-option, hence
> the compiler is invoked 7 times for building vdso32,
> 0 times for cleaning.
> (Since 57fd251c789647552d32d2fc51bedd4f90d70f9f,
> try-run is no-op for 'make clean').
>
>
>
>
> Recursive variables are expanded every time they are used.
>
> IIUC, if_changed expands the command line 3 times.
> There are 2 objects (note.o and vgettimeofday.o)
> There are 7 call-sites for cc32-option.
>
> So, the compiler is invoked 42 (3 * 2 * 7) times
> for building vdso32.

With this patch applied:
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
clean defconfig
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
clang | wc -l
55
$ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
clang | wc -l
2

Prior to this series:
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
clean defconfig
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
clang | wc -l
55
$ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
$ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
clang | wc -l
2

With patch 3 applied, we can drop CROSS_COMPILE_COMPAT, and we now get:
$ ARCH=arm64 make LLVM=1 -j72 clean defconfig
ARCH=arm64 make LLVM=1 -j72 arch/arm64/kernel/vdso32/ V=1 | tr -s ' '
| cut -d ' ' -f 2 | grep clang | wc -l
44
$ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
$ ARCH=arm64 make LLVM=1 -j72 arch/arm64/kernel/vdso32/ V=1 | tr -s '
' | cut -d ' ' -f 2 | grep clang | wc -l
2

Please confirm; perhaps my pipeline missed some invocations? Or was
there a different target you were referring to?

--
Thanks,
~Nick Desaulniers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] arm64: vdso32: lazily invoke COMPAT_CC
  2021-10-14 20:59       ` Nick Desaulniers
@ 2021-10-16 14:19         ` Masahiro Yamada
  -1 siblings, 0 replies; 22+ messages in thread
From: Masahiro Yamada @ 2021-10-16 14:19 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Catalin Marinas, Will Deacon, llvm, Linux Kernel Mailing List,
	linux-arm-kernel, Vincenzo Frascino, Lucas Henneman

On Fri, Oct 15, 2021 at 5:59 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Tue, Oct 12, 2021 at 8:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Wed, Oct 13, 2021 at 8:46 AM Nick Desaulniers
> > <ndesaulniers@google.com> wrote:
> > >
> > > When running the following command without arm-linux-gnueabi-gcc in
> > > one's $PATH, the following warning is observed:
> > >
> > > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 mrproper
> > > make[1]: arm-linux-gnueabi-gcc: No such file or directory
> > >
> > > This is because KCONFIG is not run for mrproper, so CONFIG_CC_IS_CLANG
> > > is not set, and we end up eagerly evaluating various variables that try
> > > to invoke CC_COMPAT.
> > >
> > > This is a similar problem to what was observed in
> > > commit 3ec8a5b33dea ("kbuild: do not export LDFLAGS_vmlinux")
> > >
> > > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > > Reported-by: Lucas Henneman <henneman@google.com>
> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> >
> >
> > There are two ways to fix it:
> >
> >   [1]: sink the error message to /dev/null
> >         (as in commit dc960bfeedb01cf832c5632ed1f3daed4416b142)
> >   [2]: use a recursively-expanded variable as you did.
> >
> >
> > "Simple variable (:=) vs recursive variable (=)" is a trade-off.
> >
> > Please be careful about the cost when you try the [2] approach.
> >
> >
> >
> > Simple variables are immediately expanded while parsing Makefile.
> > There are 7 call-sites for cc32-option, hence
> > the compiler is invoked 7 times for building vdso32,
> > 0 times for cleaning.
> > (Since 57fd251c789647552d32d2fc51bedd4f90d70f9f,
> > try-run is no-op for 'make clean').
> >
> >
> >
> >
> > Recursive variables are expanded every time they are used.
> >
> > IIUC, if_changed expands the command line 3 times.
> > There are 2 objects (note.o and vgettimeofday.o)
> > There are 7 call-sites for cc32-option.
> >
> > So, the compiler is invoked 42 (3 * 2 * 7) times
> > for building vdso32.
>
> With this patch applied:
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> clean defconfig
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> clang | wc -l
> 55
> $ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> clang | wc -l
> 2
>
> Prior to this series:
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> clean defconfig
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> clang | wc -l
> 55
> $ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> clang | wc -l
> 2
>
> With patch 3 applied, we can drop CROSS_COMPILE_COMPAT, and we now get:
> $ ARCH=arm64 make LLVM=1 -j72 clean defconfig
> ARCH=arm64 make LLVM=1 -j72 arch/arm64/kernel/vdso32/ V=1 | tr -s ' '
> | cut -d ' ' -f 2 | grep clang | wc -l
> 44
> $ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
> $ ARCH=arm64 make LLVM=1 -j72 arch/arm64/kernel/vdso32/ V=1 | tr -s '
> ' | cut -d ' ' -f 2 | grep clang | wc -l
> 2
>
> Please confirm; perhaps my pipeline missed some invocations? Or was
> there a different target you were referring to?






It is pointless to check the build commands.

I am talking about how many times $(call cc32-option, ) is evaluated.


How about adding the following debug code?

(Everytime cc32-option is evaluated, a file "dummy-cc32-option-<PID>"
is created)



diff --git a/arch/arm64/kernel/vdso32/Makefile
b/arch/arm64/kernel/vdso32/Makefile
index 89299a26638b..e40365f5bc38 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -26,9 +26,9 @@ LD_COMPAT ?= $(CROSS_COMPILE_COMPAT)ld
 endif

 cc32-option = $(call try-run,\
-        $(CC_COMPAT) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
+        $(CC_COMPAT) $(1) -c -x c /dev/null -o "$$TMP"; touch
dummy-cc32-option-$$$$,$(1),$(2))
 cc32-disable-warning = $(call try-run,\
-       $(CC_COMPAT) -W$(strip $(1)) -c -x c /dev/null -o
"$$TMP",-Wno-$(strip $(1)))
+       $(CC_COMPAT) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP";
touch dummy-cc32-option-$$$$,-Wno-$(strip $(1)))

 # We cannot use the global flags to compile the vDSO files, the main reason
 # being that the 32-bit compiler may be older than the main (64-bit) compiler





Without this patch:

masahiro@grover:~/ref/linux$ rm dummy-cc32-*
masahiro@grover:~/ref/linux$ make -s LLVM=1 ARCH=arm64
CROSS_COMPILE_COMPAT=arm-linux-gnueabi- defconfig clean
arch/arm64/kernel/vdso32/ -j8
masahiro@grover:~/ref/linux$ ls -1 dummy-cc32-*
dummy-cc32-disable-warning-765530
dummy-cc32-option-765495
dummy-cc32-option-765500
dummy-cc32-option-765505
dummy-cc32-option-765510
dummy-cc32-option-765515
dummy-cc32-option-765520
dummy-cc32-option-765525




With this patch:



masahiro@grover:~/ref/linux$ rm dummy-cc32-*
masahiro@grover:~/ref/linux$ make -s LLVM=1 ARCH=arm64
CROSS_COMPILE_COMPAT=arm-linux-gnueabi- defconfig clean
arch/arm64/kernel/vdso32/ -j8
masahiro@grover:~/ref/linux$ ls -1 dummy-cc32-*
dummy-cc32-disable-warning-768908
dummy-cc32-disable-warning-768949
dummy-cc32-disable-warning-768990
dummy-cc32-disable-warning-769035
dummy-cc32-disable-warning-769076
dummy-cc32-disable-warning-769117
dummy-cc32-option-768871
dummy-cc32-option-768878
dummy-cc32-option-768883
dummy-cc32-option-768888
dummy-cc32-option-768893
dummy-cc32-option-768898
dummy-cc32-option-768903
dummy-cc32-option-768914
dummy-cc32-option-768919
dummy-cc32-option-768924
dummy-cc32-option-768929
dummy-cc32-option-768934
dummy-cc32-option-768939
dummy-cc32-option-768944
dummy-cc32-option-768955
dummy-cc32-option-768960
dummy-cc32-option-768965
dummy-cc32-option-768970
dummy-cc32-option-768975
dummy-cc32-option-768980
dummy-cc32-option-768985
dummy-cc32-option-768998
dummy-cc32-option-769005
dummy-cc32-option-769010
dummy-cc32-option-769015
dummy-cc32-option-769020
dummy-cc32-option-769025
dummy-cc32-option-769030
dummy-cc32-option-769041
dummy-cc32-option-769046
dummy-cc32-option-769051
dummy-cc32-option-769056
dummy-cc32-option-769061
dummy-cc32-option-769066
dummy-cc32-option-769071
dummy-cc32-option-769082
dummy-cc32-option-769087
dummy-cc32-option-769092
dummy-cc32-option-769097
dummy-cc32-option-769102
dummy-cc32-option-769107
dummy-cc32-option-769112






The diff of  the number of expansions:

cc32-option                        7    ->   42
cc32-disable-warning         1   ->    6





-- 
Best Regards
Masahiro Yamada

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] arm64: vdso32: lazily invoke COMPAT_CC
@ 2021-10-16 14:19         ` Masahiro Yamada
  0 siblings, 0 replies; 22+ messages in thread
From: Masahiro Yamada @ 2021-10-16 14:19 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Catalin Marinas, Will Deacon, llvm, Linux Kernel Mailing List,
	linux-arm-kernel, Vincenzo Frascino, Lucas Henneman

On Fri, Oct 15, 2021 at 5:59 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Tue, Oct 12, 2021 at 8:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Wed, Oct 13, 2021 at 8:46 AM Nick Desaulniers
> > <ndesaulniers@google.com> wrote:
> > >
> > > When running the following command without arm-linux-gnueabi-gcc in
> > > one's $PATH, the following warning is observed:
> > >
> > > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 mrproper
> > > make[1]: arm-linux-gnueabi-gcc: No such file or directory
> > >
> > > This is because KCONFIG is not run for mrproper, so CONFIG_CC_IS_CLANG
> > > is not set, and we end up eagerly evaluating various variables that try
> > > to invoke CC_COMPAT.
> > >
> > > This is a similar problem to what was observed in
> > > commit 3ec8a5b33dea ("kbuild: do not export LDFLAGS_vmlinux")
> > >
> > > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > > Reported-by: Lucas Henneman <henneman@google.com>
> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> >
> >
> > There are two ways to fix it:
> >
> >   [1]: sink the error message to /dev/null
> >         (as in commit dc960bfeedb01cf832c5632ed1f3daed4416b142)
> >   [2]: use a recursively-expanded variable as you did.
> >
> >
> > "Simple variable (:=) vs recursive variable (=)" is a trade-off.
> >
> > Please be careful about the cost when you try the [2] approach.
> >
> >
> >
> > Simple variables are immediately expanded while parsing Makefile.
> > There are 7 call-sites for cc32-option, hence
> > the compiler is invoked 7 times for building vdso32,
> > 0 times for cleaning.
> > (Since 57fd251c789647552d32d2fc51bedd4f90d70f9f,
> > try-run is no-op for 'make clean').
> >
> >
> >
> >
> > Recursive variables are expanded every time they are used.
> >
> > IIUC, if_changed expands the command line 3 times.
> > There are 2 objects (note.o and vgettimeofday.o)
> > There are 7 call-sites for cc32-option.
> >
> > So, the compiler is invoked 42 (3 * 2 * 7) times
> > for building vdso32.
>
> With this patch applied:
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> clean defconfig
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> clang | wc -l
> 55
> $ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> clang | wc -l
> 2
>
> Prior to this series:
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> clean defconfig
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> clang | wc -l
> 55
> $ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> clang | wc -l
> 2
>
> With patch 3 applied, we can drop CROSS_COMPILE_COMPAT, and we now get:
> $ ARCH=arm64 make LLVM=1 -j72 clean defconfig
> ARCH=arm64 make LLVM=1 -j72 arch/arm64/kernel/vdso32/ V=1 | tr -s ' '
> | cut -d ' ' -f 2 | grep clang | wc -l
> 44
> $ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
> $ ARCH=arm64 make LLVM=1 -j72 arch/arm64/kernel/vdso32/ V=1 | tr -s '
> ' | cut -d ' ' -f 2 | grep clang | wc -l
> 2
>
> Please confirm; perhaps my pipeline missed some invocations? Or was
> there a different target you were referring to?






It is pointless to check the build commands.

I am talking about how many times $(call cc32-option, ) is evaluated.


How about adding the following debug code?

(Everytime cc32-option is evaluated, a file "dummy-cc32-option-<PID>"
is created)



diff --git a/arch/arm64/kernel/vdso32/Makefile
b/arch/arm64/kernel/vdso32/Makefile
index 89299a26638b..e40365f5bc38 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -26,9 +26,9 @@ LD_COMPAT ?= $(CROSS_COMPILE_COMPAT)ld
 endif

 cc32-option = $(call try-run,\
-        $(CC_COMPAT) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
+        $(CC_COMPAT) $(1) -c -x c /dev/null -o "$$TMP"; touch
dummy-cc32-option-$$$$,$(1),$(2))
 cc32-disable-warning = $(call try-run,\
-       $(CC_COMPAT) -W$(strip $(1)) -c -x c /dev/null -o
"$$TMP",-Wno-$(strip $(1)))
+       $(CC_COMPAT) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP";
touch dummy-cc32-option-$$$$,-Wno-$(strip $(1)))

 # We cannot use the global flags to compile the vDSO files, the main reason
 # being that the 32-bit compiler may be older than the main (64-bit) compiler





Without this patch:

masahiro@grover:~/ref/linux$ rm dummy-cc32-*
masahiro@grover:~/ref/linux$ make -s LLVM=1 ARCH=arm64
CROSS_COMPILE_COMPAT=arm-linux-gnueabi- defconfig clean
arch/arm64/kernel/vdso32/ -j8
masahiro@grover:~/ref/linux$ ls -1 dummy-cc32-*
dummy-cc32-disable-warning-765530
dummy-cc32-option-765495
dummy-cc32-option-765500
dummy-cc32-option-765505
dummy-cc32-option-765510
dummy-cc32-option-765515
dummy-cc32-option-765520
dummy-cc32-option-765525




With this patch:



masahiro@grover:~/ref/linux$ rm dummy-cc32-*
masahiro@grover:~/ref/linux$ make -s LLVM=1 ARCH=arm64
CROSS_COMPILE_COMPAT=arm-linux-gnueabi- defconfig clean
arch/arm64/kernel/vdso32/ -j8
masahiro@grover:~/ref/linux$ ls -1 dummy-cc32-*
dummy-cc32-disable-warning-768908
dummy-cc32-disable-warning-768949
dummy-cc32-disable-warning-768990
dummy-cc32-disable-warning-769035
dummy-cc32-disable-warning-769076
dummy-cc32-disable-warning-769117
dummy-cc32-option-768871
dummy-cc32-option-768878
dummy-cc32-option-768883
dummy-cc32-option-768888
dummy-cc32-option-768893
dummy-cc32-option-768898
dummy-cc32-option-768903
dummy-cc32-option-768914
dummy-cc32-option-768919
dummy-cc32-option-768924
dummy-cc32-option-768929
dummy-cc32-option-768934
dummy-cc32-option-768939
dummy-cc32-option-768944
dummy-cc32-option-768955
dummy-cc32-option-768960
dummy-cc32-option-768965
dummy-cc32-option-768970
dummy-cc32-option-768975
dummy-cc32-option-768980
dummy-cc32-option-768985
dummy-cc32-option-768998
dummy-cc32-option-769005
dummy-cc32-option-769010
dummy-cc32-option-769015
dummy-cc32-option-769020
dummy-cc32-option-769025
dummy-cc32-option-769030
dummy-cc32-option-769041
dummy-cc32-option-769046
dummy-cc32-option-769051
dummy-cc32-option-769056
dummy-cc32-option-769061
dummy-cc32-option-769066
dummy-cc32-option-769071
dummy-cc32-option-769082
dummy-cc32-option-769087
dummy-cc32-option-769092
dummy-cc32-option-769097
dummy-cc32-option-769102
dummy-cc32-option-769107
dummy-cc32-option-769112






The diff of  the number of expansions:

cc32-option                        7    ->   42
cc32-disable-warning         1   ->    6





-- 
Best Regards
Masahiro Yamada

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] arm64: vdso32: lazily invoke COMPAT_CC
  2021-10-16 14:19         ` Masahiro Yamada
@ 2021-10-18 20:34           ` Nick Desaulniers
  -1 siblings, 0 replies; 22+ messages in thread
From: Nick Desaulniers @ 2021-10-18 20:34 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Catalin Marinas, Will Deacon, llvm, Linux Kernel Mailing List,
	linux-arm-kernel, Vincenzo Frascino, Lucas Henneman

On Sat, Oct 16, 2021 at 7:20 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Fri, Oct 15, 2021 at 5:59 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Tue, Oct 12, 2021 at 8:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > On Wed, Oct 13, 2021 at 8:46 AM Nick Desaulniers
> > > <ndesaulniers@google.com> wrote:
> > > >
> > > > When running the following command without arm-linux-gnueabi-gcc in
> > > > one's $PATH, the following warning is observed:
> > > >
> > > > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 mrproper
> > > > make[1]: arm-linux-gnueabi-gcc: No such file or directory
> > > >
> > > > This is because KCONFIG is not run for mrproper, so CONFIG_CC_IS_CLANG
> > > > is not set, and we end up eagerly evaluating various variables that try
> > > > to invoke CC_COMPAT.
> > > >
> > > > This is a similar problem to what was observed in
> > > > commit 3ec8a5b33dea ("kbuild: do not export LDFLAGS_vmlinux")
> > > >
> > > > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > > > Reported-by: Lucas Henneman <henneman@google.com>
> > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > >
> > >
> > > There are two ways to fix it:
> > >
> > >   [1]: sink the error message to /dev/null
> > >         (as in commit dc960bfeedb01cf832c5632ed1f3daed4416b142)
> > >   [2]: use a recursively-expanded variable as you did.
> > >
> > >
> > > "Simple variable (:=) vs recursive variable (=)" is a trade-off.
> > >
> > > Please be careful about the cost when you try the [2] approach.
> > >
> > >
> > >
> > > Simple variables are immediately expanded while parsing Makefile.
> > > There are 7 call-sites for cc32-option, hence
> > > the compiler is invoked 7 times for building vdso32,
> > > 0 times for cleaning.
> > > (Since 57fd251c789647552d32d2fc51bedd4f90d70f9f,
> > > try-run is no-op for 'make clean').
> > >
> > >
> > >
> > >
> > > Recursive variables are expanded every time they are used.
> > >
> > > IIUC, if_changed expands the command line 3 times.
> > > There are 2 objects (note.o and vgettimeofday.o)
> > > There are 7 call-sites for cc32-option.
> > >
> > > So, the compiler is invoked 42 (3 * 2 * 7) times
> > > for building vdso32.
> >
> > With this patch applied:
> > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> > clean defconfig
> > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> > arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> > clang | wc -l
> > 55
> > $ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
> > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> > arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> > clang | wc -l
> > 2
> >
> > Prior to this series:
> > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> > clean defconfig
> > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> > arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> > clang | wc -l
> > 55
> > $ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
> > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> > arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> > clang | wc -l
> > 2
> >
> > With patch 3 applied, we can drop CROSS_COMPILE_COMPAT, and we now get:
> > $ ARCH=arm64 make LLVM=1 -j72 clean defconfig
> > ARCH=arm64 make LLVM=1 -j72 arch/arm64/kernel/vdso32/ V=1 | tr -s ' '
> > | cut -d ' ' -f 2 | grep clang | wc -l
> > 44
> > $ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
> > $ ARCH=arm64 make LLVM=1 -j72 arch/arm64/kernel/vdso32/ V=1 | tr -s '
> > ' | cut -d ' ' -f 2 | grep clang | wc -l
> > 2
> >
> > Please confirm; perhaps my pipeline missed some invocations? Or was
> > there a different target you were referring to?
>
>
>
>
>
>
> It is pointless to check the build commands.
>
> I am talking about how many times $(call cc32-option, ) is evaluated.

Of course V=1 doesn't print the cc-option invocations! /s

>
>
> How about adding the following debug code?
>
> (Everytime cc32-option is evaluated, a file "dummy-cc32-option-<PID>"
> is created)
>
>
>
> diff --git a/arch/arm64/kernel/vdso32/Makefile
> b/arch/arm64/kernel/vdso32/Makefile
> index 89299a26638b..e40365f5bc38 100644
> --- a/arch/arm64/kernel/vdso32/Makefile
> +++ b/arch/arm64/kernel/vdso32/Makefile
> @@ -26,9 +26,9 @@ LD_COMPAT ?= $(CROSS_COMPILE_COMPAT)ld
>  endif
>
>  cc32-option = $(call try-run,\
> -        $(CC_COMPAT) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
> +        $(CC_COMPAT) $(1) -c -x c /dev/null -o "$$TMP"; touch
> dummy-cc32-option-$$$$,$(1),$(2))
>  cc32-disable-warning = $(call try-run,\
> -       $(CC_COMPAT) -W$(strip $(1)) -c -x c /dev/null -o
> "$$TMP",-Wno-$(strip $(1)))
> +       $(CC_COMPAT) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP";
> touch dummy-cc32-option-$$$$,-Wno-$(strip $(1)))

heh, I usually add `($info $$VAR is [${VAR}]); \` debugging statements
to these. Touching files is another neat trick.

>
>  # We cannot use the global flags to compile the vDSO files, the main reason
>  # being that the 32-bit compiler may be older than the main (64-bit) compiler
>
>
>
>
>
> Without this patch:
>
> masahiro@grover:~/ref/linux$ rm dummy-cc32-*
> masahiro@grover:~/ref/linux$ make -s LLVM=1 ARCH=arm64
> CROSS_COMPILE_COMPAT=arm-linux-gnueabi- defconfig clean
> arch/arm64/kernel/vdso32/ -j8
> masahiro@grover:~/ref/linux$ ls -1 dummy-cc32-*
> dummy-cc32-disable-warning-765530
> dummy-cc32-option-765495
> dummy-cc32-option-765500
> dummy-cc32-option-765505
> dummy-cc32-option-765510
> dummy-cc32-option-765515
> dummy-cc32-option-765520
> dummy-cc32-option-765525
>
>
>
>
> With this patch:
>
>
>
> masahiro@grover:~/ref/linux$ rm dummy-cc32-*
> masahiro@grover:~/ref/linux$ make -s LLVM=1 ARCH=arm64
> CROSS_COMPILE_COMPAT=arm-linux-gnueabi- defconfig clean
> arch/arm64/kernel/vdso32/ -j8
> masahiro@grover:~/ref/linux$ ls -1 dummy-cc32-*
> dummy-cc32-disable-warning-768908
> dummy-cc32-disable-warning-768949
> dummy-cc32-disable-warning-768990
> dummy-cc32-disable-warning-769035
> dummy-cc32-disable-warning-769076
> dummy-cc32-disable-warning-769117
> dummy-cc32-option-768871
> dummy-cc32-option-768878
> dummy-cc32-option-768883
> dummy-cc32-option-768888
> dummy-cc32-option-768893
> dummy-cc32-option-768898
> dummy-cc32-option-768903
> dummy-cc32-option-768914
> dummy-cc32-option-768919
> dummy-cc32-option-768924
> dummy-cc32-option-768929
> dummy-cc32-option-768934
> dummy-cc32-option-768939
> dummy-cc32-option-768944
> dummy-cc32-option-768955
> dummy-cc32-option-768960
> dummy-cc32-option-768965
> dummy-cc32-option-768970
> dummy-cc32-option-768975
> dummy-cc32-option-768980
> dummy-cc32-option-768985
> dummy-cc32-option-768998
> dummy-cc32-option-769005
> dummy-cc32-option-769010
> dummy-cc32-option-769015
> dummy-cc32-option-769020
> dummy-cc32-option-769025
> dummy-cc32-option-769030
> dummy-cc32-option-769041
> dummy-cc32-option-769046
> dummy-cc32-option-769051
> dummy-cc32-option-769056
> dummy-cc32-option-769061
> dummy-cc32-option-769066
> dummy-cc32-option-769071
> dummy-cc32-option-769082
> dummy-cc32-option-769087
> dummy-cc32-option-769092
> dummy-cc32-option-769097
> dummy-cc32-option-769102
> dummy-cc32-option-769107
> dummy-cc32-option-769112
>
>
>
>
>
>
> The diff of  the number of expansions:
>
> cc32-option                        7    ->   42
> cc32-disable-warning         1   ->    6

Indeed, thanks for confirming.  An unfortunate dichotomy.  Surely
there's another way to avoid cc-option+cc-disable-warning calls for
the `clean` target?  I'll change to just redirecting errors to
/dev/null with your Suggested-by tag, but this seems a bit of an
unfortunate situation.
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] arm64: vdso32: lazily invoke COMPAT_CC
@ 2021-10-18 20:34           ` Nick Desaulniers
  0 siblings, 0 replies; 22+ messages in thread
From: Nick Desaulniers @ 2021-10-18 20:34 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Catalin Marinas, Will Deacon, llvm, Linux Kernel Mailing List,
	linux-arm-kernel, Vincenzo Frascino, Lucas Henneman

On Sat, Oct 16, 2021 at 7:20 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Fri, Oct 15, 2021 at 5:59 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Tue, Oct 12, 2021 at 8:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > On Wed, Oct 13, 2021 at 8:46 AM Nick Desaulniers
> > > <ndesaulniers@google.com> wrote:
> > > >
> > > > When running the following command without arm-linux-gnueabi-gcc in
> > > > one's $PATH, the following warning is observed:
> > > >
> > > > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 mrproper
> > > > make[1]: arm-linux-gnueabi-gcc: No such file or directory
> > > >
> > > > This is because KCONFIG is not run for mrproper, so CONFIG_CC_IS_CLANG
> > > > is not set, and we end up eagerly evaluating various variables that try
> > > > to invoke CC_COMPAT.
> > > >
> > > > This is a similar problem to what was observed in
> > > > commit 3ec8a5b33dea ("kbuild: do not export LDFLAGS_vmlinux")
> > > >
> > > > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > > > Reported-by: Lucas Henneman <henneman@google.com>
> > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > >
> > >
> > > There are two ways to fix it:
> > >
> > >   [1]: sink the error message to /dev/null
> > >         (as in commit dc960bfeedb01cf832c5632ed1f3daed4416b142)
> > >   [2]: use a recursively-expanded variable as you did.
> > >
> > >
> > > "Simple variable (:=) vs recursive variable (=)" is a trade-off.
> > >
> > > Please be careful about the cost when you try the [2] approach.
> > >
> > >
> > >
> > > Simple variables are immediately expanded while parsing Makefile.
> > > There are 7 call-sites for cc32-option, hence
> > > the compiler is invoked 7 times for building vdso32,
> > > 0 times for cleaning.
> > > (Since 57fd251c789647552d32d2fc51bedd4f90d70f9f,
> > > try-run is no-op for 'make clean').
> > >
> > >
> > >
> > >
> > > Recursive variables are expanded every time they are used.
> > >
> > > IIUC, if_changed expands the command line 3 times.
> > > There are 2 objects (note.o and vgettimeofday.o)
> > > There are 7 call-sites for cc32-option.
> > >
> > > So, the compiler is invoked 42 (3 * 2 * 7) times
> > > for building vdso32.
> >
> > With this patch applied:
> > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> > clean defconfig
> > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> > arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> > clang | wc -l
> > 55
> > $ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
> > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> > arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> > clang | wc -l
> > 2
> >
> > Prior to this series:
> > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> > clean defconfig
> > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> > arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> > clang | wc -l
> > 55
> > $ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
> > $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72
> > arch/arm64/kernel/vdso32/ V=1 | tr -s ' ' | cut -d ' ' -f 2 | grep
> > clang | wc -l
> > 2
> >
> > With patch 3 applied, we can drop CROSS_COMPILE_COMPAT, and we now get:
> > $ ARCH=arm64 make LLVM=1 -j72 clean defconfig
> > ARCH=arm64 make LLVM=1 -j72 arch/arm64/kernel/vdso32/ V=1 | tr -s ' '
> > | cut -d ' ' -f 2 | grep clang | wc -l
> > 44
> > $ find arch/arm64/kernel/vdso32/ -name \*.o | xargs rm
> > $ ARCH=arm64 make LLVM=1 -j72 arch/arm64/kernel/vdso32/ V=1 | tr -s '
> > ' | cut -d ' ' -f 2 | grep clang | wc -l
> > 2
> >
> > Please confirm; perhaps my pipeline missed some invocations? Or was
> > there a different target you were referring to?
>
>
>
>
>
>
> It is pointless to check the build commands.
>
> I am talking about how many times $(call cc32-option, ) is evaluated.

Of course V=1 doesn't print the cc-option invocations! /s

>
>
> How about adding the following debug code?
>
> (Everytime cc32-option is evaluated, a file "dummy-cc32-option-<PID>"
> is created)
>
>
>
> diff --git a/arch/arm64/kernel/vdso32/Makefile
> b/arch/arm64/kernel/vdso32/Makefile
> index 89299a26638b..e40365f5bc38 100644
> --- a/arch/arm64/kernel/vdso32/Makefile
> +++ b/arch/arm64/kernel/vdso32/Makefile
> @@ -26,9 +26,9 @@ LD_COMPAT ?= $(CROSS_COMPILE_COMPAT)ld
>  endif
>
>  cc32-option = $(call try-run,\
> -        $(CC_COMPAT) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
> +        $(CC_COMPAT) $(1) -c -x c /dev/null -o "$$TMP"; touch
> dummy-cc32-option-$$$$,$(1),$(2))
>  cc32-disable-warning = $(call try-run,\
> -       $(CC_COMPAT) -W$(strip $(1)) -c -x c /dev/null -o
> "$$TMP",-Wno-$(strip $(1)))
> +       $(CC_COMPAT) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP";
> touch dummy-cc32-option-$$$$,-Wno-$(strip $(1)))

heh, I usually add `($info $$VAR is [${VAR}]); \` debugging statements
to these. Touching files is another neat trick.

>
>  # We cannot use the global flags to compile the vDSO files, the main reason
>  # being that the 32-bit compiler may be older than the main (64-bit) compiler
>
>
>
>
>
> Without this patch:
>
> masahiro@grover:~/ref/linux$ rm dummy-cc32-*
> masahiro@grover:~/ref/linux$ make -s LLVM=1 ARCH=arm64
> CROSS_COMPILE_COMPAT=arm-linux-gnueabi- defconfig clean
> arch/arm64/kernel/vdso32/ -j8
> masahiro@grover:~/ref/linux$ ls -1 dummy-cc32-*
> dummy-cc32-disable-warning-765530
> dummy-cc32-option-765495
> dummy-cc32-option-765500
> dummy-cc32-option-765505
> dummy-cc32-option-765510
> dummy-cc32-option-765515
> dummy-cc32-option-765520
> dummy-cc32-option-765525
>
>
>
>
> With this patch:
>
>
>
> masahiro@grover:~/ref/linux$ rm dummy-cc32-*
> masahiro@grover:~/ref/linux$ make -s LLVM=1 ARCH=arm64
> CROSS_COMPILE_COMPAT=arm-linux-gnueabi- defconfig clean
> arch/arm64/kernel/vdso32/ -j8
> masahiro@grover:~/ref/linux$ ls -1 dummy-cc32-*
> dummy-cc32-disable-warning-768908
> dummy-cc32-disable-warning-768949
> dummy-cc32-disable-warning-768990
> dummy-cc32-disable-warning-769035
> dummy-cc32-disable-warning-769076
> dummy-cc32-disable-warning-769117
> dummy-cc32-option-768871
> dummy-cc32-option-768878
> dummy-cc32-option-768883
> dummy-cc32-option-768888
> dummy-cc32-option-768893
> dummy-cc32-option-768898
> dummy-cc32-option-768903
> dummy-cc32-option-768914
> dummy-cc32-option-768919
> dummy-cc32-option-768924
> dummy-cc32-option-768929
> dummy-cc32-option-768934
> dummy-cc32-option-768939
> dummy-cc32-option-768944
> dummy-cc32-option-768955
> dummy-cc32-option-768960
> dummy-cc32-option-768965
> dummy-cc32-option-768970
> dummy-cc32-option-768975
> dummy-cc32-option-768980
> dummy-cc32-option-768985
> dummy-cc32-option-768998
> dummy-cc32-option-769005
> dummy-cc32-option-769010
> dummy-cc32-option-769015
> dummy-cc32-option-769020
> dummy-cc32-option-769025
> dummy-cc32-option-769030
> dummy-cc32-option-769041
> dummy-cc32-option-769046
> dummy-cc32-option-769051
> dummy-cc32-option-769056
> dummy-cc32-option-769061
> dummy-cc32-option-769066
> dummy-cc32-option-769071
> dummy-cc32-option-769082
> dummy-cc32-option-769087
> dummy-cc32-option-769092
> dummy-cc32-option-769097
> dummy-cc32-option-769102
> dummy-cc32-option-769107
> dummy-cc32-option-769112
>
>
>
>
>
>
> The diff of  the number of expansions:
>
> cc32-option                        7    ->   42
> cc32-disable-warning         1   ->    6

Indeed, thanks for confirming.  An unfortunate dichotomy.  Surely
there's another way to avoid cc-option+cc-disable-warning calls for
the `clean` target?  I'll change to just redirecting errors to
/dev/null with your Suggested-by tag, but this seems a bit of an
unfortunate situation.
-- 
Thanks,
~Nick Desaulniers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 3/3] arm64: vdso32: require CROSS_COMPILE_COMPAT for gcc+bfd
  2021-10-12 23:46   ` Nick Desaulniers
@ 2021-10-18 22:58     ` Nathan Chancellor
  -1 siblings, 0 replies; 22+ messages in thread
From: Nathan Chancellor @ 2021-10-18 22:58 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Catalin Marinas, Will Deacon, llvm, linux-kernel,
	linux-arm-kernel, Vincenzo Frascino

On Tue, Oct 12, 2021 at 04:46:06PM -0700, Nick Desaulniers wrote:
> Similar to
> commit 231ad7f409f1 ("Makefile: infer --target from ARCH for CC=clang")
> There really is no point in setting --target based on
> $CROSS_COMPILE_COMPAT for clang when the integrated assembler is being
> used.

It might be nice to mention commit ef94340583ee ("arm64: vdso32: drop
-no-integrated-as flag") here, as that is what flipped on the integrated
assembler for this Makefile (and it cannot be turned off).

> Allows COMPAT_VDSO to be selected without setting $CROSS_COMPILE_COMPAT
> when using clang and lld together.
> 
> Before:
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 defconfig
> $ grep CONFIG_COMPAT_VDSO .config
> CONFIG_COMPAT_VDSO=y
> $ ARCH=arm64 make -j72 LLVM=1 defconfig
> $ grep CONFIG_COMPAT_VDSO .config
> $
> 
> After:
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 defconfig
> $ grep CONFIG_COMPAT_VDSO .config
> CONFIG_COMPAT_VDSO=y
> $ ARCH=arm64 make -j72 LLVM=1 defconfig
> $ grep CONFIG_COMPAT_VDSO .config
> CONFIG_COMPAT_VDSO=y
> 
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Modulo these two nits, this works for me too.

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

> ---
>  arch/arm64/Kconfig                |  3 ++-
>  arch/arm64/kernel/vdso32/Makefile | 17 +++++------------
>  2 files changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 5c7ae4c3954b..7b28dad2fb80 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1264,7 +1264,8 @@ config KUSER_HELPERS
>  
>  config COMPAT_VDSO
>  	bool "Enable vDSO for 32-bit applications"
> -	depends on !CPU_BIG_ENDIAN && "$(CROSS_COMPILE_COMPAT)" != ""
> +	depends on !CPU_BIG_ENDIAN
> +	depends on CC_IS_CLANG && LD_IS_LLD || "$(CROSS_COMPILE_COMPAT)" != ""

It works fine as is but it might be nice to add parentheses around the
new condition for ease of understanding.

>  	select GENERIC_COMPAT_VDSO
>  	default y
>  	help
> diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
> index d24b12318f4c..376261d3791f 100644
> --- a/arch/arm64/kernel/vdso32/Makefile
> +++ b/arch/arm64/kernel/vdso32/Makefile
> @@ -10,18 +10,15 @@ include $(srctree)/lib/vdso/Makefile
>  
>  # Same as cc-*option, but using CC_COMPAT instead of CC
>  ifeq ($(CONFIG_CC_IS_CLANG), y)
> -CC_COMPAT_CLANG_FLAGS := --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%))
> -
>  CC_COMPAT ?= $(CC)
> -CC_COMPAT += $(CC_COMPAT_CLANG_FLAGS)
> -
> -ifneq ($(LLVM),)
> -LD_COMPAT ?= $(LD)
> +CC_COMPAT += --target=arm-linux-gnueabi
>  else
> -LD_COMPAT ?= $(CROSS_COMPILE_COMPAT)ld
> +CC_COMPAT ?= $(CROSS_COMPILE_COMPAT)gcc
>  endif
> +
> +ifeq ($(CONFIG_LD_IS_LLD), y)
> +LD_COMPAT ?= $(LD)
>  else
> -CC_COMPAT ?= $(CROSS_COMPILE_COMPAT)gcc
>  LD_COMPAT ?= $(CROSS_COMPILE_COMPAT)ld
>  endif
>  
> @@ -44,10 +41,6 @@ VDSO_CPPFLAGS += $(LINUXINCLUDE)
>  # Common C and assembly flags
>  # From top-level Makefile
>  VDSO_CAFLAGS = $(VDSO_CPPFLAGS)
> -ifneq ($(shell $(CC_COMPAT) --version 2>&1 | head -n 1 | grep clang),)
> -VDSO_CAFLAGS += --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%))
> -endif
> -
>  VDSO_CAFLAGS += $(call cc32-option,-fno-PIE)
>  ifdef CONFIG_DEBUG_INFO
>  VDSO_CAFLAGS += -g
> -- 
> 2.33.0.882.g93a45727a2-goog
> 
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 3/3] arm64: vdso32: require CROSS_COMPILE_COMPAT for gcc+bfd
@ 2021-10-18 22:58     ` Nathan Chancellor
  0 siblings, 0 replies; 22+ messages in thread
From: Nathan Chancellor @ 2021-10-18 22:58 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Catalin Marinas, Will Deacon, llvm, linux-kernel,
	linux-arm-kernel, Vincenzo Frascino

On Tue, Oct 12, 2021 at 04:46:06PM -0700, Nick Desaulniers wrote:
> Similar to
> commit 231ad7f409f1 ("Makefile: infer --target from ARCH for CC=clang")
> There really is no point in setting --target based on
> $CROSS_COMPILE_COMPAT for clang when the integrated assembler is being
> used.

It might be nice to mention commit ef94340583ee ("arm64: vdso32: drop
-no-integrated-as flag") here, as that is what flipped on the integrated
assembler for this Makefile (and it cannot be turned off).

> Allows COMPAT_VDSO to be selected without setting $CROSS_COMPILE_COMPAT
> when using clang and lld together.
> 
> Before:
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 defconfig
> $ grep CONFIG_COMPAT_VDSO .config
> CONFIG_COMPAT_VDSO=y
> $ ARCH=arm64 make -j72 LLVM=1 defconfig
> $ grep CONFIG_COMPAT_VDSO .config
> $
> 
> After:
> $ ARCH=arm64 CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make -j72 LLVM=1 defconfig
> $ grep CONFIG_COMPAT_VDSO .config
> CONFIG_COMPAT_VDSO=y
> $ ARCH=arm64 make -j72 LLVM=1 defconfig
> $ grep CONFIG_COMPAT_VDSO .config
> CONFIG_COMPAT_VDSO=y
> 
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Modulo these two nits, this works for me too.

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

> ---
>  arch/arm64/Kconfig                |  3 ++-
>  arch/arm64/kernel/vdso32/Makefile | 17 +++++------------
>  2 files changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 5c7ae4c3954b..7b28dad2fb80 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1264,7 +1264,8 @@ config KUSER_HELPERS
>  
>  config COMPAT_VDSO
>  	bool "Enable vDSO for 32-bit applications"
> -	depends on !CPU_BIG_ENDIAN && "$(CROSS_COMPILE_COMPAT)" != ""
> +	depends on !CPU_BIG_ENDIAN
> +	depends on CC_IS_CLANG && LD_IS_LLD || "$(CROSS_COMPILE_COMPAT)" != ""

It works fine as is but it might be nice to add parentheses around the
new condition for ease of understanding.

>  	select GENERIC_COMPAT_VDSO
>  	default y
>  	help
> diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
> index d24b12318f4c..376261d3791f 100644
> --- a/arch/arm64/kernel/vdso32/Makefile
> +++ b/arch/arm64/kernel/vdso32/Makefile
> @@ -10,18 +10,15 @@ include $(srctree)/lib/vdso/Makefile
>  
>  # Same as cc-*option, but using CC_COMPAT instead of CC
>  ifeq ($(CONFIG_CC_IS_CLANG), y)
> -CC_COMPAT_CLANG_FLAGS := --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%))
> -
>  CC_COMPAT ?= $(CC)
> -CC_COMPAT += $(CC_COMPAT_CLANG_FLAGS)
> -
> -ifneq ($(LLVM),)
> -LD_COMPAT ?= $(LD)
> +CC_COMPAT += --target=arm-linux-gnueabi
>  else
> -LD_COMPAT ?= $(CROSS_COMPILE_COMPAT)ld
> +CC_COMPAT ?= $(CROSS_COMPILE_COMPAT)gcc
>  endif
> +
> +ifeq ($(CONFIG_LD_IS_LLD), y)
> +LD_COMPAT ?= $(LD)
>  else
> -CC_COMPAT ?= $(CROSS_COMPILE_COMPAT)gcc
>  LD_COMPAT ?= $(CROSS_COMPILE_COMPAT)ld
>  endif
>  
> @@ -44,10 +41,6 @@ VDSO_CPPFLAGS += $(LINUXINCLUDE)
>  # Common C and assembly flags
>  # From top-level Makefile
>  VDSO_CAFLAGS = $(VDSO_CPPFLAGS)
> -ifneq ($(shell $(CC_COMPAT) --version 2>&1 | head -n 1 | grep clang),)
> -VDSO_CAFLAGS += --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%))
> -endif
> -
>  VDSO_CAFLAGS += $(call cc32-option,-fno-PIE)
>  ifdef CONFIG_DEBUG_INFO
>  VDSO_CAFLAGS += -g
> -- 
> 2.33.0.882.g93a45727a2-goog
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread
end of thread, other threads:[~2021-10-18 23:00 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-12 23:46 [PATCH 0/3] compat vdso cleanups Nick Desaulniers
2021-10-12 23:46 ` Nick Desaulniers
2021-10-12 23:46 ` [PATCH 1/3] arm64: vdso32: drop the test for dmb ishld Nick Desaulniers
2021-10-12 23:46   ` Nick Desaulniers
2021-10-13  7:55   ` Arnd Bergmann
2021-10-13  7:55     ` Arnd Bergmann
2021-10-13 17:24   ` Christian Biesinger
2021-10-13 17:24     ` Christian Biesinger
2021-10-12 23:46 ` [PATCH 2/3] arm64: vdso32: lazily invoke COMPAT_CC Nick Desaulniers
2021-10-12 23:46   ` Nick Desaulniers
2021-10-13  3:02   ` Masahiro Yamada
2021-10-13  3:02     ` Masahiro Yamada
2021-10-14 20:59     ` Nick Desaulniers
2021-10-14 20:59       ` Nick Desaulniers
2021-10-16 14:19       ` Masahiro Yamada
2021-10-16 14:19         ` Masahiro Yamada
2021-10-18 20:34         ` Nick Desaulniers
2021-10-18 20:34           ` Nick Desaulniers
2021-10-12 23:46 ` [PATCH 3/3] arm64: vdso32: require CROSS_COMPILE_COMPAT for gcc+bfd Nick Desaulniers
2021-10-12 23:46   ` Nick Desaulniers
2021-10-18 22:58   ` Nathan Chancellor
2021-10-18 22:58     ` Nathan Chancellor

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.