Re: [for-next][PATCH 8/8] tracing/fgraph: Have wakeup and irqsoff tracers ignore graph functions too

From: Namhyung Kim
Date: Mon Dec 12 2016 - 11:41:43 EST


On Tue, Dec 13, 2016 at 01:33:42AM +0900, Namhyung Kim wrote:
> From: "Steven Rostedt (Red Hat)" <rostedt@xxxxxxxxxxx>
>
> Currently both the wakeup and irqsoff traces do not handle set_graph_notrace
> well. The ftrace infrastructure will ignore the return paths of all
> functions leaving them hanging without an end:
>
> # echo '*spin*' > set_graph_notrace
> # cat trace
> [...]
> _raw_spin_lock() {
> preempt_count_add() {
> do_raw_spin_lock() {
> update_rq_clock();
>
> Where the '*spin*' functions should have looked like this:
>
> _raw_spin_lock() {
> preempt_count_add();
> do_raw_spin_lock();
> }
> update_rq_clock();
>
> Instead, have the wakeup and irqsoff tracers ignore the functions that are
> set by the set_graph_notrace like the function_graph tracer does. Move
> the logic in the function_graph tracer into a header to allow wakeup and
> irqsoff tracers to use it as well.
>
> Signed-off-by: Steven Rostedt <rostedt@xxxxxxxxxxx>

Acked-by: Namhyung Kim <namhyung@xxxxxxxxxx>

Thanks,
Namhyung


> ---
> kernel/trace/trace.h | 11 +++++++++++
> kernel/trace/trace_functions_graph.c | 14 +++++++-------
> kernel/trace/trace_irqsoff.c | 12 ++++++++++++
> kernel/trace/trace_sched_wakeup.c | 12 ++++++++++++
> 4 files changed, 42 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 37602e722336..c2234494f40c 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -846,6 +846,17 @@ static inline int
> ftrace_graph_notrace_addr(unsigned long addr)
> return 0;
> }
> #endif /* CONFIG_DYNAMIC_FTRACE */
> +
> +extern unsigned int fgraph_max_depth;
> +
> +static inline bool ftrace_graph_ignore_func(struct ftrace_graph_ent *trace)
> +{
> + /* trace it when it is-nested-in or is a function enabled. */
> + return !(trace->depth || ftrace_graph_addr(trace->func)) ||
> + (trace->depth < 0) ||
> + (fgraph_max_depth && trace->depth >= fgraph_max_depth);
> +}
> +
> #else /* CONFIG_FUNCTION_GRAPH_TRACER */
> static inline enum print_line_t
> print_graph_function_flags(struct trace_iterator *iter, u32 flags)
> diff --git a/kernel/trace/trace_functions_graph.c
> b/kernel/trace/trace_functions_graph.c
> index 566f7327c3aa..d56123cdcc89 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -65,7 +65,7 @@ struct fgraph_data {
>
> #define TRACE_GRAPH_INDENT 2
>
> -static unsigned int max_depth;
> +unsigned int fgraph_max_depth;
>
> static struct tracer_opt trace_opts[] = {
> /* Display overruns? (for self-debug purpose) */
> @@ -384,10 +384,10 @@ int trace_graph_entry(struct ftrace_graph_ent *trace)
> if (!ftrace_trace_task(tr))
> return 0;
>
> - /* trace it when it is-nested-in or is a function enabled. */
> - if ((!(trace->depth || ftrace_graph_addr(trace->func)) ||
> - ftrace_graph_ignore_irqs()) || (trace->depth < 0) ||
> - (max_depth && trace->depth >= max_depth))
> + if (ftrace_graph_ignore_func(trace))
> + return 0;
> +
> + if (ftrace_graph_ignore_irqs())
> return 0;
>
> /*
> @@ -1500,7 +1500,7 @@ graph_depth_write(struct file *filp, const char
> __user *ubuf, size_t cnt,
> if (ret)
> return ret;
>
> - max_depth = val;
> + fgraph_max_depth = val;
>
> *ppos += cnt;
>
> @@ -1514,7 +1514,7 @@ graph_depth_read(struct file *filp, char __user
> *ubuf, size_t cnt,
> char buf[15]; /* More than enough to hold UINT_MAX + "\n"*/
> int n;
>
> - n = sprintf(buf, "%d\n", max_depth);
> + n = sprintf(buf, "%d\n", fgraph_max_depth);
>
> return simple_read_from_buffer(ubuf, cnt, ppos, buf, n);
> }
> diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
> index 03cdff84d026..86654d7e1afe 100644
> --- a/kernel/trace/trace_irqsoff.c
> +++ b/kernel/trace/trace_irqsoff.c
> @@ -175,6 +175,18 @@ static int irqsoff_graph_entry(struct
> ftrace_graph_ent *trace)
> int ret;
> int pc;
>
> + if (ftrace_graph_ignore_func(trace))
> + return 0;
> + /*
> + * Do not trace a function if it's filtered by set_graph_notrace.
> + * Make the index of ret stack negative to indicate that it should
> + * ignore further functions. But it needs its own ret stack entry
> + * to recover the original index in order to continue tracing after
> + * returning from the function.
> + */
> + if (ftrace_graph_notrace_addr(trace->func))
> + return 1;
> +
> if (!func_prolog_dec(tr, &data, &flags))
> return 0;
>
> diff --git a/kernel/trace/trace_sched_wakeup.c
> b/kernel/trace/trace_sched_wakeup.c
> index 1bf2324dc682..5d0bb025bb21 100644
> --- a/kernel/trace/trace_sched_wakeup.c
> +++ b/kernel/trace/trace_sched_wakeup.c
> @@ -239,6 +239,18 @@ static int wakeup_graph_entry(struct
> ftrace_graph_ent *trace)
> unsigned long flags;
> int pc, ret = 0;
>
> + if (ftrace_graph_ignore_func(trace))
> + return 0;
> + /*
> + * Do not trace a function if it's filtered by set_graph_notrace.
> + * Make the index of ret stack negative to indicate that it should
> + * ignore further functions. But it needs its own ret stack entry
> + * to recover the original index in order to continue tracing after
> + * returning from the function.
> + */
> + if (ftrace_graph_notrace_addr(trace->func))
> + return 1;
> +
> if (!func_prolog_preempt_disable(tr, &data, &pc))
> return 0;
>
> --
> 2.10.2