2021-10-12 13:40:11

by Heiko Carstens

[permalink] [raw]
Subject: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

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


2021-10-12 13:41:17

by Heiko Carstens

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

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 <[email protected]>
Reviewed-by: Sven Schnelle <[email protected]>
Signed-off-by: Heiko Carstens <[email protected]>
---
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

2021-10-12 13:41:47

by Heiko Carstens

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

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

Acked-by: Ilya Leoshkevich <[email protected]>
Reviewed-by: Sven Schnelle <[email protected]>
Signed-off-by: Heiko Carstens <[email protected]>
---
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

2021-10-12 13:42:08

by Heiko Carstens

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

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

Acked-by: Ilya Leoshkevich <[email protected]>
Reviewed-by: Sven Schnelle <[email protected]>
Signed-off-by: Heiko Carstens <[email protected]>
---
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

2021-10-12 13:43:20

by Heiko Carstens

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

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 <[email protected]>
Reviewed-by: Sven Schnelle <[email protected]>
Signed-off-by: Heiko Carstens <[email protected]>
---
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

2021-10-12 13:51:18

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

On Tue, 12 Oct 2021 15:37:58 +0200
Heiko Carstens <[email protected]> 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/[email protected]/

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

2021-10-12 15:03:07

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

On Tue, Oct 12, 2021 at 09:48:52AM -0400, Steven Rostedt wrote:
> On Tue, 12 Oct 2021 15:37:58 +0200
> Heiko Carstens <[email protected]> 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/[email protected]/

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.

2021-10-12 15:36:39

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

On Tue, 12 Oct 2021 16:59:02 +0200
Heiko Carstens <[email protected]> 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

2021-10-12 17:07:55

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

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 <[email protected]> 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/[email protected]/
>
> 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.
>

2021-10-12 17:23:06

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

On Tue, Oct 12, 2021 at 11:34:04AM -0400, Steven Rostedt wrote:
> On Tue, 12 Oct 2021 16:59:02 +0200
> Heiko Carstens <[email protected]> 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.

2021-10-12 18:27:29

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

On Tue, Oct 12, 2021 at 07:04:53PM +0200, Jiri Olsa wrote:
> > > See here:
> > >
> > > https://lore.kernel.org/all/[email protected]/
> >
> > 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.

2021-10-13 08:53:16

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

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/[email protected]/
> > >
> > > 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.

2021-10-19 02:08:10

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 0/4] s390: DYNAMIC_FTRACE_WITH_DIRECT_CALL support

On Wed, 13 Oct 2021 10:48:20 +0200
Heiko Carstens <[email protected]> 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/[email protected]/
> > > >
> > > > 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) <[email protected]>

for the series.

-- Steve