[PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

[PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

[Heiko Carstens]

This small series adds DYNAMIC_FTRACE_WITH_DIRECT_CALL support for
s390 and is based on linux-next 20211012.

Besides the architecture backend this also adds s390 ftrace direct
call samples, and slightly changes config option handling a bit, so
that options only have to be selected. This way also additional future
architectures can easily add their trampolines to the samples.

If ok, I'd like to get this upstream via the s390 tree with the next
merge window.

Heiko Carstens (4):
  s390/ftrace: add HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALL support
  s390: make STACK_FRAME_OVERHEAD available via asm-offsets.h
  samples: add HAVE_SAMPLE_FTRACE_DIRECT config option
  samples: add s390 support for ftrace direct call samples

 arch/s390/Kconfig                     |  2 ++
 arch/s390/include/asm/ftrace.h        | 12 ++++++++
 arch/s390/kernel/asm-offsets.c        |  1 +
 arch/s390/kernel/mcount.S             | 23 ++++++++++----
 arch/x86/Kconfig                      |  1 +
 samples/Kconfig                       |  5 ++-
 samples/ftrace/ftrace-direct-modify.c | 44 +++++++++++++++++++++++++++
 samples/ftrace/ftrace-direct-too.c    | 28 +++++++++++++++++
 samples/ftrace/ftrace-direct.c        | 28 +++++++++++++++++
 9 files changed, 137 insertions(+), 7 deletions(-)

-- 
2.25.1

[PATCH 1/4] s390/ftrace: add HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALL support

[Heiko Carstens]

This is the s390 variant of commit 562955fe6a55 ("ftrace/x86: Add
register_ftrace_direct() for custom trampolines").

Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Reviewed-by: Sven Schnelle <svens@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/Kconfig              |  1 +
 arch/s390/include/asm/ftrace.h | 12 ++++++++++++
 arch/s390/kernel/mcount.S      | 23 +++++++++++++++++------
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 87b71b91e5c9..908da6f1aa0d 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -153,6 +153,7 @@ config S390
 	select HAVE_DMA_CONTIGUOUS
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_DYNAMIC_FTRACE_WITH_ARGS
+	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS
 	select HAVE_EBPF_JIT if PACK_STACK && HAVE_MARCH_Z196_FEATURES
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS
diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index 4a721b44d4f4..267f70f4393f 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -58,6 +58,18 @@ static __always_inline void ftrace_instruction_pointer_set(struct ftrace_regs *f
 	regs->psw.addr = ip;
 }
 
+/*
+ * When an ftrace registered caller is tracing a function that is
+ * also set by a register_ftrace_direct() call, it needs to be
+ * differentiated in the ftrace_caller trampoline. To do this,
+ * place the direct caller in the ORIG_GPR2 part of pt_regs. This
+ * tells the ftrace_caller that there's a direct caller.
+ */
+static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
+{
+	regs->orig_gpr2 = addr;
+}
+
 /*
  * Even though the system call numbers are identical for s390/s390x a
  * different system call table is used for compat tasks. This may lead
diff --git a/arch/s390/kernel/mcount.S b/arch/s390/kernel/mcount.S
index cc25fbd75ea9..39bcc0e39a10 100644
--- a/arch/s390/kernel/mcount.S
+++ b/arch/s390/kernel/mcount.S
@@ -22,10 +22,11 @@ ENTRY(ftrace_stub)
 	BR_EX	%r14
 ENDPROC(ftrace_stub)
 
-#define STACK_FRAME_SIZE  (STACK_FRAME_OVERHEAD + __PT_SIZE)
-#define STACK_PTREGS	  (STACK_FRAME_OVERHEAD)
-#define STACK_PTREGS_GPRS (STACK_PTREGS + __PT_GPRS)
-#define STACK_PTREGS_PSW  (STACK_PTREGS + __PT_PSW)
+#define STACK_FRAME_SIZE	(STACK_FRAME_OVERHEAD + __PT_SIZE)
+#define STACK_PTREGS		(STACK_FRAME_OVERHEAD)
+#define STACK_PTREGS_GPRS	(STACK_PTREGS + __PT_GPRS)
+#define STACK_PTREGS_PSW	(STACK_PTREGS + __PT_PSW)
+#define STACK_PTREGS_ORIG_GPR2	(STACK_PTREGS + __PT_ORIG_GPR2)
 #ifdef __PACK_STACK
 /* allocate just enough for r14, r15 and backchain */
 #define TRACED_FUNC_FRAME_SIZE	24
@@ -51,6 +52,7 @@ ENDPROC(ftrace_stub)
 	# allocate pt_regs and stack frame for ftrace_trace_function
 	aghi	%r15,-STACK_FRAME_SIZE
 	stg	%r1,(STACK_PTREGS_GPRS+15*8)(%r15)
+	xc	STACK_PTREGS_ORIG_GPR2(8,%r15),STACK_PTREGS_ORIG_GPR2(%r15)
 
 	.if \allregs == 1
 	stg	%r14,(STACK_PTREGS_PSW)(%r15)
@@ -101,8 +103,17 @@ SYM_INNER_LABEL(ftrace_graph_caller, SYM_L_GLOBAL)
 	stg	%r2,(STACK_PTREGS_GPRS+14*8)(%r15)
 .Lftrace_graph_caller_end:
 #endif
-	lg	%r1,(STACK_PTREGS_PSW+8)(%r15)
-	lmg	%r2,%r15,(STACK_PTREGS_GPRS+2*8)(%r15)
+	lg	%r0,(STACK_PTREGS_PSW+8)(%r15)
+#ifdef CONFIG_HAVE_MARCH_Z196_FEATURES
+	ltg	%r1,STACK_PTREGS_ORIG_GPR2(%r15)
+	locgrz	%r1,%r0
+#else
+	lg	%r1,STACK_PTREGS_ORIG_GPR2(%r15)
+	ltgr	%r1,%r1
+	jnz	0f
+	lgr	%r1,%r0
+#endif
+0:	lmg	%r2,%r15,(STACK_PTREGS_GPRS+2*8)(%r15)
 	BR_EX	%r1
 SYM_CODE_END(ftrace_common)
 
-- 
2.25.1

[PATCH 2/4] s390: make STACK_FRAME_OVERHEAD available via asm-offsets.h

[Heiko Carstens]

Make STACK_FRAME_OVERHEAD available via asm-offsets.h. This allows to
add s390 specific asm code to e.g. ftrace samples, without requiring
to add random header files, which might cause all sort of problems on
other architectures. asm-offsets.h can be assumed to be non-problematic.

Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Reviewed-by: Sven Schnelle <svens@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/asm-offsets.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c
index b57da9338588..b6ee3fd7fe0c 100644
--- a/arch/s390/kernel/asm-offsets.c
+++ b/arch/s390/kernel/asm-offsets.c
@@ -45,6 +45,7 @@ int main(void)
 	OFFSET(__SF_SIE_SAVEAREA, stack_frame, empty1[2]);
 	OFFSET(__SF_SIE_REASON, stack_frame, empty1[3]);
 	OFFSET(__SF_SIE_FLAGS, stack_frame, empty1[4]);
+	DEFINE(STACK_FRAME_OVERHEAD, sizeof(struct stack_frame));
 	BLANK();
 	/* idle data offsets */
 	OFFSET(__CLOCK_IDLE_ENTER, s390_idle_data, clock_idle_enter);
-- 
2.25.1

[PATCH 3/4] samples: add HAVE_SAMPLE_FTRACE_DIRECT config option

[Heiko Carstens]

Add HAVE_SAMPLE_FTRACE_DIRECT config option which can be selected by
architectures which have support for ftrace direct call samples.

Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Reviewed-by: Sven Schnelle <svens@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/x86/Kconfig | 1 +
 samples/Kconfig  | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index a52e81cb256e..917391003c7a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -189,6 +189,7 @@ config X86
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS
 	select HAVE_DYNAMIC_FTRACE_WITH_ARGS	if X86_64
 	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+	select HAVE_SAMPLE_FTRACE_DIRECT	if X86_64
 	select HAVE_EBPF_JIT
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS
 	select HAVE_EISA
diff --git a/samples/Kconfig b/samples/Kconfig
index 8cbd6490823f..0823b97d8546 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -26,7 +26,7 @@ config SAMPLE_TRACE_PRINTK
 config SAMPLE_FTRACE_DIRECT
 	tristate "Build register_ftrace_direct() example"
 	depends on DYNAMIC_FTRACE_WITH_DIRECT_CALLS && m
-	depends on X86_64 # has x86_64 inlined asm
+	depends on HAVE_SAMPLE_FTRACE_DIRECT
 	help
 	  This builds an ftrace direct function example
 	  that hooks to wake_up_process and prints the parameters.
@@ -226,3 +226,6 @@ config SAMPLE_WATCH_QUEUE
 source "samples/rust/Kconfig"
 
 endif # SAMPLES
+
+config HAVE_SAMPLE_FTRACE_DIRECT
+	bool
-- 
2.25.1

[PATCH 4/4] samples: add s390 support for ftrace direct call samples

[Heiko Carstens]

Add s390 support for ftrace direct call samples, which also enables
ftrace direct call selftests within ftrace selftests.

Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Reviewed-by: Sven Schnelle <svens@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/Kconfig                     |  1 +
 samples/ftrace/ftrace-direct-modify.c | 44 +++++++++++++++++++++++++++
 samples/ftrace/ftrace-direct-too.c    | 28 +++++++++++++++++
 samples/ftrace/ftrace-direct.c        | 28 +++++++++++++++++
 4 files changed, 101 insertions(+)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 908da6f1aa0d..f615c3f65f5a 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -192,6 +192,7 @@ config S390
 	select HAVE_REGS_AND_STACK_ACCESS_API
 	select HAVE_RELIABLE_STACKTRACE
 	select HAVE_RSEQ
+	select HAVE_SAMPLE_FTRACE_DIRECT
 	select HAVE_SOFTIRQ_ON_OWN_STACK
 	select HAVE_SYSCALL_TRACEPOINTS
 	select HAVE_VIRT_CPU_ACCOUNTING
diff --git a/samples/ftrace/ftrace-direct-modify.c b/samples/ftrace/ftrace-direct-modify.c
index 5b9a09957c6e..690e4a9ff333 100644
--- a/samples/ftrace/ftrace-direct-modify.c
+++ b/samples/ftrace/ftrace-direct-modify.c
@@ -2,6 +2,7 @@
 #include <linux/module.h>
 #include <linux/kthread.h>
 #include <linux/ftrace.h>
+#include <asm/asm-offsets.h>
 
 void my_direct_func1(void)
 {
@@ -18,6 +19,8 @@ extern void my_tramp2(void *);
 
 static unsigned long my_ip = (unsigned long)schedule;
 
+#ifdef CONFIG_X86_64
+
 asm (
 "	.pushsection    .text, \"ax\", @progbits\n"
 "	.type		my_tramp1, @function\n"
@@ -41,6 +44,47 @@ asm (
 "	.popsection\n"
 );
 
+#endif /* CONFIG_X86_64 */
+
+#ifdef CONFIG_S390
+
+asm (
+"	.pushsection	.text, \"ax\", @progbits\n"
+"	.type		my_tramp1, @function\n"
+"	.globl		my_tramp1\n"
+"   my_tramp1:"
+"	lgr		%r1,%r15\n"
+"	stmg		%r0,%r5,"__stringify(__SF_GPRS)"(%r15)\n"
+"	stg		%r14,"__stringify(__SF_GPRS+8*8)"(%r15)\n"
+"	aghi		%r15,"__stringify(-STACK_FRAME_OVERHEAD)"\n"
+"	stg		%r1,"__stringify(__SF_BACKCHAIN)"(%r15)\n"
+"	brasl		%r14,my_direct_func1\n"
+"	aghi		%r15,"__stringify(STACK_FRAME_OVERHEAD)"\n"
+"	lmg		%r0,%r5,"__stringify(__SF_GPRS)"(%r15)\n"
+"	lg		%r14,"__stringify(__SF_GPRS+8*8)"(%r15)\n"
+"	lgr		%r1,%r0\n"
+"	br		%r1\n"
+"	.size		my_tramp1, .-my_tramp1\n"
+"	.type		my_tramp2, @function\n"
+"	.globl		my_tramp2\n"
+"   my_tramp2:"
+"	lgr		%r1,%r15\n"
+"	stmg		%r0,%r5,"__stringify(__SF_GPRS)"(%r15)\n"
+"	stg		%r14,"__stringify(__SF_GPRS+8*8)"(%r15)\n"
+"	aghi		%r15,"__stringify(-STACK_FRAME_OVERHEAD)"\n"
+"	stg		%r1,"__stringify(__SF_BACKCHAIN)"(%r15)\n"
+"	brasl		%r14,my_direct_func2\n"
+"	aghi		%r15,"__stringify(STACK_FRAME_OVERHEAD)"\n"
+"	lmg		%r0,%r5,"__stringify(__SF_GPRS)"(%r15)\n"
+"	lg		%r14,"__stringify(__SF_GPRS+8*8)"(%r15)\n"
+"	lgr		%r1,%r0\n"
+"	br		%r1\n"
+"	.size		my_tramp2, .-my_tramp2\n"
+"	.popsection\n"
+);
+
+#endif /* CONFIG_S390 */
+
 static unsigned long my_tramp = (unsigned long)my_tramp1;
 static unsigned long tramps[2] = {
 	(unsigned long)my_tramp1,
diff --git a/samples/ftrace/ftrace-direct-too.c b/samples/ftrace/ftrace-direct-too.c
index 3f0079c9bd6f..6e0de725bf22 100644
--- a/samples/ftrace/ftrace-direct-too.c
+++ b/samples/ftrace/ftrace-direct-too.c
@@ -3,6 +3,7 @@
 
 #include <linux/mm.h> /* for handle_mm_fault() */
 #include <linux/ftrace.h>
+#include <asm/asm-offsets.h>
 
 void my_direct_func(struct vm_area_struct *vma,
 			unsigned long address, unsigned int flags)
@@ -13,6 +14,8 @@ void my_direct_func(struct vm_area_struct *vma,
 
 extern void my_tramp(void *);
 
+#ifdef CONFIG_X86_64
+
 asm (
 "	.pushsection    .text, \"ax\", @progbits\n"
 "	.type		my_tramp, @function\n"
@@ -33,6 +36,31 @@ asm (
 "	.popsection\n"
 );
 
+#endif /* CONFIG_X86_64 */
+
+#ifdef CONFIG_S390
+
+asm (
+"	.pushsection	.text, \"ax\", @progbits\n"
+"	.type		my_tramp, @function\n"
+"	.globl		my_tramp\n"
+"   my_tramp:"
+"	lgr		%r1,%r15\n"
+"	stmg		%r0,%r5,"__stringify(__SF_GPRS)"(%r15)\n"
+"	stg		%r14,"__stringify(__SF_GPRS+8*8)"(%r15)\n"
+"	aghi		%r15,"__stringify(-STACK_FRAME_OVERHEAD)"\n"
+"	stg		%r1,"__stringify(__SF_BACKCHAIN)"(%r15)\n"
+"	brasl		%r14,my_direct_func\n"
+"	aghi		%r15,"__stringify(STACK_FRAME_OVERHEAD)"\n"
+"	lmg		%r0,%r5,"__stringify(__SF_GPRS)"(%r15)\n"
+"	lg		%r14,"__stringify(__SF_GPRS+8*8)"(%r15)\n"
+"	lgr		%r1,%r0\n"
+"	br		%r1\n"
+"	.size		my_tramp, .-my_tramp\n"
+"	.popsection\n"
+);
+
+#endif /* CONFIG_S390 */
 
 static int __init ftrace_direct_init(void)
 {
diff --git a/samples/ftrace/ftrace-direct.c b/samples/ftrace/ftrace-direct.c
index a2729d1ef17f..a30aa42ec76a 100644
--- a/samples/ftrace/ftrace-direct.c
+++ b/samples/ftrace/ftrace-direct.c
@@ -3,6 +3,7 @@
 
 #include <linux/sched.h> /* for wake_up_process() */
 #include <linux/ftrace.h>
+#include <asm/asm-offsets.h>
 
 void my_direct_func(struct task_struct *p)
 {
@@ -11,6 +12,8 @@ void my_direct_func(struct task_struct *p)
 
 extern void my_tramp(void *);
 
+#ifdef CONFIG_X86_64
+
 asm (
 "	.pushsection    .text, \"ax\", @progbits\n"
 "	.type		my_tramp, @function\n"
@@ -27,6 +30,31 @@ asm (
 "	.popsection\n"
 );
 
+#endif /* CONFIG_X86_64 */
+
+#ifdef CONFIG_S390
+
+asm (
+"	.pushsection	.text, \"ax\", @progbits\n"
+"	.type		my_tramp, @function\n"
+"	.globl		my_tramp\n"
+"   my_tramp:"
+"	lgr		%r1,%r15\n"
+"	stmg		%r0,%r5,"__stringify(__SF_GPRS)"(%r15)\n"
+"	stg		%r14,"__stringify(__SF_GPRS+8*8)"(%r15)\n"
+"	aghi		%r15,"__stringify(-STACK_FRAME_OVERHEAD)"\n"
+"	stg		%r1,"__stringify(__SF_BACKCHAIN)"(%r15)\n"
+"	brasl		%r14,my_direct_func\n"
+"	aghi		%r15,"__stringify(STACK_FRAME_OVERHEAD)"\n"
+"	lmg		%r0,%r5,"__stringify(__SF_GPRS)"(%r15)\n"
+"	lg		%r14,"__stringify(__SF_GPRS+8*8)"(%r15)\n"
+"	lgr		%r1,%r0\n"
+"	br		%r1\n"
+"	.size		my_tramp, .-my_tramp\n"
+"	.popsection\n"
+);
+
+#endif /* CONFIG_S390 */
 
 static int __init ftrace_direct_init(void)
 {
-- 
2.25.1

Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

[Steven Rostedt]

On Tue, 12 Oct 2021 15:37:58 +0200
Heiko Carstens <hca@linux.ibm.com> wrote:

> This small series adds DYNAMIC_FTRACE_WITH_DIRECT_CALL support for
> s390 and is based on linux-next 20211012.

Cool!

> 
> Besides the architecture backend this also adds s390 ftrace direct
> call samples, and slightly changes config option handling a bit, so
> that options only have to be selected. This way also additional future
> architectures can easily add their trampolines to the samples.

Makes sense.

> 
> If ok, I'd like to get this upstream via the s390 tree with the next
> merge window.

A quick look at the patches look fine to me. I'll do a bit more digging
before adding a Reviewed-by.

One thing you may want to note, we are working on fixing direct trampolines
that conflict with the function graph tracer, and have patches that fix it.
I'm not that familiar on how ftrace works on s390, but you may want to
investigate this, because if s390 has the issues that x86 has, where you
can't have both function graph tracing and a direct trampoline on the same
function.

See here:

  https://lore.kernel.org/all/20211008091336.33616-1-jolsa@kernel.org/

-- Steve


> 
> Heiko Carstens (4):
>   s390/ftrace: add HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALL support
>   s390: make STACK_FRAME_OVERHEAD available via asm-offsets.h
>   samples: add HAVE_SAMPLE_FTRACE_DIRECT config option
>   samples: add s390 support for ftrace direct call samples
> 
>  arch/s390/Kconfig                     |  2 ++
>  arch/s390/include/asm/ftrace.h        | 12 ++++++++
>  arch/s390/kernel/asm-offsets.c        |  1 +
>  arch/s390/kernel/mcount.S             | 23 ++++++++++----
>  arch/x86/Kconfig                      |  1 +
>  samples/Kconfig                       |  5 ++-
>  samples/ftrace/ftrace-direct-modify.c | 44 +++++++++++++++++++++++++++
>  samples/ftrace/ftrace-direct-too.c    | 28 +++++++++++++++++
>  samples/ftrace/ftrace-direct.c        | 28 +++++++++++++++++
>  9 files changed, 137 insertions(+), 7 deletions(-)
> 

Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

[Heiko Carstens]

On Tue, Oct 12, 2021 at 09:48:52AM -0400, Steven Rostedt wrote:
> On Tue, 12 Oct 2021 15:37:58 +0200
> Heiko Carstens <hca@linux.ibm.com> wrote:
> > This small series adds DYNAMIC_FTRACE_WITH_DIRECT_CALL support for
> > s390 and is based on linux-next 20211012.
...
> > Besides the architecture backend this also adds s390 ftrace direct
> > call samples, and slightly changes config option handling a bit, so
> > that options only have to be selected. This way also additional future
> > architectures can easily add their trampolines to the samples.
...
> > If ok, I'd like to get this upstream via the s390 tree with the next
> > merge window.
> 
> A quick look at the patches look fine to me. I'll do a bit more digging
> before adding a Reviewed-by.
> 
> One thing you may want to note, we are working on fixing direct trampolines
> that conflict with the function graph tracer, and have patches that fix it.
> I'm not that familiar on how ftrace works on s390, but you may want to
> investigate this, because if s390 has the issues that x86 has, where you
> can't have both function graph tracing and a direct trampoline on the same
> function.
> 
> See here:
> 
>   https://lore.kernel.org/all/20211008091336.33616-1-jolsa@kernel.org/

I applied Jiri's patch set and the newly added selftest passes.

Note: s390 will also get HAVE_DYNAMIC_FTRACE_WITH_ARGS support, which is
required for the new selftest - this is currently only in linux-next.
See commit 5740a7c71ab6 ("s390/ftrace: add HAVE_DYNAMIC_FTRACE_WITH_ARGS
support") in linux-next.

Also manually testing with loading the ftrace-direct test module and
enabling the function graph tracer seems to work correctly:

  6) + 15.138 us   |            }
  6)               |            wake_up_process() {
  6)               |              my_direct_func [ftrace_direct]() {
  6)               |                /* waking up ksoftirqd/6-44 */
  6)   0.944 us    |              }
  6)               |              try_to_wake_up() {
  6)   0.185 us    |                kthread_is_per_cpu();

One thing to note: Jiri adds a new a sample module, which obviously
will not compile for s390. Not sure if the config mechanism I propose
with this patch set is the best way to address this - it would then
require to add a config option for each new sample module.

Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

[Steven Rostedt]

On Tue, 12 Oct 2021 16:59:02 +0200
Heiko Carstens <hca@linux.ibm.com> wrote:

> One thing to note: Jiri adds a new a sample module, which obviously
> will not compile for s390. Not sure if the config mechanism I propose
> with this patch set is the best way to address this - it would then
> require to add a config option for each new sample module.

Is that really an issue?

We could just group them, as long as they have the same prefix.

HAVE_SAMPLE_FTRACE_DIRECT
HAVE_SAMPLE_FTRACE_MULTI_DIRECT

??

-- Steve

Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

[Jiri Olsa]

On Tue, Oct 12, 2021 at 04:59:02PM +0200, Heiko Carstens wrote:
> On Tue, Oct 12, 2021 at 09:48:52AM -0400, Steven Rostedt wrote:
> > On Tue, 12 Oct 2021 15:37:58 +0200
> > Heiko Carstens <hca@linux.ibm.com> wrote:
> > > This small series adds DYNAMIC_FTRACE_WITH_DIRECT_CALL support for
> > > s390 and is based on linux-next 20211012.
> ...
> > > Besides the architecture backend this also adds s390 ftrace direct
> > > call samples, and slightly changes config option handling a bit, so
> > > that options only have to be selected. This way also additional future
> > > architectures can easily add their trampolines to the samples.
> ...
> > > If ok, I'd like to get this upstream via the s390 tree with the next
> > > merge window.
> > 
> > A quick look at the patches look fine to me. I'll do a bit more digging
> > before adding a Reviewed-by.
> > 
> > One thing you may want to note, we are working on fixing direct trampolines
> > that conflict with the function graph tracer, and have patches that fix it.
> > I'm not that familiar on how ftrace works on s390, but you may want to
> > investigate this, because if s390 has the issues that x86 has, where you
> > can't have both function graph tracing and a direct trampoline on the same
> > function.
> > 
> > See here:
> > 
> >   https://lore.kernel.org/all/20211008091336.33616-1-jolsa@kernel.org/
> 
> I applied Jiri's patch set and the newly added selftest passes.

nice, could I have your Tested-by? ;-)

thanks,
jirka

> 
> Note: s390 will also get HAVE_DYNAMIC_FTRACE_WITH_ARGS support, which is
> required for the new selftest - this is currently only in linux-next.
> See commit 5740a7c71ab6 ("s390/ftrace: add HAVE_DYNAMIC_FTRACE_WITH_ARGS
> support") in linux-next.
> 
> Also manually testing with loading the ftrace-direct test module and
> enabling the function graph tracer seems to work correctly:
> 
>   6) + 15.138 us   |            }
>   6)               |            wake_up_process() {
>   6)               |              my_direct_func [ftrace_direct]() {
>   6)               |                /* waking up ksoftirqd/6-44 */
>   6)   0.944 us    |              }
>   6)               |              try_to_wake_up() {
>   6)   0.185 us    |                kthread_is_per_cpu();
> 
> One thing to note: Jiri adds a new a sample module, which obviously
> will not compile for s390. Not sure if the config mechanism I propose
> with this patch set is the best way to address this - it would then
> require to add a config option for each new sample module.
> 

Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

[Heiko Carstens]

On Tue, Oct 12, 2021 at 11:34:04AM -0400, Steven Rostedt wrote:
> On Tue, 12 Oct 2021 16:59:02 +0200
> Heiko Carstens <hca@linux.ibm.com> wrote:
> 
> > One thing to note: Jiri adds a new a sample module, which obviously
> > will not compile for s390. Not sure if the config mechanism I propose
> > with this patch set is the best way to address this - it would then
> > require to add a config option for each new sample module.
> 
> Is that really an issue?
> 
> We could just group them, as long as they have the same prefix.
> 
> HAVE_SAMPLE_FTRACE_DIRECT
> HAVE_SAMPLE_FTRACE_MULTI_DIRECT

Sure, works for me. I just thought I should mention this.

Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

[Heiko Carstens]

On Tue, Oct 12, 2021 at 07:04:53PM +0200, Jiri Olsa wrote:
> > > See here:
> > > 
> > >   https://lore.kernel.org/all/20211008091336.33616-1-jolsa@kernel.org/
> > 
> > I applied Jiri's patch set and the newly added selftest passes.
> 
> nice, could I have your Tested-by? ;-)

Well, now I added also the missing pieces to ftrace-direct-multi
sample module and when loading that and looking into
/sys/kernel/debug/tracing/trace it looks like "my_direct_func" gets
some random junk as parameter and nothing that could count as "ip".

Will look into that, probably tomorrow.

Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

[Heiko Carstens]

On Tue, Oct 12, 2021 at 08:25:14PM +0200, Heiko Carstens wrote:
> On Tue, Oct 12, 2021 at 07:04:53PM +0200, Jiri Olsa wrote:
> > > > See here:
> > > > 
> > > >   https://lore.kernel.org/all/20211008091336.33616-1-jolsa@kernel.org/
> > > 
> > > I applied Jiri's patch set and the newly added selftest passes.
> > 
> > nice, could I have your Tested-by? ;-)
> 
> Well, now I added also the missing pieces to ftrace-direct-multi
> sample module and when loading that and looking into
> /sys/kernel/debug/tracing/trace it looks like "my_direct_func" gets
> some random junk as parameter and nothing that could count as "ip".
> 
> Will look into that, probably tomorrow.

Ok, if I load the correct module, it even works. I had a bug in the
first version, fixed it, but still loaded the broken module to test
my changes. Clever me ;)

So it all works for me.

Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

[Steven Rostedt]

On Wed, 13 Oct 2021 10:48:20 +0200
Heiko Carstens <hca@linux.ibm.com> wrote:

> On Tue, Oct 12, 2021 at 08:25:14PM +0200, Heiko Carstens wrote:
> > On Tue, Oct 12, 2021 at 07:04:53PM +0200, Jiri Olsa wrote:  
> > > > > See here:
> > > > > 
> > > > >   https://lore.kernel.org/all/20211008091336.33616-1-jolsa@kernel.org/  
> > > > 
> > > > I applied Jiri's patch set and the newly added selftest passes.  
> > > 
> > > nice, could I have your Tested-by? ;-)  
> > 
> > Well, now I added also the missing pieces to ftrace-direct-multi
> > sample module and when loading that and looking into
> > /sys/kernel/debug/tracing/trace it looks like "my_direct_func" gets
> > some random junk as parameter and nothing that could count as "ip".
> > 
> > Will look into that, probably tomorrow.  
> 
> Ok, if I load the correct module, it even works. I had a bug in the
> first version, fixed it, but still loaded the broken module to test
> my changes. Clever me ;)
> 
> So it all works for me.

BTW, in case you need my ack:

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

for the series.

-- Steve