function_graph: Rewrite to allow multiple users
Subsystems (4)
Steven Rostedt
maintainerv2 cover function_graph: Rewrite to allow multiple users
The background for this is explained in the V1 version found here:
http://lkml.kernel.org/r/[email protected]
The TL;DR; is this:
The function graph tracer required a rewrite, mainly because it can only allow one callback registered at a time. The main motivation for this change is to allow kretprobes to use the code of function graph tracer, which should allow all archs that have function graph tracing to also have kretprobes with no extra work.
Masami told me that one requirement was to allow the function entry callback to store data on the shadow stack that can be retrieved by the the function return callback. I added this, as well as a per-task variable (used by one of the function graph users).
The two functions to allow the storing of data on the stack and retrieval of it are:
void *fgraph_reserve_data(int size_in_bytes)
Allows the entry function to reserve up to 4 words of data on the shadow stack. On success, a pointer to the contents is returned. This may be only called once per entry function.
void *fgraph_retrieve_data(void)
Allows the return function to retrieve the reserved data that was allocated by the entry function.
Note, this code has passed my full test suite.
Changes since v1:
- Well, the first part of that series was already merged. But that was just the preparation for this part.
- Allocate a page for the shadow stack split it up that way. When the stack is full, we stop allowing more to be added (stop tracing).
- Added the reserve and retrieve of private data on the shadow stack for individual entry/return callbacks to pass data to each other.
- Added a "per task" data that can be used by a fgraph_ops for all function callbacks for a specific task.
Steven Rostedt (VMware) (14):
function_graph: Convert ret_stack to a series of longs
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 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 | 37 +- include/linux/sched.h | 2 +- kernel/trace/fgraph.c | 862 ++++++++++++++++++++++++++++------- kernel/trace/ftrace.c | 13 +- kernel/trace/ftrace_internal.h | 2 - kernel/trace/trace.h | 132 +++--- kernel/trace/trace_functions.c | 7 + 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 | 168 ++++++- 11 files changed, 1048 insertions(+), 291 deletions(-)
Steven Rostedt
maintainerRFC PATCH v2 1/14 function_graph: Convert ret_stack to a series of longs
From: "Steven Rostedt (VMware)" <[email protected]>
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.
--- include/linux/sched.h | 2 +- kernel/trace/fgraph.c | 124 ++++++++++++++++++++++++------------------ 2 files changed, 71 insertions(+), 55 deletions(-)
include/linux/sched.h +1 -1
@@ -1113,7 +1113,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;
kernel/trace/fgraph.c +70 -54
307 lines not on this page · raw message
Signature · 1 line
2.20.1
Peter Zijlstra
maintainerRe: [RFC][PATCH 01/14 v2] function_graph: Convert ret_stack to a series of longs
kernel/trace/fgraph.c line 27
Quoted 2 lines · Steven Rostedt
kernel/trace/fgraph.c +2
-0+#define FGRAPH_RET_SIZE (sizeof(struct ftrace_ret_stack)) +#define FGRAPH_RET_INDEX (ALIGN(FGRAPH_RET_SIZE, sizeof(long)) / sizeof(long))
I think you want to write that like:
BUILD_BUG_ON(sizeof(ftrace_ret_stack) % sizeof(long));
It'd be very weird for that sizeof not to be right.
Quoted 1 line
+#define
SHADOW_STACK_SIZE(PAGE_SIZE)
Do we really need that big a shadow stack?
kernel/trace/fgraph.c line 36
Quoted 8 lines
kernel/trace/fgraph.c +8
-0+#define SHADOW_STACK_INDEX \ + (ALIGN(SHADOW_STACK_SIZE, sizeof(long)) / sizeof(long)) +/* Leave on a buffer at the end */ +#define SHADOW_STACK_MAX_INDEX (SHADOW_STACK_INDEX - FGRAPH_RET_INDEX) + +#define RET_STACK (t, index) ((struct ftrace_ret_stack *)(&(t)->ret_stack[index])) +#define RET_STACK_INC (c) ({ c += FGRAPH_RET_INDEX; }) +#define RET_STACK_DEC (c) ({ c -= FGRAPH_RET_INDEX; })
I'm thinking something like:
#define RET_PUSH(s, val) \
do { \
(s) -= sizeof(val); \
(typeof(val) *)(s) = val; \
} while (0)#define RET_POP(s, type) \
({ \
type *__ptr = (void *)(s); \
(s) += sizeof(type); \
*__ptr; \
})Would me clearer?
Steven Rostedt
maintainerRe: [RFC][PATCH 01/14 v2] function_graph: Convert ret_stack to a series of longs
Quoted 8 lines · On Fri, 24 May 2019 13:11:44 +0200 Peter Zijlstra <[email protected]> wrote:
kernel/trace/fgraph.c line 27
Quoted 2 lines · Steven Rostedt
kernel/trace/fgraph.c +2
-0+#define FGRAPH_RET_SIZE (sizeof(struct ftrace_ret_stack)) +#define FGRAPH_RET_INDEX (ALIGN(FGRAPH_RET_SIZE, sizeof(long)) / sizeof(long))I think you want to write that like:
BUILD_BUG_ON(sizeof(ftrace_ret_stack) % sizeof(long));
Sure.
Quoted 2 lines
It'd be very weird for that sizeof not to be right.
Agreed, but I was paranoid. The BUILD_BUG_ON() would also work.
Quoted 4 lines
Quoted 2 lines
+#define
SHADOW_STACK_SIZE(PAGE_SIZE)Do we really need that big a shadow stack?
Well, this is a sticky point. I allow up to 16 users at a time (although I can't imagine more than 5, but you never know), and each user adds a long and up to 4 more words (which is probably unlikely anyway). And then we can have deep call stacks (we are getting deeper each release it seems).
I figured, I start with a page size, and then in the future we can make it dynamic, or shrink it if it proves to be too much.
Quoted 18 lines
kernel/trace/fgraph.c line 36
Quoted 9 lines
kernel/trace/fgraph.c +8
-0+#define SHADOW_STACK_INDEX \ + (ALIGN(SHADOW_STACK_SIZE, sizeof(long)) / sizeof(long)) +/* Leave on a buffer at the end */ +#define SHADOW_STACK_MAX_INDEX (SHADOW_STACK_INDEX - FGRAPH_RET_INDEX) + +#define RET_STACK (t, index) ((struct ftrace_ret_stack *)(&(t)->ret_stack[index])) +#define RET_STACK_INC (c) ({ c += FGRAPH_RET_INDEX; }) +#define RET_STACK_DEC (c) ({ c -= FGRAPH_RET_INDEX; })I'm thinking something like:
#define
RET_PUSH(s, val)\do { \ (s) -= sizeof(val); \ (typeof(val) *)(s) = val; \ } while (0)
Quoted 8 lines
#define
RET_POP(s, type)\({ \ type *__ptr = (void *)(s); \ (s) += sizeof(type); \ *__ptr; \ })Would me clearer?
Due to races with interrupts, and this not being an atomic operation, I had to play tricks with moving the stack pointer and adding data to it. So I wanted to keep the changing of the stack pointer and adding and retrieving of the stack data separate.
Later patches remove the RET_STACK_INC/DEC() anyway.
Thanks for taking the time to look at these patches!
-- Steve
Masami Hiramatsu
maintainerRe: [RFC][PATCH 01/14 v2] function_graph: Convert ret_stack to a series of longs
Quoted 5 lines · On Fri, 24 May 2019 08:05:53 -0400 Steven Rostedt <[email protected]> wrote:
Quoted 4 lines
Quoted 2 lines
+#define
SHADOW_STACK_SIZE(PAGE_SIZE)Do we really need that big a shadow stack?
Quoted 8 lines
Well, this is a sticky point. I allow up to 16 users at a time (although I can't imagine more than 5, but you never know), and each user adds a long and up to 4 more words (which is probably unlikely anyway). And then we can have deep call stacks (we are getting deeper each release it seems).
I figured, I start with a page size, and then in the future we can make it dynamic, or shrink it if it proves to be too much.
I'd prefer dynamic allocation, based on the number of users or actual stack starvation.
Thank you,
Signature · 1 line
Masami Hiramatsu <[email protected]>
Steven Rostedt
maintainerRe: [RFC][PATCH 01/14 v2] function_graph: Convert ret_stack to a series of longs
Quoted 11 lines · On Mon, 3 Jun 2019 20:30:49 +0900 Masami Hiramatsu <[email protected]> wrote:
Quoted 11 lines
Quoted 4 lines
Quoted 2 lines
+#define
SHADOW_STACK_SIZE(PAGE_SIZE)Do we really need that big a shadow stack?
Well, this is a sticky point. I allow up to 16 users at a time (although I can't imagine more than 5, but you never know), and each user adds a long and up to 4 more words (which is probably unlikely anyway). And then we can have deep call stacks (we are getting deeper each release it seems).
Quoted 5 lines
Quoted 2 lines
I figured, I start with a page size, and then in the future we can make it dynamic, or shrink it if it proves to be too much.
I'd prefer dynamic allocation, based on the number of users or actual stack starvation.
As stated, it's something we can improve on in the future. I'll probably be pushing out this series for linux-next, and then we can incrementally improve it.
First on my list is to add a REGS version of function_graph such that kretprobes can use it ;-)
-- Steve
Steven Rostedt
maintainerRFC PATCH v2 2/14 function_graph: Add an array structure that will allow multiple callbacks
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
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).
--- kernel/trace/fgraph.c | 98 ++++++++++++++++++++++++++++--------------- 1 file changed, 65 insertions(+), 33 deletions(-)
kernel/trace/fgraph.c +65 -33
@@ -38,9 +38,27 @@static bool kill_ftrace_graph;int ftrace_graph_active; +#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; +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_is_dead - returns true if ftrace_graph_stop() was called *
@@ -123,7 +141,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 ;
@@ -231,7 +249,7 @@ unsigned long ftrace_return_to_handler(unsigned long frame_pointer)ftrace_pop_return_trace (&trace, &ret, frame_pointer); 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
@@ -349,11 +367,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 ; -} -/* The callbacks that hook a function */ trace_func_graph_ret_t ftrace_graph_return = (trace_func_graph_ret_t )ftrace_stub;
@@ -586,37 +599,53 @@ 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; 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;
@@ -624,19 +653,22 @@ 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 = (trace_func_graph_ret_t )ftrace_stub; - 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 ); - +if (!ftrace_graph_active) { + ftrace_graph_return = (trace_func_graph_ret_t )ftrace_stub; + 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); }
Signature · 1 line
2.20.1
Steven Rostedt
maintainerRFC PATCH v2 3/14 function_graph: Allow multiple users to attach to function graph
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
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 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 wont 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.
--- include/linux/ftrace.h | 2 + kernel/trace/fgraph.c | 343 +++++++++++++++++++++++++++++++++++------ 2 files changed, 296 insertions(+), 49 deletions(-)
include/linux/ftrace.h +2 -0
@@ -787,6 +787,8 @@ 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); +int function_graph_enter (unsigned long ret,unsigned long func, +unsigned long frame_pointer,unsigned long *retp);/* * Sometimes we don't want to trace a function with the function * graph tracer but we want them to keep traced by the usual function
kernel/trace/fgraph.c +294 -49
500 lines not on this page · raw message
Signature · 1 line
2.20.1
Peter Zijlstra
maintainerRe: [RFC][PATCH 03/14 v2] function_graph: Allow multiple users to attach to function graph
Quoted 9 lines · Steven Rostedt
From: "Steven Rostedt (VMware)" <[email protected]>
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 a function being traced is entered, each of theftrace_opsentryfunc is called and if it returns non zero, its index into the array will be added to the shadow stack.
Quoted 8 lines
On exit of the function being traced, the shadow stack will contain the indexes of the
ftrace_opson 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_opsis removed, its place on the array is replaced with aftrace_opsthat contains the stub functions and that will be called when the function finally returns.
But but but but.. why not add all the required bits to the shadow stack in the first place and do away with the array entirely?
So on ret, just keep POP'ing until either the stack is empty or the entry is for another function.
Steven Rostedt
maintainerRe: [RFC][PATCH 03/14 v2] function_graph: Allow multiple users to attach to function graph
Quoted 2 lines · On Fri, 24 May 2019 13:26:08 +0200 Peter Zijlstra <[email protected]> wrote:
But but but but.. why not add all the required bits to the shadow stack in the first place and do away with the array entirely?
What required bits would that be? The pointer to the fgraph_ops, because we need that to pass to the calling function.
Quoted 3 lines
So on ret, just keep POP'ing until either the stack is empty or the entry is for another function.
When we hit a fgraph_ops, how do we know if it was freed or not? We can't just blindly reference it.
The idea of the array, is that we can maintain state in a single location of when the fgraph_ops is freed. If we return from a function, we have an index and a counter, and if the counter doesn't match with what's in the array, then we know that the fgraph_ops is no longer around and we just drop it.
The reason for the array, is to keep track of if the fgraph_ops has been freed or not. Otherwise, when we unregister the fgraph_ops, we would need to search all shadow stacks, looking for it to unreference it.
Believe me, I rather not have that array, but I couldn't come up with a better solution to handle freeing of fgraph_ops.
-- Steve
Peter Zijlstra
maintainerRe: [RFC][PATCH 03/14 v2] function_graph: Allow multiple users to attach to function graph
Quoted 8 lines · Steven Rostedt
Quoted 2 lines · On Fri, 24 May 2019 13:26:08 +0200 Peter Zijlstra <[email protected]> wrote:
But but but but.. why not add all the required bits to the shadow stack in the first place and do away with the array entirely?
What required bits would that be? The pointer to the
fgraph_ops, because we need that to pass to the calling function.
I was thinking a smaller structure comprising of {func,callback}, which you pop, if func matches, run callback.
Quoted 12 lines
Quoted 2 lines
So on ret, just keep POP'ing until either the stack is empty or the entry is for another function.
When we hit a
fgraph_ops, how do we know if it was freed or not? We can't just blindly reference it.The idea of the array, is that we can maintain state in a single location of when the
fgraph_opsis freed. If we return from a function, we have an index and a counter, and if the counter doesn't match with what's in the array, then we know that thefgraph_opsis no longer around and we just drop it.
Quoted 7 lines
The reason for the array, is to keep track of if the
fgraph_opshas been freed or not. Otherwise, when we unregister thefgraph_ops, we would need to search all shadow stacks, looking for it to unreference it.Believe me, I rather not have that array, but I couldn't come up with a better solution to handle freeing of
fgraph_ops.
The trivial answer would be to refcount the thing, but can't we make rcu_tasks do this?
And delay the unreg until all active users are gone -- who gives a crap that can take a while.
Steven Rostedt
maintainerRe: [RFC][PATCH 03/14 v2] function_graph: Allow multiple users to attach to function graph
Quoted 5 lines · On Fri, 24 May 2019 14:27:24 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 2 lines
Believe me, I rather not have that array, but I couldn't come up with a better solution to handle freeing of
fgraph_ops.The trivial answer would be to refcount the thing, but can't we make
rcu_tasksdo this?
But wouldn't refcounts require atomic operations, something that would be excruciatingly slow for something that runs on all functions.
rcu_tasks doesn't cross voluntary sleeps, which this does.
Quoted 3 lines
And delay the unreg until all active users are gone -- who gives a crap that can take a while.
It could literally be forever (well, until the machine reboots). And something that could appear to be a memory leak, although a very slow one. But probably be hard to have more than the number of tasks on the system.
-- Steve
Peter Zijlstra
maintainerRe: [RFC][PATCH 03/14 v2] function_graph: Allow multiple users to attach to function graph
Quoted 11 lines · Steven Rostedt
Quoted 5 lines · On Fri, 24 May 2019 14:27:24 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 2 lines
Believe me, I rather not have that array, but I couldn't come up with a better solution to handle freeing of
fgraph_ops.The trivial answer would be to refcount the thing, but can't we make
rcu_tasksdo this?But wouldn't refcounts require atomic operations, something that would be excruciatingly slow for something that runs on all functions.
Obviously, which is why I suggested something else :-)
Quoted 1 line
rcu_tasksdoesn't cross voluntary sleeps, which this does.
Sure, but we can 'fix' that, surely. Alternatively we use SRCU, or something else, a blend between SRCU and percpu-rwsem for example, SRCU has that annoying smp_mb() on the read side, where percpu-rwsem doesn't have that.
Quoted 7 lines
Quoted 2 lines
And delay the unreg until all active users are gone -- who gives a crap that can take a while.
It could literally be forever (well, until the machine reboots). And something that could appear to be a memory leak, although a very slow one. But probably be hard to have more than the number of tasks on the system.
Again, who cares.. ? How often do you have return trace functions that dissapear, afaict that only happens with modules, and neither function_graph_trace nor kprobes are modules.
It'll just mean the module unload will be stuck, possibly forever. That's not something I care about. Also, if we _really_ care, we can mandate that module users use some sort of ugly trampoline that covers their asses at the cost of some performance.
Getting rid of that array makes this code far saner (and I suspect faster too).
Steven Rostedt
maintainerRe: [RFC][PATCH 03/14 v2] function_graph: Allow multiple users to attach to function graph
Quoted 3 lines · On Mon, 27 May 2019 12:10:04 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 1 line
rcu_tasksdoesn't cross voluntary sleeps, which this does.Sure, but we can 'fix' that, surely.
Well, that's the point of the rcu_tasks. To let us know when a task has voluntarily slept. I don't think we want to "fix" that.
Quoted 17 lines
Alternatively we use SRCU, or
something else, a blend between SRCU and percpu-rwsem for example, SRCU has that annoyingsmp_mb()on the read side, where percpu-rwsem doesn't have that.Quoted 7 lines
Quoted 2 lines
And delay the unreg until all active users are gone -- who gives a crap that can take a while.
It could literally be forever (well, until the machine reboots). And something that could appear to be a memory leak, although a very slow one. But probably be hard to have more than the number of tasks on the system.
Again, who cares.. ? How often do you have return trace functions that dissapear, afaict that only happens with modules, and neither
function_graph_tracenor kprobes are modules.
Quoted 7 lines
It'll just mean the module unload will be stuck, possibly forever. That's not something I care about. Also, if we _really_ care, we can mandate that module users use some sort of ugly trampoline that covers their asses at the cost of some performance.
Getting rid of that array makes this code far saner (and I suspect faster too).
The array is not the complex part of this. It was probably the easiest part of this patch series. It just shows up a lot in the beginning because I needed it to work before doing anything else. The more difficult parts came with the passing of data from entry to exit.
I plan on keeping the array for now, as it is just an internal implementation detail, that gives us only a limitation of the array size that is noticed outside of the function graph code. If we find some kind of RCU alternative, then we can switch to that in the future and remove the array limitation.
-- Steve
Steven Rostedt
maintainerRFC PATCH v2 4/14 function_graph: Remove logic around ftrace_graph_entry and return
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
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 repectively 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.
--- kernel/trace/fgraph.c | 71 +++++++++------------------------- kernel/trace/ftrace.c | 2 - kernel/trace/ftrace_internal.h | 2 - 3 files changed, 19 insertions(+), 56 deletions(-)
kernel/trace/fgraph.c +19 -52
@@ -127,6 +127,18 @@ static inline int get_fgraph_array(struct task_struct *t, int offset)FGRAPH_ARRAY_MASK ; } +/* 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) +{ +return ; +} +/* * @offset: The index into @t->ret_stack to find the ret_stack entry * @index: Where to place the index into @t->ret_stack of that entry
@@ -285,6 +297,9 @@ int function_graph_enter(unsigned long ret, unsigned long func,int cnt =0 ;int i; +if (!ftrace_ops_test (&global_ops, func,NULL )) + goto out; + trace.func = func; trace.depth = ++current->curr_ret_depth;
@@ -598,7 +613,6 @@ void ftrace_graph_sleep_time_control(bool enable)trace_func_graph_ret_t ftrace_graph_return = (trace_func_graph_ret_t )ftrace_stub;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)
@@ -680,46 +694,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
@@ -862,18 +836,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 ); }
@@ -909,7 +877,6 @@ void unregister_ftrace_graph(struct fgraph_ops *gops)if (!ftrace_graph_active) { ftrace_graph_return = (trace_func_graph_ret_t )ftrace_stub; 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 );
kernel/trace/ftrace.c +0 -2
@@ -212,8 +212,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 ;
kernel/trace/ftrace_internal.h +0 -2
@@ -63,10 +63,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 */
Signature · 1 line
2.20.1
Steven Rostedt
maintainerRFC PATCH v2 5/14 ftrace/function_graph: Pass fgraph_ops to function graph callbacks
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
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.
--- include/linux/ftrace.h | 10 +++++++--- kernel/trace/fgraph.c | 14 ++++++++------ 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, 39 insertions(+), 23 deletions(-)
include/linux/ftrace.h +7 -3
@@ -737,11 +737,15 @@ struct ftrace_graph_ret {int depth; } __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
kernel/trace/fgraph.c +8 -6
@@ -128,13 +128,13 @@ static inline int get_fgraph_array(struct task_struct *t, int offset) }/* 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) {return ; }
@@ -177,12 +177,14 @@ get_ret_stack(struct task_struct *t, int offset, int *index)/* Both enabled by default (can be cleared by function_graph tracer flags */ static bool fgraph_sleep_time = true; -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) { }
@@ -320,7 +322,7 @@ int function_graph_enter(unsigned long ret, unsigned long func,atomic_inc (¤t->trace_overrun);break ; } -if (fgraph_array[i]->entryfunc (&trace)) { +if (fgraph_array[i]->entryfunc (&trace, fgraph_array[i])) { offset = current->curr_ret_stack;/* Check the top level stored word */ type =get_fgraph_type (current, offset -1 );
@@ -487,7 +489,7 @@ unsigned long ftrace_return_to_handler(unsigned long frame_pointer) i =0 ;do { idx =get_fgraph_array (current, offset - i); - fgraph_array[idx]->retfunc (&trace); + fgraph_array[idx]->retfunc (&trace, fgraph_array[idx]); i++; }while (i < index);
kernel/trace/ftrace.c +4 -2
@@ -790,7 +790,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;
@@ -807,7 +808,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;
kernel/trace/trace.h +2 -2
@@ -737,8 +737,8 @@ void trace_default_header(struct seq_file *m);void print_trace_header (struct seq_file *m,struct trace_iterator *iter);int trace_empty (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 );
kernel/trace/trace_functions_graph.c +7 -4
@@ -124,7 +124,8 @@ static inline int ftrace_graph_ignore_irqs(void)return in_irq (); } -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;
@@ -237,7 +238,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;
@@ -274,7 +276,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);
@@ -287,7 +290,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 = {
kernel/trace/trace_irqsoff.c +4 -2
@@ -172,7 +172,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;
@@ -202,7 +203,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;
kernel/trace/trace_sched_wakeup.c +4 -2
@@ -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;
@@ -142,7 +143,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;
kernel/trace/trace_selftest.c +3 -2
@@ -724,7 +724,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 )) {
@@ -738,7 +739,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 = {
Signature · 1 line
2.20.1
Steven Rostedt
maintainerRFC PATCH v2 6/14 ftrace: Allow function_graph tracer to be enabled in instances
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
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.
--- include/linux/ftrace.h | 1 + kernel/trace/ftrace.c | 1 + kernel/trace/trace.h | 12 +++++ kernel/trace/trace_functions.c | 7 +++ kernel/trace/trace_functions_graph.c | 65 +++++++++++++++++----------- kernel/trace/trace_selftest.c | 2 +- 6 files changed, 62 insertions(+), 26 deletions(-)
include/linux/ftrace.h +1 -0
@@ -752,6 +752,7 @@ extern int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace, struct fgraphstruct fgraph_ops {trace_func_graph_ent_t entryfunc;trace_func_graph_ret_t retfunc; +void *private; };/*
kernel/trace/ftrace.c +1 -0
@@ -6224,6 +6224,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)
kernel/trace/trace.h +12 -0
@@ -313,6 +313,9 @@ struct trace_array {#ifdef CONFIG_FUNCTION_TRACER struct ftrace_ops *ops;struct trace_pid_list __rcu *function_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;
@@ -930,6 +933,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 long flags,int pc); +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 *ftrace_graph_hash;
@@ -1023,6 +1029,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;
kernel/trace/trace_functions.c +7 -0
@@ -68,6 +68,12 @@ int ftrace_create_function_files(struct trace_array *tr,if (ret)return ret; + ret =allocate_fgraph_ops (tr); +if (ret) { +kfree (tr->ops); +return ret; + } +ftrace_create_filter_files (tr->ops, parent);return 0 ;
@@ -78,6 +84,7 @@ void ftrace_destroy_function_files(struct trace_array *tr)ftrace_destroy_filter_files (tr->ops);kfree (tr->ops); tr->ops =NULL ; +free_fgraph_ops (tr); }static int function_trace_init (struct trace_array *tr)
kernel/trace/trace_functions_graph.c +40 -25
118 lines not on this page · raw message
kernel/trace/trace_selftest.c +1 -1
@@ -770,7 +770,7 @@ trace_selftest_startup_function_graph(struct tracer *trace,* to detect and recover from possible hangs */ tracing_reset_online_cpus (&tr->trace_buffer); -set_graph_array (tr); + fgraph_ops.private = tr; ret =register_ftrace_graph (&fgraph_ops);if (ret) {warn_failed_init_tracer (trace, ret);
Signature · 1 line
2.20.1
Steven Rostedt
maintainerRFC PATCH v2 7/14 ftrace: Allow ftrace startup flags exist without dynamic ftrace
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
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().
--- include/linux/ftrace.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
include/linux/ftrace.h +9 -9
@@ -286,6 +286,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 int ftrace_arch_code_modify_prepare (void );
@@ -373,15 +382,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()
Signature · 1 line
2.20.1
Steven Rostedt
maintainerRFC PATCH v2 8/14 function_graph: Have the instances use their own ftrace_ops for filtering
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
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.
--- include/linux/ftrace.h | 1 + kernel/trace/fgraph.c | 63 +++++++++++++++++----------- kernel/trace/ftrace.c | 6 +-- kernel/trace/trace.h | 16 +++---- kernel/trace/trace_functions.c | 2 +- kernel/trace/trace_functions_graph.c | 8 +++- 6 files changed, 59 insertions(+), 37 deletions(-)
include/linux/ftrace.h +1 -0
@@ -752,6 +752,7 @@ extern int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace, struct fgraphstruct fgraph_ops {trace_func_graph_ent_t entryfunc;trace_func_graph_ret_t retfunc; +struct ftrace_ops ops;/* for the hash lists */ void *private; };
kernel/trace/fgraph.c +39 -24
123 lines not on this page · raw message
kernel/trace/ftrace.c +3 -3
@@ -2673,6 +2673,8 @@ int ftrace_startup(struct ftrace_ops *ops, int command)if (unlikely (ftrace_disabled))return -ENODEV ; +ftrace_ops_init (ops); + ret =__register_ftrace_function (ops);if (ret)return ret;
@@ -6224,7 +6226,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)
@@ -6677,8 +6679,6 @@ int register_ftrace_function(struct ftrace_ops *ops) {int ret =-1 ; -ftrace_ops_init (ops); -mutex_lock (&ftrace_lock); ret =ftrace_startup (ops,0 );
kernel/trace/trace.h +9 -7
@@ -933,8 +933,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 long flags,int pc); -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
@@ -998,6 +998,7 @@ static inline int ftrace_graph_notrace_addr(unsigned long addr)preempt_enable_notrace (); return ret; } +#else static inline int ftrace_graph_addr (struct ftrace_graph_ent *trace) {
@@ -1023,18 +1024,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;
kernel/trace/trace_functions.c +1 -1
@@ -68,7 +68,7 @@ int ftrace_create_function_files(struct trace_array *tr,if (ret)return ret; - ret =allocate_fgraph_ops (tr); + ret =allocate_fgraph_ops (tr, tr->ops);if (ret) {kfree (tr->ops);return ret;
kernel/trace/trace_functions_graph.c +6 -2
@@ -287,7 +287,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;
@@ -300,6 +300,9 @@ int allocate_fgraph_ops(struct trace_array *tr) tr->gops = gops; gops->private = tr; + +fgraph_init_ops (&gops->ops, ops); +return 0 ; }
@@ -308,10 +311,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)
Signature · 1 line
2.20.1
Steven Rostedt
maintainerRFC PATCH v2 9/14 function_graph: Add "task variables" per task for fgraph_ops
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
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.
--- include/linux/ftrace.h | 2 ++ kernel/trace/fgraph.c | 73 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 1 deletion(-)
include/linux/ftrace.h +2 -0
@@ -754,6 +754,7 @@ struct fgraph_ops {trace_func_graph_ret_t retfunc;struct ftrace_ops ops;/* for the hash lists */ void *private; +int idx; };/*
@@ -792,6 +793,7 @@ 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); +unsigned long *fgraph_get_task_var (struct fgraph_ops *gops);int function_graph_enter (unsigned long ret,unsigned long func,unsigned long frame_pointer,unsigned long *retp);
kernel/trace/fgraph.c +72 -1
124 lines not on this page · raw message
Signature · 1 line
2.20.1
Steven Rostedt
maintainerRFC PATCH v2 10/14 function_graph: Move set_graph_function tests to shadow stack global var
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
The use of the task->trace_recursion for the logic used for the set_graph_funnction 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.
--- kernel/trace/trace.h | 37 +++++++++++++++++----------- kernel/trace/trace_functions_graph.c | 6 ++--- kernel/trace/trace_irqsoff.c | 4 +-- kernel/trace/trace_sched_wakeup.c | 4 +-- 4 files changed, 29 insertions(+), 22 deletions(-)
kernel/trace/trace.h +22 -15
@@ -567,9 +567,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
@@ -583,7 +580,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,
@@ -937,11 +934,16 @@ extern void init_array_fgraph_ops(struct trace_array *tr, struct ftrace_ops *opsextern 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 *ftrace_graph_hash;extern struct ftrace_hash *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 ;
@@ -954,12 +956,11 @@ static inline int ftrace_graph_addr(struct ftrace_graph_ent *trace) }if (ftrace_lookup_ip (ftrace_graph_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);/*
@@ -979,11 +980,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)
@@ -1000,7 +1004,7 @@ static inline int ftrace_graph_notrace_addr(unsigned long addr) }#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 ; }
@@ -1009,17 +1013,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); }
kernel/trace/trace_functions_graph.c +3 -3
@@ -148,7 +148,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 ())
@@ -246,7 +246,7 @@ void trace_graph_return(struct ftrace_graph_ret *trace,int cpu;int pc; -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 );
@@ -268,7 +268,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 );
kernel/trace/trace_irqsoff.c +2 -2
@@ -181,7 +181,7 @@ static int irqsoff_graph_entry(struct ftrace_graph_ent *trace,int ret;int pc; -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.
@@ -211,7 +211,7 @@ static void irqsoff_graph_return(struct ftrace_graph_ret *trace,unsigned long flags;int pc; -ftrace_graph_addr_finish (trace); +ftrace_graph_addr_finish (gops, trace);if (!func_prolog_dec (tr, &data, &flags))return ;
kernel/trace/trace_sched_wakeup.c +2 -2
@@ -120,7 +120,7 @@ static int wakeup_graph_entry(struct ftrace_graph_ent *trace,unsigned long flags;int pc, 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.
@@ -151,7 +151,7 @@ static void wakeup_graph_return(struct ftrace_graph_ret *trace,unsigned long flags;int pc; -ftrace_graph_addr_finish (trace); +ftrace_graph_addr_finish (gops, trace);if (!func_prolog_preempt_disable (tr, &data, &pc))return ;
Signature · 1 line
2.20.1
Steven Rostedt
maintainerRFC PATCH v2 11/14 function_graph: Move graph depth stored data to shadow stack global var
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
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.
--- kernel/trace/trace.h | 63 ++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 31 deletions(-)
kernel/trace/trace.h +32 -31
@@ -567,25 +567,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
@@ -598,16 +579,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
@@ -936,8 +907,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 *ftrace_graph_hash;extern struct ftrace_hash *ftrace_graph_notrace_hash;
@@ -961,7 +962,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
@@ -986,7 +987,7 @@ ftrace_graph_addr_finish(struct fgraph_ops *gops, struct ftrace_graph_ret *traceunsigned 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 ; }
Signature · 1 line
2.20.1
Steven Rostedt
maintainerRFC PATCH v2 12/14 function_graph: Move graph notrace bit to shadow stack global var
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
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.
--- kernel/trace/trace.h | 16 +++++++++------- kernel/trace/trace_functions_graph.c | 10 ++++++---- 2 files changed, 15 insertions(+), 11 deletions(-)
kernel/trace/trace.h +9 -7
@@ -566,13 +566,6 @@ enum {* can only be modified by current, we can reuse trace_recursion. */ 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, };#define trace_recursion_set (bit) do { (current)->trace_recursion |= (1<<(bit)); } while (0)
@@ -926,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 ;
kernel/trace/trace_functions_graph.c +6 -4
@@ -125,6 +125,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;
@@ -133,11 +134,11 @@ int trace_graph_entry(struct ftrace_graph_ent *trace,int cpu;int pc; -if (trace_recursion_test (TRACE_GRAPH_NOTRACE_BIT )) +if (*task_var &TRACE_GRAPH_NOTRACE )return 0 ;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.
@@ -239,6 +240,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;
@@ -248,8 +250,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 ; }
Signature · 1 line
2.20.1
Steven Rostedt
maintainerRFC PATCH v2 13/14 function_graph: Implement fgraph_reserve_data() and fgraph_retrieve_data()
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
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 4 words onto the task's shadow ret_stack and this then can be retrived by fgraph_retrieve_data() called by the corresponding retfunc().
--- include/linux/ftrace.h | 3 + kernel/trace/fgraph.c | 244 +++++++++++++++++++++++++++++++++++------ 2 files changed, 213 insertions(+), 34 deletions(-)
include/linux/ftrace.h +3 -0
@@ -757,6 +757,9 @@ struct fgraph_ops {int idx; }; +void *fgraph_reserve_data (int size_bytes); +void *fgraph_retrieve_data (void ); +/* * Stack of return addresses for functions * of a thread.
kernel/trace/fgraph.c +210 -34
334 lines not on this page · raw message
Signature · 1 line
2.20.1
Steven Rostedt
maintainerRFC PATCH v2 14/14 function_graph: Add selftest for passing local variables
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
Add boot up selftest that passes variables from a function entry to a function exit, and make sure that they do get passed around.
--- kernel/trace/trace_selftest.c | 161 ++++++++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+)
kernel/trace/trace_selftest.c +161 -0
@@ -718,6 +718,165 @@ 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 (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 ; +char *p; + + p =fgraph_retrieve_data (); +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 ; + } + +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; + +printk (KERN_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) { +printk (KERN_WARNING "Failed to init store_bytes fgraph tracing\n" ); +return -1 ; + } + +DYN_FTRACE_TEST_NAME (); + +unregister_ftrace_graph (&store_bytes); + +if (fgraph_error_str) { +printk (KERN_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
@@ -805,6 +964,8 @@ trace_selftest_startup_function_graph(struct tracer *trace, goto out; } + ret =test_graph_storage (); +/* Don't test dynamic tracing, the function tracer already did */ out:
Signature · 1 line
2.20.1
Masami Hiramatsu
maintainerQuoted 23 lines · On Mon, 20 May 2019 10:20:01 -0400 Steven Rostedt <[email protected]> wrote:
The background for this is explained in the V1 version found here:
http://lkml.kernel.org/r/[email protected]
The TL;DR; is this:
The function graph tracer required a rewrite, mainly because it can only allow one callback registered at a time. The main motivation for this change is to allow kretprobes to use the code of function graph tracer, which should allow all archs that have function graph tracing to also have kretprobes with no extra work.
Masami told me that one requirement was to allow the function entry callback to store data on the shadow stack that can be retrieved by the the function return callback. I added this, as well as a per-task variable (used by one of the function graph users).
The two functions to allow the storing of data on the stack and retrieval of it are:
void *fgraph_reserve_data(int
size_in_bytes)
Quoted 8 lines
Allows the entry function to reserve up to 4 words of data on the shadow stack. On success, a pointer to the contents is returned. This may be only called once per entry function.
void
*fgraph_retrieve_data(void)Allows the return function to retrieve the reserved data that was allocated by the entry function.
Nice! this seems good for kretprobe too. I'll review and try to port kretprobe on this framework.
Thank you!
Quoted 46 lines
Note, this code has passed my full test suite.
Changes since v1:
- Well, the first part of that series was already merged. But that was just the preparation for this part.
- Allocate a page for the shadow stack split it up that way. When the stack is full, we stop allowing more to be added (stop tracing).
- Added the reserve and retrieve of private data on the shadow stack for individual entry/return callbacks to pass data to each other.
- Added a "per task" data that can be used by a
fgraph_opsfor all function callbacks for a specific task.Steven Rostedt (VMware) (14):
function_graph: Convertret_stackto a series of longs
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 aroundftrace_graph_entryand return ftrace/function_graph: Passfgraph_opsto function graph callbacks
ftrace: Allowfunction_graphtracer to be enabled in instances
ftrace: Allow ftrace startup flags exist without dynamic ftrace
function_graph: Have the instances use their ownftrace_opsfor filtering
function_graph: Add "task variables" per task forfgraph_ops
function_graph: Moveset_graph_functiontests 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: Implementfgraph_reserve_data()andfgraph_retrieve_data()
function_graph: Add selftest for passing local variables----
include/linux/ftrace.h | 37 +- include/linux/sched.h | 2 +- kernel/trace/fgraph.c | 862 ++++++++++++++++++++++++++++------- kernel/trace/ftrace.c | 13 +- kernel/trace/ftrace_internal.h | 2 - kernel/trace/trace.h | 132 +++--- kernel/trace/trace_functions.c | 7 + 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 | 168 ++++++- 11 files changed, 1048 insertions(+), 291 deletions(-)
Signature · 1 line
Masami Hiramatsu <[email protected]>
Steven Rostedt
maintainerQuoted 6 lines · On Wed, 22 May 2019 23:19:55 +0900 Masami Hiramatsu <[email protected]> wrote:
Quoted 6 lines
void *fgraph_reserve_data(int
size_in_bytes)Allows the entry function to reserve up to 4 words of data on the shadow stack. On success, a pointer to the contents is returned. This may be only called once per entry function.
Quoted 7 lines
Quoted 4 lines
void
*fgraph_retrieve_data(void)Allows the return function to retrieve the reserved data that was allocated by the entry function.
Nice! this seems good for kretprobe too. I'll review and try to port kretprobe on this framework.
If you rather pull from my git repo and not download all the patches, they are currently available in my ftrace/fgraph-multi branch.
-- Steve
Masami Hiramatsu
maintainerQuoted 11 lines · On Wed, 22 May 2019 10:40:27 -0400 Steven Rostedt <[email protected]> wrote:
Quoted 8 lines · On Wed, 22 May 2019 23:19:55 +0900 Masami Hiramatsu <[email protected]> wrote:
Quoted 8 lines
void *fgraph_reserve_data(int
size_in_bytes)Allows the entry function to reserve up to 4 words of data on the shadow stack. On success, a pointer to the contents is returned. This may be only called once per entry function.
void
*fgraph_retrieve_data(void)
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Allows the return function to retrieve the reserved data that was allocated by the entry function.
Nice! this seems good for kretprobe too. I'll review and try to port kretprobe on this framework.
If you rather pull from my git repo and not download all the patches, they are currently available in my ftrace/fgraph-multi branch.
Hi Steve,
I found that these interfaces seem tightly coupled with fgraph_ops. But that cause a problem when I'm using it from kretprobe.
kretprobe has 2 handlers, entry handler and return handler, and both need pt_regs. But fgraph_ops's entryfunc and retfunc do not pass the pt_regs. That is the biggest issue for me on these APIs.
Can we expand fgraph_ops with regs parameter?
Thank you,
Signature · 1 line
Masami Hiramatsu <[email protected]>
Steven Rostedt
maintainerQuoted 4 lines · On Wed, 29 May 2019 15:47:40 +0900 Masami Hiramatsu <[email protected]> wrote:
Hi Steve,
I found that these interfaces seem tightly coupled with
fgraph_ops. But that cause a problem when I'm using it from kretprobe.
I was thinking that the kretprobes could use the fgraph_ops like kprobes uses ftrace_ops.
Quoted 5 lines
kretprobe has 2 handlers, entry handler and return handler, and both need
pt_regs. But fgraph_ops's entryfunc and retfunc do not pass thept_regs. That is the biggest issue for me on these APIs.
Can we expandfgraph_opswith regs parameter?
Ug. Yeah, of course you need that :-/
OK, so this series isn't enough to allow kretprobes to use it yet. OK, I plan on still keeping it because it does allow for placing function graph tracer into instances with their own filters.
I'll look into adding a REGS flag like we do with ftrace_ops.
Does the return need all regs? Or is just the return code good enough?
-- Steve
Masami Hiramatsu
maintainerQuoted 18 lines · On Wed, 29 May 2019 05:25:21 -0400 Steven Rostedt <[email protected]> wrote:
Quoted 4 lines · On Wed, 29 May 2019 15:47:40 +0900 Masami Hiramatsu <[email protected]> wrote:
Hi Steve,
I found that these interfaces seem tightly coupled with
fgraph_ops. But that cause a problem when I'm using it from kretprobe.I was thinking that the kretprobes could use the
fgraph_opslike kprobes usesftrace_ops.Quoted 5 lines
kretprobe has 2 handlers, entry handler and return handler, and both need
pt_regs. But fgraph_ops's entryfunc and retfunc do not pass thept_regs. That is the biggest issue for me on these APIs.
Can we expandfgraph_opswith regs parameter?
Quoted 5 lines
Ug. Yeah, of course you need that :-/
OK, so this series isn't enough to allow kretprobes to use it yet. OK, I plan on still keeping it because it does allow for placing function graph tracer into instances with their own filters.
OK, that will be a "regs" extension.
Quoted 4 lines
I'll look into adding a REGS flag like we do with
ftrace_ops.Does the return need all regs? Or is just the return code good enough?
Since it depends on arch, I think all regs we need. And for the entry handler, we need all.
Thank you,
Signature · 1 line
Masami Hiramatsu <[email protected]>
Steven Rostedt
maintainerQuoted 4 lines · On Thu, 30 May 2019 18:29:20 +0900 Masami Hiramatsu <[email protected]> wrote:
Quoted 3 lines
OK, so this series isn't enough to allow kretprobes to use it yet. OK, I plan on still keeping it because it does allow for placing function graph tracer into instances with their own filters.
Quoted 8 lines
OK, that will be a "regs" extension.
Quoted 4 lines
I'll look into adding a REGS flag like we do with
ftrace_ops.Does the return need all regs? Or is just the return code good enough?
Since it depends on arch, I think all regs we need. And for the entry handler, we need all.
When I get back home, I'm going to push this series to linux-next. And then I'll start adding the REGS extension for the next Linux release after this series gets in.
Thanks for looking at it.
-- Steve