| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20181123025946.GB17060@google.com>
Date: Thu, 22 Nov 2018 18:59:46 -0800
From: Joel Fernandes <joel@...lfernandes.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Thomas Gleixner <tglx@...utronix.de>,
Peter Zijlstra <peterz@...radead.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Josh Poimboeuf <jpoimboe@...hat.com>,
Frederic Weisbecker <frederic@...nel.org>,
Andy Lutomirski <luto@...nel.org>,
Mark Rutland <mark.rutland@....com>
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@...dmis.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@...dmis.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
Powered by blists - more mailing lists