* [PATCH v3 01/27] function_graph: Convert ret_stack to a series of longs
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 02/27] fgraph: Use BUILD_BUG_ON() to make sure we have structures divisible by long Steven Rostedt
` (27 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
In order to make it possible to have multiple callbacks registered with the
function_graph tracer, the retstack needs to be converted from an array of
ftrace_ret_stack structures to an array of longs. This will allow to store
the list of callbacks on the stack for the return side of the functions.
Link: https://lore.kernel.org/linux-trace-kernel/171509092742.162236.4427737821399314856.stgit@devnote2
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/sched.h | 2 +-
kernel/trace/fgraph.c | 136 +++++++++++++++++++++++++-----------------
2 files changed, 83 insertions(+), 55 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 61591ac6eab6..352939dab3a5 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1402,7 +1402,7 @@ struct task_struct {
int curr_ret_depth;
/* Stack of return addresses for return function tracing: */
- struct ftrace_ret_stack *ret_stack;
+ unsigned long *ret_stack;
/* Timestamp for last schedule: */
unsigned long long ftrace_timestamp;
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index a130b2d898f7..c62e6db718a0 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -25,6 +25,30 @@
#define ASSIGN_OPS_HASH(opsname, val)
#endif
+/*
+ * FGRAPH_FRAME_SIZE: Size in bytes of the meta data on the shadow stack
+ * FGRAPH_FRAME_OFFSET: Size in long words of the meta data frame
+ * SHADOW_STACK_SIZE: The size in bytes of the entire shadow stack
+ * SHADOW_STACK_OFFSET: The size in long words of the shadow stack
+ * SHADOW_STACK_MAX_OFFSET: The max offset of the stack for a new frame to be added
+ */
+#define FGRAPH_FRAME_SIZE sizeof(struct ftrace_ret_stack)
+#define FGRAPH_FRAME_OFFSET (ALIGN(FGRAPH_FRAME_SIZE, sizeof(long)) / sizeof(long))
+#define SHADOW_STACK_SIZE (PAGE_SIZE)
+#define SHADOW_STACK_OFFSET \
+ (ALIGN(SHADOW_STACK_SIZE, sizeof(long)) / sizeof(long))
+/* Leave on a buffer at the end */
+#define SHADOW_STACK_MAX_INDEX (SHADOW_STACK_OFFSET - FGRAPH_FRAME_OFFSET)
+
+/*
+ * RET_STACK(): Return the frame from a given @offset from task @t
+ * RET_STACK_INC(): Reserve one frame size on the stack.
+ * RET_STACK_DEC(): Remove one frame size from the stack.
+ */
+#define RET_STACK(t, index) ((struct ftrace_ret_stack *)(&(t)->ret_stack[index]))
+#define RET_STACK_INC(c) ({ c += FGRAPH_FRAME_OFFSET; })
+#define RET_STACK_DEC(c) ({ c -= FGRAPH_FRAME_OFFSET; })
+
DEFINE_STATIC_KEY_FALSE(kill_ftrace_graph);
int ftrace_graph_active;
@@ -69,6 +93,7 @@ static int
ftrace_push_return_trace(unsigned long ret, unsigned long func,
unsigned long frame_pointer, unsigned long *retp)
{
+ struct ftrace_ret_stack *ret_stack;
unsigned long long calltime;
int index;
@@ -85,23 +110,25 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func,
smp_rmb();
/* The return trace stack is full */
- if (current->curr_ret_stack == FTRACE_RETFUNC_DEPTH - 1) {
+ if (current->curr_ret_stack >= SHADOW_STACK_MAX_INDEX) {
atomic_inc(¤t->trace_overrun);
return -EBUSY;
}
calltime = trace_clock_local();
- index = ++current->curr_ret_stack;
+ index = current->curr_ret_stack;
+ RET_STACK_INC(current->curr_ret_stack);
+ ret_stack = RET_STACK(current, index);
barrier();
- current->ret_stack[index].ret = ret;
- current->ret_stack[index].func = func;
- current->ret_stack[index].calltime = calltime;
+ ret_stack->ret = ret;
+ ret_stack->func = func;
+ ret_stack->calltime = calltime;
#ifdef HAVE_FUNCTION_GRAPH_FP_TEST
- current->ret_stack[index].fp = frame_pointer;
+ ret_stack->fp = frame_pointer;
#endif
#ifdef HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
- current->ret_stack[index].retp = retp;
+ ret_stack->retp = retp;
#endif
return 0;
}
@@ -137,7 +164,7 @@ int function_graph_enter(unsigned long ret, unsigned long func,
return 0;
out_ret:
- current->curr_ret_stack--;
+ RET_STACK_DEC(current->curr_ret_stack);
out:
current->curr_ret_depth--;
return -EBUSY;
@@ -148,11 +175,13 @@ static void
ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
unsigned long frame_pointer)
{
+ struct ftrace_ret_stack *ret_stack;
int index;
index = current->curr_ret_stack;
+ RET_STACK_DEC(index);
- if (unlikely(index < 0 || index >= FTRACE_RETFUNC_DEPTH)) {
+ if (unlikely(index < 0 || index > SHADOW_STACK_MAX_INDEX)) {
ftrace_graph_stop();
WARN_ON(1);
/* Might as well panic, otherwise we have no where to go */
@@ -160,6 +189,7 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
return;
}
+ ret_stack = RET_STACK(current, index);
#ifdef HAVE_FUNCTION_GRAPH_FP_TEST
/*
* The arch may choose to record the frame pointer used
@@ -175,22 +205,22 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
* Note, -mfentry does not use frame pointers, and this test
* is not needed if CC_USING_FENTRY is set.
*/
- if (unlikely(current->ret_stack[index].fp != frame_pointer)) {
+ if (unlikely(ret_stack->fp != frame_pointer)) {
ftrace_graph_stop();
WARN(1, "Bad frame pointer: expected %lx, received %lx\n"
" from func %ps return to %lx\n",
current->ret_stack[index].fp,
frame_pointer,
- (void *)current->ret_stack[index].func,
- current->ret_stack[index].ret);
+ (void *)ret_stack->func,
+ ret_stack->ret);
*ret = (unsigned long)panic;
return;
}
#endif
- *ret = current->ret_stack[index].ret;
- trace->func = current->ret_stack[index].func;
- trace->calltime = current->ret_stack[index].calltime;
+ *ret = ret_stack->ret;
+ trace->func = ret_stack->func;
+ trace->calltime = ret_stack->calltime;
trace->overrun = atomic_read(¤t->trace_overrun);
trace->depth = current->curr_ret_depth--;
/*
@@ -251,7 +281,7 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs
* curr_ret_stack is after that.
*/
barrier();
- current->curr_ret_stack--;
+ RET_STACK_DEC(current->curr_ret_stack);
if (unlikely(!ret)) {
ftrace_graph_stop();
@@ -294,12 +324,13 @@ unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
struct ftrace_ret_stack *
ftrace_graph_get_ret_stack(struct task_struct *task, int idx)
{
- idx = task->curr_ret_stack - idx;
+ int index = task->curr_ret_stack;
- if (idx >= 0 && idx <= task->curr_ret_stack)
- return &task->ret_stack[idx];
+ index -= FGRAPH_FRAME_OFFSET * (idx + 1);
+ if (index < 0)
+ return NULL;
- return NULL;
+ return RET_STACK(task, index);
}
/**
@@ -321,18 +352,20 @@ ftrace_graph_get_ret_stack(struct task_struct *task, int idx)
unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
unsigned long ret, unsigned long *retp)
{
+ struct ftrace_ret_stack *ret_stack;
int index = task->curr_ret_stack;
int i;
if (ret != (unsigned long)dereference_kernel_function_descriptor(return_to_handler))
return ret;
- if (index < 0)
- return ret;
+ RET_STACK_DEC(index);
- for (i = 0; i <= index; i++)
- if (task->ret_stack[i].retp == retp)
- return task->ret_stack[i].ret;
+ for (i = index; i >= 0; RET_STACK_DEC(i)) {
+ ret_stack = RET_STACK(task, i);
+ if (ret_stack->retp == retp)
+ return ret_stack->ret;
+ }
return ret;
}
@@ -346,14 +379,15 @@ unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
return ret;
task_idx = task->curr_ret_stack;
+ RET_STACK_DEC(task_idx);
if (!task->ret_stack || task_idx < *idx)
return ret;
task_idx -= *idx;
- (*idx)++;
+ RET_STACK_INC(*idx);
- return task->ret_stack[task_idx].ret;
+ return RET_STACK(task, task_idx);
}
#endif /* HAVE_FUNCTION_GRAPH_RET_ADDR_PTR */
@@ -391,7 +425,7 @@ trace_func_graph_ent_t ftrace_graph_entry = ftrace_graph_entry_stub;
static trace_func_graph_ent_t __ftrace_graph_entry = ftrace_graph_entry_stub;
/* Try to assign a return stack array on FTRACE_RETSTACK_ALLOC_SIZE tasks. */
-static int alloc_retstack_tasklist(struct ftrace_ret_stack **ret_stack_list)
+static int alloc_retstack_tasklist(unsigned long **ret_stack_list)
{
int i;
int ret = 0;
@@ -399,10 +433,7 @@ static int alloc_retstack_tasklist(struct ftrace_ret_stack **ret_stack_list)
struct task_struct *g, *t;
for (i = 0; i < FTRACE_RETSTACK_ALLOC_SIZE; i++) {
- ret_stack_list[i] =
- kmalloc_array(FTRACE_RETFUNC_DEPTH,
- sizeof(struct ftrace_ret_stack),
- GFP_KERNEL);
+ ret_stack_list[i] = kmalloc(SHADOW_STACK_SIZE, GFP_KERNEL);
if (!ret_stack_list[i]) {
start = 0;
end = i;
@@ -420,9 +451,9 @@ static int alloc_retstack_tasklist(struct ftrace_ret_stack **ret_stack_list)
if (t->ret_stack == NULL) {
atomic_set(&t->trace_overrun, 0);
- t->curr_ret_stack = -1;
+ t->curr_ret_stack = 0;
t->curr_ret_depth = -1;
- /* Make sure the tasks see the -1 first: */
+ /* Make sure the tasks see the 0 first: */
smp_wmb();
t->ret_stack = ret_stack_list[start++];
}
@@ -442,6 +473,7 @@ ftrace_graph_probe_sched_switch(void *ignore, bool preempt,
struct task_struct *next,
unsigned int prev_state)
{
+ struct ftrace_ret_stack *ret_stack;
unsigned long long timestamp;
int index;
@@ -466,8 +498,11 @@ ftrace_graph_probe_sched_switch(void *ignore, bool preempt,
*/
timestamp -= next->ftrace_timestamp;
- for (index = next->curr_ret_stack; index >= 0; index--)
- next->ret_stack[index].calltime += timestamp;
+ for (index = next->curr_ret_stack - FGRAPH_FRAME_OFFSET; index >= 0; ) {
+ ret_stack = RET_STACK(next, index);
+ ret_stack->calltime += timestamp;
+ index -= FGRAPH_FRAME_OFFSET;
+ }
}
static int ftrace_graph_entry_test(struct ftrace_graph_ent *trace)
@@ -510,10 +545,10 @@ void update_function_graph_func(void)
ftrace_graph_entry = __ftrace_graph_entry;
}
-static DEFINE_PER_CPU(struct ftrace_ret_stack *, idle_ret_stack);
+static DEFINE_PER_CPU(unsigned long *, idle_ret_stack);
static void
-graph_init_task(struct task_struct *t, struct ftrace_ret_stack *ret_stack)
+graph_init_task(struct task_struct *t, unsigned long *ret_stack)
{
atomic_set(&t->trace_overrun, 0);
t->ftrace_timestamp = 0;
@@ -528,7 +563,7 @@ graph_init_task(struct task_struct *t, struct ftrace_ret_stack *ret_stack)
*/
void ftrace_graph_init_idle_task(struct task_struct *t, int cpu)
{
- t->curr_ret_stack = -1;
+ t->curr_ret_stack = 0;
t->curr_ret_depth = -1;
/*
* The idle task has no parent, it either has its own
@@ -538,14 +573,11 @@ void ftrace_graph_init_idle_task(struct task_struct *t, int cpu)
WARN_ON(t->ret_stack != per_cpu(idle_ret_stack, cpu));
if (ftrace_graph_active) {
- struct ftrace_ret_stack *ret_stack;
+ unsigned long *ret_stack;
ret_stack = per_cpu(idle_ret_stack, cpu);
if (!ret_stack) {
- ret_stack =
- kmalloc_array(FTRACE_RETFUNC_DEPTH,
- sizeof(struct ftrace_ret_stack),
- GFP_KERNEL);
+ ret_stack = kmalloc(SHADOW_STACK_SIZE, GFP_KERNEL);
if (!ret_stack)
return;
per_cpu(idle_ret_stack, cpu) = ret_stack;
@@ -559,15 +591,13 @@ void ftrace_graph_init_task(struct task_struct *t)
{
/* Make sure we do not use the parent ret_stack */
t->ret_stack = NULL;
- t->curr_ret_stack = -1;
+ t->curr_ret_stack = 0;
t->curr_ret_depth = -1;
if (ftrace_graph_active) {
- struct ftrace_ret_stack *ret_stack;
+ unsigned long *ret_stack;
- ret_stack = kmalloc_array(FTRACE_RETFUNC_DEPTH,
- sizeof(struct ftrace_ret_stack),
- GFP_KERNEL);
+ ret_stack = kmalloc(SHADOW_STACK_SIZE, GFP_KERNEL);
if (!ret_stack)
return;
graph_init_task(t, ret_stack);
@@ -576,7 +606,7 @@ void ftrace_graph_init_task(struct task_struct *t)
void ftrace_graph_exit_task(struct task_struct *t)
{
- struct ftrace_ret_stack *ret_stack = t->ret_stack;
+ unsigned long *ret_stack = t->ret_stack;
t->ret_stack = NULL;
/* NULL must become visible to IRQs before we free it: */
@@ -588,12 +618,10 @@ void ftrace_graph_exit_task(struct task_struct *t)
/* Allocate a return stack for each task */
static int start_graph_tracing(void)
{
- struct ftrace_ret_stack **ret_stack_list;
+ unsigned long **ret_stack_list;
int ret, cpu;
- ret_stack_list = kmalloc_array(FTRACE_RETSTACK_ALLOC_SIZE,
- sizeof(struct ftrace_ret_stack *),
- GFP_KERNEL);
+ ret_stack_list = kmalloc(SHADOW_STACK_SIZE, GFP_KERNEL);
if (!ret_stack_list)
return -ENOMEM;
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 02/27] fgraph: Use BUILD_BUG_ON() to make sure we have structures divisible by long
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 01/27] function_graph: Convert ret_stack to a series of longs Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 03/27] function_graph: Add an array structure that will allow multiple callbacks Steven Rostedt
` (26 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Instead of using "ALIGN()", use BUILD_BUG_ON() as the structures should
always be divisible by sizeof(long).
Co-developed with Masami Hiramatsu:
Link: https://lore.kernel.org/linux-trace-kernel/171509093949.162236.14518699447151894536.stgit@devnote2
Link: http://lkml.kernel.org/r/20190524111144.GI2589@hirez.programming.kicks-ass.net
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/fgraph.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index c62e6db718a0..fdb206aeffe3 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -33,7 +33,7 @@
* SHADOW_STACK_MAX_OFFSET: The max offset of the stack for a new frame to be added
*/
#define FGRAPH_FRAME_SIZE sizeof(struct ftrace_ret_stack)
-#define FGRAPH_FRAME_OFFSET (ALIGN(FGRAPH_FRAME_SIZE, sizeof(long)) / sizeof(long))
+#define FGRAPH_FRAME_OFFSET DIV_ROUND_UP(FGRAPH_FRAME_SIZE, sizeof(long))
#define SHADOW_STACK_SIZE (PAGE_SIZE)
#define SHADOW_STACK_OFFSET \
(ALIGN(SHADOW_STACK_SIZE, sizeof(long)) / sizeof(long))
@@ -103,6 +103,8 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func,
if (!current->ret_stack)
return -EBUSY;
+ BUILD_BUG_ON(SHADOW_STACK_SIZE % sizeof(long));
+
/*
* We must make sure the ret_stack is tested before we read
* anything else.
@@ -326,6 +328,8 @@ ftrace_graph_get_ret_stack(struct task_struct *task, int idx)
{
int index = task->curr_ret_stack;
+ BUILD_BUG_ON(FGRAPH_FRAME_SIZE % sizeof(long));
+
index -= FGRAPH_FRAME_OFFSET * (idx + 1);
if (index < 0)
return NULL;
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 03/27] function_graph: Add an array structure that will allow multiple callbacks
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 01/27] function_graph: Convert ret_stack to a series of longs Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 02/27] fgraph: Use BUILD_BUG_ON() to make sure we have structures divisible by long Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 04/27] function_graph: Allow multiple users to attach to function graph Steven Rostedt
` (25 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Add an array structure that will eventually allow the function graph tracer
to have up to 16 simultaneous callbacks attached. It's an array of 16
fgraph_ops pointers, that is assigned when one is registered. On entry of a
function the entry of the first item in the array is called, and if it
returns zero, then the callback returns non zero if it wants the return
callback to be called on exit of the function.
The array will simplify the process of having more than one callback
attached to the same function, as its index into the array can be stored on
the shadow stack. We need to only save the index, because this will allow
the fgraph_ops to be freed before the function returns (which may happen if
the function call schedule for a long time).
Co-developed with Masami Hiramatsu:
Link: https://lore.kernel.org/linux-trace-kernel/171509095075.162236.8272148192748284581.stgit@devnote2
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/fgraph.c | 114 ++++++++++++++++++++++++++++++------------
1 file changed, 81 insertions(+), 33 deletions(-)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index fdb206aeffe3..d2ce5d651cf0 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -52,6 +52,11 @@
DEFINE_STATIC_KEY_FALSE(kill_ftrace_graph);
int ftrace_graph_active;
+static int fgraph_array_cnt;
+#define FGRAPH_ARRAY_SIZE 16
+
+static struct fgraph_ops *fgraph_array[FGRAPH_ARRAY_SIZE];
+
/* Both enabled by default (can be cleared by function_graph tracer flags */
static bool fgraph_sleep_time = true;
@@ -75,6 +80,20 @@ int __weak ftrace_disable_ftrace_graph_caller(void)
}
#endif
+int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace)
+{
+ return 0;
+}
+
+static void ftrace_graph_ret_stub(struct ftrace_graph_ret *trace)
+{
+}
+
+static struct fgraph_ops fgraph_stub = {
+ .entryfunc = ftrace_graph_entry_stub,
+ .retfunc = ftrace_graph_ret_stub,
+};
+
/**
* ftrace_graph_stop - set to permanently disable function graph tracing
*
@@ -161,7 +180,7 @@ int function_graph_enter(unsigned long ret, unsigned long func,
goto out;
/* Only trace if the calling function expects to */
- if (!ftrace_graph_entry(&trace))
+ if (!fgraph_array[0]->entryfunc(&trace))
goto out_ret;
return 0;
@@ -276,7 +295,7 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs
trace.retval = fgraph_ret_regs_return_value(ret_regs);
#endif
trace.rettime = trace_clock_local();
- ftrace_graph_return(&trace);
+ fgraph_array[0]->retfunc(&trace);
/*
* The ftrace_graph_return() may still access the current
* ret_stack structure, we need to make sure the update of
@@ -412,11 +431,6 @@ void ftrace_graph_sleep_time_control(bool enable)
fgraph_sleep_time = enable;
}
-int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace)
-{
- return 0;
-}
-
/*
* Simply points to ftrace_stub, but with the proper protocol.
* Defined by the linker script in linux/vmlinux.lds.h
@@ -654,37 +668,54 @@ static int start_graph_tracing(void)
int register_ftrace_graph(struct fgraph_ops *gops)
{
int ret = 0;
+ int i;
mutex_lock(&ftrace_lock);
- /* we currently allow only one tracer registered at a time */
- if (ftrace_graph_active) {
+ if (!fgraph_array[0]) {
+ /* The array must always have real data on it */
+ for (i = 0; i < FGRAPH_ARRAY_SIZE; i++)
+ fgraph_array[i] = &fgraph_stub;
+ }
+
+ /* Look for an available spot */
+ for (i = 0; i < FGRAPH_ARRAY_SIZE; i++) {
+ if (fgraph_array[i] == &fgraph_stub)
+ break;
+ }
+ if (i >= FGRAPH_ARRAY_SIZE) {
ret = -EBUSY;
goto out;
}
- register_pm_notifier(&ftrace_suspend_notifier);
+ fgraph_array[i] = gops;
+ if (i + 1 > fgraph_array_cnt)
+ fgraph_array_cnt = i + 1;
ftrace_graph_active++;
- ret = start_graph_tracing();
- if (ret) {
- ftrace_graph_active--;
- goto out;
- }
- ftrace_graph_return = gops->retfunc;
+ if (ftrace_graph_active == 1) {
+ register_pm_notifier(&ftrace_suspend_notifier);
+ ret = start_graph_tracing();
+ if (ret) {
+ ftrace_graph_active--;
+ goto out;
+ }
+
+ ftrace_graph_return = gops->retfunc;
- /*
- * Update the indirect function to the entryfunc, and the
- * function that gets called to the entry_test first. Then
- * call the update fgraph entry function to determine if
- * the entryfunc should be called directly or not.
- */
- __ftrace_graph_entry = gops->entryfunc;
- ftrace_graph_entry = ftrace_graph_entry_test;
- update_function_graph_func();
+ /*
+ * Update the indirect function to the entryfunc, and the
+ * function that gets called to the entry_test first. Then
+ * call the update fgraph entry function to determine if
+ * the entryfunc should be called directly or not.
+ */
+ __ftrace_graph_entry = gops->entryfunc;
+ ftrace_graph_entry = ftrace_graph_entry_test;
+ update_function_graph_func();
- ret = ftrace_startup(&graph_ops, FTRACE_START_FUNC_RET);
+ ret = ftrace_startup(&graph_ops, FTRACE_START_FUNC_RET);
+ }
out:
mutex_unlock(&ftrace_lock);
return ret;
@@ -692,19 +723,36 @@ int register_ftrace_graph(struct fgraph_ops *gops)
void unregister_ftrace_graph(struct fgraph_ops *gops)
{
+ int i;
+
mutex_lock(&ftrace_lock);
if (unlikely(!ftrace_graph_active))
goto out;
- ftrace_graph_active--;
- ftrace_graph_return = ftrace_stub_graph;
- ftrace_graph_entry = ftrace_graph_entry_stub;
- __ftrace_graph_entry = ftrace_graph_entry_stub;
- ftrace_shutdown(&graph_ops, FTRACE_STOP_FUNC_RET);
- unregister_pm_notifier(&ftrace_suspend_notifier);
- unregister_trace_sched_switch(ftrace_graph_probe_sched_switch, NULL);
+ for (i = 0; i < fgraph_array_cnt; i++)
+ if (gops == fgraph_array[i])
+ break;
+ if (i >= fgraph_array_cnt)
+ goto out;
+ fgraph_array[i] = &fgraph_stub;
+ if (i + 1 == fgraph_array_cnt) {
+ for (; i >= 0; i--)
+ if (fgraph_array[i] != &fgraph_stub)
+ break;
+ fgraph_array_cnt = i + 1;
+ }
+
+ ftrace_graph_active--;
+ if (!ftrace_graph_active) {
+ ftrace_graph_return = ftrace_stub_graph;
+ ftrace_graph_entry = ftrace_graph_entry_stub;
+ __ftrace_graph_entry = ftrace_graph_entry_stub;
+ ftrace_shutdown(&graph_ops, FTRACE_STOP_FUNC_RET);
+ unregister_pm_notifier(&ftrace_suspend_notifier);
+ unregister_trace_sched_switch(ftrace_graph_probe_sched_switch, NULL);
+ }
out:
mutex_unlock(&ftrace_lock);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 04/27] function_graph: Allow multiple users to attach to function graph
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (2 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 03/27] function_graph: Add an array structure that will allow multiple callbacks Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 05/27] function_graph: Handle tail calls for stack unwinding Steven Rostedt
` (24 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Allow for multiple users to attach to function graph tracer at the same
time. Only 16 simultaneous users can attach to the tracer. This is because
there's an array that stores the pointers to the attached fgraph_ops. When
a function being traced is entered, each of the ftrace_ops entryfunc is
called and if it returns non zero, its index into the array will be added
to the shadow stack.
On exit of the function being traced, the shadow stack will contain the
indexes of the ftrace_ops on the array that want their retfunc to be
called.
Because a function may sleep for a long time (if a task sleeps itself),
the return of the function may be literally days later. If the ftrace_ops
is removed, its place on the array is replaced with a ftrace_ops that
contains the stub functions and that will be called when the function
finally returns.
If another ftrace_ops is added that happens to get the same index into the
array, its return function may be called. But that's actually the way
things current work with the old function graph tracer. If one tracer is
removed and another is added, the new one will get the return calls of the
function traced by the previous one, thus this is not a regression. This
can be fixed by adding a counter to each time the array item is updated and
save that on the shadow stack as well, such that it won't be called if the
index saved does not match the index on the array.
Note, being able to filter functions when both are called is not completely
handled yet, but that shouldn't be too hard to manage.
Co-developed with Masami Hiramatsu:
Link: https://lore.kernel.org/linux-trace-kernel/171509096221.162236.8806372072523195752.stgit@devnote2
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/ftrace.h | 3 +-
kernel/trace/fgraph.c | 379 ++++++++++++++++++++++++++++++++---------
2 files changed, 304 insertions(+), 78 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 800995c425e0..8f00a7415bd5 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1038,6 +1038,7 @@ extern int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace);
struct fgraph_ops {
trace_func_graph_ent_t entryfunc;
trace_func_graph_ret_t retfunc;
+ int idx;
};
/*
@@ -1072,7 +1073,7 @@ function_graph_enter(unsigned long ret, unsigned long func,
unsigned long frame_pointer, unsigned long *retp);
struct ftrace_ret_stack *
-ftrace_graph_get_ret_stack(struct task_struct *task, int idx);
+ftrace_graph_get_ret_stack(struct task_struct *task, int skip);
unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
unsigned long ret, unsigned long *retp);
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index d2ce5d651cf0..aae51f746828 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -7,6 +7,7 @@
*
* Highly modified by Steven Rostedt (VMware).
*/
+#include <linux/bits.h>
#include <linux/jump_label.h>
#include <linux/suspend.h>
#include <linux/ftrace.h>
@@ -28,35 +29,177 @@
/*
* FGRAPH_FRAME_SIZE: Size in bytes of the meta data on the shadow stack
* FGRAPH_FRAME_OFFSET: Size in long words of the meta data frame
- * SHADOW_STACK_SIZE: The size in bytes of the entire shadow stack
- * SHADOW_STACK_OFFSET: The size in long words of the shadow stack
- * SHADOW_STACK_MAX_OFFSET: The max offset of the stack for a new frame to be added
*/
#define FGRAPH_FRAME_SIZE sizeof(struct ftrace_ret_stack)
#define FGRAPH_FRAME_OFFSET DIV_ROUND_UP(FGRAPH_FRAME_SIZE, sizeof(long))
-#define SHADOW_STACK_SIZE (PAGE_SIZE)
-#define SHADOW_STACK_OFFSET \
- (ALIGN(SHADOW_STACK_SIZE, sizeof(long)) / sizeof(long))
-/* Leave on a buffer at the end */
-#define SHADOW_STACK_MAX_INDEX (SHADOW_STACK_OFFSET - FGRAPH_FRAME_OFFSET)
/*
- * RET_STACK(): Return the frame from a given @offset from task @t
- * RET_STACK_INC(): Reserve one frame size on the stack.
- * RET_STACK_DEC(): Remove one frame size from the stack.
+ * On entry to a function (via function_graph_enter()), a new fgraph frame
+ * (ftrace_ret_stack) is pushed onto the stack as well as a word that
+ * holds a bitmask and a type (called "bitmap"). The bitmap is defined as:
+ *
+ * bits: 0 - 9 offset in words from the previous ftrace_ret_stack
+ * Currently, this will always be set to FGRAPH_FRAME_OFFSET
+ * to get to the fgraph frame.
+ *
+ * bits: 10 - 11 Type of storage
+ * 0 - reserved
+ * 1 - bitmap of fgraph_array index
+ *
+ * For type with "bitmap of fgraph_array index" (FGRAPH_TYPE_BITMAP):
+ * bits: 12 - 27 The bitmap of fgraph_ops fgraph_array index
+ * That is, it's a bitmask of 0-15 (16 bits)
+ * where if a corresponding ops in the fgraph_array[]
+ * expects a callback from the return of the function
+ * it's corresponding bit will be set.
+ *
+ *
+ * The top of the ret_stack (when not empty) will always have a reference
+ * word that points to the last fgraph frame that was saved.
+ *
+ * That is, at the end of function_graph_enter, if the first and forth
+ * fgraph_ops on the fgraph_array[] (index 0 and 3) needs their retfunc called
+ * on the return of the function being traced, this is what will be on the
+ * task's shadow ret_stack: (the stack grows upward)
+ *
+ * ret_stack[SHADOW_STACK_MAX_OFFSET]
+ * ...
+ * | | <- task->curr_ret_stack
+ * +--------------------------------------------+
+ * | (9 << 12) | (1 << 10) | FGRAPH_FRAME_OFFSET|
+ * | *or put another way* |
+ * | (BIT(3)|BIT(0)) << FGRAPH_INDEX_SHIFT | \ |
+ * | FGRAPH_TYPE_BITMAP << FGRAPH_TYPE_SHIFT| \ |
+ * | (offset:FGRAPH_FRAME_OFFSET) | <- the offset is from here
+ * +--------------------------------------------+
+ * | struct ftrace_ret_stack |
+ * | (stores the saved ret pointer) | <- the offset points here
+ * +--------------------------------------------+
+ * | (X) | (N) | ( N words away from
+ * | | previous ret_stack)
+ * ...
+ * ret_stack[0]
+ *
+ * If a backtrace is required, and the real return pointer needs to be
+ * fetched, then it looks at the task's curr_ret_stack offset, if it
+ * is greater than zero (reserved, or right before popped), it would mask
+ * the value by FGRAPH_FRAME_OFFSET_MASK to get the offset of the
+ * ftrace_ret_stack structure stored on the shadow stack.
+ */
+
+/*
+ * The following is for the top word on the stack:
+ *
+ * FGRAPH_FRAME_OFFSET (0-9) holds the offset delta to the fgraph frame
+ * FGRAPH_TYPE (10-11) holds the type of word this is.
+ * (RESERVED or BITMAP)
+ */
+#define FGRAPH_FRAME_OFFSET_BITS 10
+#define FGRAPH_FRAME_OFFSET_MASK GENMASK(FGRAPH_FRAME_OFFSET_BITS - 1, 0)
+
+#define FGRAPH_TYPE_BITS 2
+#define FGRAPH_TYPE_MASK GENMASK(FGRAPH_TYPE_BITS - 1, 0)
+#define FGRAPH_TYPE_SHIFT FGRAPH_FRAME_OFFSET_BITS
+
+enum {
+ FGRAPH_TYPE_RESERVED = 0,
+ FGRAPH_TYPE_BITMAP = 1,
+};
+
+/*
+ * For BITMAP type:
+ * FGRAPH_INDEX (12-27) bits holding the gops index wanting return callback called
+ */
+#define FGRAPH_INDEX_BITS 16
+#define FGRAPH_INDEX_MASK GENMASK(FGRAPH_INDEX_BITS - 1, 0)
+#define FGRAPH_INDEX_SHIFT (FGRAPH_TYPE_SHIFT + FGRAPH_TYPE_BITS)
+
+#define FGRAPH_ARRAY_SIZE FGRAPH_INDEX_BITS
+
+/*
+ * SHADOW_STACK_SIZE: The size in bytes of the entire shadow stack
+ * SHADOW_STACK_OFFSET: The size in long words of the shadow stack
+ * SHADOW_STACK_MAX_OFFSET: The max offset of the stack for a new frame to be added
*/
-#define RET_STACK(t, index) ((struct ftrace_ret_stack *)(&(t)->ret_stack[index]))
-#define RET_STACK_INC(c) ({ c += FGRAPH_FRAME_OFFSET; })
-#define RET_STACK_DEC(c) ({ c -= FGRAPH_FRAME_OFFSET; })
+#define SHADOW_STACK_SIZE (PAGE_SIZE)
+#define SHADOW_STACK_OFFSET (SHADOW_STACK_SIZE / sizeof(long))
+/* Leave on a buffer at the end */
+#define SHADOW_STACK_MAX_OFFSET (SHADOW_STACK_OFFSET - (FGRAPH_FRAME_OFFSET + 1))
+
+/* RET_STACK(): Return the frame from a given @offset from task @t */
+#define RET_STACK(t, offset) ((struct ftrace_ret_stack *)(&(t)->ret_stack[offset]))
DEFINE_STATIC_KEY_FALSE(kill_ftrace_graph);
int ftrace_graph_active;
static int fgraph_array_cnt;
-#define FGRAPH_ARRAY_SIZE 16
static struct fgraph_ops *fgraph_array[FGRAPH_ARRAY_SIZE];
+/* Get the FRAME_OFFSET from the word from the @offset on ret_stack */
+static inline int get_frame_offset(struct task_struct *t, int offset)
+{
+ return t->ret_stack[offset] & FGRAPH_FRAME_OFFSET_MASK;
+}
+
+/* Get FGRAPH_TYPE from the word from the @offset at ret_stack */
+static inline int get_fgraph_type(struct task_struct *t, int offset)
+{
+ return (t->ret_stack[offset] >> FGRAPH_TYPE_SHIFT) & FGRAPH_TYPE_MASK;
+}
+
+/* For BITMAP type: get the bitmask from the @offset at ret_stack */
+static inline unsigned long
+get_bitmap_bits(struct task_struct *t, int offset)
+{
+ return (t->ret_stack[offset] >> FGRAPH_INDEX_SHIFT) & FGRAPH_INDEX_MASK;
+}
+
+/* Write the bitmap to the ret_stack at @offset (does index, offset and bitmask) */
+static inline void
+set_bitmap(struct task_struct *t, int offset, unsigned long bitmap)
+{
+ t->ret_stack[offset] = (bitmap << FGRAPH_INDEX_SHIFT) |
+ (FGRAPH_TYPE_BITMAP << FGRAPH_TYPE_SHIFT) | FGRAPH_FRAME_OFFSET;
+}
+
+/*
+ * @offset: The offset into @t->ret_stack to find the ret_stack entry
+ * @frame_offset: Where to place the offset into @t->ret_stack of that entry
+ *
+ * Returns a pointer to the previous ret_stack below @offset or NULL
+ * when it reaches the bottom of the stack.
+ *
+ * Calling this with:
+ *
+ * offset = task->curr_ret_stack;
+ * do {
+ * ret_stack = get_ret_stack(task, offset, &offset);
+ * } while (ret_stack);
+ *
+ * Will iterate through all the ret_stack entries from curr_ret_stack
+ * down to the first one.
+ */
+static inline struct ftrace_ret_stack *
+get_ret_stack(struct task_struct *t, int offset, int *frame_offset)
+{
+ int offs;
+
+ BUILD_BUG_ON(FGRAPH_FRAME_SIZE % sizeof(long));
+
+ if (unlikely(offset <= 0))
+ return NULL;
+
+ offs = get_frame_offset(t, --offset);
+ if (WARN_ON_ONCE(offs <= 0 || offs > offset))
+ return NULL;
+
+ offset -= offs;
+
+ *frame_offset = offset;
+ return RET_STACK(t, offset);
+}
+
/* Both enabled by default (can be cleared by function_graph tracer flags */
static bool fgraph_sleep_time = true;
@@ -110,11 +253,13 @@ void ftrace_graph_stop(void)
/* Add a function return address to the trace stack on thread info.*/
static int
ftrace_push_return_trace(unsigned long ret, unsigned long func,
- unsigned long frame_pointer, unsigned long *retp)
+ unsigned long frame_pointer, unsigned long *retp,
+ int fgraph_idx)
{
struct ftrace_ret_stack *ret_stack;
unsigned long long calltime;
- int index;
+ unsigned long val;
+ int offset;
if (unlikely(ftrace_graph_is_dead()))
return -EBUSY;
@@ -124,24 +269,57 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func,
BUILD_BUG_ON(SHADOW_STACK_SIZE % sizeof(long));
+ /* Set val to "reserved" with the delta to the new fgraph frame */
+ val = (FGRAPH_TYPE_RESERVED << FGRAPH_TYPE_SHIFT) | FGRAPH_FRAME_OFFSET;
+
/*
* We must make sure the ret_stack is tested before we read
* anything else.
*/
smp_rmb();
- /* The return trace stack is full */
- if (current->curr_ret_stack >= SHADOW_STACK_MAX_INDEX) {
+ /*
+ * Check if there's room on the shadow stack to fit a fraph frame
+ * and a bitmap word.
+ */
+ if (current->curr_ret_stack + FGRAPH_FRAME_OFFSET + 1 >= SHADOW_STACK_MAX_OFFSET) {
atomic_inc(¤t->trace_overrun);
return -EBUSY;
}
calltime = trace_clock_local();
- index = current->curr_ret_stack;
- RET_STACK_INC(current->curr_ret_stack);
- ret_stack = RET_STACK(current, index);
+ offset = READ_ONCE(current->curr_ret_stack);
+ ret_stack = RET_STACK(current, offset);
+ offset += FGRAPH_FRAME_OFFSET;
+
+ /* ret offset = FGRAPH_FRAME_OFFSET ; type = reserved */
+ current->ret_stack[offset] = val;
+ ret_stack->ret = ret;
+ /*
+ * The unwinders expect curr_ret_stack to point to either zero
+ * or an offset where to find the next ret_stack. Even though the
+ * ret stack might be bogus, we want to write the ret and the
+ * offset to find the ret_stack before we increment the stack point.
+ * If an interrupt comes in now before we increment the curr_ret_stack
+ * it may blow away what we wrote. But that's fine, because the
+ * offset will still be correct (even though the 'ret' won't be).
+ * What we worry about is the offset being correct after we increment
+ * the curr_ret_stack and before we update that offset, as if an
+ * interrupt comes in and does an unwind stack dump, it will need
+ * at least a correct offset!
+ */
barrier();
+ WRITE_ONCE(current->curr_ret_stack, offset + 1);
+ /*
+ * This next barrier is to ensure that an interrupt coming in
+ * will not corrupt what we are about to write.
+ */
+ barrier();
+
+ /* Still keep it reserved even if an interrupt came in */
+ current->ret_stack[offset] = val;
+
ret_stack->ret = ret;
ret_stack->func = func;
ret_stack->calltime = calltime;
@@ -151,7 +329,7 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func,
#ifdef HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
ret_stack->retp = retp;
#endif
- return 0;
+ return offset;
}
/*
@@ -168,49 +346,67 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func,
# define MCOUNT_INSN_SIZE 0
#endif
+/* If the caller does not use ftrace, call this function. */
int function_graph_enter(unsigned long ret, unsigned long func,
unsigned long frame_pointer, unsigned long *retp)
{
struct ftrace_graph_ent trace;
+ unsigned long bitmap = 0;
+ int offset;
+ int i;
trace.func = func;
trace.depth = ++current->curr_ret_depth;
- if (ftrace_push_return_trace(ret, func, frame_pointer, retp))
+ offset = ftrace_push_return_trace(ret, func, frame_pointer, retp, 0);
+ if (offset < 0)
goto out;
- /* Only trace if the calling function expects to */
- if (!fgraph_array[0]->entryfunc(&trace))
+ for (i = 0; i < fgraph_array_cnt; i++) {
+ struct fgraph_ops *gops = fgraph_array[i];
+
+ if (gops == &fgraph_stub)
+ continue;
+
+ if (gops->entryfunc(&trace))
+ bitmap |= BIT(i);
+ }
+
+ if (!bitmap)
goto out_ret;
+ /*
+ * Since this function uses fgraph_idx = 0 as a tail-call checking
+ * flag, set that bit always.
+ */
+ set_bitmap(current, offset, bitmap | BIT(0));
+
return 0;
out_ret:
- RET_STACK_DEC(current->curr_ret_stack);
+ current->curr_ret_stack -= FGRAPH_FRAME_OFFSET + 1;
out:
current->curr_ret_depth--;
return -EBUSY;
}
/* Retrieve a function return address to the trace stack on thread info.*/
-static void
+static struct ftrace_ret_stack *
ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
- unsigned long frame_pointer)
+ unsigned long frame_pointer, int *offset)
{
struct ftrace_ret_stack *ret_stack;
- int index;
- index = current->curr_ret_stack;
- RET_STACK_DEC(index);
+ ret_stack = get_ret_stack(current, current->curr_ret_stack, offset);
- if (unlikely(index < 0 || index > SHADOW_STACK_MAX_INDEX)) {
+ if (unlikely(!ret_stack)) {
ftrace_graph_stop();
- WARN_ON(1);
+ WARN(1, "Bad function graph ret_stack pointer: %d",
+ current->curr_ret_stack);
/* Might as well panic, otherwise we have no where to go */
*ret = (unsigned long)panic;
- return;
+ return NULL;
}
- ret_stack = RET_STACK(current, index);
#ifdef HAVE_FUNCTION_GRAPH_FP_TEST
/*
* The arch may choose to record the frame pointer used
@@ -230,26 +426,29 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
ftrace_graph_stop();
WARN(1, "Bad frame pointer: expected %lx, received %lx\n"
" from func %ps return to %lx\n",
- current->ret_stack[index].fp,
+ ret_stack->fp,
frame_pointer,
(void *)ret_stack->func,
ret_stack->ret);
*ret = (unsigned long)panic;
- return;
+ return NULL;
}
#endif
+ *offset += FGRAPH_FRAME_OFFSET;
*ret = ret_stack->ret;
trace->func = ret_stack->func;
trace->calltime = ret_stack->calltime;
trace->overrun = atomic_read(¤t->trace_overrun);
- trace->depth = current->curr_ret_depth--;
+ trace->depth = current->curr_ret_depth;
/*
* We still want to trace interrupts coming in if
* max_depth is set to 1. Make sure the decrement is
* seen before ftrace_graph_return.
*/
barrier();
+
+ return ret_stack;
}
/*
@@ -287,30 +486,47 @@ struct fgraph_ret_regs;
static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs,
unsigned long frame_pointer)
{
+ struct ftrace_ret_stack *ret_stack;
struct ftrace_graph_ret trace;
+ unsigned long bitmap;
unsigned long ret;
+ int offset;
+ int i;
+
+ ret_stack = ftrace_pop_return_trace(&trace, &ret, frame_pointer, &offset);
+
+ if (unlikely(!ret_stack)) {
+ ftrace_graph_stop();
+ WARN_ON(1);
+ /* Might as well panic. What else to do? */
+ return (unsigned long)panic;
+ }
- ftrace_pop_return_trace(&trace, &ret, frame_pointer);
+ trace.rettime = trace_clock_local();
#ifdef CONFIG_FUNCTION_GRAPH_RETVAL
trace.retval = fgraph_ret_regs_return_value(ret_regs);
#endif
- trace.rettime = trace_clock_local();
- fgraph_array[0]->retfunc(&trace);
+
+ bitmap = get_bitmap_bits(current, offset);
+ for (i = 0; i < FGRAPH_ARRAY_SIZE; i++) {
+ struct fgraph_ops *gops = fgraph_array[i];
+
+ if (!(bitmap & BIT(i)))
+ continue;
+ if (gops == &fgraph_stub)
+ continue;
+
+ gops->retfunc(&trace);
+ }
+
/*
* The ftrace_graph_return() may still access the current
* ret_stack structure, we need to make sure the update of
* curr_ret_stack is after that.
*/
barrier();
- RET_STACK_DEC(current->curr_ret_stack);
-
- if (unlikely(!ret)) {
- ftrace_graph_stop();
- WARN_ON(1);
- /* Might as well panic. What else to do? */
- ret = (unsigned long)panic;
- }
-
+ current->curr_ret_stack -= FGRAPH_FRAME_OFFSET + 1;
+ current->curr_ret_depth--;
return ret;
}
@@ -333,7 +549,7 @@ unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
/**
* ftrace_graph_get_ret_stack - return the entry of the shadow stack
- * @task: The task to read the shadow stack from
+ * @task: The task to read the shadow stack from.
* @idx: Index down the shadow stack
*
* Return the ret_struct on the shadow stack of the @task at the
@@ -345,15 +561,17 @@ unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
struct ftrace_ret_stack *
ftrace_graph_get_ret_stack(struct task_struct *task, int idx)
{
- int index = task->curr_ret_stack;
+ struct ftrace_ret_stack *ret_stack = NULL;
+ int offset = task->curr_ret_stack;
- BUILD_BUG_ON(FGRAPH_FRAME_SIZE % sizeof(long));
-
- index -= FGRAPH_FRAME_OFFSET * (idx + 1);
- if (index < 0)
+ if (offset < 0)
return NULL;
- return RET_STACK(task, index);
+ do {
+ ret_stack = get_ret_stack(task, offset, &offset);
+ } while (ret_stack && --idx >= 0);
+
+ return ret_stack;
}
/**
@@ -376,16 +594,15 @@ unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
unsigned long ret, unsigned long *retp)
{
struct ftrace_ret_stack *ret_stack;
- int index = task->curr_ret_stack;
- int i;
+ int i = task->curr_ret_stack;
if (ret != (unsigned long)dereference_kernel_function_descriptor(return_to_handler))
return ret;
- RET_STACK_DEC(index);
-
- for (i = index; i >= 0; RET_STACK_DEC(i)) {
- ret_stack = RET_STACK(task, i);
+ while (i > 0) {
+ ret_stack = get_ret_stack(current, i, &i);
+ if (!ret_stack)
+ break;
if (ret_stack->retp == retp)
return ret_stack->ret;
}
@@ -396,21 +613,26 @@ unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
unsigned long ret, unsigned long *retp)
{
- int task_idx;
+ struct ftrace_ret_stack *ret_stack;
+ int offset = task->curr_ret_stack;
+ int i;
if (ret != (unsigned long)dereference_kernel_function_descriptor(return_to_handler))
return ret;
- task_idx = task->curr_ret_stack;
- RET_STACK_DEC(task_idx);
-
- if (!task->ret_stack || task_idx < *idx)
+ if (!idx)
return ret;
- task_idx -= *idx;
- RET_STACK_INC(*idx);
+ i = *idx;
+ do {
+ ret_stack = get_ret_stack(task, offset, &offset);
+ i--;
+ } while (i >= 0 && ret_stack);
+
+ if (ret_stack)
+ return ret_stack->ret;
- return RET_STACK(task, task_idx);
+ return ret;
}
#endif /* HAVE_FUNCTION_GRAPH_RET_ADDR_PTR */
@@ -493,7 +715,7 @@ ftrace_graph_probe_sched_switch(void *ignore, bool preempt,
{
struct ftrace_ret_stack *ret_stack;
unsigned long long timestamp;
- int index;
+ int offset;
/*
* Does the user want to count the time a function was asleep.
@@ -516,10 +738,10 @@ ftrace_graph_probe_sched_switch(void *ignore, bool preempt,
*/
timestamp -= next->ftrace_timestamp;
- for (index = next->curr_ret_stack - FGRAPH_FRAME_OFFSET; index >= 0; ) {
- ret_stack = RET_STACK(next, index);
- ret_stack->calltime += timestamp;
- index -= FGRAPH_FRAME_OFFSET;
+ for (offset = next->curr_ret_stack; offset > 0; ) {
+ ret_stack = get_ret_stack(next, offset, &offset);
+ if (ret_stack)
+ ret_stack->calltime += timestamp;
}
}
@@ -570,6 +792,8 @@ graph_init_task(struct task_struct *t, unsigned long *ret_stack)
{
atomic_set(&t->trace_overrun, 0);
t->ftrace_timestamp = 0;
+ t->curr_ret_stack = 0;
+ t->curr_ret_depth = -1;
/* make curr_ret_stack visible before we add the ret_stack */
smp_wmb();
t->ret_stack = ret_stack;
@@ -691,6 +915,7 @@ int register_ftrace_graph(struct fgraph_ops *gops)
fgraph_array[i] = gops;
if (i + 1 > fgraph_array_cnt)
fgraph_array_cnt = i + 1;
+ gops->idx = i;
ftrace_graph_active++;
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 05/27] function_graph: Handle tail calls for stack unwinding
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (3 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 04/27] function_graph: Allow multiple users to attach to function graph Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 06/27] function_graph: Remove logic around ftrace_graph_entry and return Steven Rostedt
` (23 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
For the tail-call, there would be 2 or more ftrace_ret_stacks on the
ret_stack, which records "return_to_handler" as the return address except
for the last one. But on the real stack, there should be 1 entry because
tail-call reuses the return address on the stack and jump to the next
function.
In ftrace_graph_ret_addr() that is used for stack unwinding, skip tail
calls as a real stack unwinder would do.
Link: https://lore.kernel.org/linux-trace-kernel/171509096221.162236.8806372072523195752.stgit@devnote2
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/fgraph.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index aae51f746828..8de2a2662281 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -594,16 +594,26 @@ unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
unsigned long ret, unsigned long *retp)
{
struct ftrace_ret_stack *ret_stack;
+ unsigned long return_handler = (unsigned long)dereference_kernel_function_descriptor(return_to_handler);
int i = task->curr_ret_stack;
- if (ret != (unsigned long)dereference_kernel_function_descriptor(return_to_handler))
+ if (ret != return_handler)
return ret;
while (i > 0) {
ret_stack = get_ret_stack(current, i, &i);
if (!ret_stack)
break;
- if (ret_stack->retp == retp)
+ /*
+ * For the tail-call, there would be 2 or more ftrace_ret_stacks on
+ * the ret_stack, which records "return_to_handler" as the return
+ * address except for the last one.
+ * But on the real stack, there should be 1 entry because tail-call
+ * reuses the return address on the stack and jump to the next function.
+ * Thus we will continue to find real return address.
+ */
+ if (ret_stack->retp == retp &&
+ ret_stack->ret != return_handler)
return ret_stack->ret;
}
@@ -614,10 +624,11 @@ unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
unsigned long ret, unsigned long *retp)
{
struct ftrace_ret_stack *ret_stack;
+ unsigned long return_handler = (unsigned long)dereference_kernel_function_descriptor(return_to_handler);
int offset = task->curr_ret_stack;
int i;
- if (ret != (unsigned long)dereference_kernel_function_descriptor(return_to_handler))
+ if (ret != return_handler)
return ret;
if (!idx)
@@ -626,6 +637,8 @@ unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
i = *idx;
do {
ret_stack = get_ret_stack(task, offset, &offset);
+ if (ret_stack && ret_stack->ret == return_handler)
+ continue;
i--;
} while (i >= 0 && ret_stack);
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 06/27] function_graph: Remove logic around ftrace_graph_entry and return
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (4 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 05/27] function_graph: Handle tail calls for stack unwinding Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 07/27] ftrace/function_graph: Pass fgraph_ops to function graph callbacks Steven Rostedt
` (22 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The function pointers ftrace_graph_entry and ftrace_graph_return are no
longer called via the function_graph tracer. Instead, an array structure is
now used that will allow for multiple users of the function_graph
infrastructure. The variables are still used by the architecture code for
non dynamic ftrace configs, where a test is made against them to see if
they point to the default stub function or not. This is how the static
function tracing knows to call into the function graph tracer
infrastructure or not.
Two new stub functions are made. entry_run() and return_run(). The
ftrace_graph_entry and ftrace_graph_return are set to them respectively
when the function graph tracer is enabled, and this will trigger the
architecture specific function graph code to be executed.
This also requires checking the global_ops hash for all calls into the
function_graph tracer.
Co-developed with Masami Hiramatsu:
Link: https://lore.kernel.org/linux-trace-kernel/171509097408.162236.17387844142114638932.stgit@devnote2
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/fgraph.c | 67 ++++++++--------------------------
kernel/trace/ftrace.c | 2 -
kernel/trace/ftrace_internal.h | 2 -
3 files changed, 15 insertions(+), 56 deletions(-)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 8de2a2662281..2b52afa03ab4 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -163,6 +163,17 @@ set_bitmap(struct task_struct *t, int offset, unsigned long bitmap)
(FGRAPH_TYPE_BITMAP << FGRAPH_TYPE_SHIFT) | FGRAPH_FRAME_OFFSET;
}
+/* ftrace_graph_entry set to this to tell some archs to run function graph */
+static int entry_run(struct ftrace_graph_ent *trace)
+{
+ return 0;
+}
+
+/* ftrace_graph_return set to this to tell some archs to run function graph */
+static void return_run(struct ftrace_graph_ret *trace)
+{
+}
+
/*
* @offset: The offset into @t->ret_stack to find the ret_stack entry
* @frame_offset: Where to place the offset into @t->ret_stack of that entry
@@ -675,7 +686,6 @@ extern void ftrace_stub_graph(struct ftrace_graph_ret *);
/* The callbacks that hook a function */
trace_func_graph_ret_t ftrace_graph_return = ftrace_stub_graph;
trace_func_graph_ent_t ftrace_graph_entry = ftrace_graph_entry_stub;
-static trace_func_graph_ent_t __ftrace_graph_entry = ftrace_graph_entry_stub;
/* Try to assign a return stack array on FTRACE_RETSTACK_ALLOC_SIZE tasks. */
static int alloc_retstack_tasklist(unsigned long **ret_stack_list)
@@ -758,46 +768,6 @@ ftrace_graph_probe_sched_switch(void *ignore, bool preempt,
}
}
-static int ftrace_graph_entry_test(struct ftrace_graph_ent *trace)
-{
- if (!ftrace_ops_test(&global_ops, trace->func, NULL))
- return 0;
- return __ftrace_graph_entry(trace);
-}
-
-/*
- * The function graph tracer should only trace the functions defined
- * by set_ftrace_filter and set_ftrace_notrace. If another function
- * tracer ops is registered, the graph tracer requires testing the
- * function against the global ops, and not just trace any function
- * that any ftrace_ops registered.
- */
-void update_function_graph_func(void)
-{
- struct ftrace_ops *op;
- bool do_test = false;
-
- /*
- * The graph and global ops share the same set of functions
- * to test. If any other ops is on the list, then
- * the graph tracing needs to test if its the function
- * it should call.
- */
- do_for_each_ftrace_op(op, ftrace_ops_list) {
- if (op != &global_ops && op != &graph_ops &&
- op != &ftrace_list_end) {
- do_test = true;
- /* in double loop, break out with goto */
- goto out;
- }
- } while_for_each_ftrace_op(op);
- out:
- if (do_test)
- ftrace_graph_entry = ftrace_graph_entry_test;
- else
- ftrace_graph_entry = __ftrace_graph_entry;
-}
-
static DEFINE_PER_CPU(unsigned long *, idle_ret_stack);
static void
@@ -939,18 +909,12 @@ int register_ftrace_graph(struct fgraph_ops *gops)
ftrace_graph_active--;
goto out;
}
-
- ftrace_graph_return = gops->retfunc;
-
/*
- * Update the indirect function to the entryfunc, and the
- * function that gets called to the entry_test first. Then
- * call the update fgraph entry function to determine if
- * the entryfunc should be called directly or not.
+ * Some archs just test to see if these are not
+ * the default function
*/
- __ftrace_graph_entry = gops->entryfunc;
- ftrace_graph_entry = ftrace_graph_entry_test;
- update_function_graph_func();
+ ftrace_graph_return = return_run;
+ ftrace_graph_entry = entry_run;
ret = ftrace_startup(&graph_ops, FTRACE_START_FUNC_RET);
}
@@ -986,7 +950,6 @@ void unregister_ftrace_graph(struct fgraph_ops *gops)
if (!ftrace_graph_active) {
ftrace_graph_return = ftrace_stub_graph;
ftrace_graph_entry = ftrace_graph_entry_stub;
- __ftrace_graph_entry = ftrace_graph_entry_stub;
ftrace_shutdown(&graph_ops, FTRACE_STOP_FUNC_RET);
unregister_pm_notifier(&ftrace_suspend_notifier);
unregister_trace_sched_switch(ftrace_graph_probe_sched_switch, NULL);
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 65208d3b5ed9..789950a4f977 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -235,8 +235,6 @@ static void update_ftrace_function(void)
func = ftrace_ops_list_func;
}
- update_function_graph_func();
-
/* If there's no change, then do nothing more here */
if (ftrace_trace_function == func)
return;
diff --git a/kernel/trace/ftrace_internal.h b/kernel/trace/ftrace_internal.h
index 5012c04f92c0..19eddcb91584 100644
--- a/kernel/trace/ftrace_internal.h
+++ b/kernel/trace/ftrace_internal.h
@@ -42,10 +42,8 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip, void *regs)
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
extern int ftrace_graph_active;
-void update_function_graph_func(void);
#else /* !CONFIG_FUNCTION_GRAPH_TRACER */
# define ftrace_graph_active 0
-static inline void update_function_graph_func(void) { }
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
#else /* !CONFIG_FUNCTION_TRACER */
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 07/27] ftrace/function_graph: Pass fgraph_ops to function graph callbacks
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (5 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 06/27] function_graph: Remove logic around ftrace_graph_entry and return Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 08/27] ftrace: Allow function_graph tracer to be enabled in instances Steven Rostedt
` (21 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Pass the fgraph_ops structure to the function graph callbacks. This will
allow callbacks to add a descriptor to a fgraph_ops private field that wil
be added in the future and use it for the callbacks. This will be useful
when more than one callback can be registered to the function graph tracer.
Co-developed with Masami Hiramatsu:
Link: https://lore.kernel.org/linux-trace-kernel/171509098588.162236.4787930115997357578.stgit@devnote2
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/ftrace.h | 10 +++++++---
kernel/trace/fgraph.c | 16 +++++++++-------
kernel/trace/ftrace.c | 6 ++++--
kernel/trace/trace.h | 4 ++--
kernel/trace/trace_functions_graph.c | 11 +++++++----
kernel/trace/trace_irqsoff.c | 6 ++++--
kernel/trace/trace_sched_wakeup.c | 6 ++++--
kernel/trace/trace_selftest.c | 5 +++--
8 files changed, 40 insertions(+), 24 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 8f00a7415bd5..4d817b85ac0d 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1027,11 +1027,15 @@ struct ftrace_graph_ret {
unsigned long long rettime;
} __packed;
+struct fgraph_ops;
+
/* Type of the callback handlers for tracing function graph*/
-typedef void (*trace_func_graph_ret_t)(struct ftrace_graph_ret *); /* return */
-typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *); /* entry */
+typedef void (*trace_func_graph_ret_t)(struct ftrace_graph_ret *,
+ struct fgraph_ops *); /* return */
+typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *,
+ struct fgraph_ops *); /* entry */
-extern int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace);
+extern int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace, struct fgraph_ops *gops);
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 2b52afa03ab4..54ed2ed2036b 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -164,13 +164,13 @@ set_bitmap(struct task_struct *t, int offset, unsigned long bitmap)
}
/* ftrace_graph_entry set to this to tell some archs to run function graph */
-static int entry_run(struct ftrace_graph_ent *trace)
+static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops)
{
return 0;
}
/* ftrace_graph_return set to this to tell some archs to run function graph */
-static void return_run(struct ftrace_graph_ret *trace)
+static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops)
{
}
@@ -234,12 +234,14 @@ int __weak ftrace_disable_ftrace_graph_caller(void)
}
#endif
-int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace)
+int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
+ struct fgraph_ops *gops)
{
return 0;
}
-static void ftrace_graph_ret_stub(struct ftrace_graph_ret *trace)
+static void ftrace_graph_ret_stub(struct ftrace_graph_ret *trace,
+ struct fgraph_ops *gops)
{
}
@@ -379,7 +381,7 @@ int function_graph_enter(unsigned long ret, unsigned long func,
if (gops == &fgraph_stub)
continue;
- if (gops->entryfunc(&trace))
+ if (gops->entryfunc(&trace, gops))
bitmap |= BIT(i);
}
@@ -527,7 +529,7 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs
if (gops == &fgraph_stub)
continue;
- gops->retfunc(&trace);
+ gops->retfunc(&trace, gops);
}
/*
@@ -681,7 +683,7 @@ void ftrace_graph_sleep_time_control(bool enable)
* Simply points to ftrace_stub, but with the proper protocol.
* Defined by the linker script in linux/vmlinux.lds.h
*/
-extern void ftrace_stub_graph(struct ftrace_graph_ret *);
+void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops);
/* The callbacks that hook a function */
trace_func_graph_ret_t ftrace_graph_return = ftrace_stub_graph;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 789950a4f977..d18387c0642d 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -815,7 +815,8 @@ void ftrace_graph_graph_time_control(bool enable)
fgraph_graph_time = enable;
}
-static int profile_graph_entry(struct ftrace_graph_ent *trace)
+static int profile_graph_entry(struct ftrace_graph_ent *trace,
+ struct fgraph_ops *gops)
{
struct ftrace_ret_stack *ret_stack;
@@ -832,7 +833,8 @@ static int profile_graph_entry(struct ftrace_graph_ent *trace)
return 1;
}
-static void profile_graph_return(struct ftrace_graph_ret *trace)
+static void profile_graph_return(struct ftrace_graph_ret *trace,
+ struct fgraph_ops *gops)
{
struct ftrace_ret_stack *ret_stack;
struct ftrace_profile_stat *stat;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 749a182dab48..2575ec243350 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -679,8 +679,8 @@ void trace_latency_header(struct seq_file *m);
void trace_default_header(struct seq_file *m);
void print_trace_header(struct seq_file *m, struct trace_iterator *iter);
-void trace_graph_return(struct ftrace_graph_ret *trace);
-int trace_graph_entry(struct ftrace_graph_ent *trace);
+void trace_graph_return(struct ftrace_graph_ret *trace, struct fgraph_ops *gops);
+int trace_graph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops);
void set_graph_array(struct trace_array *tr);
void tracing_start_cmdline_record(void);
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index c35fbaab2a47..b7b142b65299 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -129,7 +129,8 @@ static inline int ftrace_graph_ignore_irqs(void)
return in_hardirq();
}
-int trace_graph_entry(struct ftrace_graph_ent *trace)
+int trace_graph_entry(struct ftrace_graph_ent *trace,
+ struct fgraph_ops *gops)
{
struct trace_array *tr = graph_array;
struct trace_array_cpu *data;
@@ -238,7 +239,8 @@ void __trace_graph_return(struct trace_array *tr,
trace_buffer_unlock_commit_nostack(buffer, event);
}
-void trace_graph_return(struct ftrace_graph_ret *trace)
+void trace_graph_return(struct ftrace_graph_ret *trace,
+ struct fgraph_ops *gops)
{
struct trace_array *tr = graph_array;
struct trace_array_cpu *data;
@@ -275,7 +277,8 @@ void set_graph_array(struct trace_array *tr)
smp_mb();
}
-static void trace_graph_thresh_return(struct ftrace_graph_ret *trace)
+static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
+ struct fgraph_ops *gops)
{
ftrace_graph_addr_finish(trace);
@@ -288,7 +291,7 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace)
(trace->rettime - trace->calltime < tracing_thresh))
return;
else
- trace_graph_return(trace);
+ trace_graph_return(trace, gops);
}
static struct fgraph_ops funcgraph_thresh_ops = {
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index ba37f768e2f2..5478f4c4f708 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -175,7 +175,8 @@ static int irqsoff_display_graph(struct trace_array *tr, int set)
return start_irqsoff_tracer(irqsoff_trace, set);
}
-static int irqsoff_graph_entry(struct ftrace_graph_ent *trace)
+static int irqsoff_graph_entry(struct ftrace_graph_ent *trace,
+ struct fgraph_ops *gops)
{
struct trace_array *tr = irqsoff_trace;
struct trace_array_cpu *data;
@@ -205,7 +206,8 @@ static int irqsoff_graph_entry(struct ftrace_graph_ent *trace)
return ret;
}
-static void irqsoff_graph_return(struct ftrace_graph_ret *trace)
+static void irqsoff_graph_return(struct ftrace_graph_ret *trace,
+ struct fgraph_ops *gops)
{
struct trace_array *tr = irqsoff_trace;
struct trace_array_cpu *data;
diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
index 0469a04a355f..49bcc812652c 100644
--- a/kernel/trace/trace_sched_wakeup.c
+++ b/kernel/trace/trace_sched_wakeup.c
@@ -112,7 +112,8 @@ static int wakeup_display_graph(struct trace_array *tr, int set)
return start_func_tracer(tr, set);
}
-static int wakeup_graph_entry(struct ftrace_graph_ent *trace)
+static int wakeup_graph_entry(struct ftrace_graph_ent *trace,
+ struct fgraph_ops *gops)
{
struct trace_array *tr = wakeup_trace;
struct trace_array_cpu *data;
@@ -141,7 +142,8 @@ static int wakeup_graph_entry(struct ftrace_graph_ent *trace)
return ret;
}
-static void wakeup_graph_return(struct ftrace_graph_ret *trace)
+static void wakeup_graph_return(struct ftrace_graph_ret *trace,
+ struct fgraph_ops *gops)
{
struct trace_array *tr = wakeup_trace;
struct trace_array_cpu *data;
diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
index e9c5058a8efd..56f269c0560a 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -762,7 +762,8 @@ trace_selftest_startup_function(struct tracer *trace, struct trace_array *tr)
static unsigned int graph_hang_thresh;
/* Wrap the real function entry probe to avoid possible hanging */
-static int trace_graph_entry_watchdog(struct ftrace_graph_ent *trace)
+static int trace_graph_entry_watchdog(struct ftrace_graph_ent *trace,
+ struct fgraph_ops *gops)
{
/* This is harmlessly racy, we want to approximately detect a hang */
if (unlikely(++graph_hang_thresh > GRAPH_MAX_FUNC_TEST)) {
@@ -776,7 +777,7 @@ static int trace_graph_entry_watchdog(struct ftrace_graph_ent *trace)
return 0;
}
- return trace_graph_entry(trace);
+ return trace_graph_entry(trace, gops);
}
static struct fgraph_ops fgraph_ops __initdata = {
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 08/27] ftrace: Allow function_graph tracer to be enabled in instances
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (6 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 07/27] ftrace/function_graph: Pass fgraph_ops to function graph callbacks Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 09/27] ftrace: Allow ftrace startup flags to exist without dynamic ftrace Steven Rostedt
` (20 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Now that function graph tracing can handle more than one user, allow it to
be enabled in the ftrace instances. Note, the filtering of the functions is
still joined by the top level set_ftrace_filter and friends, as well as the
graph and nograph files.
Co-developed with Masami Hiramatsu:
Link: https://lore.kernel.org/linux-trace-kernel/171509099743.162236.1699959255446248163.stgit@devnote2
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/ftrace.h | 1 +
kernel/trace/ftrace.c | 1 +
kernel/trace/trace.h | 13 +++++-
kernel/trace/trace_functions.c | 8 ++++
kernel/trace/trace_functions_graph.c | 65 +++++++++++++++++-----------
kernel/trace/trace_selftest.c | 4 +-
6 files changed, 64 insertions(+), 28 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 4d817b85ac0d..fd656e6d6b7c 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1042,6 +1042,7 @@ extern int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace, struct fgraph
struct fgraph_ops {
trace_func_graph_ent_t entryfunc;
trace_func_graph_ret_t retfunc;
+ void *private;
int idx;
};
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index d18387c0642d..b85f00b0ffe7 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -7327,6 +7327,7 @@ __init void ftrace_init_global_array_ops(struct trace_array *tr)
tr->ops = &global_ops;
tr->ops->private = tr;
ftrace_init_trace_array(tr);
+ init_array_fgraph_ops(tr);
}
void ftrace_init_array_ops(struct trace_array *tr, ftrace_func_t func)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 2575ec243350..a5070f9b977b 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -397,6 +397,9 @@ struct trace_array {
struct ftrace_ops *ops;
struct trace_pid_list __rcu *function_pids;
struct trace_pid_list __rcu *function_no_pids;
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ struct fgraph_ops *gops;
+#endif
#ifdef CONFIG_DYNAMIC_FTRACE
/* All of these are protected by the ftrace_lock */
struct list_head func_probes;
@@ -681,7 +684,6 @@ void print_trace_header(struct seq_file *m, struct trace_iterator *iter);
void trace_graph_return(struct ftrace_graph_ret *trace, struct fgraph_ops *gops);
int trace_graph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops);
-void set_graph_array(struct trace_array *tr);
void tracing_start_cmdline_record(void);
void tracing_stop_cmdline_record(void);
@@ -892,6 +894,9 @@ extern int __trace_graph_entry(struct trace_array *tr,
extern void __trace_graph_return(struct trace_array *tr,
struct ftrace_graph_ret *trace,
unsigned int trace_ctx);
+extern void init_array_fgraph_ops(struct trace_array *tr);
+extern int allocate_fgraph_ops(struct trace_array *tr);
+extern void free_fgraph_ops(struct trace_array *tr);
#ifdef CONFIG_DYNAMIC_FTRACE
extern struct ftrace_hash __rcu *ftrace_graph_hash;
@@ -1004,6 +1009,12 @@ print_graph_function_flags(struct trace_iterator *iter, u32 flags)
{
return TRACE_TYPE_UNHANDLED;
}
+static inline void init_array_fgraph_ops(struct trace_array *tr) { }
+static inline int allocate_fgraph_ops(struct trace_array *tr)
+{
+ return 0;
+}
+static inline void free_fgraph_ops(struct trace_array *tr) { }
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
extern struct list_head ftrace_pids;
diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
index 9f1bfbe105e8..8e8da0d0ee52 100644
--- a/kernel/trace/trace_functions.c
+++ b/kernel/trace/trace_functions.c
@@ -80,6 +80,7 @@ void ftrace_free_ftrace_ops(struct trace_array *tr)
int ftrace_create_function_files(struct trace_array *tr,
struct dentry *parent)
{
+ int ret;
/*
* The top level array uses the "global_ops", and the files are
* created on boot up.
@@ -90,6 +91,12 @@ int ftrace_create_function_files(struct trace_array *tr,
if (!tr->ops)
return -EINVAL;
+ ret = allocate_fgraph_ops(tr);
+ if (ret) {
+ kfree(tr->ops);
+ return ret;
+ }
+
ftrace_create_filter_files(tr->ops, parent);
return 0;
@@ -99,6 +106,7 @@ void ftrace_destroy_function_files(struct trace_array *tr)
{
ftrace_destroy_filter_files(tr->ops);
ftrace_free_ftrace_ops(tr);
+ free_fgraph_ops(tr);
}
static ftrace_func_t select_trace_function(u32 flags_val)
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index b7b142b65299..9ccc904a7703 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -83,8 +83,6 @@ static struct tracer_flags tracer_flags = {
.opts = trace_opts
};
-static struct trace_array *graph_array;
-
/*
* DURATION column is being also used to display IRQ signs,
* following values are used by print_graph_irq and others
@@ -132,7 +130,7 @@ static inline int ftrace_graph_ignore_irqs(void)
int trace_graph_entry(struct ftrace_graph_ent *trace,
struct fgraph_ops *gops)
{
- struct trace_array *tr = graph_array;
+ struct trace_array *tr = gops->private;
struct trace_array_cpu *data;
unsigned long flags;
unsigned int trace_ctx;
@@ -242,7 +240,7 @@ void __trace_graph_return(struct trace_array *tr,
void trace_graph_return(struct ftrace_graph_ret *trace,
struct fgraph_ops *gops)
{
- struct trace_array *tr = graph_array;
+ struct trace_array *tr = gops->private;
struct trace_array_cpu *data;
unsigned long flags;
unsigned int trace_ctx;
@@ -268,15 +266,6 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
local_irq_restore(flags);
}
-void set_graph_array(struct trace_array *tr)
-{
- graph_array = tr;
-
- /* Make graph_array visible before we start tracing */
-
- smp_mb();
-}
-
static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
struct fgraph_ops *gops)
{
@@ -294,25 +283,53 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
trace_graph_return(trace, gops);
}
-static struct fgraph_ops funcgraph_thresh_ops = {
- .entryfunc = &trace_graph_entry,
- .retfunc = &trace_graph_thresh_return,
-};
-
static struct fgraph_ops funcgraph_ops = {
.entryfunc = &trace_graph_entry,
.retfunc = &trace_graph_return,
};
+int allocate_fgraph_ops(struct trace_array *tr)
+{
+ struct fgraph_ops *gops;
+
+ gops = kzalloc(sizeof(*gops), GFP_KERNEL);
+ if (!gops)
+ return -ENOMEM;
+
+ gops->entryfunc = &trace_graph_entry;
+ gops->retfunc = &trace_graph_return;
+
+ tr->gops = gops;
+ gops->private = tr;
+ return 0;
+}
+
+void free_fgraph_ops(struct trace_array *tr)
+{
+ kfree(tr->gops);
+}
+
+__init void init_array_fgraph_ops(struct trace_array *tr)
+{
+ tr->gops = &funcgraph_ops;
+ funcgraph_ops.private = tr;
+}
+
static int graph_trace_init(struct trace_array *tr)
{
int ret;
- set_graph_array(tr);
+ tr->gops->entryfunc = trace_graph_entry;
+
if (tracing_thresh)
- ret = register_ftrace_graph(&funcgraph_thresh_ops);
+ tr->gops->retfunc = trace_graph_thresh_return;
else
- ret = register_ftrace_graph(&funcgraph_ops);
+ tr->gops->retfunc = trace_graph_return;
+
+ /* Make gops functions are visible before we start tracing */
+ smp_mb();
+
+ ret = register_ftrace_graph(tr->gops);
if (ret)
return ret;
tracing_start_cmdline_record();
@@ -323,10 +340,7 @@ static int graph_trace_init(struct trace_array *tr)
static void graph_trace_reset(struct trace_array *tr)
{
tracing_stop_cmdline_record();
- if (tracing_thresh)
- unregister_ftrace_graph(&funcgraph_thresh_ops);
- else
- unregister_ftrace_graph(&funcgraph_ops);
+ unregister_ftrace_graph(tr->gops);
}
static int graph_trace_update_thresh(struct trace_array *tr)
@@ -1365,6 +1379,7 @@ static struct tracer graph_trace __tracer_data = {
.print_header = print_graph_headers,
.flags = &tracer_flags,
.set_flag = func_graph_set_flag,
+ .allow_instances = true,
#ifdef CONFIG_FTRACE_SELFTEST
.selftest = trace_selftest_startup_function_graph,
#endif
diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
index 56f269c0560a..f8f55fd79e53 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -813,7 +813,7 @@ trace_selftest_startup_function_graph(struct tracer *trace,
* to detect and recover from possible hangs
*/
tracing_reset_online_cpus(&tr->array_buffer);
- set_graph_array(tr);
+ fgraph_ops.private = tr;
ret = register_ftrace_graph(&fgraph_ops);
if (ret) {
warn_failed_init_tracer(trace, ret);
@@ -856,7 +856,7 @@ trace_selftest_startup_function_graph(struct tracer *trace,
cond_resched();
tracing_reset_online_cpus(&tr->array_buffer);
- set_graph_array(tr);
+ fgraph_ops.private = tr;
/*
* Some archs *cough*PowerPC*cough* add characters to the
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 09/27] ftrace: Allow ftrace startup flags to exist without dynamic ftrace
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (7 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 08/27] ftrace: Allow function_graph tracer to be enabled in instances Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 10/27] ftrace: Add subops logic to allow one ops to manage many Steven Rostedt
` (19 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Some of the flags for ftrace_startup() may be exposed even when
CONFIG_DYNAMIC_FTRACE is not configured in. This is fine as the difference
between dynamic ftrace and static ftrace is done within the internals of
ftrace itself. No need to have use cases fail to compile because dynamic
ftrace is disabled.
This change is needed to move some of the logic of what is passed to
ftrace_startup() out of the parameters of ftrace_startup().
Link: https://lore.kernel.org/linux-trace-kernel/171509100890.162236.4362350342549122222.stgit@devnote2
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/ftrace.h | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index fd656e6d6b7c..586018744785 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -509,6 +509,15 @@ static inline void stack_tracer_disable(void) { }
static inline void stack_tracer_enable(void) { }
#endif
+enum {
+ FTRACE_UPDATE_CALLS = (1 << 0),
+ FTRACE_DISABLE_CALLS = (1 << 1),
+ FTRACE_UPDATE_TRACE_FUNC = (1 << 2),
+ FTRACE_START_FUNC_RET = (1 << 3),
+ FTRACE_STOP_FUNC_RET = (1 << 4),
+ FTRACE_MAY_SLEEP = (1 << 5),
+};
+
#ifdef CONFIG_DYNAMIC_FTRACE
void ftrace_arch_code_modify_prepare(void);
@@ -603,15 +612,6 @@ void ftrace_set_global_notrace(unsigned char *buf, int len, int reset);
void ftrace_free_filter(struct ftrace_ops *ops);
void ftrace_ops_set_global_filter(struct ftrace_ops *ops);
-enum {
- FTRACE_UPDATE_CALLS = (1 << 0),
- FTRACE_DISABLE_CALLS = (1 << 1),
- FTRACE_UPDATE_TRACE_FUNC = (1 << 2),
- FTRACE_START_FUNC_RET = (1 << 3),
- FTRACE_STOP_FUNC_RET = (1 << 4),
- FTRACE_MAY_SLEEP = (1 << 5),
-};
-
/*
* The FTRACE_UPDATE_* enum is used to pass information back
* from the ftrace_update_record() and ftrace_test_record()
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 10/27] ftrace: Add subops logic to allow one ops to manage many
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (8 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 09/27] ftrace: Allow ftrace startup flags to exist without dynamic ftrace Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 11/27] ftrace: Allow subops filtering to be modified Steven Rostedt
` (18 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
There are cases where a single system will use a single function callback
to handle multiple users. For example, to allow function_graph tracer to
have multiple users where each can trace their own set of functions, it is
useful to only have one ftrace_ops registered to ftrace that will call a
function by the function_graph tracer to handle the multiplexing with the
different registered function_graph tracers.
Add a "subop_list" to the ftrace_ops that will hold a list of other
ftrace_ops that the top ftrace_ops will manage.
The function ftrace_startup_subops() that takes the manager ftrace_ops and
a subop ftrace_ops it will manage. If there are no subops with the
ftrace_ops yet, it will copy the ftrace_ops subop filters to the manager
ftrace_ops and register that with ftrace_startup(), and adds the subop to
its subop_list. If the manager ops already has something registered, it
will then merge the new subop filters with what it has and enable the new
functions that covers all the subops it has.
To remove a subop, ftrace_shutdown_subops() is called which will use the
subop_list of the manager ops to rebuild all the functions it needs to
trace, and update the ftrace records to only call the functions it now has
registered. If there are no more functions registered, it will then call
ftrace_shutdown() to disable itself completely.
Note, it is up to the manager ops callback to always make sure that the
subops callbacks are called if its filter matches, as there are times in
the update where the callback could be calling more functions than those
that are currently registered.
This could be updated to handle other systems other than function_graph,
for example, fprobes could use this (but will need an interface to call
ftrace_startup_subops()).
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/ftrace.h | 1 +
kernel/trace/fgraph.c | 3 +-
kernel/trace/ftrace.c | 401 ++++++++++++++++++++++++++++++++-
kernel/trace/ftrace_internal.h | 1 +
kernel/trace/trace.h | 1 +
5 files changed, 405 insertions(+), 2 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 586018744785..978a1d3b270a 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -334,6 +334,7 @@ struct ftrace_ops {
unsigned long trampoline;
unsigned long trampoline_size;
struct list_head list;
+ struct list_head subop_list;
ftrace_ops_func_t ops_func;
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
unsigned long direct_call;
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 54ed2ed2036b..e39042c40937 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -21,7 +21,8 @@
#ifdef CONFIG_DYNAMIC_FTRACE
#define ASSIGN_OPS_HASH(opsname, val) \
.func_hash = val, \
- .local_hash.regex_lock = __MUTEX_INITIALIZER(opsname.local_hash.regex_lock),
+ .local_hash.regex_lock = __MUTEX_INITIALIZER(opsname.local_hash.regex_lock), \
+ .subop_list = LIST_HEAD_INIT(opsname.subop_list),
#else
#define ASSIGN_OPS_HASH(opsname, val)
#endif
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index b85f00b0ffe7..63b8d1fe1dd8 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -74,7 +74,8 @@
#ifdef CONFIG_DYNAMIC_FTRACE
#define INIT_OPS_HASH(opsname) \
.func_hash = &opsname.local_hash, \
- .local_hash.regex_lock = __MUTEX_INITIALIZER(opsname.local_hash.regex_lock),
+ .local_hash.regex_lock = __MUTEX_INITIALIZER(opsname.local_hash.regex_lock), \
+ .subop_list = LIST_HEAD_INIT(opsname.subop_list),
#else
#define INIT_OPS_HASH(opsname)
#endif
@@ -161,6 +162,7 @@ static inline void ftrace_ops_init(struct ftrace_ops *ops)
#ifdef CONFIG_DYNAMIC_FTRACE
if (!(ops->flags & FTRACE_OPS_FL_INITIALIZED)) {
mutex_init(&ops->local_hash.regex_lock);
+ INIT_LIST_HEAD(&ops->subop_list);
ops->func_hash = &ops->local_hash;
ops->flags |= FTRACE_OPS_FL_INITIALIZED;
}
@@ -3164,6 +3166,403 @@ int ftrace_shutdown(struct ftrace_ops *ops, int command)
return 0;
}
+/* Simply make a copy of @src and return it */
+static struct ftrace_hash *copy_hash(struct ftrace_hash *src)
+{
+ if (ftrace_hash_empty(src))
+ return EMPTY_HASH;
+
+ return alloc_and_copy_ftrace_hash(src->size_bits, src);
+}
+
+/*
+ * Append @new_hash entries to @hash:
+ *
+ * If @hash is the EMPTY_HASH then it traces all functions and nothing
+ * needs to be done.
+ *
+ * If @new_hash is the EMPTY_HASH, then make *hash the EMPTY_HASH so
+ * that it traces everything.
+ *
+ * Otherwise, go through all of @new_hash and add anything that @hash
+ * doesn't already have, to @hash.
+ *
+ * The filter_hash updates uses just the append_hash() function
+ * and the notrace_hash does not.
+ */
+static int append_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash)
+{
+ struct ftrace_func_entry *entry;
+ int size;
+ int i;
+
+ /* An empty hash does everything */
+ if (ftrace_hash_empty(*hash))
+ return 0;
+
+ /* If new_hash has everything make hash have everything */
+ if (ftrace_hash_empty(new_hash)) {
+ free_ftrace_hash(*hash);
+ *hash = EMPTY_HASH;
+ return 0;
+ }
+
+ size = 1 << new_hash->size_bits;
+ for (i = 0; i < size; i++) {
+ hlist_for_each_entry(entry, &new_hash->buckets[i], hlist) {
+ /* Only add if not already in hash */
+ if (!__ftrace_lookup_ip(*hash, entry->ip) &&
+ add_hash_entry(*hash, entry->ip) == NULL)
+ return -ENOMEM;
+ }
+ }
+ return 0;
+}
+
+/*
+ * Add to @hash only those that are in both @new_hash1 and @new_hash2
+ *
+ * The notrace_hash updates uses just the intersect_hash() function
+ * and the filter_hash does not.
+ */
+static int intersect_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash1,
+ struct ftrace_hash *new_hash2)
+{
+ struct ftrace_func_entry *entry;
+ int size;
+ int i;
+
+ /*
+ * If new_hash1 or new_hash2 is the EMPTY_HASH then make the hash
+ * empty as well as empty for notrace means none are notraced.
+ */
+ if (ftrace_hash_empty(new_hash1) || ftrace_hash_empty(new_hash2)) {
+ free_ftrace_hash(*hash);
+ *hash = EMPTY_HASH;
+ return 0;
+ }
+
+ size = 1 << new_hash1->size_bits;
+ for (i = 0; i < size; i++) {
+ hlist_for_each_entry(entry, &new_hash1->buckets[i], hlist) {
+ /* Only add if in both @new_hash1 and @new_hash2 */
+ if (__ftrace_lookup_ip(new_hash2, entry->ip) &&
+ add_hash_entry(*hash, entry->ip) == NULL)
+ return -ENOMEM;
+ }
+ }
+ /* If nothing intersects, make it the empty set */
+ if (ftrace_hash_empty(*hash)) {
+ free_ftrace_hash(*hash);
+ *hash = EMPTY_HASH;
+ }
+ return 0;
+}
+
+/* Return a new hash that has a union of all @ops->filter_hash entries */
+static struct ftrace_hash *append_hashes(struct ftrace_ops *ops)
+{
+ struct ftrace_hash *new_hash;
+ struct ftrace_ops *subops;
+ int ret;
+
+ new_hash = alloc_ftrace_hash(ops->func_hash->filter_hash->size_bits);
+ if (!new_hash)
+ return NULL;
+
+ list_for_each_entry(subops, &ops->subop_list, list) {
+ ret = append_hash(&new_hash, subops->func_hash->filter_hash);
+ if (ret < 0) {
+ free_ftrace_hash(new_hash);
+ return NULL;
+ }
+ /* Nothing more to do if new_hash is empty */
+ if (ftrace_hash_empty(new_hash))
+ break;
+ }
+ return new_hash;
+}
+
+/* Make @ops trace evenything except what all its subops do not trace */
+static struct ftrace_hash *intersect_hashes(struct ftrace_ops *ops)
+{
+ struct ftrace_hash *new_hash = NULL;
+ struct ftrace_ops *subops;
+ int size_bits;
+ int ret;
+
+ list_for_each_entry(subops, &ops->subop_list, list) {
+ struct ftrace_hash *next_hash;
+
+ if (!new_hash) {
+ size_bits = subops->func_hash->notrace_hash->size_bits;
+ new_hash = alloc_and_copy_ftrace_hash(size_bits, ops->func_hash->notrace_hash);
+ if (!new_hash)
+ return NULL;
+ continue;
+ }
+ size_bits = new_hash->size_bits;
+ next_hash = new_hash;
+ new_hash = alloc_ftrace_hash(size_bits);
+ ret = intersect_hash(&new_hash, next_hash, subops->func_hash->notrace_hash);
+ free_ftrace_hash(next_hash);
+ if (ret < 0) {
+ free_ftrace_hash(new_hash);
+ return NULL;
+ }
+ /* Nothing more to do if new_hash is empty */
+ if (ftrace_hash_empty(new_hash))
+ break;
+ }
+ return new_hash;
+}
+
+static bool ops_equal(struct ftrace_hash *A, struct ftrace_hash *B)
+{
+ struct ftrace_func_entry *entry;
+ int size;
+ int i;
+
+ if (ftrace_hash_empty(A))
+ return ftrace_hash_empty(B);
+
+ if (ftrace_hash_empty(B))
+ return ftrace_hash_empty(A);
+
+ if (A->count != B->count)
+ return false;
+
+ size = 1 << A->size_bits;
+ for (i = 0; i < size; i++) {
+ hlist_for_each_entry(entry, &A->buckets[i], hlist) {
+ if (!__ftrace_lookup_ip(B, entry->ip))
+ return false;
+ }
+ }
+
+ return true;
+}
+
+static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
+ struct ftrace_hash **orig_hash,
+ struct ftrace_hash *hash,
+ int enable);
+
+static int ftrace_update_ops(struct ftrace_ops *ops, struct ftrace_hash *filter_hash,
+ struct ftrace_hash *notrace_hash)
+{
+ int ret;
+
+ if (!ops_equal(filter_hash, ops->func_hash->filter_hash)) {
+ ret = ftrace_hash_move_and_update_ops(ops, &ops->func_hash->filter_hash,
+ filter_hash, 1);
+ if (ret < 0)
+ return ret;
+ }
+
+ if (!ops_equal(notrace_hash, ops->func_hash->notrace_hash)) {
+ ret = ftrace_hash_move_and_update_ops(ops, &ops->func_hash->notrace_hash,
+ notrace_hash, 0);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
+}
+
+/**
+ * ftrace_startup_subops - enable tracing for subops of an ops
+ * @ops: Manager ops (used to pick all the functions of its subops)
+ * @subops: A new ops to add to @ops
+ * @command: Extra commands to use to enable tracing
+ *
+ * The @ops is a manager @ops that has the filter that includes all the functions
+ * that its list of subops are tracing. Adding a new @subops will add the
+ * functions of @subops to @ops.
+ */
+int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int command)
+{
+ struct ftrace_hash *filter_hash;
+ struct ftrace_hash *notrace_hash;
+ struct ftrace_hash *save_filter_hash;
+ struct ftrace_hash *save_notrace_hash;
+ int size_bits;
+ int ret;
+
+ if (unlikely(ftrace_disabled))
+ return -ENODEV;
+
+ ftrace_ops_init(ops);
+ ftrace_ops_init(subops);
+
+ if (WARN_ON_ONCE(subops->flags & FTRACE_OPS_FL_ENABLED))
+ return -EBUSY;
+
+ /* Make everything canonical (Just in case!) */
+ if (!ops->func_hash->filter_hash)
+ ops->func_hash->filter_hash = EMPTY_HASH;
+ if (!ops->func_hash->notrace_hash)
+ ops->func_hash->notrace_hash = EMPTY_HASH;
+ if (!subops->func_hash->filter_hash)
+ subops->func_hash->filter_hash = EMPTY_HASH;
+ if (!subops->func_hash->notrace_hash)
+ subops->func_hash->notrace_hash = EMPTY_HASH;
+
+ /* For the first subops to ops just enable it normally */
+ if (list_empty(&ops->subop_list)) {
+ /* Just use the subops hashes */
+ filter_hash = copy_hash(subops->func_hash->filter_hash);
+ notrace_hash = copy_hash(subops->func_hash->notrace_hash);
+ if (!filter_hash || !notrace_hash) {
+ free_ftrace_hash(filter_hash);
+ free_ftrace_hash(notrace_hash);
+ return -ENOMEM;
+ }
+
+ save_filter_hash = ops->func_hash->filter_hash;
+ save_notrace_hash = ops->func_hash->notrace_hash;
+
+ ops->func_hash->filter_hash = filter_hash;
+ ops->func_hash->notrace_hash = notrace_hash;
+ list_add(&subops->list, &ops->subop_list);
+ ret = ftrace_startup(ops, command);
+ if (ret < 0) {
+ list_del(&subops->list);
+ ops->func_hash->filter_hash = save_filter_hash;
+ ops->func_hash->notrace_hash = save_notrace_hash;
+ free_ftrace_hash(filter_hash);
+ free_ftrace_hash(notrace_hash);
+ } else {
+ free_ftrace_hash(save_filter_hash);
+ free_ftrace_hash(save_notrace_hash);
+ subops->flags |= FTRACE_OPS_FL_ENABLED;
+ }
+ return ret;
+ }
+
+ /*
+ * Here there's already something attached. Here are the rules:
+ * o If either filter_hash is empty then the final stays empty
+ * o Otherwise, the final is a superset of both hashes
+ * o If either notrace_hash is empty then the final stays empty
+ * o Otherwise, the final is an intersection between the hashes
+ */
+ if (ftrace_hash_empty(ops->func_hash->filter_hash) ||
+ ftrace_hash_empty(subops->func_hash->filter_hash)) {
+ filter_hash = EMPTY_HASH;
+ } else {
+ size_bits = max(ops->func_hash->filter_hash->size_bits,
+ subops->func_hash->filter_hash->size_bits);
+ filter_hash = alloc_and_copy_ftrace_hash(size_bits, ops->func_hash->filter_hash);
+ if (!filter_hash)
+ return -ENOMEM;
+ ret = append_hash(&filter_hash, subops->func_hash->filter_hash);
+ if (ret < 0) {
+ free_ftrace_hash(filter_hash);
+ return ret;
+ }
+ }
+
+ if (ftrace_hash_empty(ops->func_hash->notrace_hash) ||
+ ftrace_hash_empty(subops->func_hash->notrace_hash)) {
+ notrace_hash = EMPTY_HASH;
+ } else {
+ size_bits = max(ops->func_hash->filter_hash->size_bits,
+ subops->func_hash->filter_hash->size_bits);
+ notrace_hash = alloc_ftrace_hash(size_bits);
+ if (!notrace_hash) {
+ free_ftrace_hash(filter_hash);
+ return -ENOMEM;
+ }
+
+ ret = intersect_hash(¬race_hash, ops->func_hash->filter_hash,
+ subops->func_hash->filter_hash);
+ if (ret < 0) {
+ free_ftrace_hash(filter_hash);
+ free_ftrace_hash(notrace_hash);
+ return ret;
+ }
+ }
+
+ list_add(&subops->list, &ops->subop_list);
+
+ ret = ftrace_update_ops(ops, filter_hash, notrace_hash);
+ free_ftrace_hash(filter_hash);
+ free_ftrace_hash(notrace_hash);
+ if (ret < 0)
+ list_del(&subops->list);
+ else
+ subops->flags |= FTRACE_OPS_FL_ENABLED;
+
+ return ret;
+}
+
+/**
+ * ftrace_shutdown_subops - Remove a subops from a manager ops
+ * @ops: A manager ops to remove @subops from
+ * @subops: The subops to remove from @ops
+ * @command: Any extra command flags to add to modifying the text
+ *
+ * Removes the functions being traced by the @subops from @ops. Note, it
+ * will not affect functions that are being traced by other subops that
+ * still exist in @ops.
+ *
+ * If the last subops is removed from @ops, then @ops is shutdown normally.
+ */
+int ftrace_shutdown_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int command)
+{
+ struct ftrace_hash *filter_hash;
+ struct ftrace_hash *notrace_hash;
+ int ret;
+
+ if (unlikely(ftrace_disabled))
+ return -ENODEV;
+
+ if (WARN_ON_ONCE(!(subops->flags & FTRACE_OPS_FL_ENABLED)))
+ return -EINVAL;
+
+ list_del(&subops->list);
+
+ if (list_empty(&ops->subop_list)) {
+ /* Last one, just disable the current ops */
+
+ ret = ftrace_shutdown(ops, command);
+ if (ret < 0) {
+ list_add(&subops->list, &ops->subop_list);
+ return ret;
+ }
+
+ subops->flags &= ~FTRACE_OPS_FL_ENABLED;
+
+ free_ftrace_hash(ops->func_hash->filter_hash);
+ free_ftrace_hash(ops->func_hash->notrace_hash);
+ ops->func_hash->filter_hash = EMPTY_HASH;
+ ops->func_hash->notrace_hash = EMPTY_HASH;
+
+ return 0;
+ }
+
+ /* Rebuild the hashes without subops */
+ filter_hash = append_hashes(ops);
+ notrace_hash = intersect_hashes(ops);
+ if (!filter_hash || !notrace_hash) {
+ free_ftrace_hash(filter_hash);
+ free_ftrace_hash(notrace_hash);
+ list_add(&subops->list, &ops->subop_list);
+ return -ENOMEM;
+ }
+
+ ret = ftrace_update_ops(ops, filter_hash, notrace_hash);
+ if (ret < 0)
+ list_add(&subops->list, &ops->subop_list);
+ else
+ subops->flags &= ~FTRACE_OPS_FL_ENABLED;
+
+ free_ftrace_hash(filter_hash);
+ free_ftrace_hash(notrace_hash);
+ return ret;
+}
+
static u64 ftrace_update_time;
unsigned long ftrace_update_tot_cnt;
unsigned long ftrace_number_of_pages;
diff --git a/kernel/trace/ftrace_internal.h b/kernel/trace/ftrace_internal.h
index 19eddcb91584..cdfd12c44ab4 100644
--- a/kernel/trace/ftrace_internal.h
+++ b/kernel/trace/ftrace_internal.h
@@ -15,6 +15,7 @@ extern struct ftrace_ops global_ops;
int ftrace_startup(struct ftrace_ops *ops, int command);
int ftrace_shutdown(struct ftrace_ops *ops, int command);
int ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip, void *regs);
+int ftrace_shutdown_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int command);
#else /* !CONFIG_DYNAMIC_FTRACE */
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index a5070f9b977b..9a70beb2cc46 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1136,6 +1136,7 @@ extern int ftrace_set_filter(struct ftrace_ops *ops, unsigned char *buf,
int len, int reset);
extern int ftrace_set_notrace(struct ftrace_ops *ops, unsigned char *buf,
int len, int reset);
+extern int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int command);
#else
struct ftrace_func_command;
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 11/27] ftrace: Allow subops filtering to be modified
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (9 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 10/27] ftrace: Add subops logic to allow one ops to manage many Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 12/27] function_graph: Have the instances use their own ftrace_ops for filtering Steven Rostedt
` (17 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
The subops filters use a "manager" ops to enable and disable its filters.
The manager ops can handle more than one subops, and its filter is what
controls what functions get set. Add a ftrace_hash_move_and_update_subops()
function that will update the manager ops when the subops filters change.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/ftrace.h | 3 +
kernel/trace/ftrace.c | 141 +++++++++++++++++++++++++++++++++--------
2 files changed, 116 insertions(+), 28 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 978a1d3b270a..63238a9a9270 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -227,6 +227,7 @@ ftrace_func_t ftrace_ops_get_func(struct ftrace_ops *ops);
* ftrace_enabled.
* DIRECT - Used by the direct ftrace_ops helper for direct functions
* (internal ftrace only, should not be used by others)
+ * SUBOP - Is controlled by another op in field managed.
*/
enum {
FTRACE_OPS_FL_ENABLED = BIT(0),
@@ -247,6 +248,7 @@ enum {
FTRACE_OPS_FL_TRACE_ARRAY = BIT(15),
FTRACE_OPS_FL_PERMANENT = BIT(16),
FTRACE_OPS_FL_DIRECT = BIT(17),
+ FTRACE_OPS_FL_SUBOP = BIT(18),
};
#ifndef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
@@ -336,6 +338,7 @@ struct ftrace_ops {
struct list_head list;
struct list_head subop_list;
ftrace_ops_func_t ops_func;
+ struct ftrace_ops *managed;
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
unsigned long direct_call;
#endif
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 63b8d1fe1dd8..cbb91b0afcc8 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3343,10 +3343,28 @@ static bool ops_equal(struct ftrace_hash *A, struct ftrace_hash *B)
return true;
}
-static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
- struct ftrace_hash **orig_hash,
- struct ftrace_hash *hash,
- int enable);
+static void ftrace_ops_update_code(struct ftrace_ops *ops,
+ struct ftrace_ops_hash *old_hash);
+
+static int __ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
+ struct ftrace_hash **orig_hash,
+ struct ftrace_hash *hash,
+ int enable)
+{
+ struct ftrace_ops_hash old_hash_ops;
+ struct ftrace_hash *old_hash;
+ int ret;
+
+ old_hash = *orig_hash;
+ old_hash_ops.filter_hash = ops->func_hash->filter_hash;
+ old_hash_ops.notrace_hash = ops->func_hash->notrace_hash;
+ ret = ftrace_hash_move(ops, enable, orig_hash, hash);
+ if (!ret) {
+ ftrace_ops_update_code(ops, &old_hash_ops);
+ free_ftrace_hash_rcu(old_hash);
+ }
+ return ret;
+}
static int ftrace_update_ops(struct ftrace_ops *ops, struct ftrace_hash *filter_hash,
struct ftrace_hash *notrace_hash)
@@ -3354,15 +3372,15 @@ static int ftrace_update_ops(struct ftrace_ops *ops, struct ftrace_hash *filter_
int ret;
if (!ops_equal(filter_hash, ops->func_hash->filter_hash)) {
- ret = ftrace_hash_move_and_update_ops(ops, &ops->func_hash->filter_hash,
- filter_hash, 1);
+ ret = __ftrace_hash_move_and_update_ops(ops, &ops->func_hash->filter_hash,
+ filter_hash, 1);
if (ret < 0)
return ret;
}
if (!ops_equal(notrace_hash, ops->func_hash->notrace_hash)) {
- ret = ftrace_hash_move_and_update_ops(ops, &ops->func_hash->notrace_hash,
- notrace_hash, 0);
+ ret = __ftrace_hash_move_and_update_ops(ops, &ops->func_hash->notrace_hash,
+ notrace_hash, 0);
if (ret < 0)
return ret;
}
@@ -3435,7 +3453,8 @@ int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int
} else {
free_ftrace_hash(save_filter_hash);
free_ftrace_hash(save_notrace_hash);
- subops->flags |= FTRACE_OPS_FL_ENABLED;
+ subops->flags |= FTRACE_OPS_FL_ENABLED | FTRACE_OPS_FL_SUBOP;
+ subops->managed = ops;
}
return ret;
}
@@ -3489,11 +3508,12 @@ int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int
ret = ftrace_update_ops(ops, filter_hash, notrace_hash);
free_ftrace_hash(filter_hash);
free_ftrace_hash(notrace_hash);
- if (ret < 0)
+ if (ret < 0) {
list_del(&subops->list);
- else
- subops->flags |= FTRACE_OPS_FL_ENABLED;
-
+ } else {
+ subops->flags |= FTRACE_OPS_FL_ENABLED | FTRACE_OPS_FL_SUBOP;
+ subops->managed = ops;
+ }
return ret;
}
@@ -3538,6 +3558,8 @@ int ftrace_shutdown_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, in
free_ftrace_hash(ops->func_hash->notrace_hash);
ops->func_hash->filter_hash = EMPTY_HASH;
ops->func_hash->notrace_hash = EMPTY_HASH;
+ subops->flags &= ~(FTRACE_OPS_FL_ENABLED | FTRACE_OPS_FL_SUBOP);
+ subops->managed = NULL;
return 0;
}
@@ -3553,16 +3575,65 @@ int ftrace_shutdown_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, in
}
ret = ftrace_update_ops(ops, filter_hash, notrace_hash);
- if (ret < 0)
+ if (ret < 0) {
list_add(&subops->list, &ops->subop_list);
- else
- subops->flags &= ~FTRACE_OPS_FL_ENABLED;
-
+ } else {
+ subops->flags &= ~(FTRACE_OPS_FL_ENABLED | FTRACE_OPS_FL_SUBOP);
+ subops->managed = NULL;
+ }
free_ftrace_hash(filter_hash);
free_ftrace_hash(notrace_hash);
return ret;
}
+static int ftrace_hash_move_and_update_subops(struct ftrace_ops *subops,
+ struct ftrace_hash **orig_subhash,
+ struct ftrace_hash *hash,
+ int enable)
+{
+ struct ftrace_ops *ops = subops->managed;
+ struct ftrace_hash **orig_hash;
+ struct ftrace_hash *save_hash;
+ struct ftrace_hash *new_hash;
+ int ret;
+
+ /* Manager ops can not be subops (yet) */
+ if (WARN_ON_ONCE(!ops || ops->flags & FTRACE_OPS_FL_SUBOP))
+ return -EINVAL;
+
+ /* Move the new hash over to the subops hash */
+ save_hash = *orig_subhash;
+ *orig_subhash = __ftrace_hash_move(hash);
+ if (!*orig_subhash) {
+ *orig_subhash = save_hash;
+ return -ENOMEM;
+ }
+
+ /* Create a new_hash to hold the ops new functions */
+ if (enable) {
+ orig_hash = &ops->func_hash->filter_hash;
+ new_hash = append_hashes(ops);
+ } else {
+ orig_hash = &ops->func_hash->notrace_hash;
+ new_hash = intersect_hashes(ops);
+ }
+
+ /* Move the hash over to the new hash */
+ ret = __ftrace_hash_move_and_update_ops(ops, orig_hash, new_hash, enable);
+
+ free_ftrace_hash(new_hash);
+
+ if (ret) {
+ /* Put back the original hash */
+ free_ftrace_hash_rcu(*orig_subhash);
+ *orig_subhash = save_hash;
+ } else {
+ free_ftrace_hash_rcu(save_hash);
+ }
+ return ret;
+}
+
+
static u64 ftrace_update_time;
unsigned long ftrace_update_tot_cnt;
unsigned long ftrace_number_of_pages;
@@ -4779,19 +4850,33 @@ static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
struct ftrace_hash *hash,
int enable)
{
- struct ftrace_ops_hash old_hash_ops;
- struct ftrace_hash *old_hash;
- int ret;
+ if (ops->flags & FTRACE_OPS_FL_SUBOP)
+ return ftrace_hash_move_and_update_subops(ops, orig_hash, hash, enable);
- old_hash = *orig_hash;
- old_hash_ops.filter_hash = ops->func_hash->filter_hash;
- old_hash_ops.notrace_hash = ops->func_hash->notrace_hash;
- ret = ftrace_hash_move(ops, enable, orig_hash, hash);
- if (!ret) {
- ftrace_ops_update_code(ops, &old_hash_ops);
- free_ftrace_hash_rcu(old_hash);
+ /*
+ * If this ops is not enabled, it could be sharing its filters
+ * with a subop. If that's the case, update the subop instead of
+ * this ops. Shared filters are only allowed to have one ops set
+ * at a time, and if we update the ops that is not enabled,
+ * it will not affect subops that share it.
+ */
+ if (!(ops->flags & FTRACE_OPS_FL_ENABLED)) {
+ struct ftrace_ops *op;
+
+ /* Check if any other manager subops maps to this hash */
+ do_for_each_ftrace_op(op, ftrace_ops_list) {
+ struct ftrace_ops *subops;
+
+ list_for_each_entry(subops, &op->subop_list, list) {
+ if ((subops->flags & FTRACE_OPS_FL_ENABLED) &&
+ subops->func_hash == ops->func_hash) {
+ return ftrace_hash_move_and_update_subops(subops, orig_hash, hash, enable);
+ }
+ }
+ } while_for_each_ftrace_op(op);
}
- return ret;
+
+ return __ftrace_hash_move_and_update_ops(ops, orig_hash, hash, enable);
}
static bool module_exists(const char *module)
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 12/27] function_graph: Have the instances use their own ftrace_ops for filtering
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (10 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 11/27] ftrace: Allow subops filtering to be modified Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 13/27] function_graph: Add pid tracing back to function graph tracer Steven Rostedt
` (16 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Allow for instances to have their own ftrace_ops part of the fgraph_ops
that makes the funtion_graph tracer filter on the set_ftrace_filter file
of the instance and not the top instance.
This uses the new ftrace_startup_subops(), by using graph_ops as the
"manager ops" that defines the callback function and adds the functions
defined by the filters of the ops for each trace instance. The callback
defined by the manager ops will call the registered fgraph ops that were
added to the fgraph_array.
Co-developed with Masami Hiramatsu:
Link: https://lore.kernel.org/linux-trace-kernel/171509102088.162236.15758883237657317789.stgit@devnote2
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/ftrace.h | 1 +
kernel/trace/fgraph.c | 81 +++++++++++++++++-----------
kernel/trace/ftrace.c | 2 +-
kernel/trace/trace.h | 15 +++---
kernel/trace/trace_functions.c | 2 +-
kernel/trace/trace_functions_graph.c | 8 ++-
6 files changed, 68 insertions(+), 41 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 63238a9a9270..8f865689e868 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1046,6 +1046,7 @@ extern int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace, struct fgraph
struct fgraph_ops {
trace_func_graph_ent_t entryfunc;
trace_func_graph_ret_t retfunc;
+ struct ftrace_ops ops; /* for the hash lists */
void *private;
int idx;
};
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index e39042c40937..3ef6db53c0bf 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -18,15 +18,6 @@
#include "ftrace_internal.h"
#include "trace.h"
-#ifdef CONFIG_DYNAMIC_FTRACE
-#define ASSIGN_OPS_HASH(opsname, val) \
- .func_hash = val, \
- .local_hash.regex_lock = __MUTEX_INITIALIZER(opsname.local_hash.regex_lock), \
- .subop_list = LIST_HEAD_INIT(opsname.subop_list),
-#else
-#define ASSIGN_OPS_HASH(opsname, val)
-#endif
-
/*
* FGRAPH_FRAME_SIZE: Size in bytes of the meta data on the shadow stack
* FGRAPH_FRAME_OFFSET: Size in long words of the meta data frame
@@ -156,6 +147,13 @@ get_bitmap_bits(struct task_struct *t, int offset)
return (t->ret_stack[offset] >> FGRAPH_INDEX_SHIFT) & FGRAPH_INDEX_MASK;
}
+/* For BITMAP type: set the bits in the bitmap bitmask at @offset on ret_stack */
+static inline void
+set_bitmap_bits(struct task_struct *t, int offset, unsigned long bitmap)
+{
+ t->ret_stack[offset] |= (bitmap << FGRAPH_INDEX_SHIFT);
+}
+
/* Write the bitmap to the ret_stack at @offset (does index, offset and bitmask) */
static inline void
set_bitmap(struct task_struct *t, int offset, unsigned long bitmap)
@@ -382,7 +380,8 @@ int function_graph_enter(unsigned long ret, unsigned long func,
if (gops == &fgraph_stub)
continue;
- if (gops->entryfunc(&trace, gops))
+ if (ftrace_ops_test(&gops->ops, func, NULL) &&
+ gops->entryfunc(&trace, gops))
bitmap |= BIT(i);
}
@@ -665,16 +664,28 @@ unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
static struct ftrace_ops graph_ops = {
.func = ftrace_graph_func,
- .flags = FTRACE_OPS_FL_INITIALIZED |
- FTRACE_OPS_FL_PID |
- FTRACE_OPS_GRAPH_STUB,
+ .flags = FTRACE_OPS_GRAPH_STUB,
#ifdef FTRACE_GRAPH_TRAMP_ADDR
.trampoline = FTRACE_GRAPH_TRAMP_ADDR,
/* trampoline_size is only needed for dynamically allocated tramps */
#endif
- ASSIGN_OPS_HASH(graph_ops, &global_ops.local_hash)
};
+void fgraph_init_ops(struct ftrace_ops *dst_ops,
+ struct ftrace_ops *src_ops)
+{
+ dst_ops->flags = FTRACE_OPS_FL_PID | FTRACE_OPS_GRAPH_STUB;
+
+#ifdef CONFIG_DYNAMIC_FTRACE
+ if (src_ops) {
+ dst_ops->func_hash = &src_ops->local_hash;
+ mutex_init(&dst_ops->local_hash.regex_lock);
+ INIT_LIST_HEAD(&dst_ops->subop_list);
+ dst_ops->flags |= FTRACE_OPS_FL_INITIALIZED;
+ }
+#endif
+}
+
void ftrace_graph_sleep_time_control(bool enable)
{
fgraph_sleep_time = enable;
@@ -877,6 +888,7 @@ static int start_graph_tracing(void)
int register_ftrace_graph(struct fgraph_ops *gops)
{
+ int command = 0;
int ret = 0;
int i;
@@ -894,7 +906,7 @@ int register_ftrace_graph(struct fgraph_ops *gops)
break;
}
if (i >= FGRAPH_ARRAY_SIZE) {
- ret = -EBUSY;
+ ret = -ENOSPC;
goto out;
}
@@ -908,18 +920,22 @@ int register_ftrace_graph(struct fgraph_ops *gops)
if (ftrace_graph_active == 1) {
register_pm_notifier(&ftrace_suspend_notifier);
ret = start_graph_tracing();
- if (ret) {
- ftrace_graph_active--;
- goto out;
- }
+ if (ret)
+ goto error;
/*
* Some archs just test to see if these are not
* the default function
*/
ftrace_graph_return = return_run;
ftrace_graph_entry = entry_run;
+ command = FTRACE_START_FUNC_RET;
+ }
- ret = ftrace_startup(&graph_ops, FTRACE_START_FUNC_RET);
+ ret = ftrace_startup_subops(&graph_ops, &gops->ops, command);
+error:
+ if (ret) {
+ fgraph_array[i] = &fgraph_stub;
+ ftrace_graph_active--;
}
out:
mutex_unlock(&ftrace_lock);
@@ -928,6 +944,7 @@ int register_ftrace_graph(struct fgraph_ops *gops)
void unregister_ftrace_graph(struct fgraph_ops *gops)
{
+ int command = 0;
int i;
mutex_lock(&ftrace_lock);
@@ -935,25 +952,29 @@ void unregister_ftrace_graph(struct fgraph_ops *gops)
if (unlikely(!ftrace_graph_active))
goto out;
- for (i = 0; i < fgraph_array_cnt; i++)
- if (gops == fgraph_array[i])
- break;
- if (i >= fgraph_array_cnt)
+ if (unlikely(gops->idx < 0 || gops->idx >= fgraph_array_cnt))
goto out;
- fgraph_array[i] = &fgraph_stub;
- if (i + 1 == fgraph_array_cnt) {
- for (; i >= 0; i--)
- if (fgraph_array[i] != &fgraph_stub)
- break;
+ WARN_ON_ONCE(fgraph_array[gops->idx] != gops);
+
+ fgraph_array[gops->idx] = &fgraph_stub;
+ if (gops->idx + 1 == fgraph_array_cnt) {
+ i = gops->idx;
+ while (i >= 0 && fgraph_array[i] == &fgraph_stub)
+ i--;
fgraph_array_cnt = i + 1;
}
ftrace_graph_active--;
+
+ if (!ftrace_graph_active)
+ command = FTRACE_STOP_FUNC_RET;
+
+ ftrace_shutdown_subops(&graph_ops, &gops->ops, command);
+
if (!ftrace_graph_active) {
ftrace_graph_return = ftrace_stub_graph;
ftrace_graph_entry = ftrace_graph_entry_stub;
- ftrace_shutdown(&graph_ops, FTRACE_STOP_FUNC_RET);
unregister_pm_notifier(&ftrace_suspend_notifier);
unregister_trace_sched_switch(ftrace_graph_probe_sched_switch, NULL);
}
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index cbb91b0afcc8..58e0f4bc0241 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -7811,7 +7811,7 @@ __init void ftrace_init_global_array_ops(struct trace_array *tr)
tr->ops = &global_ops;
tr->ops->private = tr;
ftrace_init_trace_array(tr);
- init_array_fgraph_ops(tr);
+ init_array_fgraph_ops(tr, tr->ops);
}
void ftrace_init_array_ops(struct trace_array *tr, ftrace_func_t func)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 9a70beb2cc46..f06b5ddd3580 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -894,8 +894,8 @@ extern int __trace_graph_entry(struct trace_array *tr,
extern void __trace_graph_return(struct trace_array *tr,
struct ftrace_graph_ret *trace,
unsigned int trace_ctx);
-extern void init_array_fgraph_ops(struct trace_array *tr);
-extern int allocate_fgraph_ops(struct trace_array *tr);
+extern void init_array_fgraph_ops(struct trace_array *tr, struct ftrace_ops *ops);
+extern int allocate_fgraph_ops(struct trace_array *tr, struct ftrace_ops *ops);
extern void free_fgraph_ops(struct trace_array *tr);
#ifdef CONFIG_DYNAMIC_FTRACE
@@ -1003,18 +1003,19 @@ static inline bool ftrace_graph_ignore_func(struct ftrace_graph_ent *trace)
(fgraph_max_depth && trace->depth >= fgraph_max_depth);
}
+void fgraph_init_ops(struct ftrace_ops *dst_ops,
+ struct ftrace_ops *src_ops);
+
#else /* CONFIG_FUNCTION_GRAPH_TRACER */
static inline enum print_line_t
print_graph_function_flags(struct trace_iterator *iter, u32 flags)
{
return TRACE_TYPE_UNHANDLED;
}
-static inline void init_array_fgraph_ops(struct trace_array *tr) { }
-static inline int allocate_fgraph_ops(struct trace_array *tr)
-{
- return 0;
-}
static inline void free_fgraph_ops(struct trace_array *tr) { }
+/* ftrace_ops may not be defined */
+#define init_array_fgraph_ops(tr, ops) do { } while (0)
+#define allocate_fgraph_ops(tr, ops) ({ 0; })
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
extern struct list_head ftrace_pids;
diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
index 8e8da0d0ee52..13bf2415245d 100644
--- a/kernel/trace/trace_functions.c
+++ b/kernel/trace/trace_functions.c
@@ -91,7 +91,7 @@ int ftrace_create_function_files(struct trace_array *tr,
if (!tr->ops)
return -EINVAL;
- ret = allocate_fgraph_ops(tr);
+ ret = allocate_fgraph_ops(tr, tr->ops);
if (ret) {
kfree(tr->ops);
return ret;
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 9ccc904a7703..7f30652f0e97 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -288,7 +288,7 @@ static struct fgraph_ops funcgraph_ops = {
.retfunc = &trace_graph_return,
};
-int allocate_fgraph_ops(struct trace_array *tr)
+int allocate_fgraph_ops(struct trace_array *tr, struct ftrace_ops *ops)
{
struct fgraph_ops *gops;
@@ -301,6 +301,9 @@ int allocate_fgraph_ops(struct trace_array *tr)
tr->gops = gops;
gops->private = tr;
+
+ fgraph_init_ops(&gops->ops, ops);
+
return 0;
}
@@ -309,10 +312,11 @@ void free_fgraph_ops(struct trace_array *tr)
kfree(tr->gops);
}
-__init void init_array_fgraph_ops(struct trace_array *tr)
+__init void init_array_fgraph_ops(struct trace_array *tr, struct ftrace_ops *ops)
{
tr->gops = &funcgraph_ops;
funcgraph_ops.private = tr;
+ fgraph_init_ops(&tr->gops->ops, ops);
}
static int graph_trace_init(struct trace_array *tr)
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 13/27] function_graph: Add pid tracing back to function graph tracer
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (11 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 12/27] function_graph: Have the instances use their own ftrace_ops for filtering Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-06 0:08 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 14/27] function_graph: Use a simple LRU for fgraph_array index number Steven Rostedt
` (15 subsequent siblings)
28 siblings, 1 reply; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
Now that the function_graph has a main callback that handles the function
graph subops tracing, it no longer honors the pid filtering of ftrace. Add
back this logic in the function_graph code to update the gops callback for
the entry function to test if it should trace the current task or not.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/ftrace.h | 2 ++
kernel/trace/fgraph.c | 40 ++++++++++++++++++++++++++++++++++
kernel/trace/ftrace.c | 5 +++--
kernel/trace/ftrace_internal.h | 2 ++
4 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 8f865689e868..e31ec8516de1 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1040,6 +1040,7 @@ typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *,
struct fgraph_ops *); /* entry */
extern int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace, struct fgraph_ops *gops);
+bool ftrace_pids_enabled(struct ftrace_ops *ops);
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
@@ -1048,6 +1049,7 @@ struct fgraph_ops {
trace_func_graph_ret_t retfunc;
struct ftrace_ops ops; /* for the hash lists */
void *private;
+ trace_func_graph_ent_t saved_func;
int idx;
};
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 3ef6db53c0bf..30bed20c655f 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -854,6 +854,41 @@ void ftrace_graph_exit_task(struct task_struct *t)
kfree(ret_stack);
}
+static int fgraph_pid_func(struct ftrace_graph_ent *trace,
+ struct fgraph_ops *gops)
+{
+ struct trace_array *tr = gops->ops.private;
+ int pid;
+
+ if (tr) {
+ pid = this_cpu_read(tr->array_buffer.data->ftrace_ignore_pid);
+ if (pid == FTRACE_PID_IGNORE)
+ return 0;
+ if (pid != FTRACE_PID_TRACE &&
+ pid != current->pid)
+ return 0;
+ }
+
+ return gops->saved_func(trace, gops);
+}
+
+void fgraph_update_pid_func(void)
+{
+ struct fgraph_ops *gops;
+ struct ftrace_ops *op;
+
+ if (!(graph_ops.flags & FTRACE_OPS_FL_INITIALIZED))
+ return;
+
+ list_for_each_entry(op, &graph_ops.subop_list, list) {
+ if (op->flags & FTRACE_OPS_FL_PID) {
+ gops = container_of(op, struct fgraph_ops, ops);
+ gops->entryfunc = ftrace_pids_enabled(op) ?
+ fgraph_pid_func : gops->saved_func;
+ }
+ }
+}
+
/* Allocate a return stack for each task */
static int start_graph_tracing(void)
{
@@ -931,11 +966,15 @@ int register_ftrace_graph(struct fgraph_ops *gops)
command = FTRACE_START_FUNC_RET;
}
+ /* Always save the function, and reset at unregistering */
+ gops->saved_func = gops->entryfunc;
+
ret = ftrace_startup_subops(&graph_ops, &gops->ops, command);
error:
if (ret) {
fgraph_array[i] = &fgraph_stub;
ftrace_graph_active--;
+ gops->saved_func = NULL;
}
out:
mutex_unlock(&ftrace_lock);
@@ -979,5 +1018,6 @@ void unregister_ftrace_graph(struct fgraph_ops *gops)
unregister_trace_sched_switch(ftrace_graph_probe_sched_switch, NULL);
}
out:
+ gops->saved_func = NULL;
mutex_unlock(&ftrace_lock);
}
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 58e0f4bc0241..da7e6abf48b4 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -100,7 +100,7 @@ struct ftrace_ops *function_trace_op __read_mostly = &ftrace_list_end;
/* What to set function_trace_op to */
static struct ftrace_ops *set_function_trace_op;
-static bool ftrace_pids_enabled(struct ftrace_ops *ops)
+bool ftrace_pids_enabled(struct ftrace_ops *ops)
{
struct trace_array *tr;
@@ -402,10 +402,11 @@ static void ftrace_update_pid_func(void)
if (op->flags & FTRACE_OPS_FL_PID) {
op->func = ftrace_pids_enabled(op) ?
ftrace_pid_func : op->saved_func;
- ftrace_update_trampoline(op);
}
} while_for_each_ftrace_op(op);
+ fgraph_update_pid_func();
+
update_ftrace_function();
}
diff --git a/kernel/trace/ftrace_internal.h b/kernel/trace/ftrace_internal.h
index cdfd12c44ab4..bfba10c2fcf1 100644
--- a/kernel/trace/ftrace_internal.h
+++ b/kernel/trace/ftrace_internal.h
@@ -43,8 +43,10 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip, void *regs)
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
extern int ftrace_graph_active;
+extern void fgraph_update_pid_func(void);
#else /* !CONFIG_FUNCTION_GRAPH_TRACER */
# define ftrace_graph_active 0
+static inline void fgraph_update_pid_func(void) {}
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
#else /* !CONFIG_FUNCTION_TRACER */
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v3 13/27] function_graph: Add pid tracing back to function graph tracer
2024-06-03 19:07 ` [PATCH v3 13/27] function_graph: Add pid tracing back to function graph tracer Steven Rostedt
@ 2024-06-06 0:08 ` Steven Rostedt
0 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-06 0:08 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
On Mon, 03 Jun 2024 15:07:17 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> +++ b/kernel/trace/ftrace.c
> @@ -100,7 +100,7 @@ struct ftrace_ops *function_trace_op __read_mostly = &ftrace_list_end;
> /* What to set function_trace_op to */
> static struct ftrace_ops *set_function_trace_op;
>
> -static bool ftrace_pids_enabled(struct ftrace_ops *ops)
> +bool ftrace_pids_enabled(struct ftrace_ops *ops)
> {
> struct trace_array *tr;
>
> @@ -402,10 +402,11 @@ static void ftrace_update_pid_func(void)
> if (op->flags & FTRACE_OPS_FL_PID) {
> op->func = ftrace_pids_enabled(op) ?
> ftrace_pid_func : op->saved_func;
> - ftrace_update_trampoline(op);
Bah, this patch accidentally removed the above function and broke pid
tracing. Hmm, not sure why this still passed the tests. Will investigate.
-- Steve
> }
> } while_for_each_ftrace_op(op);
>
> + fgraph_update_pid_func();
> +
> update_ftrace_function();
> }
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3 14/27] function_graph: Use a simple LRU for fgraph_array index number
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (12 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 13/27] function_graph: Add pid tracing back to function graph tracer Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 15/27] function_graph: Add "task variables" per task for fgraph_ops Steven Rostedt
` (14 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
Since the fgraph_array index is used for the bitmap on the shadow
stack, it may leave some entries after a function_graph instance is
removed. Thus if another instance reuses the fgraph_array index soon
after releasing it, the fgraph may confuse to call the newer callback
for the entries which are pushed by the older instance.
To avoid reusing the fgraph_array index soon after releasing, introduce
a simple LRU table for managing the index number. This will reduce the
possibility of this confusion.
Link: https://lore.kernel.org/linux-trace-kernel/171509103267.162236.6885097397289135378.stgit@devnote2
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/fgraph.c | 71 ++++++++++++++++++++++++++++++-------------
1 file changed, 50 insertions(+), 21 deletions(-)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 30bed20c655f..7fd9b03bd170 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -124,10 +124,48 @@ enum {
DEFINE_STATIC_KEY_FALSE(kill_ftrace_graph);
int ftrace_graph_active;
-static int fgraph_array_cnt;
-
static struct fgraph_ops *fgraph_array[FGRAPH_ARRAY_SIZE];
+/* LRU index table for fgraph_array */
+static int fgraph_lru_table[FGRAPH_ARRAY_SIZE];
+static int fgraph_lru_next;
+static int fgraph_lru_last;
+
+/* Initialize fgraph_lru_table with unused index */
+static void fgraph_lru_init(void)
+{
+ int i;
+
+ for (i = 0; i < FGRAPH_ARRAY_SIZE; i++)
+ fgraph_lru_table[i] = i;
+}
+
+/* Release the used index to the LRU table */
+static int fgraph_lru_release_index(int idx)
+{
+ if (idx < 0 || idx >= FGRAPH_ARRAY_SIZE ||
+ WARN_ON_ONCE(fgraph_lru_table[fgraph_lru_last] != -1))
+ return -1;
+
+ fgraph_lru_table[fgraph_lru_last] = idx;
+ fgraph_lru_last = (fgraph_lru_last + 1) % FGRAPH_ARRAY_SIZE;
+ return 0;
+}
+
+/* Allocate a new index from LRU table */
+static int fgraph_lru_alloc_index(void)
+{
+ int idx = fgraph_lru_table[fgraph_lru_next];
+
+ /* No id is available */
+ if (idx == -1)
+ return -1;
+
+ fgraph_lru_table[fgraph_lru_next] = -1;
+ fgraph_lru_next = (fgraph_lru_next + 1) % FGRAPH_ARRAY_SIZE;
+ return idx;
+}
+
/* Get the FRAME_OFFSET from the word from the @offset on ret_stack */
static inline int get_frame_offset(struct task_struct *t, int offset)
{
@@ -374,7 +412,7 @@ int function_graph_enter(unsigned long ret, unsigned long func,
if (offset < 0)
goto out;
- for (i = 0; i < fgraph_array_cnt; i++) {
+ for (i = 0; i < FGRAPH_ARRAY_SIZE; i++) {
struct fgraph_ops *gops = fgraph_array[i];
if (gops == &fgraph_stub)
@@ -925,7 +963,7 @@ int register_ftrace_graph(struct fgraph_ops *gops)
{
int command = 0;
int ret = 0;
- int i;
+ int i = -1;
mutex_lock(&ftrace_lock);
@@ -933,21 +971,16 @@ int register_ftrace_graph(struct fgraph_ops *gops)
/* The array must always have real data on it */
for (i = 0; i < FGRAPH_ARRAY_SIZE; i++)
fgraph_array[i] = &fgraph_stub;
+ fgraph_lru_init();
}
- /* Look for an available spot */
- for (i = 0; i < FGRAPH_ARRAY_SIZE; i++) {
- if (fgraph_array[i] == &fgraph_stub)
- break;
- }
- if (i >= FGRAPH_ARRAY_SIZE) {
+ i = fgraph_lru_alloc_index();
+ if (i < 0 || WARN_ON_ONCE(fgraph_array[i] != &fgraph_stub)) {
ret = -ENOSPC;
goto out;
}
fgraph_array[i] = gops;
- if (i + 1 > fgraph_array_cnt)
- fgraph_array_cnt = i + 1;
gops->idx = i;
ftrace_graph_active++;
@@ -975,6 +1008,7 @@ int register_ftrace_graph(struct fgraph_ops *gops)
fgraph_array[i] = &fgraph_stub;
ftrace_graph_active--;
gops->saved_func = NULL;
+ fgraph_lru_release_index(i);
}
out:
mutex_unlock(&ftrace_lock);
@@ -984,25 +1018,20 @@ int register_ftrace_graph(struct fgraph_ops *gops)
void unregister_ftrace_graph(struct fgraph_ops *gops)
{
int command = 0;
- int i;
mutex_lock(&ftrace_lock);
if (unlikely(!ftrace_graph_active))
goto out;
- if (unlikely(gops->idx < 0 || gops->idx >= fgraph_array_cnt))
+ if (unlikely(gops->idx < 0 || gops->idx >= FGRAPH_ARRAY_SIZE ||
+ fgraph_array[gops->idx] != gops))
goto out;
- WARN_ON_ONCE(fgraph_array[gops->idx] != gops);
+ if (fgraph_lru_release_index(gops->idx) < 0)
+ goto out;
fgraph_array[gops->idx] = &fgraph_stub;
- if (gops->idx + 1 == fgraph_array_cnt) {
- i = gops->idx;
- while (i >= 0 && fgraph_array[i] == &fgraph_stub)
- i--;
- fgraph_array_cnt = i + 1;
- }
ftrace_graph_active--;
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 15/27] function_graph: Add "task variables" per task for fgraph_ops
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (13 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 14/27] function_graph: Use a simple LRU for fgraph_array index number Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 16/27] function_graph: Move set_graph_function tests to shadow stack global var Steven Rostedt
` (13 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Add a "task variables" array on the tasks shadow ret_stack that is the
size of longs for each possible registered fgraph_ops. That's a total
of 16, taking up 8 * 16 = 128 bytes (out of a page size 4k).
This will allow for fgraph_ops to do specific features on a per task basis
having a way to maintain state for each task.
Co-developed with Masami Hiramatsu:
Link: https://lore.kernel.org/linux-trace-kernel/171509104383.162236.12239656156685718550.stgit@devnote2
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/ftrace.h | 1 +
kernel/trace/fgraph.c | 74 +++++++++++++++++++++++++++++++++++++++++-
2 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index e31ec8516de1..82cfc8e09cfd 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1089,6 +1089,7 @@ ftrace_graph_get_ret_stack(struct task_struct *task, int skip);
unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
unsigned long ret, unsigned long *retp);
+unsigned long *fgraph_get_task_var(struct fgraph_ops *gops);
/*
* Sometimes we don't want to trace a function with the function
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 7fd9b03bd170..baa08249ffd5 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -54,6 +54,10 @@
* on the return of the function being traced, this is what will be on the
* task's shadow ret_stack: (the stack grows upward)
*
+ * ret_stack[SHADOW_STACK_OFFSET]
+ * | SHADOW_STACK_TASK_VARS(ret_stack)[15] |
+ * ...
+ * | SHADOW_STACK_TASK_VARS(ret_stack)[0] |
* ret_stack[SHADOW_STACK_MAX_OFFSET]
* ...
* | | <- task->curr_ret_stack
@@ -116,11 +120,19 @@ enum {
#define SHADOW_STACK_SIZE (PAGE_SIZE)
#define SHADOW_STACK_OFFSET (SHADOW_STACK_SIZE / sizeof(long))
/* Leave on a buffer at the end */
-#define SHADOW_STACK_MAX_OFFSET (SHADOW_STACK_OFFSET - (FGRAPH_FRAME_OFFSET + 1))
+#define SHADOW_STACK_MAX_OFFSET \
+ (SHADOW_STACK_OFFSET - (FGRAPH_FRAME_OFFSET + 1 + FGRAPH_ARRAY_SIZE))
/* RET_STACK(): Return the frame from a given @offset from task @t */
#define RET_STACK(t, offset) ((struct ftrace_ret_stack *)(&(t)->ret_stack[offset]))
+/*
+ * Each fgraph_ops has a reservered unsigned long at the end (top) of the
+ * ret_stack to store task specific state.
+ */
+#define SHADOW_STACK_TASK_VARS(ret_stack) \
+ ((unsigned long *)(&(ret_stack)[SHADOW_STACK_OFFSET - FGRAPH_ARRAY_SIZE]))
+
DEFINE_STATIC_KEY_FALSE(kill_ftrace_graph);
int ftrace_graph_active;
@@ -211,6 +223,44 @@ static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops)
{
}
+static void ret_stack_set_task_var(struct task_struct *t, int idx, long val)
+{
+ unsigned long *gvals = SHADOW_STACK_TASK_VARS(t->ret_stack);
+
+ gvals[idx] = val;
+}
+
+static unsigned long *
+ret_stack_get_task_var(struct task_struct *t, int idx)
+{
+ unsigned long *gvals = SHADOW_STACK_TASK_VARS(t->ret_stack);
+
+ return &gvals[idx];
+}
+
+static void ret_stack_init_task_vars(unsigned long *ret_stack)
+{
+ unsigned long *gvals = SHADOW_STACK_TASK_VARS(ret_stack);
+
+ memset(gvals, 0, sizeof(*gvals) * FGRAPH_ARRAY_SIZE);
+}
+
+/**
+ * fgraph_get_task_var - retrieve a task specific state variable
+ * @gops: The ftrace_ops that owns the task specific variable
+ *
+ * Every registered fgraph_ops has a task state variable
+ * reserved on the task's ret_stack. This function returns the
+ * address to that variable.
+ *
+ * Returns the address to the fgraph_ops @gops tasks specific
+ * unsigned long variable.
+ */
+unsigned long *fgraph_get_task_var(struct fgraph_ops *gops)
+{
+ return ret_stack_get_task_var(current, gops->idx);
+}
+
/*
* @offset: The offset into @t->ret_stack to find the ret_stack entry
* @frame_offset: Where to place the offset into @t->ret_stack of that entry
@@ -766,6 +816,7 @@ static int alloc_retstack_tasklist(unsigned long **ret_stack_list)
if (t->ret_stack == NULL) {
atomic_set(&t->trace_overrun, 0);
+ ret_stack_init_task_vars(ret_stack_list[start]);
t->curr_ret_stack = 0;
t->curr_ret_depth = -1;
/* Make sure the tasks see the 0 first: */
@@ -826,6 +877,7 @@ static void
graph_init_task(struct task_struct *t, unsigned long *ret_stack)
{
atomic_set(&t->trace_overrun, 0);
+ ret_stack_init_task_vars(ret_stack);
t->ftrace_timestamp = 0;
t->curr_ret_stack = 0;
t->curr_ret_depth = -1;
@@ -959,6 +1011,24 @@ static int start_graph_tracing(void)
return ret;
}
+static void init_task_vars(int idx)
+{
+ struct task_struct *g, *t;
+ int cpu;
+
+ for_each_online_cpu(cpu) {
+ if (idle_task(cpu)->ret_stack)
+ ret_stack_set_task_var(idle_task(cpu), idx, 0);
+ }
+
+ read_lock(&tasklist_lock);
+ for_each_process_thread(g, t) {
+ if (t->ret_stack)
+ ret_stack_set_task_var(t, idx, 0);
+ }
+ read_unlock(&tasklist_lock);
+}
+
int register_ftrace_graph(struct fgraph_ops *gops)
{
int command = 0;
@@ -997,6 +1067,8 @@ int register_ftrace_graph(struct fgraph_ops *gops)
ftrace_graph_return = return_run;
ftrace_graph_entry = entry_run;
command = FTRACE_START_FUNC_RET;
+ } else {
+ init_task_vars(gops->idx);
}
/* Always save the function, and reset at unregistering */
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 16/27] function_graph: Move set_graph_function tests to shadow stack global var
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (14 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 15/27] function_graph: Add "task variables" per task for fgraph_ops Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 17/27] function_graph: Move graph depth stored data " Steven Rostedt
` (12 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The use of the task->trace_recursion for the logic used for the
set_graph_function was a bit of an abuse of that variable. Now that there
exists global vars that are per stack for registered graph traces, use that
instead.
Link: https://lore.kernel.org/linux-trace-kernel/171509105520.162236.10339831553995971290.stgit@devnote2
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/trace_recursion.h | 5 +----
kernel/trace/trace.h | 32 ++++++++++++++++++----------
kernel/trace/trace_functions_graph.c | 6 +++---
kernel/trace/trace_irqsoff.c | 4 ++--
kernel/trace/trace_sched_wakeup.c | 4 ++--
5 files changed, 29 insertions(+), 22 deletions(-)
diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h
index 24ea8ac049b4..02e6afc6d7fe 100644
--- a/include/linux/trace_recursion.h
+++ b/include/linux/trace_recursion.h
@@ -44,9 +44,6 @@ enum {
*/
TRACE_IRQ_BIT,
- /* Set if the function is in the set_graph_function file */
- TRACE_GRAPH_BIT,
-
/*
* In the very unlikely case that an interrupt came in
* at a start of graph tracing, and we want to trace
@@ -60,7 +57,7 @@ enum {
* that preempted a softirq start of a function that
* preempted normal context!!!! Luckily, it can't be
* greater than 3, so the next two bits are a mask
- * of what the depth is when we set TRACE_GRAPH_BIT
+ * of what the depth is when we set TRACE_GRAPH_FL
*/
TRACE_GRAPH_DEPTH_START_BIT,
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index f06b5ddd3580..73919129e57c 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -898,11 +898,16 @@ extern void init_array_fgraph_ops(struct trace_array *tr, struct ftrace_ops *ops
extern int allocate_fgraph_ops(struct trace_array *tr, struct ftrace_ops *ops);
extern void free_fgraph_ops(struct trace_array *tr);
+enum {
+ TRACE_GRAPH_FL = 1,
+};
+
#ifdef CONFIG_DYNAMIC_FTRACE
extern struct ftrace_hash __rcu *ftrace_graph_hash;
extern struct ftrace_hash __rcu *ftrace_graph_notrace_hash;
-static inline int ftrace_graph_addr(struct ftrace_graph_ent *trace)
+static inline int
+ftrace_graph_addr(unsigned long *task_var, struct ftrace_graph_ent *trace)
{
unsigned long addr = trace->func;
int ret = 0;
@@ -924,12 +929,11 @@ static inline int ftrace_graph_addr(struct ftrace_graph_ent *trace)
}
if (ftrace_lookup_ip(hash, addr)) {
-
/*
* This needs to be cleared on the return functions
* when the depth is zero.
*/
- trace_recursion_set(TRACE_GRAPH_BIT);
+ *task_var |= TRACE_GRAPH_FL;
trace_recursion_set_depth(trace->depth);
/*
@@ -949,11 +953,14 @@ static inline int ftrace_graph_addr(struct ftrace_graph_ent *trace)
return ret;
}
-static inline void ftrace_graph_addr_finish(struct ftrace_graph_ret *trace)
+static inline void
+ftrace_graph_addr_finish(struct fgraph_ops *gops, struct ftrace_graph_ret *trace)
{
- if (trace_recursion_test(TRACE_GRAPH_BIT) &&
+ unsigned long *task_var = fgraph_get_task_var(gops);
+
+ if ((*task_var & TRACE_GRAPH_FL) &&
trace->depth == trace_recursion_depth())
- trace_recursion_clear(TRACE_GRAPH_BIT);
+ *task_var &= ~TRACE_GRAPH_FL;
}
static inline int ftrace_graph_notrace_addr(unsigned long addr)
@@ -979,7 +986,7 @@ static inline int ftrace_graph_notrace_addr(unsigned long addr)
return ret;
}
#else
-static inline int ftrace_graph_addr(struct ftrace_graph_ent *trace)
+static inline int ftrace_graph_addr(unsigned long *task_var, struct ftrace_graph_ent *trace)
{
return 1;
}
@@ -988,17 +995,20 @@ static inline int ftrace_graph_notrace_addr(unsigned long addr)
{
return 0;
}
-static inline void ftrace_graph_addr_finish(struct ftrace_graph_ret *trace)
+static inline void ftrace_graph_addr_finish(struct fgraph_ops *gops, struct ftrace_graph_ret *trace)
{ }
#endif /* CONFIG_DYNAMIC_FTRACE */
extern unsigned int fgraph_max_depth;
-static inline bool ftrace_graph_ignore_func(struct ftrace_graph_ent *trace)
+static inline bool
+ftrace_graph_ignore_func(struct fgraph_ops *gops, struct ftrace_graph_ent *trace)
{
+ unsigned long *task_var = fgraph_get_task_var(gops);
+
/* trace it when it is-nested-in or is a function enabled. */
- return !(trace_recursion_test(TRACE_GRAPH_BIT) ||
- ftrace_graph_addr(trace)) ||
+ return !((*task_var & TRACE_GRAPH_FL) ||
+ ftrace_graph_addr(task_var, trace)) ||
(trace->depth < 0) ||
(fgraph_max_depth && trace->depth >= fgraph_max_depth);
}
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 7f30652f0e97..66cce73e94f8 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -160,7 +160,7 @@ int trace_graph_entry(struct ftrace_graph_ent *trace,
if (!ftrace_trace_task(tr))
return 0;
- if (ftrace_graph_ignore_func(trace))
+ if (ftrace_graph_ignore_func(gops, trace))
return 0;
if (ftrace_graph_ignore_irqs())
@@ -247,7 +247,7 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
long disabled;
int cpu;
- ftrace_graph_addr_finish(trace);
+ ftrace_graph_addr_finish(gops, trace);
if (trace_recursion_test(TRACE_GRAPH_NOTRACE_BIT)) {
trace_recursion_clear(TRACE_GRAPH_NOTRACE_BIT);
@@ -269,7 +269,7 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
struct fgraph_ops *gops)
{
- ftrace_graph_addr_finish(trace);
+ ftrace_graph_addr_finish(gops, trace);
if (trace_recursion_test(TRACE_GRAPH_NOTRACE_BIT)) {
trace_recursion_clear(TRACE_GRAPH_NOTRACE_BIT);
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index 5478f4c4f708..fce064e20570 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -184,7 +184,7 @@ static int irqsoff_graph_entry(struct ftrace_graph_ent *trace,
unsigned int trace_ctx;
int ret;
- if (ftrace_graph_ignore_func(trace))
+ if (ftrace_graph_ignore_func(gops, trace))
return 0;
/*
* Do not trace a function if it's filtered by set_graph_notrace.
@@ -214,7 +214,7 @@ static void irqsoff_graph_return(struct ftrace_graph_ret *trace,
unsigned long flags;
unsigned int trace_ctx;
- ftrace_graph_addr_finish(trace);
+ ftrace_graph_addr_finish(gops, trace);
if (!func_prolog_dec(tr, &data, &flags))
return;
diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
index 49bcc812652c..130ca7e7787e 100644
--- a/kernel/trace/trace_sched_wakeup.c
+++ b/kernel/trace/trace_sched_wakeup.c
@@ -120,7 +120,7 @@ static int wakeup_graph_entry(struct ftrace_graph_ent *trace,
unsigned int trace_ctx;
int ret = 0;
- if (ftrace_graph_ignore_func(trace))
+ if (ftrace_graph_ignore_func(gops, trace))
return 0;
/*
* Do not trace a function if it's filtered by set_graph_notrace.
@@ -149,7 +149,7 @@ static void wakeup_graph_return(struct ftrace_graph_ret *trace,
struct trace_array_cpu *data;
unsigned int trace_ctx;
- ftrace_graph_addr_finish(trace);
+ ftrace_graph_addr_finish(gops, trace);
if (!func_prolog_preempt_disable(tr, &data, &trace_ctx))
return;
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 17/27] function_graph: Move graph depth stored data to shadow stack global var
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (15 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 16/27] function_graph: Move set_graph_function tests to shadow stack global var Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 18/27] function_graph: Move graph notrace bit " Steven Rostedt
` (11 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The use of the task->trace_recursion for the logic used for the function
graph depth was a bit of an abuse of that variable. Now that there
exists global vars that are per stack for registered graph traces, use that
instead.
Link: https://lore.kernel.org/linux-trace-kernel/171509106728.162236.2398372644430125344.stgit@devnote2
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/trace_recursion.h | 29 ----------------------------
kernel/trace/trace.h | 34 +++++++++++++++++++++++++++++++--
2 files changed, 32 insertions(+), 31 deletions(-)
diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h
index 02e6afc6d7fe..fdfb6f66718a 100644
--- a/include/linux/trace_recursion.h
+++ b/include/linux/trace_recursion.h
@@ -44,25 +44,6 @@ enum {
*/
TRACE_IRQ_BIT,
- /*
- * In the very unlikely case that an interrupt came in
- * at a start of graph tracing, and we want to trace
- * the function in that interrupt, the depth can be greater
- * than zero, because of the preempted start of a previous
- * trace. In an even more unlikely case, depth could be 2
- * if a softirq interrupted the start of graph tracing,
- * followed by an interrupt preempting a start of graph
- * tracing in the softirq, and depth can even be 3
- * if an NMI came in at the start of an interrupt function
- * that preempted a softirq start of a function that
- * preempted normal context!!!! Luckily, it can't be
- * greater than 3, so the next two bits are a mask
- * of what the depth is when we set TRACE_GRAPH_FL
- */
-
- TRACE_GRAPH_DEPTH_START_BIT,
- TRACE_GRAPH_DEPTH_END_BIT,
-
/*
* To implement set_graph_notrace, if this bit is set, we ignore
* function graph tracing of called functions, until the return
@@ -78,16 +59,6 @@ enum {
#define trace_recursion_clear(bit) do { (current)->trace_recursion &= ~(1<<(bit)); } while (0)
#define trace_recursion_test(bit) ((current)->trace_recursion & (1<<(bit)))
-#define trace_recursion_depth() \
- (((current)->trace_recursion >> TRACE_GRAPH_DEPTH_START_BIT) & 3)
-#define trace_recursion_set_depth(depth) \
- do { \
- current->trace_recursion &= \
- ~(3 << TRACE_GRAPH_DEPTH_START_BIT); \
- current->trace_recursion |= \
- ((depth) & 3) << TRACE_GRAPH_DEPTH_START_BIT; \
- } while (0)
-
#define TRACE_CONTEXT_BITS 4
#define TRACE_FTRACE_START TRACE_FTRACE_BIT
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 73919129e57c..82d879dc63ff 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -900,8 +900,38 @@ extern void free_fgraph_ops(struct trace_array *tr);
enum {
TRACE_GRAPH_FL = 1,
+
+ /*
+ * In the very unlikely case that an interrupt came in
+ * at a start of graph tracing, and we want to trace
+ * the function in that interrupt, the depth can be greater
+ * than zero, because of the preempted start of a previous
+ * trace. In an even more unlikely case, depth could be 2
+ * if a softirq interrupted the start of graph tracing,
+ * followed by an interrupt preempting a start of graph
+ * tracing in the softirq, and depth can even be 3
+ * if an NMI came in at the start of an interrupt function
+ * that preempted a softirq start of a function that
+ * preempted normal context!!!! Luckily, it can't be
+ * greater than 3, so the next two bits are a mask
+ * of what the depth is when we set TRACE_GRAPH_FL
+ */
+
+ TRACE_GRAPH_DEPTH_START_BIT,
+ TRACE_GRAPH_DEPTH_END_BIT,
};
+static inline unsigned long ftrace_graph_depth(unsigned long *task_var)
+{
+ return (*task_var >> TRACE_GRAPH_DEPTH_START_BIT) & 3;
+}
+
+static inline void ftrace_graph_set_depth(unsigned long *task_var, int depth)
+{
+ *task_var &= ~(3 << TRACE_GRAPH_DEPTH_START_BIT);
+ *task_var |= (depth & 3) << TRACE_GRAPH_DEPTH_START_BIT;
+}
+
#ifdef CONFIG_DYNAMIC_FTRACE
extern struct ftrace_hash __rcu *ftrace_graph_hash;
extern struct ftrace_hash __rcu *ftrace_graph_notrace_hash;
@@ -934,7 +964,7 @@ ftrace_graph_addr(unsigned long *task_var, struct ftrace_graph_ent *trace)
* when the depth is zero.
*/
*task_var |= TRACE_GRAPH_FL;
- trace_recursion_set_depth(trace->depth);
+ ftrace_graph_set_depth(task_var, trace->depth);
/*
* If no irqs are to be traced, but a set_graph_function
@@ -959,7 +989,7 @@ ftrace_graph_addr_finish(struct fgraph_ops *gops, struct ftrace_graph_ret *trace
unsigned long *task_var = fgraph_get_task_var(gops);
if ((*task_var & TRACE_GRAPH_FL) &&
- trace->depth == trace_recursion_depth())
+ trace->depth == ftrace_graph_depth(task_var))
*task_var &= ~TRACE_GRAPH_FL;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 18/27] function_graph: Move graph notrace bit to shadow stack global var
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (16 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 17/27] function_graph: Move graph depth stored data " Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 19/27] function_graph: Implement fgraph_reserve_data() and fgraph_retrieve_data() Steven Rostedt
` (10 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The use of the task->trace_recursion for the logic used for the function
graph no-trace was a bit of an abuse of that variable. Now that there
exists global vars that are per stack for registered graph traces, use
that instead.
Link: https://lore.kernel.org/linux-trace-kernel/171509107907.162236.6564679266777519065.stgit@devnote2
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/trace_recursion.h | 7 -------
kernel/trace/trace.h | 9 +++++++++
kernel/trace/trace_functions_graph.c | 10 ++++++----
3 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h
index fdfb6f66718a..ae04054a1be3 100644
--- a/include/linux/trace_recursion.h
+++ b/include/linux/trace_recursion.h
@@ -44,13 +44,6 @@ enum {
*/
TRACE_IRQ_BIT,
- /*
- * To implement set_graph_notrace, if this bit is set, we ignore
- * function graph tracing of called functions, until the return
- * function is called to clear it.
- */
- TRACE_GRAPH_NOTRACE_BIT,
-
/* Used to prevent recursion recording from recursing. */
TRACE_RECORD_RECURSION_BIT,
};
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 82d879dc63ff..b37402e3f0c9 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -919,8 +919,17 @@ enum {
TRACE_GRAPH_DEPTH_START_BIT,
TRACE_GRAPH_DEPTH_END_BIT,
+
+ /*
+ * To implement set_graph_notrace, if this bit is set, we ignore
+ * function graph tracing of called functions, until the return
+ * function is called to clear it.
+ */
+ TRACE_GRAPH_NOTRACE_BIT,
};
+#define TRACE_GRAPH_NOTRACE (1 << TRACE_GRAPH_NOTRACE_BIT)
+
static inline unsigned long ftrace_graph_depth(unsigned long *task_var)
{
return (*task_var >> TRACE_GRAPH_DEPTH_START_BIT) & 3;
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 66cce73e94f8..13d0387ac6a6 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -130,6 +130,7 @@ static inline int ftrace_graph_ignore_irqs(void)
int trace_graph_entry(struct ftrace_graph_ent *trace,
struct fgraph_ops *gops)
{
+ unsigned long *task_var = fgraph_get_task_var(gops);
struct trace_array *tr = gops->private;
struct trace_array_cpu *data;
unsigned long flags;
@@ -138,7 +139,7 @@ int trace_graph_entry(struct ftrace_graph_ent *trace,
int ret;
int cpu;
- if (trace_recursion_test(TRACE_GRAPH_NOTRACE_BIT))
+ if (*task_var & TRACE_GRAPH_NOTRACE)
return 0;
/*
@@ -149,7 +150,7 @@ int trace_graph_entry(struct ftrace_graph_ent *trace,
* returning from the function.
*/
if (ftrace_graph_notrace_addr(trace->func)) {
- trace_recursion_set(TRACE_GRAPH_NOTRACE_BIT);
+ *task_var |= TRACE_GRAPH_NOTRACE_BIT;
/*
* Need to return 1 to have the return called
* that will clear the NOTRACE bit.
@@ -240,6 +241,7 @@ void __trace_graph_return(struct trace_array *tr,
void trace_graph_return(struct ftrace_graph_ret *trace,
struct fgraph_ops *gops)
{
+ unsigned long *task_var = fgraph_get_task_var(gops);
struct trace_array *tr = gops->private;
struct trace_array_cpu *data;
unsigned long flags;
@@ -249,8 +251,8 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
ftrace_graph_addr_finish(gops, trace);
- if (trace_recursion_test(TRACE_GRAPH_NOTRACE_BIT)) {
- trace_recursion_clear(TRACE_GRAPH_NOTRACE_BIT);
+ if (*task_var & TRACE_GRAPH_NOTRACE) {
+ *task_var &= ~TRACE_GRAPH_NOTRACE;
return;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 19/27] function_graph: Implement fgraph_reserve_data() and fgraph_retrieve_data()
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (17 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 18/27] function_graph: Move graph notrace bit " Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 20/27] function_graph: Add selftest for passing local variables Steven Rostedt
` (9 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Added functions that can be called by a fgraph_ops entryfunc and retfunc to
store state between the entry of the function being traced to the exit of
the same function. The fgraph_ops entryfunc() may call
fgraph_reserve_data() to store up to 32 words onto the task's shadow
ret_stack and this then can be retrieved by fgraph_retrieve_data() called
by the corresponding retfunc().
Co-developed with Masami Hiramatsu:
Link: https://lore.kernel.org/linux-trace-kernel/171509109089.162236.11372474169781184034.stgit@devnote2
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/ftrace.h | 3 +
kernel/trace/fgraph.c | 191 +++++++++++++++++++++++++++++++++++++++--
2 files changed, 186 insertions(+), 8 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 82cfc8e09cfd..9f61556a9491 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1053,6 +1053,9 @@ struct fgraph_ops {
int idx;
};
+void *fgraph_reserve_data(int idx, int size_bytes);
+void *fgraph_retrieve_data(int idx, int *size_bytes);
+
/*
* Stack of return addresses for functions
* of a thread.
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index baa08249ffd5..f207b7ae5f46 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -31,12 +31,11 @@
* holds a bitmask and a type (called "bitmap"). The bitmap is defined as:
*
* bits: 0 - 9 offset in words from the previous ftrace_ret_stack
- * Currently, this will always be set to FGRAPH_FRAME_OFFSET
- * to get to the fgraph frame.
*
* bits: 10 - 11 Type of storage
* 0 - reserved
* 1 - bitmap of fgraph_array index
+ * 2 - reserved data
*
* For type with "bitmap of fgraph_array index" (FGRAPH_TYPE_BITMAP):
* bits: 12 - 27 The bitmap of fgraph_ops fgraph_array index
@@ -49,10 +48,15 @@
* The top of the ret_stack (when not empty) will always have a reference
* word that points to the last fgraph frame that was saved.
*
+ * For reserved data:
+ * bits: 12 - 17 The size in words that is stored
+ * bits: 18 - 23 The index of fgraph_array, which shows who is stored
+ *
* That is, at the end of function_graph_enter, if the first and forth
* fgraph_ops on the fgraph_array[] (index 0 and 3) needs their retfunc called
- * on the return of the function being traced, this is what will be on the
- * task's shadow ret_stack: (the stack grows upward)
+ * on the return of the function being traced, and the forth fgraph_ops
+ * stored two words of data, this is what will be on the task's shadow
+ * ret_stack: (the stack grows upward)
*
* ret_stack[SHADOW_STACK_OFFSET]
* | SHADOW_STACK_TASK_VARS(ret_stack)[15] |
@@ -62,11 +66,21 @@
* ...
* | | <- task->curr_ret_stack
* +--------------------------------------------+
+ * | (3 << 12) | (3 << 10) | FGRAPH_FRAME_OFFSET|
+ * | *or put another way* |
+ * | (3 << FGRAPH_DATA_INDEX_SHIFT)| \ | This is for fgraph_ops[3].
+ * | ((2 - 1) << FGRAPH_DATA_SHIFT)| \ | The data size is 2 words.
+ * | (FGRAPH_TYPE_DATA << FGRAPH_TYPE_SHIFT)| \ |
+ * | (offset2:FGRAPH_FRAME_OFFSET+3) | <- the offset2 is from here
+ * +--------------------------------------------+ ( It is 4 words from the ret_stack)
+ * | STORED DATA WORD 2 |
+ * | STORED DATA WORD 1 |
+ * +--------------------------------------------+
* | (9 << 12) | (1 << 10) | FGRAPH_FRAME_OFFSET|
* | *or put another way* |
* | (BIT(3)|BIT(0)) << FGRAPH_INDEX_SHIFT | \ |
* | FGRAPH_TYPE_BITMAP << FGRAPH_TYPE_SHIFT| \ |
- * | (offset:FGRAPH_FRAME_OFFSET) | <- the offset is from here
+ * | (offset1:FGRAPH_FRAME_OFFSET) | <- the offset1 is from here
* +--------------------------------------------+
* | struct ftrace_ret_stack |
* | (stores the saved ret pointer) | <- the offset points here
@@ -100,6 +114,7 @@
enum {
FGRAPH_TYPE_RESERVED = 0,
FGRAPH_TYPE_BITMAP = 1,
+ FGRAPH_TYPE_DATA = 2,
};
/*
@@ -110,6 +125,26 @@ enum {
#define FGRAPH_INDEX_MASK GENMASK(FGRAPH_INDEX_BITS - 1, 0)
#define FGRAPH_INDEX_SHIFT (FGRAPH_TYPE_SHIFT + FGRAPH_TYPE_BITS)
+/*
+ * For DATA type:
+ * FGRAPH_DATA (12-17) bits hold the size of data (in words)
+ * FGRAPH_INDEX (18-23) bits hold the index for which gops->idx the data is for
+ *
+ * Note:
+ * data_size == 0 means 1 word, and 31 (=2^5 - 1) means 32 words.
+ */
+#define FGRAPH_DATA_BITS 5
+#define FGRAPH_DATA_MASK GENMASK(FGRAPH_DATA_BITS - 1, 0)
+#define FGRAPH_DATA_SHIFT (FGRAPH_TYPE_SHIFT + FGRAPH_TYPE_BITS)
+#define FGRAPH_MAX_DATA_SIZE (sizeof(long) * (1 << FGRAPH_DATA_BITS))
+
+#define FGRAPH_DATA_INDEX_BITS 4
+#define FGRAPH_DATA_INDEX_MASK GENMASK(FGRAPH_DATA_INDEX_BITS - 1, 0)
+#define FGRAPH_DATA_INDEX_SHIFT (FGRAPH_DATA_SHIFT + FGRAPH_DATA_BITS)
+
+#define FGRAPH_MAX_INDEX \
+ ((FGRAPH_INDEX_SIZE << FGRAPH_DATA_BITS) + FGRAPH_RET_INDEX)
+
#define FGRAPH_ARRAY_SIZE FGRAPH_INDEX_BITS
/*
@@ -178,16 +213,46 @@ static int fgraph_lru_alloc_index(void)
return idx;
}
+/* Get the offset to the fgraph frame from a ret_stack value */
+static inline int __get_offset(unsigned long val)
+{
+ return val & FGRAPH_FRAME_OFFSET_MASK;
+}
+
+/* Get the type of word from a ret_stack value */
+static inline int __get_type(unsigned long val)
+{
+ return (val >> FGRAPH_TYPE_SHIFT) & FGRAPH_TYPE_MASK;
+}
+
+/* Get the data_index for a DATA type ret_stack word */
+static inline int __get_data_index(unsigned long val)
+{
+ return (val >> FGRAPH_DATA_INDEX_SHIFT) & FGRAPH_DATA_INDEX_MASK;
+}
+
+/* Get the data_size for a DATA type ret_stack word */
+static inline int __get_data_size(unsigned long val)
+{
+ return ((val >> FGRAPH_DATA_SHIFT) & FGRAPH_DATA_MASK) + 1;
+}
+
+/* Get the word from the ret_stack at @offset */
+static inline unsigned long get_fgraph_entry(struct task_struct *t, int offset)
+{
+ return t->ret_stack[offset];
+}
+
/* Get the FRAME_OFFSET from the word from the @offset on ret_stack */
static inline int get_frame_offset(struct task_struct *t, int offset)
{
- return t->ret_stack[offset] & FGRAPH_FRAME_OFFSET_MASK;
+ return __get_offset(t->ret_stack[offset]);
}
/* Get FGRAPH_TYPE from the word from the @offset at ret_stack */
static inline int get_fgraph_type(struct task_struct *t, int offset)
{
- return (t->ret_stack[offset] >> FGRAPH_TYPE_SHIFT) & FGRAPH_TYPE_MASK;
+ return __get_type(t->ret_stack[offset]);
}
/* For BITMAP type: get the bitmask from the @offset at ret_stack */
@@ -212,6 +277,25 @@ set_bitmap(struct task_struct *t, int offset, unsigned long bitmap)
(FGRAPH_TYPE_BITMAP << FGRAPH_TYPE_SHIFT) | FGRAPH_FRAME_OFFSET;
}
+/* For DATA type: get the data saved under the ret_stack word at @offset */
+static inline void *get_data_type_data(struct task_struct *t, int offset)
+{
+ unsigned long val = t->ret_stack[offset];
+
+ if (__get_type(val) != FGRAPH_TYPE_DATA)
+ return NULL;
+ offset -= __get_data_size(val);
+ return (void *)&t->ret_stack[offset];
+}
+
+/* Create the ret_stack word for a DATA type */
+static inline unsigned long make_data_type_val(int idx, int size, int offset)
+{
+ return (idx << FGRAPH_DATA_INDEX_SHIFT) |
+ ((size - 1) << FGRAPH_DATA_SHIFT) |
+ (FGRAPH_TYPE_DATA << FGRAPH_TYPE_SHIFT) | offset;
+}
+
/* ftrace_graph_entry set to this to tell some archs to run function graph */
static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops)
{
@@ -245,6 +329,91 @@ static void ret_stack_init_task_vars(unsigned long *ret_stack)
memset(gvals, 0, sizeof(*gvals) * FGRAPH_ARRAY_SIZE);
}
+/**
+ * fgraph_reserve_data - Reserve storage on the task's ret_stack
+ * @idx: The index of fgraph_array
+ * @size_bytes: The size in bytes to reserve
+ *
+ * Reserves space of up to FGRAPH_MAX_DATA_SIZE bytes on the
+ * task's ret_stack shadow stack, for a given fgraph_ops during
+ * the entryfunc() call. If entryfunc() returns zero, the storage
+ * is discarded. An entryfunc() can only call this once per iteration.
+ * The fgraph_ops retfunc() can retrieve this stored data with
+ * fgraph_retrieve_data().
+ *
+ * Returns: On success, a pointer to the data on the stack.
+ * Otherwise, NULL if there's not enough space left on the
+ * ret_stack for the data, or if fgraph_reserve_data() was called
+ * more than once for a single entryfunc() call.
+ */
+void *fgraph_reserve_data(int idx, int size_bytes)
+{
+ unsigned long val;
+ void *data;
+ int curr_ret_stack = current->curr_ret_stack;
+ int data_size;
+
+ if (size_bytes > FGRAPH_MAX_DATA_SIZE)
+ return NULL;
+
+ /* Convert the data size to number of longs. */
+ data_size = (size_bytes + sizeof(long) - 1) >> (sizeof(long) == 4 ? 2 : 3);
+
+ val = get_fgraph_entry(current, curr_ret_stack - 1);
+ data = ¤t->ret_stack[curr_ret_stack];
+
+ curr_ret_stack += data_size + 1;
+ if (unlikely(curr_ret_stack >= SHADOW_STACK_MAX_OFFSET))
+ return NULL;
+
+ val = make_data_type_val(idx, data_size, __get_offset(val) + data_size + 1);
+
+ /* Set the last word to be reserved */
+ current->ret_stack[curr_ret_stack - 1] = val;
+
+ /* Make sure interrupts see this */
+ barrier();
+ current->curr_ret_stack = curr_ret_stack;
+ /* Again sync with interrupts, and reset reserve */
+ current->ret_stack[curr_ret_stack - 1] = val;
+
+ return data;
+}
+
+/**
+ * fgraph_retrieve_data - Retrieve stored data from fgraph_reserve_data()
+ * @idx: the index of fgraph_array (fgraph_ops::idx)
+ * @size_bytes: pointer to retrieved data size.
+ *
+ * This is to be called by a fgraph_ops retfunc(), to retrieve data that
+ * was stored by the fgraph_ops entryfunc() on the function entry.
+ * That is, this will retrieve the data that was reserved on the
+ * entry of the function that corresponds to the exit of the function
+ * that the fgraph_ops retfunc() is called on.
+ *
+ * Returns: The stored data from fgraph_reserve_data() called by the
+ * matching entryfunc() for the retfunc() this is called from.
+ * Or NULL if there was nothing stored.
+ */
+void *fgraph_retrieve_data(int idx, int *size_bytes)
+{
+ int offset = current->curr_ret_stack - 1;
+ unsigned long val;
+
+ val = get_fgraph_entry(current, offset);
+ while (__get_type(val) == FGRAPH_TYPE_DATA) {
+ if (__get_data_index(val) == idx)
+ goto found;
+ offset -= __get_data_size(val) + 1;
+ val = get_fgraph_entry(current, offset);
+ }
+ return NULL;
+found:
+ if (size_bytes)
+ *size_bytes = __get_data_size(val) * sizeof(long);
+ return get_data_type_data(current, offset);
+}
+
/**
* fgraph_get_task_var - retrieve a task specific state variable
* @gops: The ftrace_ops that owns the task specific variable
@@ -464,13 +633,18 @@ int function_graph_enter(unsigned long ret, unsigned long func,
for (i = 0; i < FGRAPH_ARRAY_SIZE; i++) {
struct fgraph_ops *gops = fgraph_array[i];
+ int save_curr_ret_stack;
if (gops == &fgraph_stub)
continue;
+ save_curr_ret_stack = current->curr_ret_stack;
if (ftrace_ops_test(&gops->ops, func, NULL) &&
gops->entryfunc(&trace, gops))
bitmap |= BIT(i);
+ else
+ /* Clear out any saved storage */
+ current->curr_ret_stack = save_curr_ret_stack;
}
if (!bitmap)
@@ -626,7 +800,8 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs
* curr_ret_stack is after that.
*/
barrier();
- current->curr_ret_stack -= FGRAPH_FRAME_OFFSET + 1;
+ current->curr_ret_stack = offset - FGRAPH_FRAME_OFFSET;
+
current->curr_ret_depth--;
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 20/27] function_graph: Add selftest for passing local variables
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (18 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 19/27] function_graph: Implement fgraph_reserve_data() and fgraph_retrieve_data() Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 21/27] ftrace: Add multiple fgraph storage selftest Steven Rostedt
` (8 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Add boot up selftest that passes variables from a function entry to a
function exit, and make sure that they do get passed around.
Co-developed with Masami Hiramatsu:
Link: https://lore.kernel.org/linux-trace-kernel/171509110271.162236.11047551496319744627.stgit@devnote2
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_selftest.c | 169 ++++++++++++++++++++++++++++++++++
1 file changed, 169 insertions(+)
diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
index f8f55fd79e53..fcdc744c245e 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -756,6 +756,173 @@ trace_selftest_startup_function(struct tracer *trace, struct trace_array *tr)
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+#ifdef CONFIG_DYNAMIC_FTRACE
+
+#define BYTE_NUMBER 123
+#define SHORT_NUMBER 12345
+#define WORD_NUMBER 1234567890
+#define LONG_NUMBER 1234567890123456789LL
+
+static int fgraph_store_size __initdata;
+static const char *fgraph_store_type_name __initdata;
+static char *fgraph_error_str __initdata;
+static char fgraph_error_str_buf[128] __initdata;
+
+static __init int store_entry(struct ftrace_graph_ent *trace,
+ struct fgraph_ops *gops)
+{
+ const char *type = fgraph_store_type_name;
+ int size = fgraph_store_size;
+ void *p;
+
+ p = fgraph_reserve_data(gops->idx, size);
+ if (!p) {
+ snprintf(fgraph_error_str_buf, sizeof(fgraph_error_str_buf),
+ "Failed to reserve %s\n", type);
+ fgraph_error_str = fgraph_error_str_buf;
+ return 0;
+ }
+
+ switch (fgraph_store_size) {
+ case 1:
+ *(char *)p = BYTE_NUMBER;
+ break;
+ case 2:
+ *(short *)p = SHORT_NUMBER;
+ break;
+ case 4:
+ *(int *)p = WORD_NUMBER;
+ break;
+ case 8:
+ *(long long *)p = LONG_NUMBER;
+ break;
+ }
+
+ return 1;
+}
+
+static __init void store_return(struct ftrace_graph_ret *trace,
+ struct fgraph_ops *gops)
+{
+ const char *type = fgraph_store_type_name;
+ long long expect = 0;
+ long long found = -1;
+ int size;
+ char *p;
+
+ p = fgraph_retrieve_data(gops->idx, &size);
+ if (!p) {
+ snprintf(fgraph_error_str_buf, sizeof(fgraph_error_str_buf),
+ "Failed to retrieve %s\n", type);
+ fgraph_error_str = fgraph_error_str_buf;
+ return;
+ }
+ if (fgraph_store_size > size) {
+ snprintf(fgraph_error_str_buf, sizeof(fgraph_error_str_buf),
+ "Retrieved size %d is smaller than expected %d\n",
+ size, (int)fgraph_store_size);
+ fgraph_error_str = fgraph_error_str_buf;
+ return;
+ }
+
+ switch (fgraph_store_size) {
+ case 1:
+ expect = BYTE_NUMBER;
+ found = *(char *)p;
+ break;
+ case 2:
+ expect = SHORT_NUMBER;
+ found = *(short *)p;
+ break;
+ case 4:
+ expect = WORD_NUMBER;
+ found = *(int *)p;
+ break;
+ case 8:
+ expect = LONG_NUMBER;
+ found = *(long long *)p;
+ break;
+ }
+
+ if (found != expect) {
+ snprintf(fgraph_error_str_buf, sizeof(fgraph_error_str_buf),
+ "%s returned not %lld but %lld\n", type, expect, found);
+ fgraph_error_str = fgraph_error_str_buf;
+ return;
+ }
+ fgraph_error_str = NULL;
+}
+
+static struct fgraph_ops store_bytes __initdata = {
+ .entryfunc = store_entry,
+ .retfunc = store_return,
+};
+
+static int __init test_graph_storage_type(const char *name, int size)
+{
+ char *func_name;
+ int len;
+ int ret;
+
+ fgraph_store_type_name = name;
+ fgraph_store_size = size;
+
+ snprintf(fgraph_error_str_buf, sizeof(fgraph_error_str_buf),
+ "Failed to execute storage %s\n", name);
+ fgraph_error_str = fgraph_error_str_buf;
+
+ pr_cont("PASSED\n");
+ pr_info("Testing fgraph storage of %d byte%s: ", size, size > 1 ? "s" : "");
+
+ func_name = "*" __stringify(DYN_FTRACE_TEST_NAME);
+ len = strlen(func_name);
+
+ ret = ftrace_set_filter(&store_bytes.ops, func_name, len, 1);
+ if (ret && ret != -ENODEV) {
+ pr_cont("*Could not set filter* ");
+ return -1;
+ }
+
+ ret = register_ftrace_graph(&store_bytes);
+ if (ret) {
+ pr_warn("Failed to init store_bytes fgraph tracing\n");
+ return -1;
+ }
+
+ DYN_FTRACE_TEST_NAME();
+
+ unregister_ftrace_graph(&store_bytes);
+
+ if (fgraph_error_str) {
+ pr_cont("*** %s ***", fgraph_error_str);
+ return -1;
+ }
+
+ return 0;
+}
+/* Test the storage passed across function_graph entry and return */
+static __init int test_graph_storage(void)
+{
+ int ret;
+
+ ret = test_graph_storage_type("byte", 1);
+ if (ret)
+ return ret;
+ ret = test_graph_storage_type("short", 2);
+ if (ret)
+ return ret;
+ ret = test_graph_storage_type("word", 4);
+ if (ret)
+ return ret;
+ ret = test_graph_storage_type("long long", 8);
+ if (ret)
+ return ret;
+ return 0;
+}
+#else
+static inline int test_graph_storage(void) { return 0; }
+#endif /* CONFIG_DYNAMIC_FTRACE */
+
/* Maximum number of functions to trace before diagnosing a hang */
#define GRAPH_MAX_FUNC_TEST 100000000
@@ -913,6 +1080,8 @@ trace_selftest_startup_function_graph(struct tracer *trace,
ftrace_set_global_filter(NULL, 0, 1);
#endif
+ ret = test_graph_storage();
+
/* Don't test dynamic tracing, the function tracer already did */
out:
/* Stop it if we failed */
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 21/27] ftrace: Add multiple fgraph storage selftest
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (19 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 20/27] function_graph: Add selftest for passing local variables Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 22/27] function_graph: Use for_each_set_bit() in __ftrace_return_to_handler() Steven Rostedt
` (7 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
Add a selftest for multiple function graph tracer with storage on a same
function. In this case, the shadow stack entry will be shared among those
fgraph with different data storage. So this will ensure the fgraph will
not mixed those storage data.
Link: https://lore.kernel.org/linux-trace-kernel/171509111465.162236.3795819216426570800.stgit@devnote2
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Suggested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_selftest.c | 171 +++++++++++++++++++++++++---------
1 file changed, 126 insertions(+), 45 deletions(-)
diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
index fcdc744c245e..369efc569238 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -762,28 +762,32 @@ trace_selftest_startup_function(struct tracer *trace, struct trace_array *tr)
#define SHORT_NUMBER 12345
#define WORD_NUMBER 1234567890
#define LONG_NUMBER 1234567890123456789LL
-
-static int fgraph_store_size __initdata;
-static const char *fgraph_store_type_name __initdata;
-static char *fgraph_error_str __initdata;
-static char fgraph_error_str_buf[128] __initdata;
+#define ERRSTR_BUFLEN 128
+
+struct fgraph_fixture {
+ struct fgraph_ops gops;
+ int store_size;
+ const char *store_type_name;
+ char error_str_buf[ERRSTR_BUFLEN];
+ char *error_str;
+};
static __init int store_entry(struct ftrace_graph_ent *trace,
struct fgraph_ops *gops)
{
- const char *type = fgraph_store_type_name;
- int size = fgraph_store_size;
+ struct fgraph_fixture *fixture = container_of(gops, struct fgraph_fixture, gops);
+ const char *type = fixture->store_type_name;
+ int size = fixture->store_size;
void *p;
p = fgraph_reserve_data(gops->idx, size);
if (!p) {
- snprintf(fgraph_error_str_buf, sizeof(fgraph_error_str_buf),
+ snprintf(fixture->error_str_buf, ERRSTR_BUFLEN,
"Failed to reserve %s\n", type);
- fgraph_error_str = fgraph_error_str_buf;
return 0;
}
- switch (fgraph_store_size) {
+ switch (size) {
case 1:
*(char *)p = BYTE_NUMBER;
break;
@@ -804,7 +808,8 @@ static __init int store_entry(struct ftrace_graph_ent *trace,
static __init void store_return(struct ftrace_graph_ret *trace,
struct fgraph_ops *gops)
{
- const char *type = fgraph_store_type_name;
+ struct fgraph_fixture *fixture = container_of(gops, struct fgraph_fixture, gops);
+ const char *type = fixture->store_type_name;
long long expect = 0;
long long found = -1;
int size;
@@ -812,20 +817,18 @@ static __init void store_return(struct ftrace_graph_ret *trace,
p = fgraph_retrieve_data(gops->idx, &size);
if (!p) {
- snprintf(fgraph_error_str_buf, sizeof(fgraph_error_str_buf),
+ snprintf(fixture->error_str_buf, ERRSTR_BUFLEN,
"Failed to retrieve %s\n", type);
- fgraph_error_str = fgraph_error_str_buf;
return;
}
- if (fgraph_store_size > size) {
- snprintf(fgraph_error_str_buf, sizeof(fgraph_error_str_buf),
+ if (fixture->store_size > size) {
+ snprintf(fixture->error_str_buf, ERRSTR_BUFLEN,
"Retrieved size %d is smaller than expected %d\n",
- size, (int)fgraph_store_size);
- fgraph_error_str = fgraph_error_str_buf;
+ size, (int)fixture->store_size);
return;
}
- switch (fgraph_store_size) {
+ switch (fixture->store_size) {
case 1:
expect = BYTE_NUMBER;
found = *(char *)p;
@@ -845,45 +848,44 @@ static __init void store_return(struct ftrace_graph_ret *trace,
}
if (found != expect) {
- snprintf(fgraph_error_str_buf, sizeof(fgraph_error_str_buf),
+ snprintf(fixture->error_str_buf, ERRSTR_BUFLEN,
"%s returned not %lld but %lld\n", type, expect, found);
- fgraph_error_str = fgraph_error_str_buf;
return;
}
- fgraph_error_str = NULL;
+ fixture->error_str = NULL;
}
-static struct fgraph_ops store_bytes __initdata = {
- .entryfunc = store_entry,
- .retfunc = store_return,
-};
-
-static int __init test_graph_storage_type(const char *name, int size)
+static int __init init_fgraph_fixture(struct fgraph_fixture *fixture)
{
char *func_name;
int len;
- int ret;
- fgraph_store_type_name = name;
- fgraph_store_size = size;
+ snprintf(fixture->error_str_buf, ERRSTR_BUFLEN,
+ "Failed to execute storage %s\n", fixture->store_type_name);
+ fixture->error_str = fixture->error_str_buf;
- snprintf(fgraph_error_str_buf, sizeof(fgraph_error_str_buf),
- "Failed to execute storage %s\n", name);
- fgraph_error_str = fgraph_error_str_buf;
+ func_name = "*" __stringify(DYN_FTRACE_TEST_NAME);
+ len = strlen(func_name);
+
+ return ftrace_set_filter(&fixture->gops.ops, func_name, len, 1);
+}
+
+/* Test fgraph storage for each size */
+static int __init test_graph_storage_single(struct fgraph_fixture *fixture)
+{
+ int size = fixture->store_size;
+ int ret;
pr_cont("PASSED\n");
pr_info("Testing fgraph storage of %d byte%s: ", size, size > 1 ? "s" : "");
- func_name = "*" __stringify(DYN_FTRACE_TEST_NAME);
- len = strlen(func_name);
-
- ret = ftrace_set_filter(&store_bytes.ops, func_name, len, 1);
+ ret = init_fgraph_fixture(fixture);
if (ret && ret != -ENODEV) {
pr_cont("*Could not set filter* ");
return -1;
}
- ret = register_ftrace_graph(&store_bytes);
+ ret = register_ftrace_graph(&fixture->gops);
if (ret) {
pr_warn("Failed to init store_bytes fgraph tracing\n");
return -1;
@@ -891,30 +893,109 @@ static int __init test_graph_storage_type(const char *name, int size)
DYN_FTRACE_TEST_NAME();
- unregister_ftrace_graph(&store_bytes);
+ unregister_ftrace_graph(&fixture->gops);
- if (fgraph_error_str) {
- pr_cont("*** %s ***", fgraph_error_str);
+ if (fixture->error_str) {
+ pr_cont("*** %s ***", fixture->error_str);
return -1;
}
return 0;
}
+
+static struct fgraph_fixture store_bytes[4] __initdata = {
+ [0] = {
+ .gops = {
+ .entryfunc = store_entry,
+ .retfunc = store_return,
+ },
+ .store_size = 1,
+ .store_type_name = "byte",
+ },
+ [1] = {
+ .gops = {
+ .entryfunc = store_entry,
+ .retfunc = store_return,
+ },
+ .store_size = 2,
+ .store_type_name = "short",
+ },
+ [2] = {
+ .gops = {
+ .entryfunc = store_entry,
+ .retfunc = store_return,
+ },
+ .store_size = 4,
+ .store_type_name = "word",
+ },
+ [3] = {
+ .gops = {
+ .entryfunc = store_entry,
+ .retfunc = store_return,
+ },
+ .store_size = 8,
+ .store_type_name = "long long",
+ },
+};
+
+static __init int test_graph_storage_multi(void)
+{
+ struct fgraph_fixture *fixture;
+ bool printed = false;
+ int i, ret;
+
+ pr_cont("PASSED\n");
+ pr_info("Testing multiple fgraph storage on a function: ");
+
+ for (i = 0; i < ARRAY_SIZE(store_bytes); i++) {
+ fixture = &store_bytes[i];
+ ret = init_fgraph_fixture(fixture);
+ if (ret && ret != -ENODEV) {
+ pr_cont("*Could not set filter* ");
+ printed = true;
+ goto out;
+ }
+
+ ret = register_ftrace_graph(&fixture->gops);
+ if (ret) {
+ pr_warn("Failed to init store_bytes fgraph tracing\n");
+ printed = true;
+ goto out;
+ }
+ }
+
+ DYN_FTRACE_TEST_NAME();
+out:
+ while (--i >= 0) {
+ fixture = &store_bytes[i];
+ unregister_ftrace_graph(&fixture->gops);
+
+ if (fixture->error_str && !printed) {
+ pr_cont("*** %s ***", fixture->error_str);
+ printed = true;
+ }
+ }
+ return printed ? -1 : 0;
+}
+
/* Test the storage passed across function_graph entry and return */
static __init int test_graph_storage(void)
{
int ret;
- ret = test_graph_storage_type("byte", 1);
+ ret = test_graph_storage_single(&store_bytes[0]);
+ if (ret)
+ return ret;
+ ret = test_graph_storage_single(&store_bytes[1]);
if (ret)
return ret;
- ret = test_graph_storage_type("short", 2);
+ ret = test_graph_storage_single(&store_bytes[2]);
if (ret)
return ret;
- ret = test_graph_storage_type("word", 4);
+ ret = test_graph_storage_single(&store_bytes[3]);
if (ret)
return ret;
- ret = test_graph_storage_type("long long", 8);
+ ret = test_graph_storage_multi();
if (ret)
return ret;
return 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 22/27] function_graph: Use for_each_set_bit() in __ftrace_return_to_handler()
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (20 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 21/27] ftrace: Add multiple fgraph storage selftest Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 23/27] function_graph: Use bitmask to loop on fgraph entry Steven Rostedt
` (6 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
Instead of iterating through the entire fgraph_array[] and seeing if one
of the bitmap bits are set to know to call the array's retfunc() function,
use for_each_set_bit() on the bitmap itself. This will only iterate for
the number of set bits.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/fgraph.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index f207b7ae5f46..0827b67f746d 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -783,11 +783,10 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs
#endif
bitmap = get_bitmap_bits(current, offset);
- for (i = 0; i < FGRAPH_ARRAY_SIZE; i++) {
+
+ for_each_set_bit(i, &bitmap, sizeof(bitmap) * BITS_PER_BYTE) {
struct fgraph_ops *gops = fgraph_array[i];
- if (!(bitmap & BIT(i)))
- continue;
if (gops == &fgraph_stub)
continue;
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 23/27] function_graph: Use bitmask to loop on fgraph entry
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (21 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 22/27] function_graph: Use for_each_set_bit() in __ftrace_return_to_handler() Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 24/27] function_graph: Use static_call and branch to optimize entry function Steven Rostedt
` (5 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
Instead of looping through all the elements of fgraph_array[] to see if
there's an gops attached to one and then calling its gops->func(). Create
a fgraph_array_bitmask that sets bits when an index in the array is
reserved (via the simple lru algorithm). Then only the bits set in this
bitmask needs to be looked at where only elements in the array that have
ops registered need to be looked at.
Note, we do not care about races. If a bit is set before the gops is
assigned, it only wastes time looking at the element and ignoring it (as
it did before this bitmask is added).
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/fgraph.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 0827b67f746d..4d566a0a741d 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -172,6 +172,7 @@ DEFINE_STATIC_KEY_FALSE(kill_ftrace_graph);
int ftrace_graph_active;
static struct fgraph_ops *fgraph_array[FGRAPH_ARRAY_SIZE];
+static unsigned long fgraph_array_bitmask;
/* LRU index table for fgraph_array */
static int fgraph_lru_table[FGRAPH_ARRAY_SIZE];
@@ -196,6 +197,8 @@ static int fgraph_lru_release_index(int idx)
fgraph_lru_table[fgraph_lru_last] = idx;
fgraph_lru_last = (fgraph_lru_last + 1) % FGRAPH_ARRAY_SIZE;
+
+ clear_bit(idx, &fgraph_array_bitmask);
return 0;
}
@@ -210,6 +213,8 @@ static int fgraph_lru_alloc_index(void)
fgraph_lru_table[fgraph_lru_next] = -1;
fgraph_lru_next = (fgraph_lru_next + 1) % FGRAPH_ARRAY_SIZE;
+
+ set_bit(idx, &fgraph_array_bitmask);
return idx;
}
@@ -631,7 +636,8 @@ int function_graph_enter(unsigned long ret, unsigned long func,
if (offset < 0)
goto out;
- for (i = 0; i < FGRAPH_ARRAY_SIZE; i++) {
+ for_each_set_bit(i, &fgraph_array_bitmask,
+ sizeof(fgraph_array_bitmask) * BITS_PER_BYTE) {
struct fgraph_ops *gops = fgraph_array[i];
int save_curr_ret_stack;
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 24/27] function_graph: Use static_call and branch to optimize entry function
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (22 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 23/27] function_graph: Use bitmask to loop on fgraph entry Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 25/27] function_graph: Use static_call and branch to optimize return function Steven Rostedt
` (4 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
In most cases function graph is used by a single user. Instead of calling
a loop to call function graph callbacks in this case, call the function
entry callback directly.
Add a static_key that will be used to set the function graph logic to
either do the loop (when more than one callback is registered) or to call
the callback directly if there is only one registered callback.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/fgraph.c | 77 ++++++++++++++++++++++++++++++++++++-------
1 file changed, 66 insertions(+), 11 deletions(-)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 4d566a0a741d..7c3b0261b1bb 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -11,6 +11,7 @@
#include <linux/jump_label.h>
#include <linux/suspend.h>
#include <linux/ftrace.h>
+#include <linux/static_call.h>
#include <linux/slab.h>
#include <trace/events/sched.h>
@@ -511,6 +512,10 @@ static struct fgraph_ops fgraph_stub = {
.retfunc = ftrace_graph_ret_stub,
};
+static struct fgraph_ops *fgraph_direct_gops = &fgraph_stub;
+DEFINE_STATIC_CALL(fgraph_func, ftrace_graph_entry_stub);
+DEFINE_STATIC_KEY_TRUE(fgraph_do_direct);
+
/**
* ftrace_graph_stop - set to permanently disable function graph tracing
*
@@ -636,21 +641,34 @@ int function_graph_enter(unsigned long ret, unsigned long func,
if (offset < 0)
goto out;
- for_each_set_bit(i, &fgraph_array_bitmask,
- sizeof(fgraph_array_bitmask) * BITS_PER_BYTE) {
- struct fgraph_ops *gops = fgraph_array[i];
- int save_curr_ret_stack;
-
- if (gops == &fgraph_stub)
- continue;
+#ifdef CONFIG_HAVE_STATIC_CALL
+ if (static_branch_likely(&fgraph_do_direct)) {
+ int save_curr_ret_stack = current->curr_ret_stack;
- save_curr_ret_stack = current->curr_ret_stack;
- if (ftrace_ops_test(&gops->ops, func, NULL) &&
- gops->entryfunc(&trace, gops))
- bitmap |= BIT(i);
+ if (static_call(fgraph_func)(&trace, fgraph_direct_gops))
+ bitmap |= BIT(fgraph_direct_gops->idx);
else
/* Clear out any saved storage */
current->curr_ret_stack = save_curr_ret_stack;
+ } else
+#endif
+ {
+ for_each_set_bit(i, &fgraph_array_bitmask,
+ sizeof(fgraph_array_bitmask) * BITS_PER_BYTE) {
+ struct fgraph_ops *gops = fgraph_array[i];
+ int save_curr_ret_stack;
+
+ if (gops == &fgraph_stub)
+ continue;
+
+ save_curr_ret_stack = current->curr_ret_stack;
+ if (ftrace_ops_test(&gops->ops, func, NULL) &&
+ gops->entryfunc(&trace, gops))
+ bitmap |= BIT(i);
+ else
+ /* Clear out any saved storage */
+ current->curr_ret_stack = save_curr_ret_stack;
+ }
}
if (!bitmap)
@@ -1155,6 +1173,8 @@ void fgraph_update_pid_func(void)
gops = container_of(op, struct fgraph_ops, ops);
gops->entryfunc = ftrace_pids_enabled(op) ?
fgraph_pid_func : gops->saved_func;
+ if (ftrace_graph_active == 1)
+ static_call_update(fgraph_func, gops->entryfunc);
}
}
}
@@ -1209,6 +1229,32 @@ static void init_task_vars(int idx)
read_unlock(&tasklist_lock);
}
+static void ftrace_graph_enable_direct(bool enable_branch)
+{
+ trace_func_graph_ent_t func = NULL;
+ int i;
+
+ for_each_set_bit(i, &fgraph_array_bitmask,
+ sizeof(fgraph_array_bitmask) * BITS_PER_BYTE) {
+ func = fgraph_array[i]->entryfunc;
+ fgraph_direct_gops = fgraph_array[i];
+ }
+ if (WARN_ON_ONCE(!func))
+ return;
+
+ static_call_update(fgraph_func, func);
+ if (enable_branch)
+ static_branch_disable(&fgraph_do_direct);
+}
+
+static void ftrace_graph_disable_direct(bool disable_branch)
+{
+ if (disable_branch)
+ static_branch_disable(&fgraph_do_direct);
+ static_call_update(fgraph_func, ftrace_graph_entry_stub);
+ fgraph_direct_gops = &fgraph_stub;
+}
+
int register_ftrace_graph(struct fgraph_ops *gops)
{
int command = 0;
@@ -1235,7 +1281,11 @@ int register_ftrace_graph(struct fgraph_ops *gops)
ftrace_graph_active++;
+ if (ftrace_graph_active == 2)
+ ftrace_graph_disable_direct(true);
+
if (ftrace_graph_active == 1) {
+ ftrace_graph_enable_direct(false);
register_pm_notifier(&ftrace_suspend_notifier);
ret = start_graph_tracing();
if (ret)
@@ -1292,6 +1342,11 @@ void unregister_ftrace_graph(struct fgraph_ops *gops)
ftrace_shutdown_subops(&graph_ops, &gops->ops, command);
+ if (ftrace_graph_active == 1)
+ ftrace_graph_enable_direct(true);
+ else if (!ftrace_graph_active)
+ ftrace_graph_disable_direct(false);
+
if (!ftrace_graph_active) {
ftrace_graph_return = ftrace_stub_graph;
ftrace_graph_entry = ftrace_graph_entry_stub;
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 25/27] function_graph: Use static_call and branch to optimize return function
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (23 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 24/27] function_graph: Use static_call and branch to optimize entry function Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 26/27] selftests/ftrace: Add function_graph tracer to func-filter-pid test Steven Rostedt
` (3 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
In most cases function graph is used by a single user. Instead of calling
a loop to call function graph callbacks in this case, call the function
return callback directly.
Use the static_key that is set when the function graph tracer has less
than 2 callbacks registered. It will do the direct call in that case, and
will do the loop over all callers when there are 2 or more callbacks
registered.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/fgraph.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 7c3b0261b1bb..4bf91eebbb08 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -514,6 +514,7 @@ static struct fgraph_ops fgraph_stub = {
static struct fgraph_ops *fgraph_direct_gops = &fgraph_stub;
DEFINE_STATIC_CALL(fgraph_func, ftrace_graph_entry_stub);
+DEFINE_STATIC_CALL(fgraph_retfunc, ftrace_graph_ret_stub);
DEFINE_STATIC_KEY_TRUE(fgraph_do_direct);
/**
@@ -808,13 +809,21 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs
bitmap = get_bitmap_bits(current, offset);
- for_each_set_bit(i, &bitmap, sizeof(bitmap) * BITS_PER_BYTE) {
- struct fgraph_ops *gops = fgraph_array[i];
+#ifdef CONFIG_HAVE_STATIC_CALL
+ if (static_branch_likely(&fgraph_do_direct)) {
+ if (test_bit(fgraph_direct_gops->idx, &bitmap))
+ static_call(fgraph_retfunc)(&trace, fgraph_direct_gops);
+ } else
+#endif
+ {
+ for_each_set_bit(i, &bitmap, sizeof(bitmap) * BITS_PER_BYTE) {
+ struct fgraph_ops *gops = fgraph_array[i];
- if (gops == &fgraph_stub)
- continue;
+ if (gops == &fgraph_stub)
+ continue;
- gops->retfunc(&trace, gops);
+ gops->retfunc(&trace, gops);
+ }
}
/*
@@ -1232,17 +1241,20 @@ static void init_task_vars(int idx)
static void ftrace_graph_enable_direct(bool enable_branch)
{
trace_func_graph_ent_t func = NULL;
+ trace_func_graph_ret_t retfunc = NULL;
int i;
for_each_set_bit(i, &fgraph_array_bitmask,
sizeof(fgraph_array_bitmask) * BITS_PER_BYTE) {
func = fgraph_array[i]->entryfunc;
+ retfunc = fgraph_array[i]->retfunc;
fgraph_direct_gops = fgraph_array[i];
}
if (WARN_ON_ONCE(!func))
return;
static_call_update(fgraph_func, func);
+ static_call_update(fgraph_retfunc, retfunc);
if (enable_branch)
static_branch_disable(&fgraph_do_direct);
}
@@ -1252,6 +1264,7 @@ static void ftrace_graph_disable_direct(bool disable_branch)
if (disable_branch)
static_branch_disable(&fgraph_do_direct);
static_call_update(fgraph_func, ftrace_graph_entry_stub);
+ static_call_update(fgraph_retfunc, ftrace_graph_ret_stub);
fgraph_direct_gops = &fgraph_stub;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 26/27] selftests/ftrace: Add function_graph tracer to func-filter-pid test
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (24 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 25/27] function_graph: Use static_call and branch to optimize return function Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-03 19:07 ` [PATCH v3 27/27] selftests/ftrace: Add fgraph-multi.tc test Steven Rostedt
` (2 subsequent siblings)
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
The function tracer is tested to see if pid filtering works. Add a test to
test function_graph tracer as well, but only if the function_graph tracer
is enabled for the top level or instance.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
.../ftrace/test.d/ftrace/func-filter-pid.tc | 27 +++++++++++++++----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc
index 2f7211254529..c6fc9d31a496 100644
--- a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc
+++ b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc
@@ -14,6 +14,11 @@ if [ ! -f options/function-fork ]; then
echo "no option for function-fork found. Option will not be tested."
fi
+if [ ! -f options/funcgraph-proc ]; then
+ do_funcgraph_proc=0
+ echo "no option for function-fork found. Option will not be tested."
+fi
+
read PID _ < /proc/self/stat
if [ $do_function_fork -eq 1 ]; then
@@ -21,12 +26,18 @@ if [ $do_function_fork -eq 1 ]; then
orig_value=`grep function-fork trace_options`
fi
+if [ $do_funcgraph_proc -eq 1 ]; then
+ orig_value2=`cat options/funcgraph-proc`
+fi
+
do_reset() {
- if [ $do_function_fork -eq 0 ]; then
- return
+ if [ $do_function_fork -eq 1 ]; then
+ echo $orig_value > trace_options
fi
- echo $orig_value > trace_options
+ if [ $do_funcgraph_proc -eq 1 ]; then
+ echo $orig_value2 > options/funcgraph-proc
+ fi
}
fail() { # msg
@@ -36,13 +47,15 @@ fail() { # msg
}
do_test() {
+ TRACER=$1
+
disable_tracing
echo do_execve* > set_ftrace_filter
echo $FUNCTION_FORK >> set_ftrace_filter
echo $PID > set_ftrace_pid
- echo function > current_tracer
+ echo $TRACER > current_tracer
if [ $do_function_fork -eq 1 ]; then
# don't allow children to be traced
@@ -82,7 +95,11 @@ do_test() {
fi
}
-do_test
+do_test function
+if grep -s function_graph available_tracers; then
+ do_test function_graph
+fi
+
do_reset
exit 0
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 27/27] selftests/ftrace: Add fgraph-multi.tc test
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (25 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 26/27] selftests/ftrace: Add function_graph tracer to func-filter-pid test Steven Rostedt
@ 2024-06-03 19:07 ` Steven Rostedt
2024-06-04 12:18 ` [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
2024-06-05 14:07 ` Mark Rutland
28 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-03 19:07 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Alexei Starovoitov, Florent Revest, Martin KaFai Lau, bpf,
Sven Schnelle, Alexei Starovoitov, Jiri Olsa,
Arnaldo Carvalho de Melo, Daniel Borkmann, Alan Maguire,
Peter Zijlstra, Thomas Gleixner, Guo Ren
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
Add a test that creates 3 instances and enables function_graph tracer in
each as well as the top instance, where each will enable a filter (but one
that traces all functions) and check that they are filtering properly.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
.../ftrace/test.d/ftrace/fgraph-multi.tc | 103 ++++++++++++++++++
1 file changed, 103 insertions(+)
create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/fgraph-multi.tc
diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-multi.tc b/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-multi.tc
new file mode 100644
index 000000000000..ff88f97e41fb
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-multi.tc
@@ -0,0 +1,103 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: ftrace - function graph filters
+# requires: set_ftrace_filter function_graph:tracer
+
+# Make sure that function graph filtering works
+
+INSTANCE1="instances/test1_$$"
+INSTANCE2="instances/test2_$$"
+INSTANCE3="instances/test3_$$"
+
+WD=`pwd`
+
+do_reset() {
+ cd $WD
+ if [ -d $INSTANCE1 ]; then
+ echo nop > $INSTANCE1/current_tracer
+ rmdir $INSTANCE1
+ fi
+ if [ -d $INSTANCE2 ]; then
+ echo nop > $INSTANCE2/current_tracer
+ rmdir $INSTANCE2
+ fi
+ if [ -d $INSTANCE3 ]; then
+ echo nop > $INSTANCE3/current_tracer
+ rmdir $INSTANCE3
+ fi
+}
+
+mkdir $INSTANCE1
+if ! grep -q function_graph $INSTANCE1/available_tracers; then
+ echo "function_graph not allowed with instances"
+ rmdir $INSTANCE1
+ exit_unsupported
+fi
+
+mkdir $INSTANCE2
+mkdir $INSTANCE3
+
+fail() { # msg
+ do_reset
+ echo $1
+ exit_fail
+}
+
+disable_tracing
+clear_trace
+
+do_test() {
+ REGEX=$1
+ TEST=$2
+
+ # filter something, schedule is always good
+ if ! echo "$REGEX" > set_ftrace_filter; then
+ fail "can not enable filter $REGEX"
+ fi
+
+ echo > trace
+ echo function_graph > current_tracer
+ enable_tracing
+ sleep 1
+ # search for functions (has "{" or ";" on the line)
+ echo 0 > tracing_on
+ count=`cat trace | grep -v '^#' | grep -e '{' -e ';' | grep -v "$TEST" | wc -l`
+ echo 1 > tracing_on
+ if [ $count -ne 0 ]; then
+ fail "Graph filtering not working by itself against $TEST?"
+ fi
+
+ # Make sure we did find something
+ echo 0 > tracing_on
+ count=`cat trace | grep -v '^#' | grep -e '{' -e ';' | grep "$TEST" | wc -l`
+ echo 1 > tracing_on
+ if [ $count -eq 0 ]; then
+ fail "No traces found with $TEST?"
+ fi
+}
+
+do_test '*sched*' 'sched'
+cd $INSTANCE1
+do_test '*lock*' 'lock'
+cd $WD
+cd $INSTANCE2
+do_test '*rcu*' 'rcu'
+cd $WD
+cd $INSTANCE3
+echo function_graph > current_tracer
+
+sleep 1
+count=`cat trace | grep -v '^#' | grep -e '{' -e ';' | grep "$TEST" | wc -l`
+if [ $count -eq 0 ]; then
+ fail "No traces found with all tracing?"
+fi
+
+cd $WD
+echo nop > current_tracer
+echo nop > $INSTANCE1/current_tracer
+echo nop > $INSTANCE2/current_tracer
+echo nop > $INSTANCE3/current_tracer
+
+do_reset
+
+exit 0
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (26 preceding siblings ...)
2024-06-03 19:07 ` [PATCH v3 27/27] selftests/ftrace: Add fgraph-multi.tc test Steven Rostedt
@ 2024-06-04 12:18 ` Steven Rostedt
2024-06-04 14:23 ` Masami Hiramatsu
2024-06-04 14:44 ` Mark Rutland
2024-06-05 14:07 ` Mark Rutland
28 siblings, 2 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-04 12:18 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: linux-kernel, linux-trace-kernel, Mark Rutland, Mathieu Desnoyers,
Andrew Morton, Alexei Starovoitov, Florent Revest,
Martin KaFai Lau, bpf, Sven Schnelle, Alexei Starovoitov,
Jiri Olsa, Arnaldo Carvalho de Melo, Daniel Borkmann,
Alan Maguire, Peter Zijlstra, Thomas Gleixner, Guo Ren
Masami,
This series passed all my tests, are you comfortable with me pushing
them to linux-next?
-- Steve
On Mon, 03 Jun 2024 15:07:04 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> This is a continuation of the function graph multi user code.
> I wrote a proof of concept back in 2019 of this code[1] and
> Masami started cleaning it up. I started from Masami's work v10
> that can be found here:
>
> https://lore.kernel.org/linux-trace-kernel/171509088006.162236.7227326999861366050.stgit@devnote2/
>
> This is *only* the code that allows multiple users of function
> graph tracing. This is not the fprobe work that Masami is working
> to add on top of it. As Masami took my proof of concept, there
> was still several things I disliked about that code. Instead of
> having Masami clean it up even more, I decided to take over on just
> my code and change it up a bit.
>
> Changes since v2: https://lore.kernel.org/linux-trace-kernel/20240602033744.563858532@goodmis.org
>
> - Added comments describing which hashes the append and intersect
> functions were used for.
>
> - Replaced checks of (NULL or EMPTY_HASH) with ftrace_hash_empty()
> helper function.
>
> - Added check at the end of intersect_hash() to convert the hash
> to EMPTY hash if it doesn't have any functions.
>
> - Renamed compare_ops() to ops_equal() and return boolean (inversed return
> value).
>
> - Broke out __ftrace_hash_move_and_update_ops() to use in both
> ftrace_hash_move_and_update_ops() and ftrace_hash_move_and_update_subops().
>
> Diff between last version at end of this email.
>
> Masami Hiramatsu (Google) (3):
> function_graph: Handle tail calls for stack unwinding
> function_graph: Use a simple LRU for fgraph_array index number
> ftrace: Add multiple fgraph storage selftest
>
> Steven Rostedt (Google) (9):
> ftrace: Add subops logic to allow one ops to manage many
> ftrace: Allow subops filtering to be modified
> function_graph: Add pid tracing back to function graph tracer
> function_graph: Use for_each_set_bit() in __ftrace_return_to_handler()
> function_graph: Use bitmask to loop on fgraph entry
> function_graph: Use static_call and branch to optimize entry function
> function_graph: Use static_call and branch to optimize return function
> selftests/ftrace: Add function_graph tracer to func-filter-pid test
> selftests/ftrace: Add fgraph-multi.tc test
>
> Steven Rostedt (VMware) (15):
> function_graph: Convert ret_stack to a series of longs
> fgraph: Use BUILD_BUG_ON() to make sure we have structures divisible by long
> function_graph: Add an array structure that will allow multiple callbacks
> function_graph: Allow multiple users to attach to function graph
> function_graph: Remove logic around ftrace_graph_entry and return
> ftrace/function_graph: Pass fgraph_ops to function graph callbacks
> ftrace: Allow function_graph tracer to be enabled in instances
> ftrace: Allow ftrace startup flags to exist without dynamic ftrace
> function_graph: Have the instances use their own ftrace_ops for filtering
> function_graph: Add "task variables" per task for fgraph_ops
> function_graph: Move set_graph_function tests to shadow stack global var
> function_graph: Move graph depth stored data to shadow stack global var
> function_graph: Move graph notrace bit to shadow stack global var
> function_graph: Implement fgraph_reserve_data() and fgraph_retrieve_data()
> function_graph: Add selftest for passing local variables
>
> ----
> include/linux/ftrace.h | 43 +-
> include/linux/sched.h | 2 +-
> include/linux/trace_recursion.h | 39 -
> kernel/trace/fgraph.c | 1044 ++++++++++++++++----
> kernel/trace/ftrace.c | 522 +++++++++-
> kernel/trace/ftrace_internal.h | 5 +-
> kernel/trace/trace.h | 94 +-
> kernel/trace/trace_functions.c | 8 +
> kernel/trace/trace_functions_graph.c | 96 +-
> kernel/trace/trace_irqsoff.c | 10 +-
> kernel/trace/trace_sched_wakeup.c | 10 +-
> kernel/trace/trace_selftest.c | 259 ++++-
> .../selftests/ftrace/test.d/ftrace/fgraph-multi.tc | 103 ++
> .../ftrace/test.d/ftrace/func-filter-pid.tc | 27 +-
> 14 files changed, 1945 insertions(+), 317 deletions(-)
> create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/fgraph-multi.tc
>
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 41fabc6d30e4..da7e6abf48b4 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -3170,7 +3170,7 @@ int ftrace_shutdown(struct ftrace_ops *ops, int command)
> /* Simply make a copy of @src and return it */
> static struct ftrace_hash *copy_hash(struct ftrace_hash *src)
> {
> - if (!src || src == EMPTY_HASH)
> + if (ftrace_hash_empty(src))
> return EMPTY_HASH;
>
> return alloc_and_copy_ftrace_hash(src->size_bits, src);
> @@ -3187,6 +3187,9 @@ static struct ftrace_hash *copy_hash(struct ftrace_hash *src)
> *
> * Otherwise, go through all of @new_hash and add anything that @hash
> * doesn't already have, to @hash.
> + *
> + * The filter_hash updates uses just the append_hash() function
> + * and the notrace_hash does not.
> */
> static int append_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash)
> {
> @@ -3195,11 +3198,11 @@ static int append_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash)
> int i;
>
> /* An empty hash does everything */
> - if (!*hash || *hash == EMPTY_HASH)
> + if (ftrace_hash_empty(*hash))
> return 0;
>
> /* If new_hash has everything make hash have everything */
> - if (!new_hash || new_hash == EMPTY_HASH) {
> + if (ftrace_hash_empty(new_hash)) {
> free_ftrace_hash(*hash);
> *hash = EMPTY_HASH;
> return 0;
> @@ -3217,7 +3220,12 @@ static int append_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash)
> return 0;
> }
>
> -/* Add to @hash only those that are in both @new_hash1 and @new_hash2 */
> +/*
> + * Add to @hash only those that are in both @new_hash1 and @new_hash2
> + *
> + * The notrace_hash updates uses just the intersect_hash() function
> + * and the filter_hash does not.
> + */
> static int intersect_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash1,
> struct ftrace_hash *new_hash2)
> {
> @@ -3229,8 +3237,7 @@ static int intersect_hash(struct ftrace_hash **hash, struct ftrace_hash *new_has
> * If new_hash1 or new_hash2 is the EMPTY_HASH then make the hash
> * empty as well as empty for notrace means none are notraced.
> */
> - if (!new_hash1 || new_hash1 == EMPTY_HASH ||
> - !new_hash2 || new_hash2 == EMPTY_HASH) {
> + if (ftrace_hash_empty(new_hash1) || ftrace_hash_empty(new_hash2)) {
> free_ftrace_hash(*hash);
> *hash = EMPTY_HASH;
> return 0;
> @@ -3245,6 +3252,11 @@ static int intersect_hash(struct ftrace_hash **hash, struct ftrace_hash *new_has
> return -ENOMEM;
> }
> }
> + /* If nothing intersects, make it the empty set */
> + if (ftrace_hash_empty(*hash)) {
> + free_ftrace_hash(*hash);
> + *hash = EMPTY_HASH;
> + }
> return 0;
> }
>
> @@ -3266,7 +3278,7 @@ static struct ftrace_hash *append_hashes(struct ftrace_ops *ops)
> return NULL;
> }
> /* Nothing more to do if new_hash is empty */
> - if (new_hash == EMPTY_HASH)
> + if (ftrace_hash_empty(new_hash))
> break;
> }
> return new_hash;
> @@ -3300,59 +3312,76 @@ static struct ftrace_hash *intersect_hashes(struct ftrace_ops *ops)
> return NULL;
> }
> /* Nothing more to do if new_hash is empty */
> - if (new_hash == EMPTY_HASH)
> + if (ftrace_hash_empty(new_hash))
> break;
> }
> return new_hash;
> }
>
> -/* Returns 0 on equal or non-zero on non-equal */
> -static int compare_ops(struct ftrace_hash *A, struct ftrace_hash *B)
> +static bool ops_equal(struct ftrace_hash *A, struct ftrace_hash *B)
> {
> struct ftrace_func_entry *entry;
> int size;
> int i;
>
> - if (!A || A == EMPTY_HASH)
> - return !(!B || B == EMPTY_HASH);
> + if (ftrace_hash_empty(A))
> + return ftrace_hash_empty(B);
>
> - if (!B || B == EMPTY_HASH)
> - return !(!A || A == EMPTY_HASH);
> + if (ftrace_hash_empty(B))
> + return ftrace_hash_empty(A);
>
> if (A->count != B->count)
> - return 1;
> + return false;
>
> size = 1 << A->size_bits;
> for (i = 0; i < size; i++) {
> hlist_for_each_entry(entry, &A->buckets[i], hlist) {
> if (!__ftrace_lookup_ip(B, entry->ip))
> - return 1;
> + return false;
> }
> }
>
> - return 0;
> + return true;
> }
>
> -static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
> - struct ftrace_hash **orig_hash,
> - struct ftrace_hash *hash,
> - int enable);
> +static void ftrace_ops_update_code(struct ftrace_ops *ops,
> + struct ftrace_ops_hash *old_hash);
> +
> +static int __ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
> + struct ftrace_hash **orig_hash,
> + struct ftrace_hash *hash,
> + int enable)
> +{
> + struct ftrace_ops_hash old_hash_ops;
> + struct ftrace_hash *old_hash;
> + int ret;
> +
> + old_hash = *orig_hash;
> + old_hash_ops.filter_hash = ops->func_hash->filter_hash;
> + old_hash_ops.notrace_hash = ops->func_hash->notrace_hash;
> + ret = ftrace_hash_move(ops, enable, orig_hash, hash);
> + if (!ret) {
> + ftrace_ops_update_code(ops, &old_hash_ops);
> + free_ftrace_hash_rcu(old_hash);
> + }
> + return ret;
> +}
>
> static int ftrace_update_ops(struct ftrace_ops *ops, struct ftrace_hash *filter_hash,
> struct ftrace_hash *notrace_hash)
> {
> int ret;
>
> - if (compare_ops(filter_hash, ops->func_hash->filter_hash)) {
> - ret = ftrace_hash_move_and_update_ops(ops, &ops->func_hash->filter_hash,
> - filter_hash, 1);
> + if (!ops_equal(filter_hash, ops->func_hash->filter_hash)) {
> + ret = __ftrace_hash_move_and_update_ops(ops, &ops->func_hash->filter_hash,
> + filter_hash, 1);
> if (ret < 0)
> return ret;
> }
>
> - if (compare_ops(notrace_hash, ops->func_hash->notrace_hash)) {
> - ret = ftrace_hash_move_and_update_ops(ops, &ops->func_hash->notrace_hash,
> - notrace_hash, 0);
> + if (!ops_equal(notrace_hash, ops->func_hash->notrace_hash)) {
> + ret = __ftrace_hash_move_and_update_ops(ops, &ops->func_hash->notrace_hash,
> + notrace_hash, 0);
> if (ret < 0)
> return ret;
> }
> @@ -3438,8 +3467,8 @@ int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int
> * o If either notrace_hash is empty then the final stays empty
> * o Otherwise, the final is an intersection between the hashes
> */
> - if (ops->func_hash->filter_hash == EMPTY_HASH ||
> - subops->func_hash->filter_hash == EMPTY_HASH) {
> + if (ftrace_hash_empty(ops->func_hash->filter_hash) ||
> + ftrace_hash_empty(subops->func_hash->filter_hash)) {
> filter_hash = EMPTY_HASH;
> } else {
> size_bits = max(ops->func_hash->filter_hash->size_bits,
> @@ -3454,8 +3483,8 @@ int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int
> }
> }
>
> - if (ops->func_hash->notrace_hash == EMPTY_HASH ||
> - subops->func_hash->notrace_hash == EMPTY_HASH) {
> + if (ftrace_hash_empty(ops->func_hash->notrace_hash) ||
> + ftrace_hash_empty(subops->func_hash->notrace_hash)) {
> notrace_hash = EMPTY_HASH;
> } else {
> size_bits = max(ops->func_hash->filter_hash->size_bits,
> @@ -3591,7 +3620,7 @@ static int ftrace_hash_move_and_update_subops(struct ftrace_ops *subops,
> }
>
> /* Move the hash over to the new hash */
> - ret = ftrace_hash_move_and_update_ops(ops, orig_hash, new_hash, enable);
> + ret = __ftrace_hash_move_and_update_ops(ops, orig_hash, new_hash, enable);
>
> free_ftrace_hash(new_hash);
>
> @@ -4822,11 +4851,6 @@ static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
> struct ftrace_hash *hash,
> int enable)
> {
> - struct ftrace_ops_hash old_hash_ops;
> - struct ftrace_hash *old_hash;
> - struct ftrace_ops *op;
> - int ret;
> -
> if (ops->flags & FTRACE_OPS_FL_SUBOP)
> return ftrace_hash_move_and_update_subops(ops, orig_hash, hash, enable);
>
> @@ -4838,6 +4862,8 @@ static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
> * it will not affect subops that share it.
> */
> if (!(ops->flags & FTRACE_OPS_FL_ENABLED)) {
> + struct ftrace_ops *op;
> +
> /* Check if any other manager subops maps to this hash */
> do_for_each_ftrace_op(op, ftrace_ops_list) {
> struct ftrace_ops *subops;
> @@ -4851,15 +4877,7 @@ static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
> } while_for_each_ftrace_op(op);
> }
>
> - old_hash = *orig_hash;
> - old_hash_ops.filter_hash = ops->func_hash->filter_hash;
> - old_hash_ops.notrace_hash = ops->func_hash->notrace_hash;
> - ret = ftrace_hash_move(ops, enable, orig_hash, hash);
> - if (!ret) {
> - ftrace_ops_update_code(ops, &old_hash_ops);
> - free_ftrace_hash_rcu(old_hash);
> - }
> - return ret;
> + return __ftrace_hash_move_and_update_ops(ops, orig_hash, hash, enable);
> }
>
> static bool module_exists(const char *module)
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing
2024-06-04 12:18 ` [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
@ 2024-06-04 14:23 ` Masami Hiramatsu
2024-06-04 14:44 ` Mark Rutland
1 sibling, 0 replies; 37+ messages in thread
From: Masami Hiramatsu @ 2024-06-04 14:23 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, linux-trace-kernel, Mark Rutland, Mathieu Desnoyers,
Andrew Morton, Alexei Starovoitov, Florent Revest,
Martin KaFai Lau, bpf, Sven Schnelle, Alexei Starovoitov,
Jiri Olsa, Arnaldo Carvalho de Melo, Daniel Borkmann,
Alan Maguire, Peter Zijlstra, Thomas Gleixner, Guo Ren
On Tue, 4 Jun 2024 08:18:50 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Masami,
>
> This series passed all my tests, are you comfortable with me pushing
> them to linux-next?
Yes, this series looks good to me too.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
for the series.
Thank you!
>
> -- Steve
>
>
> On Mon, 03 Jun 2024 15:07:04 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > This is a continuation of the function graph multi user code.
> > I wrote a proof of concept back in 2019 of this code[1] and
> > Masami started cleaning it up. I started from Masami's work v10
> > that can be found here:
> >
> > https://lore.kernel.org/linux-trace-kernel/171509088006.162236.7227326999861366050.stgit@devnote2/
> >
> > This is *only* the code that allows multiple users of function
> > graph tracing. This is not the fprobe work that Masami is working
> > to add on top of it. As Masami took my proof of concept, there
> > was still several things I disliked about that code. Instead of
> > having Masami clean it up even more, I decided to take over on just
> > my code and change it up a bit.
> >
> > Changes since v2: https://lore.kernel.org/linux-trace-kernel/20240602033744.563858532@goodmis.org
> >
> > - Added comments describing which hashes the append and intersect
> > functions were used for.
> >
> > - Replaced checks of (NULL or EMPTY_HASH) with ftrace_hash_empty()
> > helper function.
> >
> > - Added check at the end of intersect_hash() to convert the hash
> > to EMPTY hash if it doesn't have any functions.
> >
> > - Renamed compare_ops() to ops_equal() and return boolean (inversed return
> > value).
> >
> > - Broke out __ftrace_hash_move_and_update_ops() to use in both
> > ftrace_hash_move_and_update_ops() and ftrace_hash_move_and_update_subops().
> >
> > Diff between last version at end of this email.
> >
> > Masami Hiramatsu (Google) (3):
> > function_graph: Handle tail calls for stack unwinding
> > function_graph: Use a simple LRU for fgraph_array index number
> > ftrace: Add multiple fgraph storage selftest
> >
> > Steven Rostedt (Google) (9):
> > ftrace: Add subops logic to allow one ops to manage many
> > ftrace: Allow subops filtering to be modified
> > function_graph: Add pid tracing back to function graph tracer
> > function_graph: Use for_each_set_bit() in __ftrace_return_to_handler()
> > function_graph: Use bitmask to loop on fgraph entry
> > function_graph: Use static_call and branch to optimize entry function
> > function_graph: Use static_call and branch to optimize return function
> > selftests/ftrace: Add function_graph tracer to func-filter-pid test
> > selftests/ftrace: Add fgraph-multi.tc test
> >
> > Steven Rostedt (VMware) (15):
> > function_graph: Convert ret_stack to a series of longs
> > fgraph: Use BUILD_BUG_ON() to make sure we have structures divisible by long
> > function_graph: Add an array structure that will allow multiple callbacks
> > function_graph: Allow multiple users to attach to function graph
> > function_graph: Remove logic around ftrace_graph_entry and return
> > ftrace/function_graph: Pass fgraph_ops to function graph callbacks
> > ftrace: Allow function_graph tracer to be enabled in instances
> > ftrace: Allow ftrace startup flags to exist without dynamic ftrace
> > function_graph: Have the instances use their own ftrace_ops for filtering
> > function_graph: Add "task variables" per task for fgraph_ops
> > function_graph: Move set_graph_function tests to shadow stack global var
> > function_graph: Move graph depth stored data to shadow stack global var
> > function_graph: Move graph notrace bit to shadow stack global var
> > function_graph: Implement fgraph_reserve_data() and fgraph_retrieve_data()
> > function_graph: Add selftest for passing local variables
> >
> > ----
> > include/linux/ftrace.h | 43 +-
> > include/linux/sched.h | 2 +-
> > include/linux/trace_recursion.h | 39 -
> > kernel/trace/fgraph.c | 1044 ++++++++++++++++----
> > kernel/trace/ftrace.c | 522 +++++++++-
> > kernel/trace/ftrace_internal.h | 5 +-
> > kernel/trace/trace.h | 94 +-
> > kernel/trace/trace_functions.c | 8 +
> > kernel/trace/trace_functions_graph.c | 96 +-
> > kernel/trace/trace_irqsoff.c | 10 +-
> > kernel/trace/trace_sched_wakeup.c | 10 +-
> > kernel/trace/trace_selftest.c | 259 ++++-
> > .../selftests/ftrace/test.d/ftrace/fgraph-multi.tc | 103 ++
> > .../ftrace/test.d/ftrace/func-filter-pid.tc | 27 +-
> > 14 files changed, 1945 insertions(+), 317 deletions(-)
> > create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/fgraph-multi.tc
> >
> >
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index 41fabc6d30e4..da7e6abf48b4 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -3170,7 +3170,7 @@ int ftrace_shutdown(struct ftrace_ops *ops, int command)
> > /* Simply make a copy of @src and return it */
> > static struct ftrace_hash *copy_hash(struct ftrace_hash *src)
> > {
> > - if (!src || src == EMPTY_HASH)
> > + if (ftrace_hash_empty(src))
> > return EMPTY_HASH;
> >
> > return alloc_and_copy_ftrace_hash(src->size_bits, src);
> > @@ -3187,6 +3187,9 @@ static struct ftrace_hash *copy_hash(struct ftrace_hash *src)
> > *
> > * Otherwise, go through all of @new_hash and add anything that @hash
> > * doesn't already have, to @hash.
> > + *
> > + * The filter_hash updates uses just the append_hash() function
> > + * and the notrace_hash does not.
> > */
> > static int append_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash)
> > {
> > @@ -3195,11 +3198,11 @@ static int append_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash)
> > int i;
> >
> > /* An empty hash does everything */
> > - if (!*hash || *hash == EMPTY_HASH)
> > + if (ftrace_hash_empty(*hash))
> > return 0;
> >
> > /* If new_hash has everything make hash have everything */
> > - if (!new_hash || new_hash == EMPTY_HASH) {
> > + if (ftrace_hash_empty(new_hash)) {
> > free_ftrace_hash(*hash);
> > *hash = EMPTY_HASH;
> > return 0;
> > @@ -3217,7 +3220,12 @@ static int append_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash)
> > return 0;
> > }
> >
> > -/* Add to @hash only those that are in both @new_hash1 and @new_hash2 */
> > +/*
> > + * Add to @hash only those that are in both @new_hash1 and @new_hash2
> > + *
> > + * The notrace_hash updates uses just the intersect_hash() function
> > + * and the filter_hash does not.
> > + */
> > static int intersect_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash1,
> > struct ftrace_hash *new_hash2)
> > {
> > @@ -3229,8 +3237,7 @@ static int intersect_hash(struct ftrace_hash **hash, struct ftrace_hash *new_has
> > * If new_hash1 or new_hash2 is the EMPTY_HASH then make the hash
> > * empty as well as empty for notrace means none are notraced.
> > */
> > - if (!new_hash1 || new_hash1 == EMPTY_HASH ||
> > - !new_hash2 || new_hash2 == EMPTY_HASH) {
> > + if (ftrace_hash_empty(new_hash1) || ftrace_hash_empty(new_hash2)) {
> > free_ftrace_hash(*hash);
> > *hash = EMPTY_HASH;
> > return 0;
> > @@ -3245,6 +3252,11 @@ static int intersect_hash(struct ftrace_hash **hash, struct ftrace_hash *new_has
> > return -ENOMEM;
> > }
> > }
> > + /* If nothing intersects, make it the empty set */
> > + if (ftrace_hash_empty(*hash)) {
> > + free_ftrace_hash(*hash);
> > + *hash = EMPTY_HASH;
> > + }
> > return 0;
> > }
> >
> > @@ -3266,7 +3278,7 @@ static struct ftrace_hash *append_hashes(struct ftrace_ops *ops)
> > return NULL;
> > }
> > /* Nothing more to do if new_hash is empty */
> > - if (new_hash == EMPTY_HASH)
> > + if (ftrace_hash_empty(new_hash))
> > break;
> > }
> > return new_hash;
> > @@ -3300,59 +3312,76 @@ static struct ftrace_hash *intersect_hashes(struct ftrace_ops *ops)
> > return NULL;
> > }
> > /* Nothing more to do if new_hash is empty */
> > - if (new_hash == EMPTY_HASH)
> > + if (ftrace_hash_empty(new_hash))
> > break;
> > }
> > return new_hash;
> > }
> >
> > -/* Returns 0 on equal or non-zero on non-equal */
> > -static int compare_ops(struct ftrace_hash *A, struct ftrace_hash *B)
> > +static bool ops_equal(struct ftrace_hash *A, struct ftrace_hash *B)
> > {
> > struct ftrace_func_entry *entry;
> > int size;
> > int i;
> >
> > - if (!A || A == EMPTY_HASH)
> > - return !(!B || B == EMPTY_HASH);
> > + if (ftrace_hash_empty(A))
> > + return ftrace_hash_empty(B);
> >
> > - if (!B || B == EMPTY_HASH)
> > - return !(!A || A == EMPTY_HASH);
> > + if (ftrace_hash_empty(B))
> > + return ftrace_hash_empty(A);
> >
> > if (A->count != B->count)
> > - return 1;
> > + return false;
> >
> > size = 1 << A->size_bits;
> > for (i = 0; i < size; i++) {
> > hlist_for_each_entry(entry, &A->buckets[i], hlist) {
> > if (!__ftrace_lookup_ip(B, entry->ip))
> > - return 1;
> > + return false;
> > }
> > }
> >
> > - return 0;
> > + return true;
> > }
> >
> > -static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
> > - struct ftrace_hash **orig_hash,
> > - struct ftrace_hash *hash,
> > - int enable);
> > +static void ftrace_ops_update_code(struct ftrace_ops *ops,
> > + struct ftrace_ops_hash *old_hash);
> > +
> > +static int __ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
> > + struct ftrace_hash **orig_hash,
> > + struct ftrace_hash *hash,
> > + int enable)
> > +{
> > + struct ftrace_ops_hash old_hash_ops;
> > + struct ftrace_hash *old_hash;
> > + int ret;
> > +
> > + old_hash = *orig_hash;
> > + old_hash_ops.filter_hash = ops->func_hash->filter_hash;
> > + old_hash_ops.notrace_hash = ops->func_hash->notrace_hash;
> > + ret = ftrace_hash_move(ops, enable, orig_hash, hash);
> > + if (!ret) {
> > + ftrace_ops_update_code(ops, &old_hash_ops);
> > + free_ftrace_hash_rcu(old_hash);
> > + }
> > + return ret;
> > +}
> >
> > static int ftrace_update_ops(struct ftrace_ops *ops, struct ftrace_hash *filter_hash,
> > struct ftrace_hash *notrace_hash)
> > {
> > int ret;
> >
> > - if (compare_ops(filter_hash, ops->func_hash->filter_hash)) {
> > - ret = ftrace_hash_move_and_update_ops(ops, &ops->func_hash->filter_hash,
> > - filter_hash, 1);
> > + if (!ops_equal(filter_hash, ops->func_hash->filter_hash)) {
> > + ret = __ftrace_hash_move_and_update_ops(ops, &ops->func_hash->filter_hash,
> > + filter_hash, 1);
> > if (ret < 0)
> > return ret;
> > }
> >
> > - if (compare_ops(notrace_hash, ops->func_hash->notrace_hash)) {
> > - ret = ftrace_hash_move_and_update_ops(ops, &ops->func_hash->notrace_hash,
> > - notrace_hash, 0);
> > + if (!ops_equal(notrace_hash, ops->func_hash->notrace_hash)) {
> > + ret = __ftrace_hash_move_and_update_ops(ops, &ops->func_hash->notrace_hash,
> > + notrace_hash, 0);
> > if (ret < 0)
> > return ret;
> > }
> > @@ -3438,8 +3467,8 @@ int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int
> > * o If either notrace_hash is empty then the final stays empty
> > * o Otherwise, the final is an intersection between the hashes
> > */
> > - if (ops->func_hash->filter_hash == EMPTY_HASH ||
> > - subops->func_hash->filter_hash == EMPTY_HASH) {
> > + if (ftrace_hash_empty(ops->func_hash->filter_hash) ||
> > + ftrace_hash_empty(subops->func_hash->filter_hash)) {
> > filter_hash = EMPTY_HASH;
> > } else {
> > size_bits = max(ops->func_hash->filter_hash->size_bits,
> > @@ -3454,8 +3483,8 @@ int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int
> > }
> > }
> >
> > - if (ops->func_hash->notrace_hash == EMPTY_HASH ||
> > - subops->func_hash->notrace_hash == EMPTY_HASH) {
> > + if (ftrace_hash_empty(ops->func_hash->notrace_hash) ||
> > + ftrace_hash_empty(subops->func_hash->notrace_hash)) {
> > notrace_hash = EMPTY_HASH;
> > } else {
> > size_bits = max(ops->func_hash->filter_hash->size_bits,
> > @@ -3591,7 +3620,7 @@ static int ftrace_hash_move_and_update_subops(struct ftrace_ops *subops,
> > }
> >
> > /* Move the hash over to the new hash */
> > - ret = ftrace_hash_move_and_update_ops(ops, orig_hash, new_hash, enable);
> > + ret = __ftrace_hash_move_and_update_ops(ops, orig_hash, new_hash, enable);
> >
> > free_ftrace_hash(new_hash);
> >
> > @@ -4822,11 +4851,6 @@ static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
> > struct ftrace_hash *hash,
> > int enable)
> > {
> > - struct ftrace_ops_hash old_hash_ops;
> > - struct ftrace_hash *old_hash;
> > - struct ftrace_ops *op;
> > - int ret;
> > -
> > if (ops->flags & FTRACE_OPS_FL_SUBOP)
> > return ftrace_hash_move_and_update_subops(ops, orig_hash, hash, enable);
> >
> > @@ -4838,6 +4862,8 @@ static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
> > * it will not affect subops that share it.
> > */
> > if (!(ops->flags & FTRACE_OPS_FL_ENABLED)) {
> > + struct ftrace_ops *op;
> > +
> > /* Check if any other manager subops maps to this hash */
> > do_for_each_ftrace_op(op, ftrace_ops_list) {
> > struct ftrace_ops *subops;
> > @@ -4851,15 +4877,7 @@ static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
> > } while_for_each_ftrace_op(op);
> > }
> >
> > - old_hash = *orig_hash;
> > - old_hash_ops.filter_hash = ops->func_hash->filter_hash;
> > - old_hash_ops.notrace_hash = ops->func_hash->notrace_hash;
> > - ret = ftrace_hash_move(ops, enable, orig_hash, hash);
> > - if (!ret) {
> > - ftrace_ops_update_code(ops, &old_hash_ops);
> > - free_ftrace_hash_rcu(old_hash);
> > - }
> > - return ret;
> > + return __ftrace_hash_move_and_update_ops(ops, orig_hash, hash, enable);
> > }
> >
> > static bool module_exists(const char *module)
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing
2024-06-04 12:18 ` [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
2024-06-04 14:23 ` Masami Hiramatsu
@ 2024-06-04 14:44 ` Mark Rutland
2024-06-04 16:31 ` Steven Rostedt
1 sibling, 1 reply; 37+ messages in thread
From: Mark Rutland @ 2024-06-04 14:44 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, linux-kernel, linux-trace-kernel,
Mathieu Desnoyers, Andrew Morton, Alexei Starovoitov,
Florent Revest, Martin KaFai Lau, bpf, Sven Schnelle,
Alexei Starovoitov, Jiri Olsa, Arnaldo Carvalho de Melo,
Daniel Borkmann, Alan Maguire, Peter Zijlstra, Thomas Gleixner,
Guo Ren
Hi Steve, Masami,
On Tue, Jun 04, 2024 at 08:18:50AM -0400, Steven Rostedt wrote:
>
> Masami,
>
> This series passed all my tests, are you comfortable with me pushing
> them to linux-next?
As a heads-up (and not to block pushing this into next), I just gave
this a spin on arm64 atop v6.10-rc2, and running the selftests I see:
ftrace - function pid filters
(instance) ftrace - function pid filters
... both go from [PASS] to [FAIL].
Everything else looks good -- I'll go dig into why that's happening.
It's possible that's just something odd with the filesystem I'm using
(e.g. the wnership test failed because this lacks 'stat').
Mark.
>
> -- Steve
>
>
> On Mon, 03 Jun 2024 15:07:04 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > This is a continuation of the function graph multi user code.
> > I wrote a proof of concept back in 2019 of this code[1] and
> > Masami started cleaning it up. I started from Masami's work v10
> > that can be found here:
> >
> > https://lore.kernel.org/linux-trace-kernel/171509088006.162236.7227326999861366050.stgit@devnote2/
> >
> > This is *only* the code that allows multiple users of function
> > graph tracing. This is not the fprobe work that Masami is working
> > to add on top of it. As Masami took my proof of concept, there
> > was still several things I disliked about that code. Instead of
> > having Masami clean it up even more, I decided to take over on just
> > my code and change it up a bit.
> >
> > Changes since v2: https://lore.kernel.org/linux-trace-kernel/20240602033744.563858532@goodmis.org
> >
> > - Added comments describing which hashes the append and intersect
> > functions were used for.
> >
> > - Replaced checks of (NULL or EMPTY_HASH) with ftrace_hash_empty()
> > helper function.
> >
> > - Added check at the end of intersect_hash() to convert the hash
> > to EMPTY hash if it doesn't have any functions.
> >
> > - Renamed compare_ops() to ops_equal() and return boolean (inversed return
> > value).
> >
> > - Broke out __ftrace_hash_move_and_update_ops() to use in both
> > ftrace_hash_move_and_update_ops() and ftrace_hash_move_and_update_subops().
> >
> > Diff between last version at end of this email.
> >
> > Masami Hiramatsu (Google) (3):
> > function_graph: Handle tail calls for stack unwinding
> > function_graph: Use a simple LRU for fgraph_array index number
> > ftrace: Add multiple fgraph storage selftest
> >
> > Steven Rostedt (Google) (9):
> > ftrace: Add subops logic to allow one ops to manage many
> > ftrace: Allow subops filtering to be modified
> > function_graph: Add pid tracing back to function graph tracer
> > function_graph: Use for_each_set_bit() in __ftrace_return_to_handler()
> > function_graph: Use bitmask to loop on fgraph entry
> > function_graph: Use static_call and branch to optimize entry function
> > function_graph: Use static_call and branch to optimize return function
> > selftests/ftrace: Add function_graph tracer to func-filter-pid test
> > selftests/ftrace: Add fgraph-multi.tc test
> >
> > Steven Rostedt (VMware) (15):
> > function_graph: Convert ret_stack to a series of longs
> > fgraph: Use BUILD_BUG_ON() to make sure we have structures divisible by long
> > function_graph: Add an array structure that will allow multiple callbacks
> > function_graph: Allow multiple users to attach to function graph
> > function_graph: Remove logic around ftrace_graph_entry and return
> > ftrace/function_graph: Pass fgraph_ops to function graph callbacks
> > ftrace: Allow function_graph tracer to be enabled in instances
> > ftrace: Allow ftrace startup flags to exist without dynamic ftrace
> > function_graph: Have the instances use their own ftrace_ops for filtering
> > function_graph: Add "task variables" per task for fgraph_ops
> > function_graph: Move set_graph_function tests to shadow stack global var
> > function_graph: Move graph depth stored data to shadow stack global var
> > function_graph: Move graph notrace bit to shadow stack global var
> > function_graph: Implement fgraph_reserve_data() and fgraph_retrieve_data()
> > function_graph: Add selftest for passing local variables
> >
> > ----
> > include/linux/ftrace.h | 43 +-
> > include/linux/sched.h | 2 +-
> > include/linux/trace_recursion.h | 39 -
> > kernel/trace/fgraph.c | 1044 ++++++++++++++++----
> > kernel/trace/ftrace.c | 522 +++++++++-
> > kernel/trace/ftrace_internal.h | 5 +-
> > kernel/trace/trace.h | 94 +-
> > kernel/trace/trace_functions.c | 8 +
> > kernel/trace/trace_functions_graph.c | 96 +-
> > kernel/trace/trace_irqsoff.c | 10 +-
> > kernel/trace/trace_sched_wakeup.c | 10 +-
> > kernel/trace/trace_selftest.c | 259 ++++-
> > .../selftests/ftrace/test.d/ftrace/fgraph-multi.tc | 103 ++
> > .../ftrace/test.d/ftrace/func-filter-pid.tc | 27 +-
> > 14 files changed, 1945 insertions(+), 317 deletions(-)
> > create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/fgraph-multi.tc
> >
> >
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index 41fabc6d30e4..da7e6abf48b4 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -3170,7 +3170,7 @@ int ftrace_shutdown(struct ftrace_ops *ops, int command)
> > /* Simply make a copy of @src and return it */
> > static struct ftrace_hash *copy_hash(struct ftrace_hash *src)
> > {
> > - if (!src || src == EMPTY_HASH)
> > + if (ftrace_hash_empty(src))
> > return EMPTY_HASH;
> >
> > return alloc_and_copy_ftrace_hash(src->size_bits, src);
> > @@ -3187,6 +3187,9 @@ static struct ftrace_hash *copy_hash(struct ftrace_hash *src)
> > *
> > * Otherwise, go through all of @new_hash and add anything that @hash
> > * doesn't already have, to @hash.
> > + *
> > + * The filter_hash updates uses just the append_hash() function
> > + * and the notrace_hash does not.
> > */
> > static int append_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash)
> > {
> > @@ -3195,11 +3198,11 @@ static int append_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash)
> > int i;
> >
> > /* An empty hash does everything */
> > - if (!*hash || *hash == EMPTY_HASH)
> > + if (ftrace_hash_empty(*hash))
> > return 0;
> >
> > /* If new_hash has everything make hash have everything */
> > - if (!new_hash || new_hash == EMPTY_HASH) {
> > + if (ftrace_hash_empty(new_hash)) {
> > free_ftrace_hash(*hash);
> > *hash = EMPTY_HASH;
> > return 0;
> > @@ -3217,7 +3220,12 @@ static int append_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash)
> > return 0;
> > }
> >
> > -/* Add to @hash only those that are in both @new_hash1 and @new_hash2 */
> > +/*
> > + * Add to @hash only those that are in both @new_hash1 and @new_hash2
> > + *
> > + * The notrace_hash updates uses just the intersect_hash() function
> > + * and the filter_hash does not.
> > + */
> > static int intersect_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash1,
> > struct ftrace_hash *new_hash2)
> > {
> > @@ -3229,8 +3237,7 @@ static int intersect_hash(struct ftrace_hash **hash, struct ftrace_hash *new_has
> > * If new_hash1 or new_hash2 is the EMPTY_HASH then make the hash
> > * empty as well as empty for notrace means none are notraced.
> > */
> > - if (!new_hash1 || new_hash1 == EMPTY_HASH ||
> > - !new_hash2 || new_hash2 == EMPTY_HASH) {
> > + if (ftrace_hash_empty(new_hash1) || ftrace_hash_empty(new_hash2)) {
> > free_ftrace_hash(*hash);
> > *hash = EMPTY_HASH;
> > return 0;
> > @@ -3245,6 +3252,11 @@ static int intersect_hash(struct ftrace_hash **hash, struct ftrace_hash *new_has
> > return -ENOMEM;
> > }
> > }
> > + /* If nothing intersects, make it the empty set */
> > + if (ftrace_hash_empty(*hash)) {
> > + free_ftrace_hash(*hash);
> > + *hash = EMPTY_HASH;
> > + }
> > return 0;
> > }
> >
> > @@ -3266,7 +3278,7 @@ static struct ftrace_hash *append_hashes(struct ftrace_ops *ops)
> > return NULL;
> > }
> > /* Nothing more to do if new_hash is empty */
> > - if (new_hash == EMPTY_HASH)
> > + if (ftrace_hash_empty(new_hash))
> > break;
> > }
> > return new_hash;
> > @@ -3300,59 +3312,76 @@ static struct ftrace_hash *intersect_hashes(struct ftrace_ops *ops)
> > return NULL;
> > }
> > /* Nothing more to do if new_hash is empty */
> > - if (new_hash == EMPTY_HASH)
> > + if (ftrace_hash_empty(new_hash))
> > break;
> > }
> > return new_hash;
> > }
> >
> > -/* Returns 0 on equal or non-zero on non-equal */
> > -static int compare_ops(struct ftrace_hash *A, struct ftrace_hash *B)
> > +static bool ops_equal(struct ftrace_hash *A, struct ftrace_hash *B)
> > {
> > struct ftrace_func_entry *entry;
> > int size;
> > int i;
> >
> > - if (!A || A == EMPTY_HASH)
> > - return !(!B || B == EMPTY_HASH);
> > + if (ftrace_hash_empty(A))
> > + return ftrace_hash_empty(B);
> >
> > - if (!B || B == EMPTY_HASH)
> > - return !(!A || A == EMPTY_HASH);
> > + if (ftrace_hash_empty(B))
> > + return ftrace_hash_empty(A);
> >
> > if (A->count != B->count)
> > - return 1;
> > + return false;
> >
> > size = 1 << A->size_bits;
> > for (i = 0; i < size; i++) {
> > hlist_for_each_entry(entry, &A->buckets[i], hlist) {
> > if (!__ftrace_lookup_ip(B, entry->ip))
> > - return 1;
> > + return false;
> > }
> > }
> >
> > - return 0;
> > + return true;
> > }
> >
> > -static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
> > - struct ftrace_hash **orig_hash,
> > - struct ftrace_hash *hash,
> > - int enable);
> > +static void ftrace_ops_update_code(struct ftrace_ops *ops,
> > + struct ftrace_ops_hash *old_hash);
> > +
> > +static int __ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
> > + struct ftrace_hash **orig_hash,
> > + struct ftrace_hash *hash,
> > + int enable)
> > +{
> > + struct ftrace_ops_hash old_hash_ops;
> > + struct ftrace_hash *old_hash;
> > + int ret;
> > +
> > + old_hash = *orig_hash;
> > + old_hash_ops.filter_hash = ops->func_hash->filter_hash;
> > + old_hash_ops.notrace_hash = ops->func_hash->notrace_hash;
> > + ret = ftrace_hash_move(ops, enable, orig_hash, hash);
> > + if (!ret) {
> > + ftrace_ops_update_code(ops, &old_hash_ops);
> > + free_ftrace_hash_rcu(old_hash);
> > + }
> > + return ret;
> > +}
> >
> > static int ftrace_update_ops(struct ftrace_ops *ops, struct ftrace_hash *filter_hash,
> > struct ftrace_hash *notrace_hash)
> > {
> > int ret;
> >
> > - if (compare_ops(filter_hash, ops->func_hash->filter_hash)) {
> > - ret = ftrace_hash_move_and_update_ops(ops, &ops->func_hash->filter_hash,
> > - filter_hash, 1);
> > + if (!ops_equal(filter_hash, ops->func_hash->filter_hash)) {
> > + ret = __ftrace_hash_move_and_update_ops(ops, &ops->func_hash->filter_hash,
> > + filter_hash, 1);
> > if (ret < 0)
> > return ret;
> > }
> >
> > - if (compare_ops(notrace_hash, ops->func_hash->notrace_hash)) {
> > - ret = ftrace_hash_move_and_update_ops(ops, &ops->func_hash->notrace_hash,
> > - notrace_hash, 0);
> > + if (!ops_equal(notrace_hash, ops->func_hash->notrace_hash)) {
> > + ret = __ftrace_hash_move_and_update_ops(ops, &ops->func_hash->notrace_hash,
> > + notrace_hash, 0);
> > if (ret < 0)
> > return ret;
> > }
> > @@ -3438,8 +3467,8 @@ int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int
> > * o If either notrace_hash is empty then the final stays empty
> > * o Otherwise, the final is an intersection between the hashes
> > */
> > - if (ops->func_hash->filter_hash == EMPTY_HASH ||
> > - subops->func_hash->filter_hash == EMPTY_HASH) {
> > + if (ftrace_hash_empty(ops->func_hash->filter_hash) ||
> > + ftrace_hash_empty(subops->func_hash->filter_hash)) {
> > filter_hash = EMPTY_HASH;
> > } else {
> > size_bits = max(ops->func_hash->filter_hash->size_bits,
> > @@ -3454,8 +3483,8 @@ int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int
> > }
> > }
> >
> > - if (ops->func_hash->notrace_hash == EMPTY_HASH ||
> > - subops->func_hash->notrace_hash == EMPTY_HASH) {
> > + if (ftrace_hash_empty(ops->func_hash->notrace_hash) ||
> > + ftrace_hash_empty(subops->func_hash->notrace_hash)) {
> > notrace_hash = EMPTY_HASH;
> > } else {
> > size_bits = max(ops->func_hash->filter_hash->size_bits,
> > @@ -3591,7 +3620,7 @@ static int ftrace_hash_move_and_update_subops(struct ftrace_ops *subops,
> > }
> >
> > /* Move the hash over to the new hash */
> > - ret = ftrace_hash_move_and_update_ops(ops, orig_hash, new_hash, enable);
> > + ret = __ftrace_hash_move_and_update_ops(ops, orig_hash, new_hash, enable);
> >
> > free_ftrace_hash(new_hash);
> >
> > @@ -4822,11 +4851,6 @@ static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
> > struct ftrace_hash *hash,
> > int enable)
> > {
> > - struct ftrace_ops_hash old_hash_ops;
> > - struct ftrace_hash *old_hash;
> > - struct ftrace_ops *op;
> > - int ret;
> > -
> > if (ops->flags & FTRACE_OPS_FL_SUBOP)
> > return ftrace_hash_move_and_update_subops(ops, orig_hash, hash, enable);
> >
> > @@ -4838,6 +4862,8 @@ static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
> > * it will not affect subops that share it.
> > */
> > if (!(ops->flags & FTRACE_OPS_FL_ENABLED)) {
> > + struct ftrace_ops *op;
> > +
> > /* Check if any other manager subops maps to this hash */
> > do_for_each_ftrace_op(op, ftrace_ops_list) {
> > struct ftrace_ops *subops;
> > @@ -4851,15 +4877,7 @@ static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
> > } while_for_each_ftrace_op(op);
> > }
> >
> > - old_hash = *orig_hash;
> > - old_hash_ops.filter_hash = ops->func_hash->filter_hash;
> > - old_hash_ops.notrace_hash = ops->func_hash->notrace_hash;
> > - ret = ftrace_hash_move(ops, enable, orig_hash, hash);
> > - if (!ret) {
> > - ftrace_ops_update_code(ops, &old_hash_ops);
> > - free_ftrace_hash_rcu(old_hash);
> > - }
> > - return ret;
> > + return __ftrace_hash_move_and_update_ops(ops, orig_hash, hash, enable);
> > }
> >
> > static bool module_exists(const char *module)
>
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing
2024-06-04 14:44 ` Mark Rutland
@ 2024-06-04 16:31 ` Steven Rostedt
2024-06-04 17:04 ` Mark Rutland
0 siblings, 1 reply; 37+ messages in thread
From: Steven Rostedt @ 2024-06-04 16:31 UTC (permalink / raw)
To: Mark Rutland
Cc: Masami Hiramatsu, linux-kernel, linux-trace-kernel,
Mathieu Desnoyers, Andrew Morton, Alexei Starovoitov,
Florent Revest, Martin KaFai Lau, bpf, Sven Schnelle,
Alexei Starovoitov, Jiri Olsa, Arnaldo Carvalho de Melo,
Daniel Borkmann, Alan Maguire, Peter Zijlstra, Thomas Gleixner,
Guo Ren
On Tue, 4 Jun 2024 15:44:40 +0100
Mark Rutland <mark.rutland@arm.com> wrote:
> Hi Steve, Masami,
>
> On Tue, Jun 04, 2024 at 08:18:50AM -0400, Steven Rostedt wrote:
> >
> > Masami,
> >
> > This series passed all my tests, are you comfortable with me pushing
> > them to linux-next?
>
> As a heads-up (and not to block pushing this into next), I just gave
> this a spin on arm64 atop v6.10-rc2, and running the selftests I see:
>
> ftrace - function pid filters
> (instance) ftrace - function pid filters
>
> ... both go from [PASS] to [FAIL].
>
> Everything else looks good -- I'll go dig into why that's happening.
>
> It's possible that's just something odd with the filesystem I'm using
> (e.g. the wnership test failed because this lacks 'stat').
Thanks for the update. I could be something I missed in patch 13 that had
to put back the pid code.
There may have been something arch specific that I'm unaware about. I'll
look at that deeper.
-- Steve
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing
2024-06-04 16:31 ` Steven Rostedt
@ 2024-06-04 17:04 ` Mark Rutland
2024-06-04 18:57 ` Steven Rostedt
0 siblings, 1 reply; 37+ messages in thread
From: Mark Rutland @ 2024-06-04 17:04 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, linux-kernel, linux-trace-kernel,
Mathieu Desnoyers, Andrew Morton, Alexei Starovoitov,
Florent Revest, Martin KaFai Lau, bpf, Sven Schnelle,
Alexei Starovoitov, Jiri Olsa, Arnaldo Carvalho de Melo,
Daniel Borkmann, Alan Maguire, Peter Zijlstra, Thomas Gleixner,
Guo Ren
On Tue, Jun 04, 2024 at 12:31:24PM -0400, Steven Rostedt wrote:
> On Tue, 4 Jun 2024 15:44:40 +0100
> Mark Rutland <mark.rutland@arm.com> wrote:
>
> > Hi Steve, Masami,
> >
> > On Tue, Jun 04, 2024 at 08:18:50AM -0400, Steven Rostedt wrote:
> > >
> > > Masami,
> > >
> > > This series passed all my tests, are you comfortable with me pushing
> > > them to linux-next?
> >
> > As a heads-up (and not to block pushing this into next), I just gave
> > this a spin on arm64 atop v6.10-rc2, and running the selftests I see:
> >
> > ftrace - function pid filters
> > (instance) ftrace - function pid filters
> >
> > ... both go from [PASS] to [FAIL].
> >
> > Everything else looks good -- I'll go dig into why that's happening.
> >
> > It's possible that's just something odd with the filesystem I'm using
> > (e.g. the wnership test failed because this lacks 'stat').
>
> Thanks for the update. I could be something I missed in patch 13 that had
> to put back the pid code.
>
> There may have been something arch specific that I'm unaware about. I'll
> look at that deeper.
It looks like e are lines in the trace that it doesn't expect:
+ cat trace
+ grep -v ^#
+ grep 970
+ wc -l
+ count_pid=0
+ cat trace
+ grep -v ^#
+ grep -v 970
+ wc -l
+ count_other=3
+ [ 0 -eq 0 -o 3 -ne 0 ]
+ fail PID filtering not working?
... where we expect that count_other to be 0.
I hacked in a 'cat trace' just before the 'fail' and that shows:
+ cat trace
# tracer: function_graph
#
# CPU DURATION FUNCTION CALLS
# | | | | | | |
3) ! 143.685 us | kernel_clone();
3) ! 127.055 us | kernel_clone();
1) ! 127.170 us | kernel_clone();
3) ! 126.840 us | kernel_clone();
I'm not sure if that's legitimate output the test is failing to account
for or if that indicates a kernel-side issue.
Mark.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing
2024-06-04 17:04 ` Mark Rutland
@ 2024-06-04 18:57 ` Steven Rostedt
2024-06-04 19:18 ` Steven Rostedt
0 siblings, 1 reply; 37+ messages in thread
From: Steven Rostedt @ 2024-06-04 18:57 UTC (permalink / raw)
To: Mark Rutland
Cc: Masami Hiramatsu, linux-kernel, linux-trace-kernel,
Mathieu Desnoyers, Andrew Morton, Alexei Starovoitov,
Florent Revest, Martin KaFai Lau, bpf, Sven Schnelle,
Alexei Starovoitov, Jiri Olsa, Arnaldo Carvalho de Melo,
Daniel Borkmann, Alan Maguire, Peter Zijlstra, Thomas Gleixner,
Guo Ren
On Tue, 4 Jun 2024 18:04:22 +0100
Mark Rutland <mark.rutland@arm.com> wrote:
> > There may have been something arch specific that I'm unaware about. I'll
> > look at that deeper.
>
> It looks like e are lines in the trace that it doesn't expect:
>
> + cat trace
> + grep -v ^#
> + grep 970
> + wc -l
> + count_pid=0
> + cat trace
> + grep -v ^#
> + grep -v 970
> + wc -l
> + count_other=3
> + [ 0 -eq 0 -o 3 -ne 0 ]
> + fail PID filtering not working?
>
> ... where we expect that count_other to be 0.
>
> I hacked in a 'cat trace' just before the 'fail' and that shows:
>
> + cat trace
> # tracer: function_graph
> #
> # CPU DURATION FUNCTION CALLS
> # | | | | | | |
> 3) ! 143.685 us | kernel_clone();
> 3) ! 127.055 us | kernel_clone();
> 1) ! 127.170 us | kernel_clone();
> 3) ! 126.840 us | kernel_clone();
>
> I'm not sure if that's legitimate output the test is failing to account
> for or if that indicates a kernel-side issue.
Bah, I just ran the test.d/ftrace/func-filter-pid.tc and it fails too. This
did pass my other tests that do run ftracetests. Hmm, I just ran it on my
test box that does the tests and it passes there. I wonder if there's some
config option that makes it fail :-/
Well, now that I see it fail, I can investigate.
-- Steve
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing
2024-06-04 18:57 ` Steven Rostedt
@ 2024-06-04 19:18 ` Steven Rostedt
0 siblings, 0 replies; 37+ messages in thread
From: Steven Rostedt @ 2024-06-04 19:18 UTC (permalink / raw)
To: Mark Rutland
Cc: Masami Hiramatsu, linux-kernel, linux-trace-kernel,
Mathieu Desnoyers, Andrew Morton, Alexei Starovoitov,
Florent Revest, Martin KaFai Lau, bpf, Sven Schnelle,
Alexei Starovoitov, Jiri Olsa, Arnaldo Carvalho de Melo,
Daniel Borkmann, Alan Maguire, Peter Zijlstra, Thomas Gleixner,
Guo Ren
On Tue, 4 Jun 2024 14:57:42 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> Bah, I just ran the test.d/ftrace/func-filter-pid.tc and it fails too. This
> did pass my other tests that do run ftracetests. Hmm, I just ran it on my
> test box that does the tests and it passes there. I wonder if there's some
> config option that makes it fail :-/
>
> Well, now that I see it fail, I can investigate.
Ah, figured it out. The updated pid test isn't working. This explains why
my test machine didn't fail, as it doesn't have the updated ftracetests.
The problem was that I never set the funcgraph-proc option, even though I
saved it :-p
That is, it shows:
> + cat trace
> # tracer: function_graph
> #
> # CPU DURATION FUNCTION CALLS
> # | | | | | | |
> 3) ! 143.685 us | kernel_clone();
> 3) ! 127.055 us | kernel_clone();
> 1) ! 127.170 us | kernel_clone();
> 3) ! 126.840 us | kernel_clone();
But when you do: echo 1 > options/funcgraph-proc
You get:
# cat trace
# tracer: function_graph
#
# CPU TASK/PID DURATION FUNCTION CALLS
# | | | | | | | | |
4) bash-939 | # 1070.009 us | kernel_clone();
4) bash-939 | # 1116.903 us | kernel_clone();
5) bash-939 | ! 976.133 us | kernel_clone();
5) bash-939 | ! 954.012 us | kernel_clone();
5) bash-939 | ! 905.825 us | kernel_clone();
5) bash-939 | # 1130.922 us | kernel_clone();
7) bash-939 | # 1097.648 us | kernel_clone();
0) bash-939 | # 1008.000 us | kernel_clone();
3) bash-939 | # 1023.391 us | kernel_clone();
4) bash-939 | # 1033.008 us | kernel_clone();
4) bash-939 | ! 949.072 us | kernel_clone();
4) bash-939 | # 1027.990 us | kernel_clone();
4) bash-939 | ! 954.678 us | kernel_clone();
4) bash-939 | ! 996.557 us | kernel_clone();
Without that option, function graph does no show what process is being
recorded (except at sched switch)
Can you add this patch to the test and see if it works again?
-- Steve
diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc
index c6fc9d31a496..8dcce001881d 100644
--- a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc
+++ b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc
@@ -8,6 +8,7 @@
# Also test it on an instance directory
do_function_fork=1
+do_funcgraph_proc=1
if [ ! -f options/function-fork ]; then
do_function_fork=0
@@ -28,6 +29,7 @@ fi
if [ $do_funcgraph_proc -eq 1 ]; then
orig_value2=`cat options/funcgraph-proc`
+ echo 1 > options/funcgraph-proc
fi
do_reset() {
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing
2024-06-03 19:07 [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
` (27 preceding siblings ...)
2024-06-04 12:18 ` [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing Steven Rostedt
@ 2024-06-05 14:07 ` Mark Rutland
28 siblings, 0 replies; 37+ messages in thread
From: Mark Rutland @ 2024-06-05 14:07 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, linux-trace-kernel, Masami Hiramatsu,
Mathieu Desnoyers, Andrew Morton, Alexei Starovoitov,
Florent Revest, Martin KaFai Lau, bpf, Sven Schnelle,
Alexei Starovoitov, Jiri Olsa, Arnaldo Carvalho de Melo,
Daniel Borkmann, Alan Maguire, Peter Zijlstra, Thomas Gleixner,
Guo Ren
On Mon, Jun 03, 2024 at 03:07:04PM -0400, Steven Rostedt wrote:
> This is a continuation of the function graph multi user code.
> I wrote a proof of concept back in 2019 of this code[1] and
> Masami started cleaning it up. I started from Masami's work v10
> that can be found here:
>
> https://lore.kernel.org/linux-trace-kernel/171509088006.162236.7227326999861366050.stgit@devnote2/
>
> This is *only* the code that allows multiple users of function
> graph tracing. This is not the fprobe work that Masami is working
> to add on top of it. As Masami took my proof of concept, there
> was still several things I disliked about that code. Instead of
> having Masami clean it up even more, I decided to take over on just
> my code and change it up a bit.
FWIW, this is useful to me as-is for testing stacktracing.
Before this series I had some horrid hacks to manipulate the global
filters, and after this series I just need to manipulate the filters in
fgraph_ops::ops, which is *much* nicer.
I've pushed out my WIP to:
https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=stacktrace/tests
... which is all to say, this is useful and I've given this some testing
beyond the usual ftrace tests.
Mark.
^ permalink raw reply [flat|nested] 37+ messages in thread