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
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 <[email protected]>
---
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
Only function traces can be exported to other destinations currently.
This patch exports event trace as well.
Signed-off-by: Tingwei Zhang <[email protected]>
---
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
To avoid mixup of packets from differnt ftrace packets simultaneously,
use different channel for packets from different CPU.
Signed-off-by: Tingwei Zhang <[email protected]>
---
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
Add the support to route trace_marker buffer to other destination
via trace_export.
Signed-off-by: Tingwei Zhang <[email protected]>
---
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
On Mon, 20 Jul 2020 10:21:15 +0800
Tingwei Zhang <[email protected]> wrote:
> Only function traces can be exported to other destinations currently.
> This patch exports event trace as well.
>
> Signed-off-by: Tingwei Zhang <[email protected]>
> ---
> 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,
On Tue, 21 Jul 2020 12:37:33 -0400
Steven Rostedt <[email protected]> wrote:
> On Mon, 20 Jul 2020 10:21:15 +0800
> Tingwei Zhang <[email protected]> wrote:
>
> > Only function traces can be exported to other destinations currently.
> > This patch exports event trace as well.
> >
> > Signed-off-by: Tingwei Zhang <[email protected]>
> > ---
> > 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,
>
On Mon, 20 Jul 2020 10:21:17 +0800
Tingwei Zhang <[email protected]> wrote:
> To avoid mixup of packets from differnt ftrace packets simultaneously,
> use different channel for packets from different CPU.
>
> Signed-off-by: Tingwei Zhang <[email protected]>
> ---
> 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)
On 2020-07-22 00:37, Steven Rostedt wrote:
> On Mon, 20 Jul 2020 10:21:15 +0800
> Tingwei Zhang <[email protected]> wrote:
>
>> Only function traces can be exported to other destinations currently.
>> This patch exports event trace as well.
>>
>> Signed-off-by: Tingwei Zhang <[email protected]>
>> ---
>> 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,
On 2020-07-22 00:43, Steven Rostedt wrote:
> On Tue, 21 Jul 2020 12:37:33 -0400
> Steven Rostedt <[email protected]> wrote:
>
>> On Mon, 20 Jul 2020 10:21:15 +0800
>> Tingwei Zhang <[email protected]> wrote:
>>
>> > Only function traces can be exported to other destinations currently.
>> > This patch exports event trace as well.
>> >
>> > Signed-off-by: Tingwei Zhang <[email protected]>
>> > ---
>> > 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,
>>