[PATCH/RFC 0/3] ftrace: allow arch specific compile options

[PATCH/RFC 0/3] ftrace: allow arch specific compile options

[Heiko Carstens]

This patch set enables s390 to use gcc's s390 specific "--mhotpatch" compile
option if the kernel is instrumented for function tracing.

The normal -pg compile option adds 24 bytes to each function prologue.
Performance measurements have shown that, depending on the workload, we have
an impact of up to 2% - 10% additional cpu time spent as compared to a kernel
which is compiled without the -pg option.

In both cases function tracing is disabled. What we see is the overhead
caused by simply adding 24 additional bytes to each function, where the
first instruction is patched to skip the rest of the code block, which
besides other effects causes instruction cache pollution.

These patches change code generation on s390 to only add a single six byte
nop to each function trace enabled function. With this change the overhead
gets reduced close to zero.

I tried to minimize the common code changes, which resulted mainly in a
new CC_FTRACE_FLAGS makefile variable and a new "nohotpatch" define.
The rest is s390 specific. The patches are against linux-next as of today.

The gcc hotpatch feature patch, which changes the existing hotpatch mechanism
to allow to specify the number of halfwords before/after the beginning of a
function is not yet upstream, but should be soon.

Heiko Carstens (3):
  ftrace: allow architectures to specify ftrace compile options
  ftrace: introduce nohotpatch function attribute
  s390/ftrace: hotpatch support for function tracing

 Makefile                       |  6 +++++-
 arch/s390/Kconfig              |  1 -
 arch/s390/Makefile             | 10 ++++++++++
 arch/s390/include/asm/ftrace.h | 15 +++++++++++++++
 arch/s390/kernel/Makefile      |  4 ++--
 arch/s390/kernel/ftrace.c      | 15 ++++++++++++++-
 arch/s390/kernel/kprobes.c     |  3 ++-
 arch/s390/kernel/mcount.S      |  2 ++
 include/linux/compiler.h       |  8 +++++++-
 kernel/Makefile                |  4 ++--
 kernel/events/Makefile         |  2 +-
 kernel/locking/Makefile        |  8 ++++----
 kernel/sched/Makefile          |  2 +-
 kernel/trace/Makefile          |  4 ++--
 lib/Makefile                   |  2 +-
 scripts/Makefile.build         |  5 +++--
 scripts/recordmcount.pl        |  9 +++++++--
 17 files changed, 78 insertions(+), 22 deletions(-)

-- 
2.1.4

[PATCH/RFC 1/3] ftrace: allow architectures to specify ftrace compile options

[Heiko Carstens]

If the kernel is compiled with function tracer support the -pg compile option
is passed to gcc to generate extra code into the prologue of each function.

This patch replaces the "open-coded" -pg compile flag with a CC_FLAGS_FTRACE
makefile variable which architectures can override if a different option
should be used for code generation.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 Makefile                | 6 +++++-
 kernel/Makefile         | 4 ++--
 kernel/events/Makefile  | 2 +-
 kernel/locking/Makefile | 8 ++++----
 kernel/sched/Makefile   | 2 +-
 kernel/trace/Makefile   | 4 ++--
 lib/Makefile            | 2 +-
 scripts/Makefile.build  | 5 +++--
 8 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/Makefile b/Makefile
index 109c17397e4e..069599c9c96b 100644
--- a/Makefile
+++ b/Makefile
@@ -742,10 +742,14 @@ KBUILD_CFLAGS 	+= $(call cc-option, -femit-struct-debug-baseonly) \
 endif
 
 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)
 endif
-KBUILD_CFLAGS	+= -pg $(CC_USING_FENTRY)
+KBUILD_CFLAGS	+= $(CC_FLAGS_FTRACE) $(CC_USING_FENTRY)
 KBUILD_AFLAGS	+= $(CC_USING_FENTRY)
 ifdef CONFIG_DYNAMIC_FTRACE
 	ifdef CONFIG_HAVE_C_RECORDMCOUNT
diff --git a/kernel/Makefile b/kernel/Makefile
index 616994f0a76f..07737e50fe6e 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -13,8 +13,8 @@ obj-y     = fork.o exec_domain.o panic.o \
 
 ifdef CONFIG_FUNCTION_TRACER
 # Do not trace debug files and internal ftrace files
-CFLAGS_REMOVE_cgroup-debug.o = -pg
-CFLAGS_REMOVE_irq_work.o = -pg
+CFLAGS_REMOVE_cgroup-debug.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_irq_work.o = $(CC_FLAGS_FTRACE)
 endif
 
 # cond_syscall is currently not LTO compatible
diff --git a/kernel/events/Makefile b/kernel/events/Makefile
index 103f5d147b2f..2925188f50ea 100644
--- a/kernel/events/Makefile
+++ b/kernel/events/Makefile
@@ -1,5 +1,5 @@
 ifdef CONFIG_FUNCTION_TRACER
-CFLAGS_REMOVE_core.o = -pg
+CFLAGS_REMOVE_core.o = $(CC_FLAGS_FTRACE)
 endif
 
 obj-y := core.o ring_buffer.o callchain.o
diff --git a/kernel/locking/Makefile b/kernel/locking/Makefile
index 4ca8eb151975..de7a416cca2a 100644
--- a/kernel/locking/Makefile
+++ b/kernel/locking/Makefile
@@ -2,10 +2,10 @@
 obj-y += mutex.o semaphore.o rwsem.o
 
 ifdef CONFIG_FUNCTION_TRACER
-CFLAGS_REMOVE_lockdep.o = -pg
-CFLAGS_REMOVE_lockdep_proc.o = -pg
-CFLAGS_REMOVE_mutex-debug.o = -pg
-CFLAGS_REMOVE_rtmutex-debug.o = -pg
+CFLAGS_REMOVE_lockdep.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_lockdep_proc.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_mutex-debug.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_rtmutex-debug.o = $(CC_FLAGS_FTRACE)
 endif
 
 obj-$(CONFIG_DEBUG_MUTEXES) += mutex-debug.o
diff --git a/kernel/sched/Makefile b/kernel/sched/Makefile
index ab32b7b0db5c..46be87024875 100644
--- a/kernel/sched/Makefile
+++ b/kernel/sched/Makefile
@@ -1,5 +1,5 @@
 ifdef CONFIG_FUNCTION_TRACER
-CFLAGS_REMOVE_clock.o = -pg
+CFLAGS_REMOVE_clock.o = $(CC_FLAGS_FTRACE)
 endif
 
 ifneq ($(CONFIG_SCHED_OMIT_FRAME_POINTER),y)
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 979ccde26720..98f26588255e 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -3,11 +3,11 @@
 
 ifdef CONFIG_FUNCTION_TRACER
 ORIG_CFLAGS := $(KBUILD_CFLAGS)
-KBUILD_CFLAGS = $(subst -pg,,$(ORIG_CFLAGS))
+KBUILD_CFLAGS = $(subst $(CC_FLAGS_FTRACE),,$(ORIG_CFLAGS))
 
 ifdef CONFIG_FTRACE_SELFTEST
 # selftest needs instrumentation
-CFLAGS_trace_selftest_dynamic.o = -pg
+CFLAGS_trace_selftest_dynamic.o = $(CC_FLAGS_FTRACE)
 obj-y += trace_selftest_dynamic.o
 endif
 endif
diff --git a/lib/Makefile b/lib/Makefile
index 3dca86fa7dc9..b1dbda74ef65 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -4,7 +4,7 @@
 
 ifdef CONFIG_FUNCTION_TRACER
 ORIG_CFLAGS := $(KBUILD_CFLAGS)
-KBUILD_CFLAGS = $(subst -pg,,$(ORIG_CFLAGS))
+KBUILD_CFLAGS = $(subst $(CC_FLAGS_FTRACE),,$(ORIG_CFLAGS))
 endif
 
 lib-y := ctype.o string.o vsprintf.o cmdline.o \
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 649ce6844033..1a170cbaa4e5 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -234,8 +234,9 @@ sub_cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH
 	"$(if $(part-of-module),1,0)" "$(@)";
 recordmcount_source := $(srctree)/scripts/recordmcount.pl
 endif
-cmd_record_mcount = 						\
-	if [ "$(findstring -pg,$(_c_flags))" = "-pg" ]; then	\
+cmd_record_mcount =						\
+	if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" =	\
+	     "$(CC_FLAGS_FTRACE)" ]; then 		    	\
 		$(sub_cmd_record_mcount)			\
 	fi;
 endif
-- 
2.1.4

[PATCH/RFC 2/3] ftrace: introduce nohotpatch function attribute

[Heiko Carstens]

gcc supports an s390 specific function attribute called "hotpatch".
It can be used to specify the number of halfwords that shall be added before
and after a function that shall be filled with nops for runtime patching.

s390 will use the hotpatch attribute for function tracing, therefore
introduce a nohotpatch define, depending on CC_USING_HOTPATCH, and add it
to the existing notrace define.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 include/linux/compiler.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index b4fd7013c9f4..b4ab98e0e10b 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -54,7 +54,13 @@ extern void __chk_io_ptr(const volatile void __iomem *);
 #include <linux/compiler-gcc.h>
 #endif
 
-#define notrace __attribute__((no_instrument_function))
+#ifdef CC_USING_HOTPATCH
+#define nohotpatch __attribute__((hotpatch(0,0)))
+#else
+#define nohotpatch
+#endif
+
+#define notrace __attribute__((no_instrument_function)) nohotpatch
 
 /* Intel compiler defines __GNUC__. So we will overwrite implementations
  * coming from above header files here
-- 
2.1.4

[PATCH/RFC 3/3] s390/ftrace: hotpatch support for function tracing

[Heiko Carstens]

Make use of gcc's hotpatch support to generate better code for ftrace
function tracing.
The generated code now contains only a six byte nop in each function
prologue instead of a 24 byte code block which will be runtime patched to
support function tracing.
With the new code generation the runtime overhead for supporting function
tracing is close to zero, while the original code did show a significant
performance impact.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 arch/s390/Kconfig              |  1 -
 arch/s390/Makefile             | 10 ++++++++++
 arch/s390/include/asm/ftrace.h | 15 +++++++++++++++
 arch/s390/kernel/Makefile      |  4 ++--
 arch/s390/kernel/ftrace.c      | 15 ++++++++++++++-
 arch/s390/kernel/kprobes.c     |  3 ++-
 arch/s390/kernel/mcount.S      |  2 ++
 scripts/recordmcount.pl        |  9 +++++++--
 8 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 7eba5b5723e9..8d11babf9aa5 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -117,7 +117,6 @@ config S390
 	select HAVE_BPF_JIT if 64BIT && PACK_STACK
 	select HAVE_CMPXCHG_DOUBLE
 	select HAVE_CMPXCHG_LOCAL
-	select HAVE_C_RECORDMCOUNT
 	select HAVE_DEBUG_KMEMLEAK
 	select HAVE_DYNAMIC_FTRACE if 64BIT
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS if 64BIT
diff --git a/arch/s390/Makefile b/arch/s390/Makefile
index e742ec5de9a7..acb6859c6a95 100644
--- a/arch/s390/Makefile
+++ b/arch/s390/Makefile
@@ -87,6 +87,16 @@ ifeq ($(call cc-option-yn,-mwarn-dynamicstack),y)
 cflags-$(CONFIG_WARN_DYNAMIC_STACK) += -mwarn-dynamicstack
 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
+endif
+
 KBUILD_CFLAGS	+= -mbackchain -msoft-float $(cflags-y)
 KBUILD_CFLAGS	+= -pipe -fno-strength-reduce -Wno-sign-compare
 KBUILD_AFLAGS	+= $(aflags-y)
diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index abb618f1ead2..836c56290499 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -3,8 +3,12 @@
 
 #define ARCH_SUPPORTS_FTRACE_OPS 1
 
+#ifdef CC_USING_HOTPATCH
+#define MCOUNT_INSN_SIZE	6
+#else
 #define MCOUNT_INSN_SIZE	24
 #define MCOUNT_RETURN_FIXUP	18
+#endif
 
 #ifndef __ASSEMBLY__
 
@@ -37,18 +41,29 @@ struct ftrace_insn {
 static inline void ftrace_generate_nop_insn(struct ftrace_insn *insn)
 {
 #ifdef CONFIG_FUNCTION_TRACER
+#ifdef CC_USING_HOTPATCH
+	/* brcl 0,0 */
+	insn->opc = 0xc004;
+	insn->disp = 0;
+#else
 	/* jg .+24 */
 	insn->opc = 0xc0f4;
 	insn->disp = MCOUNT_INSN_SIZE / 2;
 #endif
+#endif
 }
 
 static inline int is_ftrace_nop(struct ftrace_insn *insn)
 {
 #ifdef CONFIG_FUNCTION_TRACER
+#ifdef CC_USING_HOTPATCH
+	if (insn->disp == 0)
+		return 1;
+#else
 	if (insn->disp == MCOUNT_INSN_SIZE / 2)
 		return 1;
 #endif
+#endif
 	return 0;
 }
 
diff --git a/arch/s390/kernel/Makefile b/arch/s390/kernel/Makefile
index 204c43a4c245..31fab2676fe9 100644
--- a/arch/s390/kernel/Makefile
+++ b/arch/s390/kernel/Makefile
@@ -4,8 +4,8 @@
 
 ifdef CONFIG_FUNCTION_TRACER
 # Don't trace early setup code and tracing code
-CFLAGS_REMOVE_early.o = -pg
-CFLAGS_REMOVE_ftrace.o = -pg
+CFLAGS_REMOVE_early.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE)
 endif
 
 #
diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
index 3dabcae40e04..82c19899574f 100644
--- a/arch/s390/kernel/ftrace.c
+++ b/arch/s390/kernel/ftrace.c
@@ -46,6 +46,13 @@
  *	lg	%r14,8(%r15)		# offset 18
  * The jg instruction branches to offset 24 to skip as many instructions
  * as possible.
+ * In case we use gcc's hotpatch feature the original and also the disabled
+ * function prologue contains only a single six byte instruction and looks
+ * like this:
+ * >	brcl	0,0			# offset 0
+ * To enable ftrace the code gets patched like above and afterwards looks
+ * like this:
+ * >	brasl	%r0,ftrace_caller	# offset 0
  */
 
 unsigned long ftrace_plt;
@@ -64,9 +71,15 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
 	if (probe_kernel_read(&old, (void *) rec->ip, sizeof(old)))
 		return -EFAULT;
 	if (addr == MCOUNT_ADDR) {
-		/* Initial code replacement; we expect to see stg r14,8(r15) */
+		/* Initial code replacement */
+#ifdef CC_USING_HOTPATCH
+		/* We expect to see brcl 0,0 */
+		ftrace_generate_nop_insn(&orig);
+#else
+		/* We expect to see stg r14,8(r15) */
 		orig.opc = 0xe3e0;
 		orig.disp = 0xf0080024;
+#endif
 		ftrace_generate_nop_insn(&new);
 	} else if (old.opc == BREAKPOINT_INSTRUCTION) {
 		/*
diff --git a/arch/s390/kernel/kprobes.c b/arch/s390/kernel/kprobes.c
index 1e4c710dfb92..f516edc1fbe3 100644
--- a/arch/s390/kernel/kprobes.c
+++ b/arch/s390/kernel/kprobes.c
@@ -69,7 +69,8 @@ static void copy_instruction(struct kprobe *p)
 		/*
 		 * If kprobes patches the instruction that is morphed by
 		 * ftrace make sure that kprobes always sees the branch
-		 * "jg .+24" that skips the mcount block
+		 * "jg .+24" that skips the mcount block or the "brcl 0,0"
+		 * in case of hotpatch.
 		 */
 		ftrace_generate_nop_insn((struct ftrace_insn *)p->ainsn.insn);
 		p->ainsn.is_ftrace_insn = 1;
diff --git a/arch/s390/kernel/mcount.S b/arch/s390/kernel/mcount.S
index b6dfc5bfcb89..e499370fbccb 100644
--- a/arch/s390/kernel/mcount.S
+++ b/arch/s390/kernel/mcount.S
@@ -27,7 +27,9 @@ ENTRY(ftrace_caller)
 	.globl	ftrace_regs_caller
 	.set	ftrace_regs_caller,ftrace_caller
 	lgr	%r1,%r15
+#ifndef CC_USING_HOTPATCH
 	aghi	%r0,MCOUNT_RETURN_FIXUP
+#endif
 	aghi	%r15,-STACK_FRAME_SIZE
 	stg	%r1,__SF_BACKCHAIN(%r15)
 	stg	%r1,(STACK_PTREGS_GPRS+15*8)(%r15)
diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 537c38ca2e1c..826470d7f000 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -242,8 +242,13 @@ if ($arch eq "x86_64") {
     $cc .= " -m32";
 
 } elsif ($arch eq "s390" && $bits == 64) {
-    $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*R_390_(PC|PLT)32DBL\\s+_mcount\\+0x2\$";
-    $mcount_adjust = -14;
+    if ($cc =~ /-DCC_USING_HOTPATCH/) {
+	$mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*brcl\\s*0,[0-9a-f]+ <([^\+]*)>\$";
+	$mcount_adjust = 0;
+    } else {
+	$mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*R_390_(PC|PLT)32DBL\\s+_mcount\\+0x2\$";
+	$mcount_adjust = -14;
+    }
     $alignment = 8;
     $type = ".quad";
     $ld .= " -m elf64_s390";
-- 
2.1.4

Re: [PATCH/RFC 2/3] ftrace: introduce nohotpatch function attribute

[Steven Rostedt]

On Mon, 26 Jan 2015 13:54:53 +0100
Heiko Carstens <heiko.carstens@de.ibm.com> wrote:

> gcc supports an s390 specific function attribute called "hotpatch".
> It can be used to specify the number of halfwords that shall be added before
> and after a function that shall be filled with nops for runtime patching.
> 
> s390 will use the hotpatch attribute for function tracing, therefore
> introduce a nohotpatch define, depending on CC_USING_HOTPATCH, and add it
> to the existing notrace define.

Are the two mutually exclusive? That is, can you have -pg and hotpatch
together? Reason why I ask is, if you have either -pg or hotpatch, then
we only need "no_instrument_function" or "hotpatch" in the notrace
define, not both. But I could be wrong.

-- Steve

> 
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> ---
>  include/linux/compiler.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index b4fd7013c9f4..b4ab98e0e10b 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -54,7 +54,13 @@ extern void __chk_io_ptr(const volatile void __iomem *);
>  #include <linux/compiler-gcc.h>
>  #endif
>  
> -#define notrace __attribute__((no_instrument_function))
> +#ifdef CC_USING_HOTPATCH
> +#define nohotpatch __attribute__((hotpatch(0,0)))
> +#else
> +#define nohotpatch
> +#endif
> +
> +#define notrace __attribute__((no_instrument_function)) nohotpatch
>  
>  /* Intel compiler defines __GNUC__. So we will overwrite implementations
>   * coming from above header files here

Re: [PATCH/RFC 2/3] ftrace: introduce nohotpatch function attribute

[Heiko Carstens]

On Mon, Jan 26, 2015 at 09:37:01AM -0500, Steven Rostedt wrote:
> On Mon, 26 Jan 2015 13:54:53 +0100
> Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> 
> > gcc supports an s390 specific function attribute called "hotpatch".
> > It can be used to specify the number of halfwords that shall be added before
> > and after a function that shall be filled with nops for runtime patching.
> > 
> > s390 will use the hotpatch attribute for function tracing, therefore
> > introduce a nohotpatch define, depending on CC_USING_HOTPATCH, and add it
> > to the existing notrace define.
> 
> Are the two mutually exclusive? That is, can you have -pg and hotpatch
> together? Reason why I ask is, if you have either -pg or hotpatch, then
> we only need "no_instrument_function" or "hotpatch" in the notrace
> define, not both. But I could be wrong.

Actually they should be mutually exclusive. I just merged them "just in case".

So something like this

#ifdef CC_USING_HOTPATCH
#define notrace __attribute__((hotpatch(0,0)))
#else
#define notrace __attribute__((no_instrument_function))
#endif

will work as well (just tested).

Re: [PATCH/RFC 2/3] ftrace: introduce nohotpatch function attribute

[Steven Rostedt]

On Mon, 26 Jan 2015 16:03:19 +0100
Heiko Carstens <heiko.carstens@de.ibm.com> wrote:

> Actually they should be mutually exclusive. I just merged them "just in case".
> 
> So something like this
> 
> #ifdef CC_USING_HOTPATCH
> #define notrace __attribute__((hotpatch(0,0)))
> #else
> #define notrace __attribute__((no_instrument_function))
> #endif
> 
> will work as well (just tested).

I think the above change looks cleaner.

-- Steve

Re: [PATCH/RFC 2/3] ftrace: introduce nohotpatch function attribute

[Heiko Carstens]

On Mon, Jan 26, 2015 at 10:22:26AM -0500, Steven Rostedt wrote:
> On Mon, 26 Jan 2015 16:03:19 +0100
> Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> 
> > Actually they should be mutually exclusive. I just merged them "just in case".
> > 
> > So something like this
> > 
> > #ifdef CC_USING_HOTPATCH
> > #define notrace __attribute__((hotpatch(0,0)))
> > #else
> > #define notrace __attribute__((no_instrument_function))
> > #endif
> > 
> > will work as well (just tested).
> 
> I think the above change looks cleaner.

Yes, I agree with you and will change the patch.

Re: [PATCH/RFC 2/3] ftrace: introduce nohotpatch function attribute

[Dominik Vogt]

On Mon, Jan 26, 2015 at 09:37:01AM -0500, Steven Rostedt wrote:
> On Mon, 26 Jan 2015 13:54:53 +0100
> Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> > s390 will use the hotpatch attribute for function tracing, therefore
> > introduce a nohotpatch define, depending on CC_USING_HOTPATCH, and add it
> > to the existing notrace define.
> 
> Are the two mutually exclusive? That is, can you have -pg and hotpatch
> together? Reason why I ask is, if you have either -pg or hotpatch, then
> we only need "no_instrument_function" or "hotpatch" in the notrace
> define, not both. But I could be wrong.

While the kernel may use only profiling or hotpatch at the same
time, Gcc is able to generate both for the same function.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

Re: [PATCH/RFC 2/3] ftrace: introduce nohotpatch function attribute

[Steven Rostedt]

On Tue, 27 Jan 2015 07:19:42 +0100
Dominik Vogt <vogt@linux.vnet.ibm.com> wrote:
 
> While the kernel may use only profiling or hotpatch at the same
> time, Gcc is able to generate both for the same function.

Understood, but would that be useful for the kernel?

-- Steve

Re: [PATCH/RFC 2/3] ftrace: introduce nohotpatch function attribute

[Heiko Carstens]

On Tue, Jan 27, 2015 at 09:42:28AM -0500, Steven Rostedt wrote:
> On Tue, 27 Jan 2015 07:19:42 +0100
> Dominik Vogt <vogt@linux.vnet.ibm.com> wrote:
>  
> > While the kernel may use only profiling or hotpatch at the same
> > time, Gcc is able to generate both for the same function.
> 
> Understood, but would that be useful for the kernel?

Right now not. We may change that anyway in the future if needed.

Steven, how do we proceed with this small series?

Since there don't seem to be any objections, I'll repost with the
changed notrace define. However I think the whole stuff should go
upstream via the ftrace tree. Is that ok with you?

FWIW, the s390 gcc hotpatch feature was merged in the meantime:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=11762b8363737591bfb9c66093bc2edf289b917f

Re: [PATCH/RFC 2/3] ftrace: introduce nohotpatch function attribute

[Steven Rostedt]

On Wed, 28 Jan 2015 06:36:45 +0100
Heiko Carstens <heiko.carstens@de.ibm.com> wrote:

> On Tue, Jan 27, 2015 at 09:42:28AM -0500, Steven Rostedt wrote:
> > On Tue, 27 Jan 2015 07:19:42 +0100
> > Dominik Vogt <vogt@linux.vnet.ibm.com> wrote:
> >  
> > > While the kernel may use only profiling or hotpatch at the same
> > > time, Gcc is able to generate both for the same function.
> > 
> > Understood, but would that be useful for the kernel?
> 
> Right now not. We may change that anyway in the future if needed.
> 
> Steven, how do we proceed with this small series?
> 
> Since there don't seem to be any objections, I'll repost with the
> changed notrace define. However I think the whole stuff should go
> upstream via the ftrace tree. Is that ok with you?

I can take it or you can, I'm fine either way. If you want to take it,
just add my Acked-by. If you prefer me to take it, just let me know.

-- Steve

> 
> FWIW, the s390 gcc hotpatch feature was merged in the meantime:
> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=11762b8363737591bfb9c66093bc2edf289b917f

Re: [PATCH/RFC 2/3] ftrace: introduce nohotpatch function attribute

[Heiko Carstens]

On Wed, Jan 28, 2015 at 06:57:54AM -0500, Steven Rostedt wrote:
> On Wed, 28 Jan 2015 06:36:45 +0100
> Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> 
> > On Tue, Jan 27, 2015 at 09:42:28AM -0500, Steven Rostedt wrote:
> > > On Tue, 27 Jan 2015 07:19:42 +0100
> > > Dominik Vogt <vogt@linux.vnet.ibm.com> wrote:
> > >  
> > > > While the kernel may use only profiling or hotpatch at the same
> > > > time, Gcc is able to generate both for the same function.
> > > 
> > > Understood, but would that be useful for the kernel?
> > 
> > Right now not. We may change that anyway in the future if needed.
> > 
> > Steven, how do we proceed with this small series?
> > 
> > Since there don't seem to be any objections, I'll repost with the
> > changed notrace define. However I think the whole stuff should go
> > upstream via the ftrace tree. Is that ok with you?
> 
> I can take it or you can, I'm fine either way. If you want to take it,
> just add my Acked-by. If you prefer me to take it, just let me know.

Ok, then we'll put that on the s390 tree for the next merge window, so
we can fix any potential fallout easier.
I'll add your Acked-by to all three patches. The notrace patch is the
only one that I changed (see below).

Thanks!

>From a24aba18536b64f5395d4901499152191cbc4fae Mon Sep 17 00:00:00 2001
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Date: Sun, 18 Jan 2015 16:45:42 +0100
Subject: [PATCH 1/3] ftrace: let notrace function attribute disable
 hotpatching if necessary

gcc supports an s390 specific function attribute called "hotpatch".
It can be used to specify the number of halfwords that shall be added before
and after a function and which shall be filled with nops for runtime patching.

s390 will use the hotpatch attribute for function tracing, therefore make
sure that the notrace function attribute either disables the mcount call
or in case of hotpatch nop generation.

Acked-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 include/linux/compiler.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 33063f872ee3..1a31ab4fbdca 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -54,7 +54,11 @@ extern void __chk_io_ptr(const volatile void __iomem *);
 #include <linux/compiler-gcc.h>
 #endif
 
+#ifdef CC_USING_HOTPATCH
+#define notrace __attribute__((hotpatch(0,0)))
+#else
 #define notrace __attribute__((no_instrument_function))
+#endif
 
 /* Intel compiler defines __GNUC__. So we will overwrite implementations
  * coming from above header files here
-- 
2.1.4

Re: [PATCH/RFC 2/3] ftrace: introduce nohotpatch function attribute

[Steven Rostedt]

On Wed, 28 Jan 2015 13:18:33 +0100
Heiko Carstens <heiko.carstens@de.ibm.com> wrote:


> Ok, then we'll put that on the s390 tree for the next merge window, so
> we can fix any potential fallout easier.
> I'll add your Acked-by to all three patches. The notrace patch is the
> only one that I changed (see below).
> 

Yep, looks fine.

-- Steve