2021-12-05 23:20:43

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 0/3] ftrace: Add ftrace-direct-multi-modify sample module

hi,
this patchset adds ftrace-direct-multi-modify.ko kernel module
that shows the usage of modify_ftrace_direct_multi API. Plus
two additional small fixes for ftrace direct code.

thanks,
jirka


---
Jiri Olsa (3):
ftrace: Use direct_ops hash in unregister_ftrace_direct
ftrace: Add cleanup to unregister_ftrace_direct_multi
ftrace/samples: Add module to test multi direct modify interface

kernel/trace/ftrace.c | 8 +++++++-
samples/Kconfig | 8 ++++++++
samples/ftrace/Makefile | 1 +
samples/ftrace/ftrace-direct-multi-modify.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 121 insertions(+), 1 deletion(-)
create mode 100644 samples/ftrace/ftrace-direct-multi-modify.c



2021-12-05 23:20:51

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 1/3] ftrace: Use direct_ops hash in unregister_ftrace_direct

Now when we have *direct_multi interface the direct_functions
hash is no longer owned just by direct_ops. It's also used by
any other ftrace_ops passed to *direct_multi interface.

Thus to find out that we are unregistering the last function
from direct_ops, we need to check directly direct_ops's hash.

Fixes: f64dd4627ec6 ("ftrace: Add multi direct register/unregister interface")
Signed-off-by: Jiri Olsa <[email protected]>
---
kernel/trace/ftrace.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 30bc880c3849..7f0594e28226 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5217,6 +5217,7 @@ int unregister_ftrace_direct(unsigned long ip, unsigned long addr)
{
struct ftrace_direct_func *direct;
struct ftrace_func_entry *entry;
+ struct ftrace_hash *hash;
int ret = -ENODEV;

mutex_lock(&direct_mutex);
@@ -5225,7 +5226,8 @@ int unregister_ftrace_direct(unsigned long ip, unsigned long addr)
if (!entry)
goto out_unlock;

- if (direct_functions->count == 1)
+ hash = direct_ops.func_hash->filter_hash;
+ if (hash->count == 1)
unregister_ftrace_function(&direct_ops);

ret = ftrace_set_filter_ip(&direct_ops, ip, 1, 0);
--
2.33.1


2021-12-05 23:20:56

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 2/3] ftrace: Add cleanup to unregister_ftrace_direct_multi

Adding ops cleanup to unregister_ftrace_direct_multi,
so it can be reused in another register call.

Fixes: f64dd4627ec6 ("ftrace: Add multi direct register/unregister interface")
Signed-off-by: Jiri Olsa <[email protected]>
---
kernel/trace/ftrace.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 7f0594e28226..be5f6b32a012 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5542,6 +5542,10 @@ int unregister_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
err = unregister_ftrace_function(ops);
remove_direct_functions_hash(hash, addr);
mutex_unlock(&direct_mutex);
+
+ /* cleanup for possible another register call */
+ ops->func = NULL;
+ ops->trampoline = 0;
return err;
}
EXPORT_SYMBOL_GPL(unregister_ftrace_direct_multi);
--
2.33.1


2021-12-05 23:21:01

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 3/3] ftrace/samples: Add module to test multi direct modify interface

Adding ftrace-direct-multi-modify.ko kernel module that uses
modify_ftrace_direct_multi API. The core functionality is taken
from ftrace-direct-modify.ko kernel module and changed to fit
multi direct interface.

The init function creates kthread that periodically calls
modify_ftrace_direct_multi to change the trampoline address
for the direct ftrace_ops. The ftrace trace_pipe then shows
trace from both trampolines.

Also adding SAMPLE_FTRACE_MULTI_DIRECT to enable build of
direct multi interface sample modules. It's used in Makefile,
but not defined at the moment.

Same as for ftrace-direct-multi.ko, the new module is enabled
only for x86_64, so there's no need to ifdef the inlined assembly.

Signed-off-by: Jiri Olsa <[email protected]>
---
samples/Kconfig | 8 ++
samples/ftrace/Makefile | 1 +
samples/ftrace/ftrace-direct-multi-modify.c | 105 ++++++++++++++++++++
3 files changed, 114 insertions(+)
create mode 100644 samples/ftrace/ftrace-direct-multi-modify.c

diff --git a/samples/Kconfig b/samples/Kconfig
index bec3528aa2de..38daae5a06b5 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -31,6 +31,14 @@ config SAMPLE_FTRACE_DIRECT
This builds an ftrace direct function example
that hooks to wake_up_process and prints the parameters.

+config SAMPLE_FTRACE_MULTI_DIRECT
+ tristate "Build register_ftrace_direct_multi() examples"
+ depends on DYNAMIC_FTRACE_WITH_DIRECT_CALLS && m
+ depends on HAVE_SAMPLE_FTRACE_MULTI_DIRECT
+ help
+ This builds an ftrace direct multi function examples
+ that hooks trampolines to multiple functions.
+
config SAMPLE_TRACE_ARRAY
tristate "Build sample module for kernel access to Ftrace instancess"
depends on EVENT_TRACING && m
diff --git a/samples/ftrace/Makefile b/samples/ftrace/Makefile
index e8a3f8520a44..027d375890f8 100644
--- a/samples/ftrace/Makefile
+++ b/samples/ftrace/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_SAMPLE_FTRACE_DIRECT) += ftrace-direct.o
obj-$(CONFIG_SAMPLE_FTRACE_DIRECT) += ftrace-direct-too.o
obj-$(CONFIG_SAMPLE_FTRACE_DIRECT) += ftrace-direct-modify.o
obj-$(CONFIG_SAMPLE_FTRACE_MULTI_DIRECT) += ftrace-direct-multi.o
+obj-$(CONFIG_SAMPLE_FTRACE_MULTI_DIRECT) += ftrace-direct-multi-modify.o

CFLAGS_sample-trace-array.o := -I$(src)
obj-$(CONFIG_SAMPLE_TRACE_ARRAY) += sample-trace-array.o
diff --git a/samples/ftrace/ftrace-direct-multi-modify.c b/samples/ftrace/ftrace-direct-multi-modify.c
new file mode 100644
index 000000000000..ba309cb33c77
--- /dev/null
+++ b/samples/ftrace/ftrace-direct-multi-modify.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/module.h>
+#include <linux/kthread.h>
+#include <linux/ftrace.h>
+
+void my_direct_func1(unsigned long ip)
+{
+ trace_printk("my direct func1 ip %lx\n", ip);
+}
+
+void my_direct_func2(unsigned long ip)
+{
+ trace_printk("my direct func2 ip %lx\n", ip);
+}
+
+extern void my_tramp1(void *);
+extern void my_tramp2(void *);
+
+asm (
+" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp1, @function\n"
+" .globl my_tramp1\n"
+" my_tramp1:"
+" pushq %rbp\n"
+" movq %rsp, %rbp\n"
+" pushq %rdi\n"
+" movq 8(%rbp), %rdi\n"
+" call my_direct_func1\n"
+" popq %rdi\n"
+" leave\n"
+" ret\n"
+" .size my_tramp1, .-my_tramp1\n"
+" .type my_tramp2, @function\n"
+"\n"
+" .globl my_tramp2\n"
+" my_tramp2:"
+" pushq %rbp\n"
+" movq %rsp, %rbp\n"
+" pushq %rdi\n"
+" movq 8(%rbp), %rdi\n"
+" call my_direct_func2\n"
+" popq %rdi\n"
+" leave\n"
+" ret\n"
+" .size my_tramp2, .-my_tramp2\n"
+" .popsection\n"
+);
+
+static unsigned long my_tramp = (unsigned long)my_tramp1;
+static unsigned long tramps[2] = {
+ (unsigned long)my_tramp1,
+ (unsigned long)my_tramp2,
+};
+
+static struct ftrace_ops direct;
+
+static int simple_thread(void *arg)
+{
+ static int t;
+ int ret = 0;
+
+ while (!kthread_should_stop()) {
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule_timeout(2 * HZ);
+
+ if (ret)
+ continue;
+ t ^= 1;
+ ret = modify_ftrace_direct_multi(&direct, tramps[t]);
+ if (!ret)
+ my_tramp = tramps[t];
+ WARN_ON_ONCE(ret);
+ }
+
+ return 0;
+}
+
+static struct task_struct *simple_tsk;
+
+static int __init ftrace_direct_multi_init(void)
+{
+ int ret;
+
+ ftrace_set_filter_ip(&direct, (unsigned long) wake_up_process, 0, 0);
+ ftrace_set_filter_ip(&direct, (unsigned long) schedule, 0, 0);
+
+ ret = register_ftrace_direct_multi(&direct, my_tramp);
+
+ if (!ret)
+ simple_tsk = kthread_run(simple_thread, NULL, "event-sample-fn");
+ return ret;
+}
+
+static void __exit ftrace_direct_multi_exit(void)
+{
+ kthread_stop(simple_tsk);
+ unregister_ftrace_direct_multi(&direct, my_tramp);
+}
+
+module_init(ftrace_direct_multi_init);
+module_exit(ftrace_direct_multi_exit);
+
+MODULE_AUTHOR("Jiri Olsa");
+MODULE_DESCRIPTION("Example use case of using modify_ftrace_direct_multi()");
+MODULE_LICENSE("GPL");
--
2.33.1


2021-12-06 11:26:04

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH 3/3] ftrace/samples: Add module to test multi direct modify interface

On Mon, Dec 06, 2021 at 12:20:36AM +0100, Jiri Olsa wrote:
> Adding ftrace-direct-multi-modify.ko kernel module that uses
> modify_ftrace_direct_multi API. The core functionality is taken
> from ftrace-direct-modify.ko kernel module and changed to fit
> multi direct interface.
>
> The init function creates kthread that periodically calls
> modify_ftrace_direct_multi to change the trampoline address
> for the direct ftrace_ops. The ftrace trace_pipe then shows
> trace from both trampolines.
>
> Also adding SAMPLE_FTRACE_MULTI_DIRECT to enable build of
> direct multi interface sample modules. It's used in Makefile,
> but not defined at the moment.
>
> Same as for ftrace-direct-multi.ko, the new module is enabled
> only for x86_64, so there's no need to ifdef the inlined assembly.
>
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
> samples/Kconfig | 8 ++
> samples/ftrace/Makefile | 1 +
> samples/ftrace/ftrace-direct-multi-modify.c | 105 ++++++++++++++++++++
> 3 files changed, 114 insertions(+)
> create mode 100644 samples/ftrace/ftrace-direct-multi-modify.c

I think your series is based on something before 5.16-rc2?

Because there are:
503e45108451 ("ftrace/samples: add missing Kconfig option for ftrace direct multi sample")
890e3dc8bb6e ("ftrace/samples: add s390 support for ftrace direct multi sample")

Which would conflict with your patches.

2021-12-06 12:28:57

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 3/3] ftrace/samples: Add module to test multi direct modify interface

On Mon, Dec 06, 2021 at 12:25:37PM +0100, Heiko Carstens wrote:
> On Mon, Dec 06, 2021 at 12:20:36AM +0100, Jiri Olsa wrote:
> > Adding ftrace-direct-multi-modify.ko kernel module that uses
> > modify_ftrace_direct_multi API. The core functionality is taken
> > from ftrace-direct-modify.ko kernel module and changed to fit
> > multi direct interface.
> >
> > The init function creates kthread that periodically calls
> > modify_ftrace_direct_multi to change the trampoline address
> > for the direct ftrace_ops. The ftrace trace_pipe then shows
> > trace from both trampolines.
> >
> > Also adding SAMPLE_FTRACE_MULTI_DIRECT to enable build of
> > direct multi interface sample modules. It's used in Makefile,
> > but not defined at the moment.
> >
> > Same as for ftrace-direct-multi.ko, the new module is enabled
> > only for x86_64, so there's no need to ifdef the inlined assembly.
> >
> > Signed-off-by: Jiri Olsa <[email protected]>
> > ---
> > samples/Kconfig | 8 ++
> > samples/ftrace/Makefile | 1 +
> > samples/ftrace/ftrace-direct-multi-modify.c | 105 ++++++++++++++++++++
> > 3 files changed, 114 insertions(+)
> > create mode 100644 samples/ftrace/ftrace-direct-multi-modify.c
>
> I think your series is based on something before 5.16-rc2?
>
> Because there are:
> 503e45108451 ("ftrace/samples: add missing Kconfig option for ftrace direct multi sample")
> 890e3dc8bb6e ("ftrace/samples: add s390 support for ftrace direct multi sample")
>
> Which would conflict with your patches.

ah so we have some of the changes already.. I'll pick it up,
rebase and resend

thanks,
jirka