Messages in this thread | | | Date | Thu, 22 Nov 2018 18:59:46 -0800 | From | Joel Fernandes <> | Subject | Re: [RFC][PATCH 07/14] fgraph: Add new fgraph_ops structure to enable function graph hooks |
| |
On Wed, Nov 21, 2018 at 08:27:15PM -0500, Steven Rostedt wrote: > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org> > > Currently the registering of function graph is to pass in a entry and return > function. We need to have a way to associate those functions together where > the entry can determine to run the return hook. Having a structure that > contains both functions will facilitate the process of converting the code > to be able to do such. > > This is similar to the way function hooks are enabled (it passes in > ftrace_ops). Instead of passing in the functions to use, a single structure > is passed in to the registering function. > > The unregister function is now passed in the fgraph_ops handle. When we > allow more than one callback to the function graph hooks, this will let the > system know which one to remove. > > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > --- > include/linux/ftrace.h | 24 +++++++++++++++++------- > kernel/trace/fgraph.c | 9 ++++----- > kernel/trace/ftrace.c | 10 +++++++--- > kernel/trace/trace_functions_graph.c | 21 ++++++++++++++++----- > kernel/trace/trace_irqsoff.c | 10 +++++++--- > kernel/trace/trace_sched_wakeup.c | 10 +++++++--- > kernel/trace/trace_selftest.c | 8 ++++++-- > 7 files changed, 64 insertions(+), 28 deletions(-) > > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > index f98063e273e5..477ff9412d26 100644 > --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -749,6 +749,18 @@ typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *); /* entry */ > > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > > +struct fgraph_ops { > + trace_func_graph_ent_t entryfunc; > + trace_func_graph_ret_t retfunc; > + struct fgraph_ops __rcu *next; > + unsigned long flags; > + void *private; > +#ifdef CONFIG_DYNAMIC_FTRACE > + struct ftrace_ops_hash local_hash; > + struct ftrace_ops_hash *func_hash; > +#endif > +}; > + > /* > * Stack of return addresses for functions > * of a thread. > @@ -792,8 +804,9 @@ unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx, > > #define FTRACE_RETFUNC_DEPTH 50 > #define FTRACE_RETSTACK_ALLOC_SIZE 32 > -extern int register_ftrace_graph(trace_func_graph_ret_t retfunc, > - trace_func_graph_ent_t entryfunc); > + > +extern int register_ftrace_graph(struct fgraph_ops *ops); > +extern void unregister_ftrace_graph(struct fgraph_ops *ops); > > extern bool ftrace_graph_is_dead(void); > extern void ftrace_graph_stop(void); > @@ -802,8 +815,6 @@ extern void ftrace_graph_stop(void); > extern trace_func_graph_ret_t ftrace_graph_return; > extern trace_func_graph_ent_t ftrace_graph_entry; > > -extern void unregister_ftrace_graph(void); > - > extern void ftrace_graph_init_task(struct task_struct *t); > extern void ftrace_graph_exit_task(struct task_struct *t); > extern void ftrace_graph_init_idle_task(struct task_struct *t, int cpu); > @@ -830,12 +841,11 @@ static inline void ftrace_graph_init_task(struct task_struct *t) { } > static inline void ftrace_graph_exit_task(struct task_struct *t) { } > static inline void ftrace_graph_init_idle_task(struct task_struct *t, int cpu) { } > > -static inline int register_ftrace_graph(trace_func_graph_ret_t retfunc, > - trace_func_graph_ent_t entryfunc) > +static inline int register_ftrace_graph(struct fgraph_ops *ops); > { > return -1; > } > -static inline void unregister_ftrace_graph(void) { } > +static inline void unregister_ftrace_graph(struct fgraph_ops *ops) { } > > static inline int task_curr_ret_stack(struct task_struct *tsk) > { > diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c > index b9c7dbbbdd96..f3a89ecac671 100644 > --- a/kernel/trace/fgraph.c > +++ b/kernel/trace/fgraph.c > @@ -491,8 +491,7 @@ static int start_graph_tracing(void) > return ret; > } > > -int register_ftrace_graph(trace_func_graph_ret_t retfunc, > - trace_func_graph_ent_t entryfunc) > +int register_ftrace_graph(struct fgraph_ops *gops) > { > int ret = 0; > > @@ -513,7 +512,7 @@ int register_ftrace_graph(trace_func_graph_ret_t retfunc, > goto out; > } > > - ftrace_graph_return = retfunc; > + ftrace_graph_return = gops->retfunc; > > /* > * Update the indirect function to the entryfunc, and the > @@ -521,7 +520,7 @@ int register_ftrace_graph(trace_func_graph_ret_t retfunc, > * call the update fgraph entry function to determine if > * the entryfunc should be called directly or not. > */ > - __ftrace_graph_entry = entryfunc; > + __ftrace_graph_entry = gops->entryfunc; > ftrace_graph_entry = ftrace_graph_entry_test; > update_function_graph_func(); > > @@ -531,7 +530,7 @@ int register_ftrace_graph(trace_func_graph_ret_t retfunc, > return ret; > } > > -void unregister_ftrace_graph(void) > +void unregister_ftrace_graph(struct fgraph_ops *gops) > { > mutex_lock(&ftrace_lock); > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 64e635994648..d057dde081e7 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -849,15 +849,19 @@ static void profile_graph_return(struct ftrace_graph_ret *trace) > local_irq_restore(flags); > } > > +static struct fgraph_ops fprofiler_ops = { > + .entryfunc = &profile_graph_entry, > + .retfunc = &profile_graph_return, > +}; > + > static int register_ftrace_profiler(void) > { > - return register_ftrace_graph(&profile_graph_return, > - &profile_graph_entry); > + return register_ftrace_graph(&fprofiler_ops); > } > > static void unregister_ftrace_profiler(void) > { > - unregister_ftrace_graph(); > + unregister_ftrace_graph(&fprofiler_ops); > } > #else > static struct ftrace_ops ftrace_profile_ops __read_mostly = { > diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c > index 0e0ff08357cf..7c7fd13d2373 100644 > --- a/kernel/trace/trace_functions_graph.c > +++ b/kernel/trace/trace_functions_graph.c > @@ -336,17 +336,25 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace) > trace_graph_return(trace); > } > > +static struct fgraph_ops funcgraph_threash_ops = {
minor nit: should be funcgraph_thresh_ops ?
thanks,
- Joel
| |