linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] x86: Asm macros fixes
@ 2018-11-12 15:02 Nadav Amit
  2018-11-12 15:02 ` [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros Nadav Amit
  2018-11-12 15:02 ` [PATCH 2/2] x86: set a dependency on macros.S Nadav Amit
  0 siblings, 2 replies; 18+ messages in thread
From: Nadav Amit @ 2018-11-12 15:02 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Masahiro Yamada, Michal Marek, Thomas Gleixner, Borislav Petkov,
	H. Peter Anvin, x86, linux-kbuild, linux-kernel, Nadav Amit

There has been a complaint that the recent use of assembly macros in C
files broke distcc. The first patch fixes this issue.

The second patch adds a dependency for all C files on macros.S, to
trigger their recompilation when the relevant macros change.

Nadav Amit (2):
  Makefile: Fix distcc compilation with x86 macros
  x86: set a dependency on macros.S

 Makefile               |  4 +++-
 arch/x86/Makefile      |  7 +++++--
 scripts/Makefile.build | 33 +++++++++++++++++++++++++++++----
 3 files changed, 37 insertions(+), 7 deletions(-)

-- 
2.17.1


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

* [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
  2018-11-12 15:02 [PATCH 0/2] x86: Asm macros fixes Nadav Amit
@ 2018-11-12 15:02 ` Nadav Amit
  2018-11-13 11:30   ` Ingo Molnar
  2018-11-12 15:02 ` [PATCH 2/2] x86: set a dependency on macros.S Nadav Amit
  1 sibling, 1 reply; 18+ messages in thread
From: Nadav Amit @ 2018-11-12 15:02 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Masahiro Yamada, Michal Marek, Thomas Gleixner, Borislav Petkov,
	H. Peter Anvin, x86, linux-kbuild, linux-kernel, Nadav Amit

Introducing the use of asm macros in c-code broke distcc, since it only
sends the preprocessed source file. The solution is to break the
compilation into two separate phases of compilation and assembly, and
between the two concatanate the assembly macros and the compiled (yet
not assembled) source file. Since this is less efficient, this
compilation mode is only used when make is called with the "DISTCC=y"
parameter.

Note that the assembly stage should also be distributed, if distcc is
configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE".

Reported-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: Nadav Amit <namit@vmware.com>
---
 Makefile               |  4 +++-
 arch/x86/Makefile      |  7 +++++--
 scripts/Makefile.build | 29 +++++++++++++++++++++++++++--
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 9fce8b91c15f..c07349fc38c7 100644
--- a/Makefile
+++ b/Makefile
@@ -743,7 +743,9 @@ KBUILD_CFLAGS   += $(call cc-option, -gsplit-dwarf, -g)
 else
 KBUILD_CFLAGS	+= -g
 endif
-KBUILD_AFLAGS	+= -Wa,-gdwarf-2
+AFLAGS_DEBUG_INFO = -Wa,-gdwarf-2
+export AFLAGS_DEBUG_INFO
+KBUILD_AFLAGS	+= $(AFLAGS_DEBUG_INFO)
 endif
 ifdef CONFIG_DEBUG_INFO_DWARF4
 KBUILD_CFLAGS	+= $(call cc-option, -gdwarf-4,)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index f5d7f4134524..b5953cbcc9c8 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -235,10 +235,13 @@ archscripts: scripts_basic
 archheaders:
 	$(Q)$(MAKE) $(build)=arch/x86/entry/syscalls all
 
+ASM_MACRO_FILE = arch/x86/kernel/macros.s
+export ASM_MACRO_FILE
+
 archmacros:
-	$(Q)$(MAKE) $(build)=arch/x86/kernel arch/x86/kernel/macros.s
+	$(Q)$(MAKE) $(build)=arch/x86/kernel $(ASM_MACRO_FILE)
 
-ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s
+ASM_MACRO_FLAGS = -Wa,$(ASM_MACRO_FILE)
 export ASM_MACRO_FLAGS
 KBUILD_CFLAGS += $(ASM_MACRO_FLAGS)
 
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 6a6be9f440cf..d2213b041408 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -155,8 +155,33 @@ $(obj)/%.ll: $(src)/%.c FORCE
 
 quiet_cmd_cc_o_c = CC $(quiet_modtag)  $@
 
+# If distcc is used, then when an assembly macro files is needed, the
+# compilation stage and the assembly stage need to be separated. Providing
+# "DISTCC=y" option enables the separate compilation and assembly.
+cmd_cc_o_c_direct = $(CC) $(c_flags) -c -o $(1) $<
+
+ifeq ($(DISTCC),y)
+a_flags_no_debug = $(filter-out $(AFLAGS_DEBUG_INFO), $(a_flags))
+c_flags_no_macros = $(filter-out $(ASM_MACRO_FLAGS), $(c_flags))
+
+cmd_cc_o_c_two_steps =							\
+	$(CC) $(c_flags_no_macros) $(DISABLE_LTO) -fverbose-asm -S 	\
+		-o $(@D)/.$(@F:.o=.s) $< ;				\
+	cat $(ASM_MACRO_FILE) $(@D)/.$(@F:.o=.s) > 			\
+		$(@D)/.tmp_$(@F:.o=.s);					\
+	$(CC) $(a_flags_no_debug) -c -o $(1) $(@D)/.tmp_$(@F:.o=.s) ; 	\
+	rm -f $(@D)/.$(@F:.o=.s) $(@D)/.tmp_$(@F:.o=.s)			\
+
+cmd_cc_o_c_helper =    							\
+	$(if $(findstring $(ASM_MACRO_FLAGS),$(c_flags)),		\
+		$(call cmd_cc_o_c_two_steps, $(1)),			\
+		$(call cmd_cc_o_c_direct, $(1)))
+else
+cmd_cc_o_c_helper = $(call cmd_cc_o_c_direct, $(1))
+endif
+
 ifndef CONFIG_MODVERSIONS
-cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $<
+cmd_cc_o_c = $(call cmd_cc_o_c_helper,$@)
 
 else
 # When module versioning is enabled the following steps are executed:
@@ -171,7 +196,7 @@ else
 #   replace the unresolved symbols __crc_exported_symbol with
 #   the actual value of the checksum generated by genksyms
 
-cmd_cc_o_c = $(CC) $(c_flags) -c -o $(@D)/.tmp_$(@F) $<
+cmd_cc_o_c = $(call cmd_cc_o_c_helper,$(@D)/.tmp_$(@F))
 
 cmd_modversions_c =								\
 	if $(OBJDUMP) -h $(@D)/.tmp_$(@F) | grep -q __ksymtab; then		\
-- 
2.17.1


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

* [PATCH 2/2] x86: set a dependency on macros.S
  2018-11-12 15:02 [PATCH 0/2] x86: Asm macros fixes Nadav Amit
  2018-11-12 15:02 ` [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros Nadav Amit
@ 2018-11-12 15:02 ` Nadav Amit
  2018-11-13 11:30   ` Ingo Molnar
  1 sibling, 1 reply; 18+ messages in thread
From: Nadav Amit @ 2018-11-12 15:02 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Masahiro Yamada, Michal Marek, Thomas Gleixner, Borislav Petkov,
	H. Peter Anvin, x86, linux-kbuild, linux-kernel, Nadav Amit

Changes in macros.S should trigger the recompilation of all C files, as
the macros might need to affect their compilation.

Signed-off-by: Nadav Amit <namit@vmware.com>
---
 scripts/Makefile.build | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index d2213b041408..ffe3e1a01210 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -312,13 +312,13 @@ cmd_undef_syms = echo
 endif
 
 # Built-in and composite module parts
-$(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
+$(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) $(ASM_MACRO_FILE:.s=.S) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 
 # Single-part modules are special since we need to mark them in $(MODVERDIR)
 
-$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
+$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) $(ASM_MACRO_FILE:.s=.S) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 	@{ echo $(@:.o=.ko); echo $@; \
-- 
2.17.1


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

* Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
  2018-11-12 15:02 ` [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros Nadav Amit
@ 2018-11-13 11:30   ` Ingo Molnar
  2018-11-13 17:55     ` Nadav Amit
  0 siblings, 1 reply; 18+ messages in thread
From: Ingo Molnar @ 2018-11-13 11:30 UTC (permalink / raw)
  To: Nadav Amit
  Cc: Ingo Molnar, Masahiro Yamada, Michal Marek, Thomas Gleixner,
	Borislav Petkov, H. Peter Anvin, x86, linux-kbuild, linux-kernel


* Nadav Amit <namit@vmware.com> wrote:

> Introducing the use of asm macros in c-code broke distcc, since it only
> sends the preprocessed source file. The solution is to break the
> compilation into two separate phases of compilation and assembly, and
> between the two concatanate the assembly macros and the compiled (yet

s/concatenate

> not assembled) source file. Since this is less efficient, this
> compilation mode is only used when make is called with the "DISTCC=y"
> parameter.
> 
> Note that the assembly stage should also be distributed, if distcc is
> configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE".

It's a bit sad that we regressed distcc performance ...

> +# If distcc is used, then when an assembly macro files is needed, the
> +# compilation stage and the assembly stage need to be separated. Providing
> +# "DISTCC=y" option enables the separate compilation and assembly.

Let's fix the various typos:

  > +# If distcc is used, and when assembly macro files are needed, the
  > +# compilation stage and the assembly stage needs to be separated. 
  > +# Providing the "DISTCC=y" option enables separate compilation and 
  > +# assembly.



> +cmd_cc_o_c_direct = $(CC) $(c_flags) -c -o $(1) $<
> +
> +ifeq ($(DISTCC),y)
> +a_flags_no_debug = $(filter-out $(AFLAGS_DEBUG_INFO), $(a_flags))
> +c_flags_no_macros = $(filter-out $(ASM_MACRO_FLAGS), $(c_flags))
> +
> +cmd_cc_o_c_two_steps =							\
> +	$(CC) $(c_flags_no_macros) $(DISABLE_LTO) -fverbose-asm -S	\
> +		-o $(@D)/.$(@F:.o=.s) $< ;				\
> +	cat $(ASM_MACRO_FILE) $(@D)/.$(@F:.o=.s) >			\
> +		$(@D)/.tmp_$(@F:.o=.s);					\
> +	$(CC) $(a_flags_no_debug) -c -o $(1) $(@D)/.tmp_$(@F:.o=.s) ; 	\
> +	rm -f $(@D)/.$(@F:.o=.s) $(@D)/.tmp_$(@F:.o=.s)			\
> +
> +cmd_cc_o_c_helper =    							\
> +	$(if $(findstring $(ASM_MACRO_FLAGS),$(c_flags)),		\
> +		$(call cmd_cc_o_c_two_steps, $(1)),			\
> +		$(call cmd_cc_o_c_direct, $(1)))

There are various stray <space>+<tab> whitespace noise errors in the 
chunks above.

Thanks,

	Ingo

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

* Re: [PATCH 2/2] x86: set a dependency on macros.S
  2018-11-12 15:02 ` [PATCH 2/2] x86: set a dependency on macros.S Nadav Amit
@ 2018-11-13 11:30   ` Ingo Molnar
  0 siblings, 0 replies; 18+ messages in thread
From: Ingo Molnar @ 2018-11-13 11:30 UTC (permalink / raw)
  To: Nadav Amit
  Cc: Ingo Molnar, Masahiro Yamada, Michal Marek, Thomas Gleixner,
	Borislav Petkov, H. Peter Anvin, x86, linux-kbuild, linux-kernel


* Nadav Amit <namit@vmware.com> wrote:

> Changes in macros.S should trigger the recompilation of all C files, as
> the macros might need to affect their compilation.
> 
> Signed-off-by: Nadav Amit <namit@vmware.com>
> ---
>  scripts/Makefile.build | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index d2213b041408..ffe3e1a01210 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -312,13 +312,13 @@ cmd_undef_syms = echo
>  endif
>  
>  # Built-in and composite module parts
> -$(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
> +$(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) $(ASM_MACRO_FILE:.s=.S) FORCE
>  	$(call cmd,force_checksrc)
>  	$(call if_changed_rule,cc_o_c)
>  
>  # Single-part modules are special since we need to mark them in $(MODVERDIR)
>  
> -$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
> +$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) $(ASM_MACRO_FILE:.s=.S) FORCE
>  	$(call cmd,force_checksrc)
>  	$(call if_changed_rule,cc_o_c)
>  	@{ echo $(@:.o=.ko); echo $@; \

Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

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

* Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
  2018-11-13 11:30   ` Ingo Molnar
@ 2018-11-13 17:55     ` Nadav Amit
  2018-11-13 18:34       ` Nadav Amit
  0 siblings, 1 reply; 18+ messages in thread
From: Nadav Amit @ 2018-11-13 17:55 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Ingo Molnar, Masahiro Yamada, Michal Marek, Thomas Gleixner,
	Borislav Petkov, H. Peter Anvin, X86 ML,
	Linux Kbuild mailing list, LKML, Logan Gunthorpe

From: Ingo Molnar
Sent: November 13, 2018 at 11:30:00 AM GMT
> To: Nadav Amit <namit@vmware.com>
> Cc: Ingo Molnar <mingo@redhat.com>, Masahiro Yamada <yamada.masahiro@socionext.com>, Michal Marek <michal.lkml@markovi.net>, Thomas Gleixner <tglx@linutronix.de>, Borislav Petkov <bp@alien8.de>, H. Peter Anvin <hpa@zytor.com>, x86@kernel.org, linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
> 
> 
> 
> * Nadav Amit <namit@vmware.com> wrote:
> 
>> Introducing the use of asm macros in c-code broke distcc, since it only
>> sends the preprocessed source file. The solution is to break the
>> compilation into two separate phases of compilation and assembly, and
>> between the two concatanate the assembly macros and the compiled (yet
> 
> s/concatenate
> 
>> not assembled) source file. Since this is less efficient, this
>> compilation mode is only used when make is called with the "DISTCC=y"
>> parameter.
>> 
>> Note that the assembly stage should also be distributed, if distcc is
>> configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE".
> 
> It's a bit sad that we regressed distcc performance …

I don’t know what the actual impact is, but Logan, who reported the bug says
there is an alternative solution for when distcc-pump is used (which
presumably would have ~zero performance degradation). distcc is really
fragile IMHO - it’s enough that it finds what looks like two source files in
the compiler command arguments for it to fall back to local compilation.

[ In this regard, the distcc-pump solution would *not* work if distcc is
built with support for distributed assembly, since it will consider the .s
file as a second source file. ]

>> +# If distcc is used, then when an assembly macro files is needed, the
>> +# compilation stage and the assembly stage need to be separated. Providing
>> +# "DISTCC=y" option enables the separate compilation and assembly.
> 
> Let's fix the various typos:
> 
>> +# If distcc is used, and when assembly macro files are needed, the
>> +# compilation stage and the assembly stage needs to be separated. 
>> +# Providing the "DISTCC=y" option enables separate compilation and 
>> +# assembly.

That’s grammar, not typos ;-)

Sorry for that - I will fix it an send v2 (as well as the whitespace noise).

Regards,
Nadav

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

* Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
  2018-11-13 17:55     ` Nadav Amit
@ 2018-11-13 18:34       ` Nadav Amit
  2018-11-14  7:29         ` Logan Gunthorpe
  0 siblings, 1 reply; 18+ messages in thread
From: Nadav Amit @ 2018-11-13 18:34 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Ingo Molnar, Masahiro Yamada, Michal Marek, Thomas Gleixner,
	Borislav Petkov, H. Peter Anvin, X86 ML,
	Linux Kbuild mailing list, LKML, Logan Gunthorpe

From: Nadav Amit
Sent: November 13, 2018 at 5:55:34 PM GMT
> To: Ingo Molnar <mingo@kernel.org>
> Cc: Ingo Molnar <mingo@redhat.com>, Masahiro Yamada <yamada.masahiro@socionext.com>, Michal Marek <michal.lkml@markovi.net>, Thomas Gleixner <tglx@linutronix.de>, Borislav Petkov <bp@alien8.de>, H. Peter Anvin <hpa@zytor.com>, X86 ML <x86@kernel.org>, Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, Logan Gunthorpe <logang@deltatee.com>
> Subject: Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
> 
> 
> From: Ingo Molnar
> Sent: November 13, 2018 at 11:30:00 AM GMT
>> To: Nadav Amit <namit@vmware.com>
>> Cc: Ingo Molnar <mingo@redhat.com>, Masahiro Yamada <yamada.masahiro@socionext.com>, Michal Marek <michal.lkml@markovi.net>, Thomas Gleixner <tglx@linutronix.de>, Borislav Petkov <bp@alien8.de>, H. Peter Anvin <hpa@zytor.com>, x86@kernel.org, linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
>> 
>> 
>> 
>> * Nadav Amit <namit@vmware.com> wrote:
>> 
>>> Introducing the use of asm macros in c-code broke distcc, since it only
>>> sends the preprocessed source file. The solution is to break the
>>> compilation into two separate phases of compilation and assembly, and
>>> between the two concatanate the assembly macros and the compiled (yet
>> 
>> s/concatenate
>> 
>>> not assembled) source file. Since this is less efficient, this
>>> compilation mode is only used when make is called with the "DISTCC=y"
>>> parameter.
>>> 
>>> Note that the assembly stage should also be distributed, if distcc is
>>> configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE".
>> 
>> It's a bit sad that we regressed distcc performance …
> 
> I don’t know what the actual impact is, but Logan, who reported the bug says
> there is an alternative solution for when distcc-pump is used (which
> presumably would have ~zero performance degradation). distcc is really
> fragile IMHO - it’s enough that it finds what looks like two source files in
> the compiler command arguments for it to fall back to local compilation.
> 
> [ In this regard, the distcc-pump solution would *not* work if distcc is
> built with support for distributed assembly, since it will consider the .s
> file as a second source file. ]
> 
>>> +# If distcc is used, then when an assembly macro files is needed, the
>>> +# compilation stage and the assembly stage need to be separated. Providing
>>> +# "DISTCC=y" option enables the separate compilation and assembly.
>> 
>> Let's fix the various typos:
>> 
>>> +# If distcc is used, and when assembly macro files are needed, the
>>> +# compilation stage and the assembly stage needs to be separated. 
>>> +# Providing the "DISTCC=y" option enables separate compilation and 
>>> +# assembly.
> 
> That’s grammar, not typos ;-)
> 
> Sorry for that - I will fix it an send v2 (as well as the whitespace noise).

Just one question before I send v2, since I have second thoughts. Does it
make sense to require the “DISTCC” make parameter, or should it be set in
the Kconfig? It can be detected automatically, the same way gcc/clang are
detected or manually through a config option.


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

* Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
  2018-11-13 18:34       ` Nadav Amit
@ 2018-11-14  7:29         ` Logan Gunthorpe
  2018-11-14 17:46           ` Nadav Amit
  0 siblings, 1 reply; 18+ messages in thread
From: Logan Gunthorpe @ 2018-11-14  7:29 UTC (permalink / raw)
  To: Nadav Amit, Ingo Molnar
  Cc: Ingo Molnar, Masahiro Yamada, Michal Marek, Thomas Gleixner,
	Borislav Petkov, H. Peter Anvin, X86 ML,
	Linux Kbuild mailing list, LKML



On 13/11/18 11:34 AM, Nadav Amit wrote:
> Just one question before I send v2, since I have second thoughts. Does it
> make sense to require the “DISTCC” make parameter, or should it be set in
> the Kconfig? It can be detected automatically, the same way gcc/clang are
> detected or manually through a config option.

I very much prefer the make variable as it can be set in the environment
without having to change the Kconfig.

Logan


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

* Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
  2018-11-14  7:29         ` Logan Gunthorpe
@ 2018-11-14 17:46           ` Nadav Amit
  2018-11-15  1:19             ` Logan Gunthorpe
  0 siblings, 1 reply; 18+ messages in thread
From: Nadav Amit @ 2018-11-14 17:46 UTC (permalink / raw)
  To: Logan Gunthorpe, Ingo Molnar
  Cc: Ingo Molnar, Masahiro Yamada, Michal Marek, Thomas Gleixner,
	Borislav Petkov, H. Peter Anvin, X86 ML,
	Linux Kbuild mailing list, LKML

From: Logan Gunthorpe
Sent: November 14, 2018 at 7:29:38 AM GMT
> To: Nadav Amit <namit@vmware.com>, Ingo Molnar <mingo@kernel.org>
> Cc: Ingo Molnar <mingo@redhat.com>, Masahiro Yamada <yamada.masahiro@socionext.com>, Michal Marek <michal.lkml@markovi.net>, Thomas Gleixner <tglx@linutronix.de>, Borislav Petkov <bp@alien8.de>, H. Peter Anvin <hpa@zytor.com>, X86 ML <x86@kernel.org>, Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
> 
> 
> 
> 
> On 13/11/18 11:34 AM, Nadav Amit wrote:
>> Just one question before I send v2, since I have second thoughts. Does it
>> make sense to require the “DISTCC” make parameter, or should it be set in
>> the Kconfig? It can be detected automatically, the same way gcc/clang are
>> detected or manually through a config option.
> 
> I very much prefer the make variable as it can be set in the environment
> without having to change the Kconfig.

Actually, we can just figure out whether distcc or icecc are used in the
Makefile according to the “version”, similarly to the way clang is detected.
This would neither require new Makefile arguments or Kconfig options.

What do you say about the following?

-- >8 --

Subject: [PATCH] Makefile: Fix distcc compilation with x86 macros

Introducing the use of asm macros in c-code broke distcc, since it only
sends the preprocessed source file. The solution is to break the
compilation into two separate phases of compilation and assembly, and
between the two concatenate the assembly macros and the compiled (yet
not assembled) source file. Since this is less efficient, this
compilation mode is only used when distcc or icecc are used.

Note that the assembly stage should also be distributed, if distcc is
configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE".

Reported-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: Nadav Amit <namit@vmware.com>
---
 Makefile               |  4 +++-
 arch/x86/Makefile      |  7 +++++--
 scripts/Makefile.build | 30 ++++++++++++++++++++++++++++--
 3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 9fce8b91c15f..c07349fc38c7 100644
--- a/Makefile
+++ b/Makefile
@@ -743,7 +743,9 @@ KBUILD_CFLAGS   += $(call cc-option, -gsplit-dwarf, -g)
 else
 KBUILD_CFLAGS	+= -g
 endif
-KBUILD_AFLAGS	+= -Wa,-gdwarf-2
+AFLAGS_DEBUG_INFO = -Wa,-gdwarf-2
+export AFLAGS_DEBUG_INFO
+KBUILD_AFLAGS	+= $(AFLAGS_DEBUG_INFO)
 endif
 ifdef CONFIG_DEBUG_INFO_DWARF4
 KBUILD_CFLAGS	+= $(call cc-option, -gdwarf-4,)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index f5d7f4134524..b5953cbcc9c8 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -235,10 +235,13 @@ archscripts: scripts_basic
 archheaders:
 	$(Q)$(MAKE) $(build)=arch/x86/entry/syscalls all
 
+ASM_MACRO_FILE = arch/x86/kernel/macros.s
+export ASM_MACRO_FILE
+
 archmacros:
-	$(Q)$(MAKE) $(build)=arch/x86/kernel arch/x86/kernel/macros.s
+	$(Q)$(MAKE) $(build)=arch/x86/kernel $(ASM_MACRO_FILE)
 
-ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s
+ASM_MACRO_FLAGS = -Wa,$(ASM_MACRO_FILE)
 export ASM_MACRO_FLAGS
 KBUILD_CFLAGS += $(ASM_MACRO_FLAGS)
 
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 6a6be9f440cf..bac761c07bf8 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -155,8 +155,34 @@ $(obj)/%.ll: $(src)/%.c FORCE
 
 quiet_cmd_cc_o_c = CC $(quiet_modtag)  $@
 
+# If distcc is used, and when assembly macro files are needed, the compilation
+# stage and the assembly stage needs to be separated. Providing the "DISTCC=y"
+# option enables separate compilation and assembly.
+
+cmd_cc_o_c_direct = $(CC) $(c_flags) -c -o $(1) $<
+
+ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep -E 'distcc|icecc'),)
+a_flags_no_debug = $(filter-out $(AFLAGS_DEBUG_INFO), $(a_flags))
+c_flags_no_macros = $(filter-out $(ASM_MACRO_FLAGS), $(c_flags))
+
+cmd_cc_o_c_two_steps =							\
+	$(CC) $(c_flags_no_macros) $(DISABLE_LTO) -fverbose-asm -S	\
+		-o $(@D)/.$(@F:.o=.s) $<;				\
+	cat $(ASM_MACRO_FILE) $(@D)/.$(@F:.o=.s) >			\
+		$(@D)/.tmp_$(@F:.o=.s);					\
+	$(CC) $(a_flags_no_debug) -c -o $(1) $(@D)/.tmp_$(@F:.o=.s);	\
+	rm -f $(@D)/.$(@F:.o=.s) $(@D)/.tmp_$(@F:.o=.s)			\
+
+cmd_cc_o_c_helper =							\
+	$(if $(findstring $(ASM_MACRO_FLAGS),$(c_flags)),		\
+		$(call cmd_cc_o_c_two_steps, $(1)),			\
+		$(call cmd_cc_o_c_direct, $(1)))
+else
+cmd_cc_o_c_helper = $(call cmd_cc_o_c_direct, $(1))
+endif
+
 ifndef CONFIG_MODVERSIONS
-cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $<
+cmd_cc_o_c = $(call cmd_cc_o_c_helper,$@)
 
 else
 # When module versioning is enabled the following steps are executed:
@@ -171,7 +197,7 @@ else
 #   replace the unresolved symbols __crc_exported_symbol with
 #   the actual value of the checksum generated by genksyms
 
-cmd_cc_o_c = $(CC) $(c_flags) -c -o $(@D)/.tmp_$(@F) $<
+cmd_cc_o_c = $(call cmd_cc_o_c_helper,$(@D)/.tmp_$(@F))
 
 cmd_modversions_c =								\
 	if $(OBJDUMP) -h $(@D)/.tmp_$(@F) | grep -q __ksymtab; then		\
-- 
2.17.1

 


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

* Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
  2018-11-14 17:46           ` Nadav Amit
@ 2018-11-15  1:19             ` Logan Gunthorpe
  2018-11-15  1:57               ` Nadav Amit
  0 siblings, 1 reply; 18+ messages in thread
From: Logan Gunthorpe @ 2018-11-15  1:19 UTC (permalink / raw)
  To: Nadav Amit, Ingo Molnar
  Cc: Ingo Molnar, Masahiro Yamada, Michal Marek, Thomas Gleixner,
	Borislav Petkov, H. Peter Anvin, X86 ML,
	Linux Kbuild mailing list, LKML



On 14/11/18 10:46 AM, Nadav Amit wrote:
> 
> Actually, we can just figure out whether distcc or icecc are used in the
> Makefile according to the “version”, similarly to the way clang is detected.
> This would neither require new Makefile arguments or Kconfig options.
> 
> What do you say about the following?

That may be a good idea, but I was kind of hoping to be able to add
support for this to at least a future version of icecc (though I'm not
committing to that...). So it would be nice to have a way to force it
one way or the other with an environment variable.

Thanks,

Logan

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

* Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
  2018-11-15  1:19             ` Logan Gunthorpe
@ 2018-11-15  1:57               ` Nadav Amit
  2018-11-15  2:00                 ` Logan Gunthorpe
  2018-11-28 23:09                 ` Logan Gunthorpe
  0 siblings, 2 replies; 18+ messages in thread
From: Nadav Amit @ 2018-11-15  1:57 UTC (permalink / raw)
  To: Logan Gunthorpe, Ingo Molnar
  Cc: Ingo Molnar, Masahiro Yamada, Michal Marek, Thomas Gleixner,
	Borislav Petkov, H. Peter Anvin, X86 ML,
	Linux Kbuild mailing list, LKML

From: Logan Gunthorpe
Sent: November 15, 2018 at 1:19:45 AM GMT
> To: Nadav Amit <namit@vmware.com>, Ingo Molnar <mingo@kernel.org>
> Cc: Ingo Molnar <mingo@redhat.com>, Masahiro Yamada <yamada.masahiro@socionext.com>, Michal Marek <michal.lkml@markovi.net>, Thomas Gleixner <tglx@linutronix.de>, Borislav Petkov <bp@alien8.de>, H. Peter Anvin <hpa@zytor.com>, X86 ML <x86@kernel.org>, Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
> 
> 
> 
> 
> On 14/11/18 10:46 AM, Nadav Amit wrote:
>> Actually, we can just figure out whether distcc or icecc are used in the
>> Makefile according to the “version”, similarly to the way clang is detected.
>> This would neither require new Makefile arguments or Kconfig options.
>> 
>> What do you say about the following?
> 
> That may be a good idea, but I was kind of hoping to be able to add
> support for this to at least a future version of icecc (though I'm not
> committing to that...). So it would be nice to have a way to force it
> one way or the other with an environment variable.

As long as the argument was *required* to get distcc to work at all, you
could expect people would figure out an argument is needed. In this case, I
suspect nobody will ever know about this argument (except you).

Eventually, if you get a fix into icecc, we will need to change the
Makefile, consider the version number and act accordingly.

Anyhow, I’ll add an argument as you asked, and let Ingo (& others) be the
judge(s).


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

* Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
  2018-11-15  1:57               ` Nadav Amit
@ 2018-11-15  2:00                 ` Logan Gunthorpe
  2018-11-28 23:09                 ` Logan Gunthorpe
  1 sibling, 0 replies; 18+ messages in thread
From: Logan Gunthorpe @ 2018-11-15  2:00 UTC (permalink / raw)
  To: Nadav Amit, Ingo Molnar
  Cc: Ingo Molnar, Masahiro Yamada, Michal Marek, Thomas Gleixner,
	Borislav Petkov, H. Peter Anvin, X86 ML,
	Linux Kbuild mailing list, LKML



On 14/11/18 06:57 PM, Nadav Amit wrote:
> As long as the argument was *required* to get distcc to work at all, you
> could expect people would figure out an argument is needed. In this case, I
> suspect nobody will ever know about this argument (except you).

I agree with this completely.

> Eventually, if you get a fix into icecc, we will need to change the
> Makefile, consider the version number and act accordingly.

Yeah, that's probably a sensible way forward.

> Anyhow, I’ll add an argument as you asked, and let Ingo (& others) be the

An argument to force it to a regular compile would at least help with
experimenting with this stuff.

Logan

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

* Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
  2018-11-15  1:57               ` Nadav Amit
  2018-11-15  2:00                 ` Logan Gunthorpe
@ 2018-11-28 23:09                 ` Logan Gunthorpe
  2018-11-29  0:38                   ` Nadav Amit
  1 sibling, 1 reply; 18+ messages in thread
From: Logan Gunthorpe @ 2018-11-28 23:09 UTC (permalink / raw)
  To: Nadav Amit, Ingo Molnar
  Cc: Ingo Molnar, Masahiro Yamada, Michal Marek, Thomas Gleixner,
	Borislav Petkov, H. Peter Anvin, X86 ML,
	Linux Kbuild mailing list, LKML



On 2018-11-14 6:57 p.m., Nadav Amit wrote:
> Eventually, if you get a fix into icecc, we will need to change the
> Makefile, consider the version number and act accordingly.

I got a fix pulled into icecc[1] and it works quite well. It ought to be
included in the next version of icecc. And distcc (with pump at least)
has a patch too. In my experience, icecc does a much better job
distributing the kernel build though.

Logan

[1] https://github.com/icecc/icecream/pull/430

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

* Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
  2018-11-28 23:09                 ` Logan Gunthorpe
@ 2018-11-29  0:38                   ` Nadav Amit
  2018-11-29  0:49                     ` Logan Gunthorpe
  0 siblings, 1 reply; 18+ messages in thread
From: Nadav Amit @ 2018-11-29  0:38 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: Ingo Molnar, Ingo Molnar, Masahiro Yamada, Michal Marek,
	Thomas Gleixner, Borislav Petkov, H. Peter Anvin, X86 ML,
	Linux Kbuild mailing list, LKML

> On Nov 28, 2018, at 3:09 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
> 
> 
> 
> On 2018-11-14 6:57 p.m., Nadav Amit wrote:
>> Eventually, if you get a fix into icecc, we will need to change the
>> Makefile, consider the version number and act accordingly.
> 
> I got a fix pulled into icecc[1] and it works quite well. It ought to be
> included in the next version of icecc. And distcc (with pump at least)
> has a patch too. In my experience, icecc does a much better job
> distributing the kernel build though.
> 
> Logan
> 
> [1] https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ficecc%2Ficecream%2Fpull%2F430&amp;data=02%7C01%7Cnamit%40vmware.com%7C044d795be30143ae26ef08d65586953b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636790433884222635&amp;sdata=4h72yLjvOF29kcAC5CM7CB9ukH3HoALNUfgKA62E4Gs%3D&amp;reserved=0

Thanks for taking care of it. This change is certainly simpler and cleaner
than the one I came with.

So what’s your take? Would you think this patch is still needed? Should it
only be enabled automatically for distcc and not for distcc-pump?


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

* Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
  2018-11-29  0:38                   ` Nadav Amit
@ 2018-11-29  0:49                     ` Logan Gunthorpe
  2018-11-29  1:31                       ` Nadav Amit
  0 siblings, 1 reply; 18+ messages in thread
From: Logan Gunthorpe @ 2018-11-29  0:49 UTC (permalink / raw)
  To: Nadav Amit
  Cc: Ingo Molnar, Ingo Molnar, Masahiro Yamada, Michal Marek,
	Thomas Gleixner, Borislav Petkov, H. Peter Anvin, X86 ML,
	Linux Kbuild mailing list, LKML



On 2018-11-28 5:38 p.m., Nadav Amit wrote:
> So what’s your take? Would you think this patch is still needed? Should it
> only be enabled automatically for distcc and not for distcc-pump?

Not sure. The patch will probably slow things down a lot (seeing
assembly is always done locally and there are twice as many compile
steps) and will create some confusion once it's possible to disable it
for the new versions. Maybe hold off and see if anyone else complains?

I don't really know how you'd detect whether pump is in use or not and
I'm uncertain as to whether any of the auto detection can actually be
made to be reliable.

Logan



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

* Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
  2018-11-29  0:49                     ` Logan Gunthorpe
@ 2018-11-29  1:31                       ` Nadav Amit
  2018-11-29 16:43                         ` Logan Gunthorpe
  0 siblings, 1 reply; 18+ messages in thread
From: Nadav Amit @ 2018-11-29  1:31 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: Ingo Molnar, Ingo Molnar, Masahiro Yamada, Michal Marek,
	Thomas Gleixner, Borislav Petkov, H. Peter Anvin, X86 ML,
	Linux Kbuild mailing list, LKML

> On Nov 28, 2018, at 4:49 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
> 
> 
> 
> On 2018-11-28 5:38 p.m., Nadav Amit wrote:
>> So what’s your take? Would you think this patch is still needed? Should it
>> only be enabled automatically for distcc and not for distcc-pump?
> 
> Not sure. The patch will probably slow things down a lot (seeing
> assembly is always done locally and there are twice as many compile
> steps) and will create some confusion once it's possible to disable it
> for the new versions. Maybe hold off and see if anyone else complains?
> 
> I don't really know how you'd detect whether pump is in use or not and
> I'm uncertain as to whether any of the auto detection can actually be
> made to be reliable.

A silly `$(CC) —version | grep pump ` test.

Anyhow, it is up to Masahiro.


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

* Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
  2018-11-29  1:31                       ` Nadav Amit
@ 2018-11-29 16:43                         ` Logan Gunthorpe
  2018-12-01  6:29                           ` Nadav Amit
  0 siblings, 1 reply; 18+ messages in thread
From: Logan Gunthorpe @ 2018-11-29 16:43 UTC (permalink / raw)
  To: Nadav Amit
  Cc: Ingo Molnar, Ingo Molnar, Masahiro Yamada, Michal Marek,
	Thomas Gleixner, Borislav Petkov, H. Peter Anvin, X86 ML,
	Linux Kbuild mailing list, LKML



On 2018-11-28 6:31 p.m., Nadav Amit wrote:
>> On Nov 28, 2018, at 4:49 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
>>
>>
>>
>> On 2018-11-28 5:38 p.m., Nadav Amit wrote:
>>> So what’s your take? Would you think this patch is still needed? Should it
>>> only be enabled automatically for distcc and not for distcc-pump?
>>
>> Not sure. The patch will probably slow things down a lot (seeing
>> assembly is always done locally and there are twice as many compile
>> steps) and will create some confusion once it's possible to disable it
>> for the new versions. Maybe hold off and see if anyone else complains?
>>
>> I don't really know how you'd detect whether pump is in use or not and
>> I'm uncertain as to whether any of the auto detection can actually be
>> made to be reliable.
> 
> A silly `$(CC) —version | grep pump ` test.

Actually I'm not sure that's going to work in all cases. If CC="distcc
gcc", then "$(CC) --version" just looks like "gcc --version"...

Logan

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

* Re: [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros
  2018-11-29 16:43                         ` Logan Gunthorpe
@ 2018-12-01  6:29                           ` Nadav Amit
  0 siblings, 0 replies; 18+ messages in thread
From: Nadav Amit @ 2018-12-01  6:29 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: Ingo Molnar, Ingo Molnar, Masahiro Yamada, Michal Marek,
	Thomas Gleixner, Borislav Petkov, H. Peter Anvin, X86 ML,
	Linux Kbuild mailing list, LKML

> On Nov 29, 2018, at 8:43 AM, Logan Gunthorpe <logang@deltatee.com> wrote:
> 
> 
> 
> On 2018-11-28 6:31 p.m., Nadav Amit wrote:
>>> On Nov 28, 2018, at 4:49 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
>>> 
>>> 
>>> 
>>> On 2018-11-28 5:38 p.m., Nadav Amit wrote:
>>>> So what’s your take? Would you think this patch is still needed? Should it
>>>> only be enabled automatically for distcc and not for distcc-pump?
>>> 
>>> Not sure. The patch will probably slow things down a lot (seeing
>>> assembly is always done locally and there are twice as many compile
>>> steps) and will create some confusion once it's possible to disable it
>>> for the new versions. Maybe hold off and see if anyone else complains?
>>> 
>>> I don't really know how you'd detect whether pump is in use or not and
>>> I'm uncertain as to whether any of the auto detection can actually be
>>> made to be reliable.
>> 
>> A silly `$(CC) —version | grep pump ` test.
> 
> Actually I'm not sure that's going to work in all cases. If CC="distcc
> gcc", then "$(CC) --version" just looks like "gcc --version"...

Err.. You’re right. I just tried distcc --version.


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

end of thread, other threads:[~2018-12-01  6:29 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-12 15:02 [PATCH 0/2] x86: Asm macros fixes Nadav Amit
2018-11-12 15:02 ` [PATCH 1/2] Makefile: Fix distcc compilation with x86 macros Nadav Amit
2018-11-13 11:30   ` Ingo Molnar
2018-11-13 17:55     ` Nadav Amit
2018-11-13 18:34       ` Nadav Amit
2018-11-14  7:29         ` Logan Gunthorpe
2018-11-14 17:46           ` Nadav Amit
2018-11-15  1:19             ` Logan Gunthorpe
2018-11-15  1:57               ` Nadav Amit
2018-11-15  2:00                 ` Logan Gunthorpe
2018-11-28 23:09                 ` Logan Gunthorpe
2018-11-29  0:38                   ` Nadav Amit
2018-11-29  0:49                     ` Logan Gunthorpe
2018-11-29  1:31                       ` Nadav Amit
2018-11-29 16:43                         ` Logan Gunthorpe
2018-12-01  6:29                           ` Nadav Amit
2018-11-12 15:02 ` [PATCH 2/2] x86: set a dependency on macros.S Nadav Amit
2018-11-13 11:30   ` Ingo Molnar

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