[PATCH 0/4] tracing: export event trace and trace_marker

[PATCH 0/4] tracing: export event trace and trace_marker

[Tingwei Zhang]

Ftrace has ability to export trace packets to other destionation. Currently,
only function trace can be exported. This series extends the support to
event trace and trace_maker. STM is one possible destination to export ftrace.
Use seperate channel for each CPU to avoid mixing up packets from different
CPUs together.

Tingwei Zhang (4):
  stm class: ftrace: change dependency to TRACING
  tracing: add trace_export support for event trace
  tracing: add trace_export support for trace_marker
  stm class: ftrace: use different channel accroding to CPU

 drivers/hwtracing/stm/Kconfig  |  2 +-
 drivers/hwtracing/stm/ftrace.c |  5 +++--
 kernel/trace/trace.c           | 26 +++++++++++++++-----------
 3 files changed, 19 insertions(+), 14 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 1/4] stm class: ftrace: change dependency to TRACING

[Tingwei Zhang]

We will support copying event trace to STM. Change
STM_SOURCE_FTRACE to depend on TRACING since we will
support multiple tracers.

Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
---
 drivers/hwtracing/stm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwtracing/stm/Kconfig b/drivers/hwtracing/stm/Kconfig
index d0e92a8a045c..aad594fe79cc 100644
--- a/drivers/hwtracing/stm/Kconfig
+++ b/drivers/hwtracing/stm/Kconfig
@@ -71,7 +71,7 @@ config STM_SOURCE_HEARTBEAT
 
 config STM_SOURCE_FTRACE
 	tristate "Copy the output from kernel Ftrace to STM engine"
-	depends on FUNCTION_TRACER
+	depends on TRACING
 	help
 	  This option can be used to copy the output from kernel Ftrace
 	  to STM engine. Enabling this option will introduce a slight
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/4] tracing: add trace_export support for event trace

[Tingwei Zhang]

Only function traces can be exported to other destinations currently.
This patch exports event trace as well.

Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
---
 kernel/trace/trace.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index bb62269724d5..aef6330836e2 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2697,17 +2697,6 @@ int tracepoint_printk_sysctl(struct ctl_table *table, int write,
 	return ret;
 }
 
-void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
-{
-	if (static_key_false(&tracepoint_printk_key.key))
-		output_printk(fbuffer);
-
-	event_trigger_unlock_commit_regs(fbuffer->trace_file, fbuffer->buffer,
-				    fbuffer->event, fbuffer->entry,
-				    fbuffer->flags, fbuffer->pc, fbuffer->regs);
-}
-EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
-
 /*
  * Skip 3:
  *
@@ -2868,6 +2857,19 @@ int unregister_ftrace_export(struct trace_export *export)
 }
 EXPORT_SYMBOL_GPL(unregister_ftrace_export);
 
+void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
+{
+	if (static_key_false(&tracepoint_printk_key.key))
+		output_printk(fbuffer);
+
+	if (static_branch_unlikely(&ftrace_exports_enabled))
+		ftrace_exports(fbuffer->event);
+	event_trigger_unlock_commit_regs(fbuffer->trace_file, fbuffer->buffer,
+				    fbuffer->event, fbuffer->entry,
+				    fbuffer->flags, fbuffer->pc, fbuffer->regs);
+}
+EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
+
 void
 trace_function(struct trace_array *tr,
 	       unsigned long ip, unsigned long parent_ip, unsigned long flags,
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 2/4] tracing: add trace_export support for event trace

[Steven Rostedt]

On Mon, 20 Jul 2020 10:21:15 +0800
Tingwei Zhang <tingwei@codeaurora.org> wrote:

> Only function traces can be exported to other destinations currently.
> This patch exports event trace as well.
> 
> Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
> ---
>  kernel/trace/trace.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index bb62269724d5..aef6330836e2 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -2697,17 +2697,6 @@ int tracepoint_printk_sysctl(struct ctl_table *table, int write,
>  	return ret;
>  }
>  
> -void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
> -{
> -	if (static_key_false(&tracepoint_printk_key.key))
> -		output_printk(fbuffer);
> -
> -	event_trigger_unlock_commit_regs(fbuffer->trace_file, fbuffer->buffer,
> -				    fbuffer->event, fbuffer->entry,
> -				    fbuffer->flags, fbuffer->pc, fbuffer->regs);
> -}
> -EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
> -

Please move the ftrace_exports routines up, instead of moving the
trace_event_buffer_commit() down. As it fits better where it is (next
to the other buffer_commit code).

-- Steve


>  /*
>   * Skip 3:
>   *
> @@ -2868,6 +2857,19 @@ int unregister_ftrace_export(struct
> trace_export *export) }
>  EXPORT_SYMBOL_GPL(unregister_ftrace_export);
>  
> +void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
> +{
> +	if (static_key_false(&tracepoint_printk_key.key))
> +		output_printk(fbuffer);
> +
> +	if (static_branch_unlikely(&ftrace_exports_enabled))
> +		ftrace_exports(fbuffer->event);
> +	event_trigger_unlock_commit_regs(fbuffer->trace_file,
> fbuffer->buffer,
> +				    fbuffer->event, fbuffer->entry,
> +				    fbuffer->flags, fbuffer->pc,
> fbuffer->regs); +}
> +EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
> +
>  void
>  trace_function(struct trace_array *tr,
>  	       unsigned long ip, unsigned long parent_ip, unsigned
> long flags,

Re: [PATCH 2/4] tracing: add trace_export support for event trace

[Steven Rostedt]

On Tue, 21 Jul 2020 12:37:33 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon, 20 Jul 2020 10:21:15 +0800
> Tingwei Zhang <tingwei@codeaurora.org> wrote:
> 
> > Only function traces can be exported to other destinations currently.
> > This patch exports event trace as well.
> > 
> > Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
> > ---
> >  kernel/trace/trace.c | 24 +++++++++++++-----------
> >  1 file changed, 13 insertions(+), 11 deletions(-)
> > 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index bb62269724d5..aef6330836e2 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -2697,17 +2697,6 @@ int tracepoint_printk_sysctl(struct ctl_table *table, int write,
> >  	return ret;
> >  }
> >  
> > -void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
> > -{
> > -	if (static_key_false(&tracepoint_printk_key.key))
> > -		output_printk(fbuffer);
> > -
> > -	event_trigger_unlock_commit_regs(fbuffer->trace_file, fbuffer->buffer,
> > -				    fbuffer->event, fbuffer->entry,
> > -				    fbuffer->flags, fbuffer->pc, fbuffer->regs);
> > -}
> > -EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
> > -  
> 
> Please move the ftrace_exports routines up, instead of moving the
> trace_event_buffer_commit() down. As it fits better where it is (next
> to the other buffer_commit code).
> 
> -- Steve
> 
> 
> >  /*
> >   * Skip 3:
> >   *
> > @@ -2868,6 +2857,19 @@ int unregister_ftrace_export(struct
> > trace_export *export) }
> >  EXPORT_SYMBOL_GPL(unregister_ftrace_export);
> >  
> > +void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
> > +{
> > +	if (static_key_false(&tracepoint_printk_key.key))
> > +		output_printk(fbuffer);
> > +
> > +	if (static_branch_unlikely(&ftrace_exports_enabled))
> > +		ftrace_exports(fbuffer->event);

I would also recommend making each static branch a separate variable.
That way you could pick and choose what you want to enable. If you only
want events, and functions are being traced, it will add a bit of
overhead to handle the functions to just ignore them.

That is:

 ftrace_exports_enabled, trace_event_exports_enabled,
 trace_marker_exports_enabled.

-- Steve


> > +	event_trigger_unlock_commit_regs(fbuffer->trace_file,
> > fbuffer->buffer,
> > +				    fbuffer->event, fbuffer->entry,
> > +				    fbuffer->flags, fbuffer->pc,
> > fbuffer->regs); +}
> > +EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
> > +
> >  void
> >  trace_function(struct trace_array *tr,
> >  	       unsigned long ip, unsigned long parent_ip, unsigned
> > long flags,  
> 

Re: [PATCH 2/4] tracing: add trace_export support for event trace

[tingwei]

On 2020-07-22 00:43, Steven Rostedt wrote:
> On Tue, 21 Jul 2020 12:37:33 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
>> On Mon, 20 Jul 2020 10:21:15 +0800
>> Tingwei Zhang <tingwei@codeaurora.org> wrote:
>> 
>> > Only function traces can be exported to other destinations currently.
>> > This patch exports event trace as well.
>> >
>> > Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
>> > ---
>> >  kernel/trace/trace.c | 24 +++++++++++++-----------
>> >  1 file changed, 13 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
>> > index bb62269724d5..aef6330836e2 100644
>> > --- a/kernel/trace/trace.c
>> > +++ b/kernel/trace/trace.c
>> > @@ -2697,17 +2697,6 @@ int tracepoint_printk_sysctl(struct ctl_table
> *table, int write,
>> >  	return ret;
>> >  }
>> >
>> > -void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
>> > -{
>> > -	if (static_key_false(&tracepoint_printk_key.key))
>> > -		output_printk(fbuffer);
>> > -
>> > -	event_trigger_unlock_commit_regs(fbuffer->trace_file,
> fbuffer->buffer,
>> > -				    fbuffer->event, fbuffer->entry,
>> > -				    fbuffer->flags, fbuffer->pc,
> fbuffer->regs);
>> > -}
>> > -EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
>> > -
>> 
>> Please move the ftrace_exports routines up, instead of moving the
>> trace_event_buffer_commit() down. As it fits better where it is (next
>> to the other buffer_commit code).
>> 
>> -- Steve
>> 
>> 
>> >  /*
>> >   * Skip 3:
>> >   *
>> > @@ -2868,6 +2857,19 @@ int unregister_ftrace_export(struct
>> > trace_export *export) }
>> >  EXPORT_SYMBOL_GPL(unregister_ftrace_export);
>> >
>> > +void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
>> > +{
>> > +	if (static_key_false(&tracepoint_printk_key.key))
>> > +		output_printk(fbuffer);
>> > +
>> > +	if (static_branch_unlikely(&ftrace_exports_enabled))
>> > +		ftrace_exports(fbuffer->event);
> 
> I would also recommend making each static branch a separate variable.
> That way you could pick and choose what you want to enable. If you only
> want events, and functions are being traced, it will add a bit of
> overhead to handle the functions to just ignore them.
> 
> That is:
> 
>  ftrace_exports_enabled, trace_event_exports_enabled,
>  trace_marker_exports_enabled.
> 
> -- Steve
> 
That's a good idea.  Let me add that to next revision.

> 
>> > +	event_trigger_unlock_commit_regs(fbuffer->trace_file,
>> > fbuffer->buffer,
>> > +				    fbuffer->event, fbuffer->entry,
>> > +				    fbuffer->flags, fbuffer->pc,
>> > fbuffer->regs); +}
>> > +EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
>> > +
>> >  void
>> >  trace_function(struct trace_array *tr,
>> >  	       unsigned long ip, unsigned long parent_ip, unsigned
>> > long flags,
>> 

Re: [PATCH 2/4] tracing: add trace_export support for event trace

[tingwei]

On 2020-07-22 00:37, Steven Rostedt wrote:
> On Mon, 20 Jul 2020 10:21:15 +0800
> Tingwei Zhang <tingwei@codeaurora.org> wrote:
> 
>> Only function traces can be exported to other destinations currently.
>> This patch exports event trace as well.
>> 
>> Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
>> ---
>>  kernel/trace/trace.c | 24 +++++++++++++-----------
>>  1 file changed, 13 insertions(+), 11 deletions(-)
>> 
>> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
>> index bb62269724d5..aef6330836e2 100644
>> --- a/kernel/trace/trace.c
>> +++ b/kernel/trace/trace.c
>> @@ -2697,17 +2697,6 @@ int tracepoint_printk_sysctl(struct ctl_table
> *table, int write,
>>  	return ret;
>>  }
>> 
>> -void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
>> -{
>> -	if (static_key_false(&tracepoint_printk_key.key))
>> -		output_printk(fbuffer);
>> -
>> -	event_trigger_unlock_commit_regs(fbuffer->trace_file,
> fbuffer->buffer,
>> -				    fbuffer->event, fbuffer->entry,
>> -				    fbuffer->flags, fbuffer->pc,
> fbuffer->regs);
>> -}
>> -EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
>> -
> 
> Please move the ftrace_exports routines up, instead of moving the
> trace_event_buffer_commit() down. As it fits better where it is (next
> to the other buffer_commit code).
> 
> -- Steve
> 

Thanks, Steve. I'll fix this.

> 
>>  /*
>>   * Skip 3:
>>   *
>> @@ -2868,6 +2857,19 @@ int unregister_ftrace_export(struct
>> trace_export *export) }
>>  EXPORT_SYMBOL_GPL(unregister_ftrace_export);
>> 
>> +void trace_event_buffer_commit(struct trace_event_buffer *fbuffer)
>> +{
>> +	if (static_key_false(&tracepoint_printk_key.key))
>> +		output_printk(fbuffer);
>> +
>> +	if (static_branch_unlikely(&ftrace_exports_enabled))
>> +		ftrace_exports(fbuffer->event);
>> +	event_trigger_unlock_commit_regs(fbuffer->trace_file,
>> fbuffer->buffer,
>> +				    fbuffer->event, fbuffer->entry,
>> +				    fbuffer->flags, fbuffer->pc,
>> fbuffer->regs); +}
>> +EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
>> +
>>  void
>>  trace_function(struct trace_array *tr,
>>  	       unsigned long ip, unsigned long parent_ip, unsigned
>> long flags,

[PATCH 3/4] tracing: add trace_export support for trace_marker

[Tingwei Zhang]

Add the support to route trace_marker buffer to other destination
via trace_export.

Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
---
 kernel/trace/trace.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index aef6330836e2..ac18e0ee9246 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -6639,6 +6639,8 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
 	} else
 		entry->buf[cnt] = '\0';
 
+	if (static_branch_unlikely(&ftrace_exports_enabled))
+		ftrace_exports(event);
 	__buffer_unlock_commit(buffer, event);
 
 	if (tt)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 4/4] stm class: ftrace: use different channel accroding to CPU

[Tingwei Zhang]

To avoid mixup of packets from differnt ftrace packets simultaneously,
use different channel for packets from different CPU.

Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
---
 drivers/hwtracing/stm/ftrace.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/stm/ftrace.c b/drivers/hwtracing/stm/ftrace.c
index ce868e095410..6c3bce11c8d3 100644
--- a/drivers/hwtracing/stm/ftrace.c
+++ b/drivers/hwtracing/stm/ftrace.c
@@ -10,7 +10,7 @@
 #include <linux/stm.h>
 #include <linux/trace.h>
 
-#define STM_FTRACE_NR_CHANNELS 1
+#define STM_FTRACE_NR_CHANNELS NR_CPUS
 #define STM_FTRACE_CHAN 0
 
 static int stm_ftrace_link(struct stm_source_data *data);
@@ -37,8 +37,9 @@ static void notrace
 stm_ftrace_write(struct trace_export *export, const void *buf, unsigned int len)
 {
 	struct stm_ftrace *stm = container_of(export, struct stm_ftrace, ftrace);
+	unsigned int cpu = smp_processor_id();
 
-	stm_source_write(&stm->data, STM_FTRACE_CHAN, buf, len);
+	stm_source_write(&stm->data, STM_FTRACE_CHAN + cpu, buf, len);
 }
 
 static int stm_ftrace_link(struct stm_source_data *data)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 4/4] stm class: ftrace: use different channel accroding to CPU

[Steven Rostedt]

On Mon, 20 Jul 2020 10:21:17 +0800
Tingwei Zhang <tingwei@codeaurora.org> wrote:

> To avoid mixup of packets from differnt ftrace packets simultaneously,
> use different channel for packets from different CPU.
> 
> Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
> ---
>  drivers/hwtracing/stm/ftrace.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/stm/ftrace.c b/drivers/hwtracing/stm/ftrace.c
> index ce868e095410..6c3bce11c8d3 100644
> --- a/drivers/hwtracing/stm/ftrace.c
> +++ b/drivers/hwtracing/stm/ftrace.c
> @@ -10,7 +10,7 @@
>  #include <linux/stm.h>
>  #include <linux/trace.h>
>  
> -#define STM_FTRACE_NR_CHANNELS 1
> +#define STM_FTRACE_NR_CHANNELS NR_CPUS

I would make this dynamic. If this causes any kind of allocation, you
do not want NR_CPUS. That's hardcoded by the distribution and is the
max number of CPUs. With machines with hundreds of CPUs today, that
could easily be 256 or 512 or more.

Instead, in the stm_ftrace_init() I would just assign it to
nr_online_cpus().

	stm_ftrace.data.nr_chans = nr_online_cpus();

Or nr_possible_cpus().

-- Steve


>  #define STM_FTRACE_CHAN 0
>  
>  static int stm_ftrace_link(struct stm_source_data *data);
> @@ -37,8 +37,9 @@ static void notrace
>  stm_ftrace_write(struct trace_export *export, const void *buf, unsigned int len)
>  {
>  	struct stm_ftrace *stm = container_of(export, struct stm_ftrace, ftrace);
> +	unsigned int cpu = smp_processor_id();
>  
> -	stm_source_write(&stm->data, STM_FTRACE_CHAN, buf, len);
> +	stm_source_write(&stm->data, STM_FTRACE_CHAN + cpu, buf, len);
>  }
>  
>  static int stm_ftrace_link(struct stm_source_data *data)