Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1261031 > unrolled thread

[PATCH 2/3] ftrace: add ftrace-buffer option

Started byMathieu Desnoyers <mathieu.desnoyers@efficios.com>
First post2015-11-02 23:50 +0100
Last post2015-11-03 00:20 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/3] ftrace: add ftrace-buffer option Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2015-11-02 23:50 +0100
    Re: [PATCH 2/3] ftrace: add ftrace-buffer option Steven Rostedt <rostedt@goodmis.org> - 2015-11-03 00:00 +0100
      Re: [PATCH 2/3] ftrace: add ftrace-buffer option Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2015-11-03 00:10 +0100
        Re: [PATCH 2/3] ftrace: add ftrace-buffer option Steven Rostedt <rostedt@goodmis.org> - 2015-11-03 00:20 +0100

#1261031 — [PATCH 2/3] ftrace: add ftrace-buffer option

FromMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date2015-11-02 23:50 +0100
Subject[PATCH 2/3] ftrace: add ftrace-buffer option
Message-ID<qqw5k-2HW-9@gated-at.bofh.it>
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.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@redhat.com>
CC: Peter Zijlstra <peterz@infradead.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);
-- 
2.1.4

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

[toc] | [next] | [standalone]

#1261039

FromSteven Rostedt <rostedt@goodmis.org>
Date2015-11-03 00:00 +0100
Message-ID<qqwf0-2M7-3@gated-at.bofh.it>
In reply to#1261031
On Mon,  2 Nov 2015 17:42:43 -0500
Mathieu Desnoyers <mathieu.desnoyers@efficios.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.

Also, the trace_options code have been redesigned, and this won't apply
to it (see linux-next). Also, if this is only for irqsoff latency
tracers, it should be a tracer option, not a global one.

-- Steve


> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: Peter Zijlstra <peterz@infradead.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);

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

[toc] | [prev] | [next] | [standalone]

#1261046

FromMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date2015-11-03 00:10 +0100
Message-ID<qqwoF-34I-1@gated-at.bofh.it>
In reply to#1261039
----- On Nov 2, 2015, at 5:56 PM, rostedt rostedt@goodmis.org wrote:

> On Mon,  2 Nov 2015 17:42:43 -0500
> Mathieu Desnoyers <mathieu.desnoyers@efficios.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@efficios.com>
>> CC: Thomas Gleixner <tglx@linutronix.de>
>> CC: Steven Rostedt <rostedt@goodmis.org>
>> CC: Ingo Molnar <mingo@redhat.com>
>> CC: Peter Zijlstra <peterz@infradead.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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1261050

FromSteven Rostedt <rostedt@goodmis.org>
Date2015-11-03 00:20 +0100
Message-ID<qqwym-37L-5@gated-at.bofh.it>
In reply to#1261046
On Mon, 2 Nov 2015 23:01:08 +0000 (UTC)
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:

> ----- On Nov 2, 2015, at 5:56 PM, rostedt rostedt@goodmis.org wrote:
> 
> > On Mon,  2 Nov 2015 17:42:43 -0500
> > Mathieu Desnoyers <mathieu.desnoyers@efficios.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 ?
> 
>

Tomorrow I'll take a deeper look at patch 3 to try to understand.

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web