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] [thread-next>] [day] [month] [year] [list]
Message-ID: <508280247.33625.1446505268165.JavaMail.zimbra@efficios.com>
Date:	Mon, 2 Nov 2015 23:01:08 +0000 (UTC)
From:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:	rostedt <rostedt@...dmis.org>
Cc:	linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH 2/3] ftrace: add ftrace-buffer option

----- On Nov 2, 2015, at 5:56 PM, rostedt rostedt@...dmis.org wrote:

> On Mon,  2 Nov 2015 17:42:43 -0500
> Mathieu Desnoyers <mathieu.desnoyers@...icios.com> wrote:
> 
>> In order to use ftrace tracers to generate tracepoints without doing
>> tracing to its own hardcoded ring buffers, add a ftrace-buffer option
>> (default: 1). When set to 0, it disables tracing into the ftrace
>> hardcoded buffers.
>> 
> 
> This needs a more in depth change log. I have no idea why this is
> needed.

I can do an updated change log. This is needed for the next patch
in this series, which adds tracepoints in the ftrace irqs and preempt
off tracer, so other tracers such as Perf and LTTng can use them.

> 
> Also, the trace_options code have been redesigned, and this won't apply
> to it (see linux-next).

I can rebase my work on top of linux-next if you are OK with
the general idea.

> Also, if this is only for irqsoff latency
> tracers, it should be a tracer option, not a global one.

I've currently only done the irqsoff latency tracer
part, but I'm wondering whether:

1- we should make it a irqsoff latency tracer option,
2- we should keep it as a global ftrace option, and apply it to
   other ftrace tracers as well,

Thoughts ?

Thanks,

Mathieu

> 
> -- Steve
> 
> 
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
>> CC: Thomas Gleixner <tglx@...utronix.de>
>> CC: Steven Rostedt <rostedt@...dmis.org>
>> CC: Ingo Molnar <mingo@...hat.com>
>> CC: Peter Zijlstra <peterz@...radead.org>
>> ---
>>  kernel/trace/trace.c         |  4 +++-
>>  kernel/trace/trace.h         |  1 +
>>  kernel/trace/trace_irqsoff.c | 19 +++++++++++--------
>>  3 files changed, 15 insertions(+), 9 deletions(-)
>> 
>> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
>> index 6e79408..d664f0e 100644
>> --- a/kernel/trace/trace.c
>> +++ b/kernel/trace/trace.c
>> @@ -472,7 +472,8 @@ static inline void trace_access_lock_init(void)
>>  unsigned long trace_flags = TRACE_ITER_PRINT_PARENT | TRACE_ITER_PRINTK |
>>  	TRACE_ITER_ANNOTATE | TRACE_ITER_CONTEXT_INFO | TRACE_ITER_SLEEP_TIME |
>>  	TRACE_ITER_GRAPH_TIME | TRACE_ITER_RECORD_CMD | TRACE_ITER_OVERWRITE |
>> -	TRACE_ITER_IRQ_INFO | TRACE_ITER_MARKERS | TRACE_ITER_FUNCTION;
>> +	TRACE_ITER_IRQ_INFO | TRACE_ITER_MARKERS | TRACE_ITER_FUNCTION |
>> +	TRACE_ITER_FTRACE_BUFFER;
>>  
>>  static void tracer_tracing_on(struct trace_array *tr)
>>  {
>> @@ -862,6 +863,7 @@ static const char *trace_options[] = {
>>  	"irq-info",
>>  	"markers",
>>  	"function-trace",
>> +	"ftrace-buffer",
>>  	NULL
>>  };
>>  
>> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
>> index 74bde81..29968e1 100644
>> --- a/kernel/trace/trace.h
>> +++ b/kernel/trace/trace.h
>> @@ -930,6 +930,7 @@ enum trace_iterator_flags {
>>  	TRACE_ITER_IRQ_INFO		= 0x800000,
>>  	TRACE_ITER_MARKERS		= 0x1000000,
>>  	TRACE_ITER_FUNCTION		= 0x2000000,
>> +	TRACE_ITER_FTRACE_BUFFER	= 0x4000000,
>>  };
>>  
>>  /*
>> diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
>> index 8523ea3..3e5635d 100644
>> --- a/kernel/trace/trace_irqsoff.c
>> +++ b/kernel/trace/trace_irqsoff.c
>> @@ -334,9 +334,11 @@ check_critical_timing(struct trace_array *tr,
>>  	if (!report_latency(tr, delta))
>>  		goto out_unlock;
>>  
>> -	__trace_function(tr, CALLER_ADDR0, parent_ip, flags, pc);
>> -	/* Skip 5 functions to get to the irq/preempt enable function */
>> -	__trace_stack(tr, flags, 5, pc);
>> +	if (trace_flags & TRACE_ITER_FTRACE_BUFFER) {
>> +		__trace_function(tr, CALLER_ADDR0, parent_ip, flags, pc);
>> +		/* Skip 5 functions to get to the irq/preempt enable function */
>> +		__trace_stack(tr, flags, 5, pc);
>> +	}
>>  
>>  	if (data->critical_sequence != max_sequence)
>>  		goto out_unlock;
>> @@ -356,7 +358,8 @@ out_unlock:
>>  out:
>>  	data->critical_sequence = max_sequence;
>>  	data->preempt_timestamp = ftrace_now(cpu);
>> -	__trace_function(tr, CALLER_ADDR0, parent_ip, flags, pc);
>> +	if (trace_flags & TRACE_ITER_FTRACE_BUFFER)
>> +		__trace_function(tr, CALLER_ADDR0, parent_ip, flags, pc);
>>  }
>>  
>>  static inline void
>> @@ -387,9 +390,8 @@ start_critical_timing(unsigned long ip, unsigned long
>> parent_ip)
>>  	data->critical_start = parent_ip ? : ip;
>>  
>>  	local_save_flags(flags);
>> -
>> -	__trace_function(tr, ip, parent_ip, flags, preempt_count());
>> -
>> +	if (trace_flags & TRACE_ITER_FTRACE_BUFFER)
>> +		__trace_function(tr, ip, parent_ip, flags, preempt_count());
>>  	per_cpu(tracing_cpu, cpu) = 1;
>>  
>>  	atomic_dec(&data->disabled);
>> @@ -422,7 +424,8 @@ stop_critical_timing(unsigned long ip, unsigned long
>> parent_ip)
>>  	atomic_inc(&data->disabled);
>>  
>>  	local_save_flags(flags);
>> -	__trace_function(tr, ip, parent_ip, flags, preempt_count());
>> +	if (trace_flags & TRACE_ITER_FTRACE_BUFFER)
>> +		__trace_function(tr, ip, parent_ip, flags, preempt_count());
>>  	check_critical_timing(tr, data, parent_ip ? : ip, cpu);
>>  	data->critical_start = 0;
> >  	atomic_dec(&data->disabled);

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ