linux-kernel - Re: [PATCH] tracing, function graph: Remove the noisy output when use function graph filter

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1276790123.3556.111.camel@gandalf.stny.rr.com>
Date:	Thu, 17 Jun 2010 11:55:23 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Lai Jiangshan <laijs@...fujitsu.com>
Cc:	Frederic Weisbecker <fweisbec@...il.com>,
	LKML <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH] tracing, function graph: Remove the noisy output when
 use function graph filter

On Thu, 2010-06-17 at 16:58 +0800, Lai Jiangshan wrote:
> If we use function graph filter(echo some_functions >> set_graph_function)
> we are only interested in the graph of some_functions.
> 
> But if a interrupt occurs when these functions are being traced(or preparing),
> the graphs of the handlers of this interrupt are also recorded.
> These events that we aren't interested in are totally noisy.

Umm, sorry, I prefer the noise.  When analyzing the network stack, I
like to see the interrupts that preempt the code.

What you could do is add an function_graph option that disables it, but
do not make it the default.

> 
> This patch remove them(very very seldom noise is still left,
> It's OK for tracing, doesn't it?).
> 
> Signed-off-by: Lai Jiangshan <laijs@...fujitsu.com>
> ---
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index 79f4bac..9c130b3 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -12,6 +12,8 @@
>  #include <linux/slab.h>
>  #include <linux/fs.h>
>  
> +#include <asm/irq_regs.h>
> +
>  #include "trace.h"
>  #include "trace_output.h"
>  
> @@ -213,13 +215,19 @@ int trace_graph_entry(struct ftrace_graph_ent *trace)
>  	int ret;
>  	int cpu;
>  	int pc;
> +	int depth = trace->depth;
> +	unsigned long long cntxt = (unsigned long)(void *)get_irq_regs();
>  
>  	if (!ftrace_trace_task(current))
>  		return 0;
>  
>  	/* trace it when it is-nested-in or is a function enabled. */
> -	if (!(trace->depth || ftrace_graph_addr(trace->func)))
> -		return 0;
> +	if (!ftrace_graph_addr(trace->func)) {
> +		if (!depth || current->ret_stack[depth - 1].subtime != cntxt)
> +			return 0;
> +	}

This will slow down the tracing when inside the function. You are
testing every function to see if we should show the trace, even when we
are inside a function we are tracing.


> +	/* We don't use subtime here, reuse it to save context. */
> +	current->ret_stack[depth].subtime = cntxt;

This should then be a union if it is used for two things. That documents
it better.


>  
>  	local_irq_save(flags);
>  	cpu = raw_smp_processor_id();

If you make the above into an option, then we can have

	int no_irqs = tracer_flags.val & TRACE_GRAPH_NO_IRQS;
	[...]

	if (!(trace->depth || ftrace_graph_addr(trace->func)))
		return 0;

	if (no_irqs) {
		cntx = (unsigned long)(void *)get_irq_regs();
		if (trace->depth &&
		    current->ret_stack[depth - 1].subtime != cntxt)
			return 0;
		current->ret_stack[depth].subtime = cntxt;
	}

Also, I'm not really happy about the double use of subtime. It pevents
the function graph from ever ignoring the time it calls the child
functions.

-- Steve


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/