[PATCH 0/4] ftrace build improvements

[PATCH 0/4] ftrace build improvements

[Vasily Gorbik]

Support for -fentry -mrecord-mcount and -mnop-mcount has been added for
s390 in gcc master branch. An attempt to build 4.18 for s390 with that
gcc would produce a kernel with no __mcount_loc though because of the
new condition in scripts/Makefile.build:210 which disables recordmcount
tool if compiler supports -mrecord-mcount (s390 relies on combination
of -mhotpatch=0,3 gcc flag and recordmcount.pl).

This patch series adds s390 ftrace support based on combination of -pg
-fentry -mrecord-mcount and -mnop-mcount gcc flags (that fixes issue
mentioned above). At the same time this patch series fixes couple of
minor issues and adds -mnop-mcount gcc flag support (utilized by s390).

Vasily Gorbik (4):
  trace: handle CC_FLAGS_FTRACE more accurately
  trace: avoid calling cc-option -mrecord-mcount for every Makefile
  trace: add -mcount-nop option support
  s390/ftrace: add -mfentry and -mnop-mcount support

 Makefile                       | 24 ++++++++++++++++++++----
 arch/s390/Kconfig              |  2 ++
 arch/s390/Makefile             | 16 +++++++++-------
 arch/s390/include/asm/ftrace.h |  6 +++---
 arch/s390/kernel/ftrace.c      |  2 +-
 arch/s390/kernel/mcount.S      |  2 +-
 kernel/trace/Kconfig           |  5 +++++
 kernel/trace/ftrace.c          |  2 ++
 scripts/Makefile.build         |  9 +++------
 9 files changed, 46 insertions(+), 22 deletions(-)

-- 
2.18.0.13.gd42ae10

[PATCH 1/4] trace: handle CC_FLAGS_FTRACE more accurately

[Vasily Gorbik]

CC_FLAGS_FTRACE is exported and later used to remove ftrace relevant
build flags from files which should be built without ftrace support.
For that reason add -mfentry to CC_FLAGS_FTRACE as well. That fixes
a problem with vdso32 build on s390, where -mfentry could not be used
together with -m31 flag.

At the same time flags like -pg and -mfentry are not relevant for asm
files, so avoid adding them to KBUILD_AFLAGS.

Introduce CC_FLAGS_USING instead of CC_USING_FENTRY to collect
-DCC_USING_FENTRY (and future alike) which are relevant for both
KBUILD_CFLAGS and KBUILD_AFLAGS.

Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
---
 Makefile | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 7a3c4548162b..503f533277c7 100644
--- a/Makefile
+++ b/Makefile
@@ -743,12 +743,15 @@ ifdef CONFIG_FUNCTION_TRACER
 ifndef CC_FLAGS_FTRACE
 CC_FLAGS_FTRACE := -pg
 endif
-export CC_FLAGS_FTRACE
 ifdef CONFIG_HAVE_FENTRY
-CC_USING_FENTRY	:= $(call cc-option, -mfentry -DCC_USING_FENTRY)
+  ifeq ($(call cc-option-yn, -mfentry),y)
+    CC_FLAGS_FTRACE	+= -mfentry
+    CC_FLAGS_USING	+= -DCC_USING_FENTRY
+  endif
 endif
-KBUILD_CFLAGS	+= $(CC_FLAGS_FTRACE) $(CC_USING_FENTRY)
-KBUILD_AFLAGS	+= $(CC_USING_FENTRY)
+export CC_FLAGS_FTRACE
+KBUILD_CFLAGS	+= $(CC_FLAGS_FTRACE) $(CC_FLAGS_USING)
+KBUILD_AFLAGS	+= $(CC_FLAGS_USING)
 ifdef CONFIG_DYNAMIC_FTRACE
 	ifdef CONFIG_HAVE_C_RECORDMCOUNT
 		BUILD_C_RECORDMCOUNT := y
-- 
2.18.0.13.gd42ae10

[PATCH 2/4] trace: avoid calling cc-option -mrecord-mcount for every Makefile

[Vasily Gorbik]

Currently if CONFIG_FTRACE_MCOUNT_RECORD is enabled -mrecord-mcount
compiler flag support is tested for every Makefile.

Top 4 cc-option usages:
    511 -mrecord-mcount
     11  -fno-stack-protector
      9 -Wno-override-init
      2 -fsched-pressure

To address that move cc-option from scripts/Makefile.build to top Makefile
and export CC_USING_RECORD_MCOUNT to be used in original place.

While doing that also add -mrecord-mcount to CC_FLAGS_FTRACE (if gcc
actually supports it).

Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
---
 Makefile               | 7 +++++++
 scripts/Makefile.build | 9 +++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 503f533277c7..621ebdbfbf89 100644
--- a/Makefile
+++ b/Makefile
@@ -743,6 +743,13 @@ ifdef CONFIG_FUNCTION_TRACER
 ifndef CC_FLAGS_FTRACE
 CC_FLAGS_FTRACE := -pg
 endif
+ifdef CONFIG_FTRACE_MCOUNT_RECORD
+  # gcc 5 supports generating the mcount tables directly
+  ifeq ($(call cc-option-yn,-mrecord-mcount),y)
+    CC_FLAGS_FTRACE	+= -mrecord-mcount
+    export CC_USING_RECORD_MCOUNT := 1
+  endif
+endif
 ifdef CONFIG_HAVE_FENTRY
   ifeq ($(call cc-option-yn, -mfentry),y)
     CC_FLAGS_FTRACE	+= -mfentry
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 514ed63ff571..42ecb8cf7666 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -206,11 +206,8 @@ cmd_modversions_c =								\
 endif
 
 ifdef CONFIG_FTRACE_MCOUNT_RECORD
-# gcc 5 supports generating the mcount tables directly
-ifneq ($(call cc-option,-mrecord-mcount,y),y)
-KBUILD_CFLAGS += -mrecord-mcount
-else
-# else do it all manually
+ifndef CC_USING_RECORD_MCOUNT
+# compiler will not generate __mcount_loc use recordmcount or recordmcount.pl
 ifdef BUILD_C_RECORDMCOUNT
 ifeq ("$(origin RECORDMCOUNT_WARN)", "command line")
   RECORDMCOUNT_FLAGS = -w
@@ -239,7 +236,7 @@ cmd_record_mcount =						\
 	     "$(CC_FLAGS_FTRACE)" ]; then			\
 		$(sub_cmd_record_mcount)			\
 	fi;
-endif # -record-mcount
+endif # CC_USING_RECORD_MCOUNT
 endif # CONFIG_FTRACE_MCOUNT_RECORD
 
 ifdef CONFIG_STACK_VALIDATION
-- 
2.18.0.13.gd42ae10

Re: [PATCH 2/4] trace: avoid calling cc-option -mrecord-mcount for every Makefile

[Andi Kleen]

On Mon, Aug 06, 2018 at 03:17:44PM +0200, Vasily Gorbik wrote:
> Currently if CONFIG_FTRACE_MCOUNT_RECORD is enabled -mrecord-mcount
> compiler flag support is tested for every Makefile.

Good catch.  Does it make a measurable compile time difference?

> 
> Top 4 cc-option usages:
>     511 -mrecord-mcount
>      11  -fno-stack-protector
>       9 -Wno-override-init
>       2 -fsched-pressure
> 
> To address that move cc-option from scripts/Makefile.build to top Makefile
> and export CC_USING_RECORD_MCOUNT to be used in original place.
> 
> While doing that also add -mrecord-mcount to CC_FLAGS_FTRACE (if gcc
> actually supports it).
> 
> Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>

Acked-by: Andi Kleen <ak@linux.intel.com>

-Andi

Re: [PATCH 2/4] trace: avoid calling cc-option -mrecord-mcount for every Makefile

[Vasily Gorbik]

On Mon, Aug 06, 2018 at 11:07:40AM -0700, Andi Kleen wrote:
> On Mon, Aug 06, 2018 at 03:17:44PM +0200, Vasily Gorbik wrote:
> > Currently if CONFIG_FTRACE_MCOUNT_RECORD is enabled -mrecord-mcount
> > compiler flag support is tested for every Makefile.
> 
> Good catch.  Does it make a measurable compile time difference?
> 

A bit more than couple of seconds, but it does make a difference for a
delta build:
with patch:
real    0m9.405s
user    0m6.966s
sys     0m2.287s

without:
real    0m12.153s
user    0m9.058s
sys     0m3.026s

with -j48 on s390 24 cores lpar:
with patch:
real    0m1.949s
user    0m7.034s
sys     0m2.400s

without:
real    0m2.255s
user    0m9.243s
sys     0m3.266s

> > 
> > Top 4 cc-option usages:
> >     511 -mrecord-mcount

[PATCH 3/4] trace: add -mcount-nop option support

[Vasily Gorbik]

-mcount-nop gcc option generates the calls to the profiling functions
as nops which allows to avoid patching mcount jump with NOP instructions
initially.

-mcount-nop gcc option will be activated if platform selects
HAVE_NOP_MCOUNT and gcc actually supports it.
In addition to that CC_USING_NOP_MCOUNT is defined and could be used by
architectures to adapt ftrace patching behavior.

Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
---
 Makefile              | 6 ++++++
 kernel/trace/Kconfig  | 5 +++++
 kernel/trace/ftrace.c | 2 ++
 3 files changed, 13 insertions(+)

diff --git a/Makefile b/Makefile
index 621ebdbfbf89..281f4ededf3a 100644
--- a/Makefile
+++ b/Makefile
@@ -749,6 +749,12 @@ ifdef CONFIG_FTRACE_MCOUNT_RECORD
     CC_FLAGS_FTRACE	+= -mrecord-mcount
     export CC_USING_RECORD_MCOUNT := 1
   endif
+  ifdef CONFIG_HAVE_NOP_MCOUNT
+    ifeq ($(call cc-option-yn, -mnop-mcount),y)
+      CC_FLAGS_FTRACE	+= -mnop-mcount
+      CC_FLAGS_USING	+= -DCC_USING_NOP_MCOUNT
+    endif
+  endif
 endif
 ifdef CONFIG_HAVE_FENTRY
   ifeq ($(call cc-option-yn, -mfentry),y)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index dcc0166d1997..855109214e3d 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -47,6 +47,11 @@ config HAVE_FENTRY
 	help
 	  Arch supports the gcc options -pg with -mfentry
 
+config HAVE_NOP_MCOUNT
+	bool
+	help
+	  Arch supports the gcc options -pg with -mrecord-mcount and -nop-mcount
+
 config HAVE_C_RECORDMCOUNT
 	bool
 	help
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index caf9cbf35816..310dfcb20d5b 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2981,12 +2981,14 @@ static int ftrace_update_code(struct module *mod, struct ftrace_page *new_pgs)
 			p = &pg->records[i];
 			p->flags = rec_flags;
 
+#ifndef CC_USING_NOP_MCOUNT
 			/*
 			 * Do the initial record conversion from mcount jump
 			 * to the NOP instructions.
 			 */
 			if (!ftrace_code_disable(mod, p))
 				break;
+#endif
 
 			update_cnt++;
 		}
-- 
2.18.0.13.gd42ae10

[PATCH 4/4] s390/ftrace: add -mfentry and -mnop-mcount support

[Vasily Gorbik]

Utilize -mfentry and -mnop-mcount gcc options together with
-mrecord-mcount to get compiler generated calls to the profiling functions
as nops which are compatible with current -mhotpatch=0,3 approach.  At the
same time -mrecord-mcount enables __mcount_loc section generation by
the compiler which allows to avoid using scripts/recordmcount.pl script.

Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
---
 arch/s390/Kconfig              |  2 ++
 arch/s390/Makefile             | 16 +++++++++-------
 arch/s390/include/asm/ftrace.h |  6 +++---
 arch/s390/kernel/ftrace.c      |  2 +-
 arch/s390/kernel/mcount.S      |  2 +-
 5 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 8a1863d9ed53..71c2d9de379f 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -136,6 +136,7 @@ config S390
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS
+	select HAVE_FENTRY
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_TRACER
@@ -157,6 +158,7 @@ config S390
 	select HAVE_MEMBLOCK_NODE_MAP
 	select HAVE_MEMBLOCK_PHYS_MAP
 	select HAVE_MOD_ARCH_SPECIFIC
+	select HAVE_NOP_MCOUNT
 	select HAVE_OPROFILE
 	select HAVE_PERF_EVENTS
 	select HAVE_REGS_AND_STACK_ACCESS_API
diff --git a/arch/s390/Makefile b/arch/s390/Makefile
index 68a690442be0..8498babb5dad 100644
--- a/arch/s390/Makefile
+++ b/arch/s390/Makefile
@@ -86,13 +86,15 @@ ifdef CONFIG_EXPOLINE
 endif
 
 ifdef CONFIG_FUNCTION_TRACER
-# make use of hotpatch feature if the compiler supports it
-cc_hotpatch	:= -mhotpatch=0,3
-ifeq ($(call cc-option-yn,$(cc_hotpatch)),y)
-CC_FLAGS_FTRACE := $(cc_hotpatch)
-KBUILD_AFLAGS	+= -DCC_USING_HOTPATCH
-KBUILD_CFLAGS	+= -DCC_USING_HOTPATCH
-endif
+  ifeq ($(call cc-option-yn,-mfentry -mnop-mcount),n)
+    # make use of hotpatch feature if the compiler supports it
+    cc_hotpatch	:= -mhotpatch=0,3
+    ifeq ($(call cc-option-yn,$(cc_hotpatch)),y)
+      CC_FLAGS_FTRACE := $(cc_hotpatch)
+      KBUILD_AFLAGS	+= -DCC_USING_HOTPATCH
+      KBUILD_CFLAGS	+= -DCC_USING_HOTPATCH
+    endif
+  endif
 endif
 
 # Test CFI features of binutils
diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index cfccc0edd00d..8ea270fdc7fb 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -4,7 +4,7 @@
 
 #define ARCH_SUPPORTS_FTRACE_OPS 1
 
-#ifdef CC_USING_HOTPATCH
+#if defined(CC_USING_HOTPATCH) || defined(CC_USING_NOP_MCOUNT)
 #define MCOUNT_INSN_SIZE	6
 #else
 #define MCOUNT_INSN_SIZE	24
@@ -42,7 +42,7 @@ struct ftrace_insn {
 static inline void ftrace_generate_nop_insn(struct ftrace_insn *insn)
 {
 #ifdef CONFIG_FUNCTION_TRACER
-#ifdef CC_USING_HOTPATCH
+#if defined(CC_USING_HOTPATCH) || defined(CC_USING_NOP_MCOUNT)
 	/* brcl 0,0 */
 	insn->opc = 0xc004;
 	insn->disp = 0;
@@ -57,7 +57,7 @@ static inline void ftrace_generate_nop_insn(struct ftrace_insn *insn)
 static inline int is_ftrace_nop(struct ftrace_insn *insn)
 {
 #ifdef CONFIG_FUNCTION_TRACER
-#ifdef CC_USING_HOTPATCH
+#if defined(CC_USING_HOTPATCH) || defined(CC_USING_NOP_MCOUNT)
 	if (insn->disp == 0)
 		return 1;
 #else
diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
index dc76d813e420..84be7f02d0c2 100644
--- a/arch/s390/kernel/ftrace.c
+++ b/arch/s390/kernel/ftrace.c
@@ -61,7 +61,7 @@ unsigned long ftrace_plt;
 
 static inline void ftrace_generate_orig_insn(struct ftrace_insn *insn)
 {
-#ifdef CC_USING_HOTPATCH
+#if defined(CC_USING_HOTPATCH) || defined(CC_USING_NOP_MCOUNT)
 	/* brcl 0,0 */
 	insn->opc = 0xc004;
 	insn->disp = 0;
diff --git a/arch/s390/kernel/mcount.S b/arch/s390/kernel/mcount.S
index 27110f3294ed..e93fbf02490c 100644
--- a/arch/s390/kernel/mcount.S
+++ b/arch/s390/kernel/mcount.S
@@ -35,7 +35,7 @@ ENTRY(ftrace_caller)
 	.globl	ftrace_regs_caller
 	.set	ftrace_regs_caller,ftrace_caller
 	lgr	%r1,%r15
-#ifndef CC_USING_HOTPATCH
+#if !(defined(CC_USING_HOTPATCH) || defined(CC_USING_NOP_MCOUNT))
 	aghi	%r0,MCOUNT_RETURN_FIXUP
 #endif
 	aghi	%r15,-STACK_FRAME_SIZE
-- 
2.18.0.13.gd42ae10

Re: [PATCH 4/4] s390/ftrace: add -mfentry and -mnop-mcount support

[Heiko Carstens]

On Mon, Aug 06, 2018 at 03:17:47PM +0200, Vasily Gorbik wrote:
> Utilize -mfentry and -mnop-mcount gcc options together with
> -mrecord-mcount to get compiler generated calls to the profiling functions
> as nops which are compatible with current -mhotpatch=0,3 approach.  At the
> same time -mrecord-mcount enables __mcount_loc section generation by
> the compiler which allows to avoid using scripts/recordmcount.pl script.
> 
> Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
> ---
>  arch/s390/Kconfig              |  2 ++
>  arch/s390/Makefile             | 16 +++++++++-------
>  arch/s390/include/asm/ftrace.h |  6 +++---
>  arch/s390/kernel/ftrace.c      |  2 +-
>  arch/s390/kernel/mcount.S      |  2 +-
>  5 files changed, 16 insertions(+), 12 deletions(-)

Do you have numbers which tell how much this reduces the compile time of
the kernel on s390? I assume this change makes quite some difference.

Reviewed-by: Heiko Carstens <heiko.carstens@de.ibm.com>

Re: [PATCH 4/4] s390/ftrace: add -mfentry and -mnop-mcount support

[Vasily Gorbik]

On Tue, Aug 07, 2018 at 07:30:17AM +0200, Heiko Carstens wrote:
> On Mon, Aug 06, 2018 at 03:17:47PM +0200, Vasily Gorbik wrote:
> > Utilize -mfentry and -mnop-mcount gcc options together with
> > -mrecord-mcount to get compiler generated calls to the profiling functions
> > as nops which are compatible with current -mhotpatch=0,3 approach.  At the
> > same time -mrecord-mcount enables __mcount_loc section generation by
> > the compiler which allows to avoid using scripts/recordmcount.pl script.
> > 
> > Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
> > ---
> >  arch/s390/Kconfig              |  2 ++
> >  arch/s390/Makefile             | 16 +++++++++-------
> >  arch/s390/include/asm/ftrace.h |  6 +++---
> >  arch/s390/kernel/ftrace.c      |  2 +-
> >  arch/s390/kernel/mcount.S      |  2 +-
> >  5 files changed, 16 insertions(+), 12 deletions(-)
> 
> Do you have numbers which tell how much this reduces the compile time of
> the kernel on s390? I assume this change makes quite some difference.
> 

Very roughly, best time across several runs. With -j24 on 24 core lpar
with -mfentry -mrecord-mcount etc
real    0m54.748s
user    12m32.041s
sys     1m17.778s

vs -mhotpatch=0,3 + scripts/recordmcount.pl:
real    0m56.752s
user    12m52.386s
sys     1m35.873s