2020-07-26 03:02:22

by Tingwei Zhang

[permalink] [raw]
Subject: [PATCH v2 0/6] tracing: export event trace and trace_marker

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.

Change from v1:
All changes are suggested by Steven Rostedt.
User seperate flag to control function trace, event trace and trace mark.
Allocate channels according to num_possible_cpu() dynamically.
Move ftrace_exports routines up so all ftrace can use them.

Tingwei Zhang (6):
stm class: ftrace: change dependency to TRACING
tracing: add flag to control different traces
tracing: add trace_export support for event trace
tracing: add trace_export support for trace_marker
stm class: ftrace: enable supported trace export flag
stm class: ftrace: use different channel accroding to CPU

drivers/hwtracing/stm/Kconfig | 2 +-
drivers/hwtracing/stm/ftrace.c | 6 +-
include/linux/trace.h | 7 +
kernel/trace/trace.c | 270 ++++++++++++++++++---------------
4 files changed, 158 insertions(+), 127 deletions(-)

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


2020-07-26 03:02:40

by Tingwei Zhang

[permalink] [raw]
Subject: [PATCH v2 6/6] stm class: ftrace: use different channel accroding to CPU

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 | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/stm/ftrace.c b/drivers/hwtracing/stm/ftrace.c
index c694a6e692d1..9893a0e8eced 100644
--- a/drivers/hwtracing/stm/ftrace.c
+++ b/drivers/hwtracing/stm/ftrace.c
@@ -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)
@@ -63,6 +64,7 @@ static int __init stm_ftrace_init(void)
{
int ret;

+ stm_ftrace.data.nr_chans = num_possible_cpus();
ret = stm_source_register_device(NULL, &stm_ftrace.data);
if (ret)
pr_err("Failed to register stm_source - ftrace.\n");
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2020-07-26 03:02:48

by Tingwei Zhang

[permalink] [raw]
Subject: [PATCH v2 4/6] tracing: add trace_export support for trace_marker

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

Signed-off-by: Tingwei Zhang <[email protected]>
---
include/linux/trace.h | 1 +
kernel/trace/trace.c | 9 +++++++++
2 files changed, 10 insertions(+)

diff --git a/include/linux/trace.h b/include/linux/trace.h
index 91f6f1c0f2db..4bb6918fbf44 100644
--- a/include/linux/trace.h
+++ b/include/linux/trace.h
@@ -6,6 +6,7 @@

#define TRACE_EXPORT_FUNCTION BIT_ULL(0)
#define TRACE_EXPORT_EVENT BIT_ULL(1)
+#define TRACE_EXPORT_MARKER BIT_ULL(2)

/*
* The trace export - an export of Ftrace output. The trace_export
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 2f9302a8b322..67993abda394 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -271,6 +271,7 @@ static struct trace_export __rcu *ftrace_exports_list __read_mostly;

static DEFINE_STATIC_KEY_FALSE(trace_function_exports_enabled);
static DEFINE_STATIC_KEY_FALSE(trace_event_exports_enabled);
+static DEFINE_STATIC_KEY_FALSE(trace_marker_exports_enabled);

static inline void ftrace_exports_enable(struct trace_export *export)
{
@@ -279,6 +280,9 @@ static inline void ftrace_exports_enable(struct trace_export *export)

if (export->flags & TRACE_EXPORT_EVENT)
static_branch_inc(&trace_event_exports_enabled);
+
+ if (export->flags & TRACE_EXPORT_MARKER)
+ static_branch_inc(&trace_marker_exports_enabled);
}

static inline void ftrace_exports_disable(struct trace_export *export)
@@ -288,6 +292,9 @@ static inline void ftrace_exports_disable(struct trace_export *export)

if (export->flags & TRACE_EXPORT_EVENT)
static_branch_dec(&trace_event_exports_enabled);
+
+ if (export->flags & TRACE_EXPORT_MARKER)
+ static_branch_dec(&trace_marker_exports_enabled);
}

static void ftrace_exports(struct ring_buffer_event *event, int flag)
@@ -6648,6 +6655,8 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
} else
entry->buf[cnt] = '\0';

+ if (static_branch_unlikely(&trace_marker_exports_enabled))
+ ftrace_exports(event, TRACE_EXPORT_MARKER);
__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

2020-07-26 03:03:48

by Tingwei Zhang

[permalink] [raw]
Subject: [PATCH v2 2/6] tracing: add flag to control different traces

More traces like event trace or trace marker will be supported.
Add flag for difference traces, so that they can be controlled
separately. Move current function trace to it's own flag
instead of global ftrace enable flag.

Signed-off-by: Tingwei Zhang <[email protected]>
---
include/linux/trace.h | 5 +++++
kernel/trace/trace.c | 36 +++++++++++++++++++-----------------
2 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/include/linux/trace.h b/include/linux/trace.h
index 7fd86d3c691f..d2fdf9be84b5 100644
--- a/include/linux/trace.h
+++ b/include/linux/trace.h
@@ -3,6 +3,9 @@
#define _LINUX_TRACE_H

#ifdef CONFIG_TRACING
+
+#define TRACE_EXPORT_FUNCTION BIT_ULL(0)
+
/*
* The trace export - an export of Ftrace output. The trace_export
* can process traces and export them to a registered destination as
@@ -15,10 +18,12 @@
* next - pointer to the next trace_export
* write - copy traces which have been delt with ->commit() to
* the destination
+ * flags - which ftrace to be exported
*/
struct trace_export {
struct trace_export __rcu *next;
void (*write)(struct trace_export *, const void *, unsigned int);
+ int flags;
};

int register_ftrace_export(struct trace_export *export);
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index bb62269724d5..8f1e66831e9e 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2747,33 +2747,37 @@ trace_buffer_unlock_commit_nostack(struct trace_buffer *buffer,

static void
trace_process_export(struct trace_export *export,
- struct ring_buffer_event *event)
+ struct ring_buffer_event *event, int flag)
{
struct trace_entry *entry;
unsigned int size = 0;

- entry = ring_buffer_event_data(event);
- size = ring_buffer_event_length(event);
- export->write(export, entry, size);
+ if (export->flags & flag) {
+ entry = ring_buffer_event_data(event);
+ size = ring_buffer_event_length(event);
+ export->write(export, entry, size);
+ }
}

static DEFINE_MUTEX(ftrace_export_lock);

static struct trace_export __rcu *ftrace_exports_list __read_mostly;

-static DEFINE_STATIC_KEY_FALSE(ftrace_exports_enabled);
+static DEFINE_STATIC_KEY_FALSE(trace_function_exports_enabled);

-static inline void ftrace_exports_enable(void)
+static inline void ftrace_exports_enable(struct trace_export *export)
{
- static_branch_enable(&ftrace_exports_enabled);
+ if (export->flags & TRACE_EXPORT_FUNCTION)
+ static_branch_inc(&trace_function_exports_enabled);
}

-static inline void ftrace_exports_disable(void)
+static inline void ftrace_exports_disable(struct trace_export *export)
{
- static_branch_disable(&ftrace_exports_enabled);
+ if (export->flags & TRACE_EXPORT_FUNCTION)
+ static_branch_dec(&trace_function_exports_enabled);
}

-static void ftrace_exports(struct ring_buffer_event *event)
+static void ftrace_exports(struct ring_buffer_event *event, int flag)
{
struct trace_export *export;

@@ -2781,7 +2785,7 @@ static void ftrace_exports(struct ring_buffer_event *event)

export = rcu_dereference_raw_check(ftrace_exports_list);
while (export) {
- trace_process_export(export, event);
+ trace_process_export(export, event, flag);
export = rcu_dereference_raw_check(export->next);
}

@@ -2821,8 +2825,7 @@ rm_trace_export(struct trace_export **list, struct trace_export *export)
static inline void
add_ftrace_export(struct trace_export **list, struct trace_export *export)
{
- if (*list == NULL)
- ftrace_exports_enable();
+ ftrace_exports_enable(export);

add_trace_export(list, export);
}
@@ -2833,8 +2836,7 @@ rm_ftrace_export(struct trace_export **list, struct trace_export *export)
int ret;

ret = rm_trace_export(list, export);
- if (*list == NULL)
- ftrace_exports_disable();
+ ftrace_exports_disable(export);

return ret;
}
@@ -2887,8 +2889,8 @@ trace_function(struct trace_array *tr,
entry->parent_ip = parent_ip;

if (!call_filter_check_discard(call, entry, buffer, event)) {
- if (static_branch_unlikely(&ftrace_exports_enabled))
- ftrace_exports(event);
+ if (static_branch_unlikely(&trace_function_exports_enabled))
+ ftrace_exports(event, TRACE_EXPORT_FUNCTION);
__buffer_unlock_commit(buffer, event);
}
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2020-07-27 19:23:58

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v2 2/6] tracing: add flag to control different traces

On Sun, 26 Jul 2020 10:59:27 +0800
Tingwei Zhang <[email protected]> wrote:


> diff --git a/include/linux/trace.h b/include/linux/trace.h
> index 7fd86d3c691f..d2fdf9be84b5 100644
> --- a/include/linux/trace.h
> +++ b/include/linux/trace.h
> @@ -3,6 +3,9 @@
> #define _LINUX_TRACE_H
>
> #ifdef CONFIG_TRACING
> +
> +#define TRACE_EXPORT_FUNCTION BIT_ULL(0)

All the flags variables below are defined as "int". Why the BIT_ULL()?
BIT(0) should work just fine. The ULL makes one think these flags will
be used for unsigned long variables, which it is not.

-- Steve


> +

> struct trace_export {
> struct trace_export __rcu *next;
> void (*write)(struct trace_export *, const void *, unsigned int);
> + int flags;
> };
>
> int register_ftrace_export(struct trace_export *export);
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index bb62269724d5..8f1e66831e9e 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -2747,33 +2747,37 @@ trace_buffer_unlock_commit_nostack(struct trace_buffer *buffer,
>
> static void
> trace_process_export(struct trace_export *export,
> - struct ring_buffer_event *event)
> + struct ring_buffer_event *event, int flag)
> {
> struct trace_entry *entry;
> unsigned int size = 0;
>
> - entry = ring_buffer_event_data(event);
> - size = ring_buffer_event_length(event);
> - export->write(export, entry, size);
> + if (export->flags & flag) {
> + entry = ring_buffer_event_data(event);
> + size = ring_buffer_event_length(event);
> + export->write(export, entry, size);
> + }
> }

2020-07-27 19:32:57

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] stm class: ftrace: use different channel accroding to CPU

On Sun, 26 Jul 2020 10:59:31 +0800
Tingwei Zhang <[email protected]> wrote:

> --- a/drivers/hwtracing/stm/ftrace.c
> +++ b/drivers/hwtracing/stm/ftrace.c
> @@ -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();

Probably should add a comment to the above stating that this is called
from the tracing system with preemption disabled.

Other than my two comments:

Reviewed-by: Steven Rostedt (VMware) <[email protected]>

-- Steve


>
> - 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)

2020-07-28 00:30:27

by Tingwei Zhang

[permalink] [raw]
Subject: Re: [PATCH v2 2/6] tracing: add flag to control different traces

Hi Steve,
On Tue, Jul 28, 2020 at 02:45:50AM +0800, Steven Rostedt wrote:
> On Sun, 26 Jul 2020 10:59:27 +0800
> Tingwei Zhang <[email protected]> wrote:
>
>
> > diff --git a/include/linux/trace.h b/include/linux/trace.h
> > index 7fd86d3c691f..d2fdf9be84b5 100644
> > --- a/include/linux/trace.h
> > +++ b/include/linux/trace.h
> > @@ -3,6 +3,9 @@
> > #define _LINUX_TRACE_H
> >
> > #ifdef CONFIG_TRACING
> > +
> > +#define TRACE_EXPORT_FUNCTION BIT_ULL(0)
>
> All the flags variables below are defined as "int". Why the BIT_ULL()?
> BIT(0) should work just fine. The ULL makes one think these flags will
> be used for unsigned long variables, which it is not.
>

That's my mistake. Thanks for point that out. I'll fix it in next
revision.

> -- Steve
>
>
> > +
>
> > struct trace_export {
> > struct trace_export __rcu *next;
> > void (*write)(struct trace_export *, const void *, unsigned int);
> > + int flags;
> > };
> >
> > int register_ftrace_export(struct trace_export *export);
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index bb62269724d5..8f1e66831e9e 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -2747,33 +2747,37 @@ trace_buffer_unlock_commit_nostack(struct
> trace_buffer *buffer,
> >
> > static void
> > trace_process_export(struct trace_export *export,
> > - struct ring_buffer_event *event)
> > + struct ring_buffer_event *event, int flag)
> > {
> > struct trace_entry *entry;
> > unsigned int size = 0;
> >
> > - entry = ring_buffer_event_data(event);
> > - size = ring_buffer_event_length(event);
> > - export->write(export, entry, size);
> > + if (export->flags & flag) {
> > + entry = ring_buffer_event_data(event);
> > + size = ring_buffer_event_length(event);
> > + export->write(export, entry, size);
> > + }
> > }

2020-07-28 00:31:45

by Tingwei Zhang

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] stm class: ftrace: use different channel accroding to CPU

Hi Steve,
On Tue, Jul 28, 2020 at 03:08:12AM +0800, Steven Rostedt wrote:
> On Sun, 26 Jul 2020 10:59:31 +0800
> Tingwei Zhang <[email protected]> wrote:
>
> > --- a/drivers/hwtracing/stm/ftrace.c
> > +++ b/drivers/hwtracing/stm/ftrace.c
> > @@ -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();
>
> Probably should add a comment to the above stating that this is called
> from the tracing system with preemption disabled.
>

Good point. I'll add that comment in next revision.

Thanks,
Tingwei

> Other than my two comments:
>
> Reviewed-by: Steven Rostedt (VMware) <[email protected]>
>
> -- Steve
>
>
> >
> > - 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)