All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree
@ 2019-07-15  5:11 Masami Hiramatsu
  2019-07-15  5:11 ` [RFC PATCH v2 01/15] tracing: Apply soft-disabled and filter to tracepoints printk Masami Hiramatsu
                   ` (15 more replies)
  0 siblings, 16 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:11 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Hello,

Here is the 2nd version of RFC series to add boot-time tracing using
devicetree. Previous thread is here.

https://lkml.kernel.org/r/156113387975.28344.16009584175308192243.stgit@devnote2

In this version, I moved the ftrace node under /chosen/linux,ftrace
and remove compatible property, because it must be in fixed place.
Also this version has following features;

 - Introduce "instance" node, which can have events nodes for setting
   events filters and actions for the trace instance.
 - Introduce "cpumask" property
 - Introduce "ftrace-filters" and "ftrace-notraces"
 - Introduce "fgraph-filters", "fgraph-notraces" and "fgraph-max-depth"

At this moment, this feature is only available on the architecutre which
supports devicetree. For x86, we can use it on qemu with --dtb option,
or apply below patch on grub to add devicetree support on grub-x86.

https://github.com/mhiramat/grub/commit/644c35bfd2d18c772cc353b74215344f8264923a

Note that the devicetree for x86 must contain the nodes only under
/chosen/, or it may cause a problem if it conflicts with ACPI.
(Maybe we need to disable the FDT nodes except for nodes under /chosen
 on boot if ACPI exists.)

This series can be applied on Steve's tracing tree (ftrace/core) or
available on below

https://github.com/mhiramat/linux.git ftrace-devicetree-v2

Usage
======
With this series, we can setup new kprobe and synthetic events, more
complicated event filters and trigger actions including histogram
via devicetree.

For example, following kernel parameters

trace_options=sym-addr trace_event=initcall:* tp_printk trace_buf_size=1M ftrace=function ftrace_filter="vfs*"

it can be written in devicetree like below.

/{
chosen {
	...
	ftrace {
		options = "sym-addr";
		events = "initcall:*";
		tp-printk;
		buffer-size-kb = <0x400>;	// 1024KB == 1MB
		ftrace-filters = "vfs*";
	};

Moreover, now we can expand it to add filters for events, kprobe events,
and synthetic events with histogram like below.

	ftrace {
		...
		event0 {
			event = "task:task_newtask";
			filter = "pid < 128";	// adding filters
			enable;
		};
		event1 {
			event = "kprobes:vfs_read";
			probes = "vfs_read $arg1 $arg2"; // add kprobes
			filter = "common_pid < 200";
			enable;
		};
		event2 {
			event = "initcall_latency";	// add synth event
			fields = "unsigned long func", "u64 lat";
			// with histogram
			actions = "hist:keys=func.sym,lat:vals=lat:sort=lat";
		};
		// and synthetic event callers
		event3 {
			event = "initcall:initcall_start";
			actions = "hist:keys=func:ts0=common_timestamp.usecs";
		};
		event4 {
			event = "initcall:initcall_finish";
			actions = "hist:keys=func:lat=common_timestamp.usecs-$ts0:onmatch(initcall.initcall_start).initcall_latency(func,$lat)";
		};
	};

Also, this version supports "instance" node, which allows us to
run several tracers for different purpose at once. For example,
one tracer is for tracing functions in module alpha, and others
tracing module beta, you can write followings.

	ftrace {
		instance0 {
			instance = "foo";
			tracer = "function";
			ftrace-filters = "*:mod:alpha";
		};
		instance1 {
			instance = "bar";
			tracer = "function";
			ftrace-filters = "*:mod:beta";
			
		};
	};

The instance node also accepts event nodes so that each instance
can customize its event tracing.

Discussion
=====
On the previous thread, we discussed that the this devicetree usage
itself was acceptable or not. Fortunately, I had a chance to discuss
it in a F2F meeting with Frank and Tim last week.

I think the advantages of using devicetree are,

- reuse devicetree's structured syntax for complicated tracefs settings
- reuse OF-APIs in linux kernel to accept and parse it
- reuse dtc complier to compile it and validate syntax. (with yaml schema,
  we can enhance it)
- reuse current bootloader (and qemu) to load it

And we talked about some other ideas to avoid using devicetree.

- expand kernel command line (ascii command strings)
- expand kernel command line with base64 encoded comressed ascii command 
   strings
- load (compressed) ascii command strings to somewhere on memory and pass
   the address via kernel cmdline
- load (compressed) ascii command strings to somewhere on memory and pass
   the address via /chosen node (as same as initrd)
- load binary C data and point it from kernel cmdline
- load binary C data and point it from /chosen node (as same as initrd)
- load binary C data as a section of kernel image

The first 2 ideas expand the kernel's cmdline to pass some "magic" command
to setup ftrace. In both case, the problems are the maximal size of cmdline
and the issues related to the complexity of commands.
My example showed that the ftrace settings becomes long even if making one
histogram, which can be longer than 256 bytes. The long and complex data
can easily lead mis-typing, but cmdline has no syntax validator, it just
ignores the mis-typed commands.
(Of course even with the devicetree, it must be smaller than 2 pages)

Next 2 ideas are similar, but load the commands on some other memory area
and pass only address via cmdline. This solves the size limitation issue,
but still no syntax validation. Of course we can make a new structured
syntax validator similar to (or just forked from) dt-validate.
The problem (or disadvantage) of these (and following) ideas, is to change
the kernel and boot loaders to load another binary blobs on memory.

Maybe if we introduce a generic structured kernel boot arguments, which is
a kind of /chosen node of devicetree. (But if there is already such hook,
why we make another one...?)
Also, this "GSKBA" may introduce a parser and access APIs which will be
very similar to OF-APIs. This also seems redundant to me.

So the last 3 ideas will avoid introducing new parser and APIs, we just
compile the data as C data and point it from cmdline or somewhere else.
With these ideas, we still need to expand boot loaders to support
loading new binary blobs. (And the last one requires to add elf header
parser/modifier to boot loader too)

>From the above reasons, I think using devicetree's /chosen node is 
the least intrusive way to introduce this boot-time tracing feature.

Any suggestions, thoughts?

Thank you,

---

Masami Hiramatsu (15):
      tracing: Apply soft-disabled and filter to tracepoints printk
      tracing: kprobes: Output kprobe event to printk buffer
      tracing: Expose EXPORT_SYMBOL_GPL symbol
      tracing: kprobes: Register to dynevent earlier stage
      tracing: Accept different type for synthetic event fields
      tracing: Add NULL trace-array check in print_synth_event()
      dt-bindings: tracing: Add ftrace binding document
      tracing: of: Add setup tracing by devicetree support
      tracing: of: Add trace event settings
      tracing: of: Add kprobe event support
      tracing: of: Add synthetic event support
      tracing: of: Add instance node support
      tracing: of: Add cpumask property support
      tracing: of: Add function tracer filter properties
      tracing: of: Add function-graph tracer option properties


 .../devicetree/bindings/chosen/linux,ftrace.yaml   |  306 ++++++++++++
 include/linux/trace_events.h                       |    1 
 kernel/trace/Kconfig                               |   10 
 kernel/trace/Makefile                              |    1 
 kernel/trace/ftrace.c                              |   85 ++-
 kernel/trace/trace.c                               |   90 ++--
 kernel/trace/trace_events.c                        |    3 
 kernel/trace/trace_events_hist.c                   |   14 -
 kernel/trace/trace_events_trigger.c                |    2 
 kernel/trace/trace_kprobe.c                        |   81 ++-
 kernel/trace/trace_of.c                            |  507 ++++++++++++++++++++
 11 files changed, 1004 insertions(+), 96 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/chosen/linux,ftrace.yaml
 create mode 100644 kernel/trace/trace_of.c

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 01/15] tracing: Apply soft-disabled and filter to tracepoints printk
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
@ 2019-07-15  5:11 ` Masami Hiramatsu
  2019-07-15  5:11 ` [RFC PATCH v2 02/15] tracing: kprobes: Output kprobe event to printk buffer Masami Hiramatsu
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:11 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Apply soft-disabled and the filter rule of the trace events to
the printk output of tracepoints (a.k.a. tp_printk kernel parameter)
as same as trace buffer output.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/trace.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6b62e1718548..047f37816bf1 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2502,6 +2502,7 @@ static DEFINE_MUTEX(tracepoint_printk_mutex);
 static void output_printk(struct trace_event_buffer *fbuffer)
 {
 	struct trace_event_call *event_call;
+	struct trace_event_file *file;
 	struct trace_event *event;
 	unsigned long flags;
 	struct trace_iterator *iter = tracepoint_print_iter;
@@ -2515,6 +2516,12 @@ static void output_printk(struct trace_event_buffer *fbuffer)
 	    !event_call->event.funcs->trace)
 		return;
 
+	file = fbuffer->trace_file;
+	if (test_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &file->flags) ||
+	    (unlikely(file->flags & EVENT_FILE_FL_FILTERED) &&
+	     !filter_match_preds(file->filter, fbuffer->entry)))
+		return;
+
 	event = &fbuffer->trace_file->event_call->event;
 
 	spin_lock_irqsave(&tracepoint_iter_lock, flags);


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 02/15] tracing: kprobes: Output kprobe event to printk buffer
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
  2019-07-15  5:11 ` [RFC PATCH v2 01/15] tracing: Apply soft-disabled and filter to tracepoints printk Masami Hiramatsu
@ 2019-07-15  5:11 ` Masami Hiramatsu
  2019-07-15  5:11 ` [RFC PATCH v2 03/15] tracing: Expose EXPORT_SYMBOL_GPL symbol Masami Hiramatsu
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:11 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Since kprobe-events use event_trigger_unlock_commit_regs() directly,
that events doesn't show up in printk buffer if "tp_printk" is set.

Use trace_event_buffer_commit() in kprobe events so that it can
invoke output_printk() as same as other trace events.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 include/linux/trace_events.h |    1 +
 kernel/trace/trace.c         |    4 +--
 kernel/trace/trace_events.c  |    1 +
 kernel/trace/trace_kprobe.c  |   57 +++++++++++++++++++++---------------------
 4 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 5150436783e8..8912ccdb3d4b 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -211,6 +211,7 @@ struct trace_event_buffer {
 	void				*entry;
 	unsigned long			flags;
 	int				pc;
+	struct pt_regs			*regs;
 };
 
 void *trace_event_buffer_reserve(struct trace_event_buffer *fbuffer,
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 047f37816bf1..30f09c058f79 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2572,9 +2572,9 @@ 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(fbuffer->trace_file, fbuffer->buffer,
+	event_trigger_unlock_commit_regs(fbuffer->trace_file, fbuffer->buffer,
 				    fbuffer->event, fbuffer->entry,
-				    fbuffer->flags, fbuffer->pc);
+				    fbuffer->flags, fbuffer->pc, fbuffer->regs);
 }
 EXPORT_SYMBOL_GPL(trace_event_buffer_commit);
 
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index c7506bc81b75..22cf08bd2317 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -271,6 +271,7 @@ void *trace_event_buffer_reserve(struct trace_event_buffer *fbuffer,
 	if (!fbuffer->event)
 		return NULL;
 
+	fbuffer->regs = NULL;
 	fbuffer->entry = ring_buffer_event_data(fbuffer->event);
 	return fbuffer->entry;
 }
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 9d483ad9bb6c..6c5145525f90 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -988,10 +988,8 @@ __kprobe_trace_func(struct trace_kprobe *tk, struct pt_regs *regs,
 		    struct trace_event_file *trace_file)
 {
 	struct kprobe_trace_entry_head *entry;
-	struct ring_buffer_event *event;
-	struct ring_buffer *buffer;
-	int size, dsize, pc;
-	unsigned long irq_flags;
+	struct trace_event_buffer fbuffer;
+	int dsize;
 	struct trace_event_call *call = trace_probe_event_call(&tk->tp);
 
 	WARN_ON(call != trace_file->event_call);
@@ -999,24 +997,26 @@ __kprobe_trace_func(struct trace_kprobe *tk, struct pt_regs *regs,
 	if (trace_trigger_soft_disabled(trace_file))
 		return;
 
-	local_save_flags(irq_flags);
-	pc = preempt_count();
+	local_save_flags(fbuffer.flags);
+	fbuffer.pc = preempt_count();
+	fbuffer.trace_file = trace_file;
 
 	dsize = __get_data_size(&tk->tp, regs);
-	size = sizeof(*entry) + tk->tp.size + dsize;
 
-	event = trace_event_buffer_lock_reserve(&buffer, trace_file,
-						call->event.type,
-						size, irq_flags, pc);
-	if (!event)
+	fbuffer.event =
+		trace_event_buffer_lock_reserve(&fbuffer.buffer, trace_file,
+					call->event.type,
+					sizeof(*entry) + tk->tp.size + dsize,
+					fbuffer.flags, fbuffer.pc);
+	if (!fbuffer.event)
 		return;
 
-	entry = ring_buffer_event_data(event);
+	fbuffer.regs = regs;
+	entry = fbuffer.entry = ring_buffer_event_data(fbuffer.event);
 	entry->ip = (unsigned long)tk->rp.kp.addr;
 	store_trace_args(&entry[1], &tk->tp, regs, sizeof(*entry), dsize);
 
-	event_trigger_unlock_commit_regs(trace_file, buffer, event,
-					 entry, irq_flags, pc, regs);
+	trace_event_buffer_commit(&fbuffer);
 }
 
 static void
@@ -1036,10 +1036,8 @@ __kretprobe_trace_func(struct trace_kprobe *tk, struct kretprobe_instance *ri,
 		       struct trace_event_file *trace_file)
 {
 	struct kretprobe_trace_entry_head *entry;
-	struct ring_buffer_event *event;
-	struct ring_buffer *buffer;
-	int size, pc, dsize;
-	unsigned long irq_flags;
+	struct trace_event_buffer fbuffer;
+	int dsize;
 	struct trace_event_call *call = trace_probe_event_call(&tk->tp);
 
 	WARN_ON(call != trace_file->event_call);
@@ -1047,25 +1045,26 @@ __kretprobe_trace_func(struct trace_kprobe *tk, struct kretprobe_instance *ri,
 	if (trace_trigger_soft_disabled(trace_file))
 		return;
 
-	local_save_flags(irq_flags);
-	pc = preempt_count();
+	local_save_flags(fbuffer.flags);
+	fbuffer.pc = preempt_count();
+	fbuffer.trace_file = trace_file;
 
 	dsize = __get_data_size(&tk->tp, regs);
-	size = sizeof(*entry) + tk->tp.size + dsize;
-
-	event = trace_event_buffer_lock_reserve(&buffer, trace_file,
-						call->event.type,
-						size, irq_flags, pc);
-	if (!event)
+	fbuffer.event =
+		trace_event_buffer_lock_reserve(&fbuffer.buffer, trace_file,
+					call->event.type,
+					sizeof(*entry) + tk->tp.size + dsize,
+					fbuffer.flags, fbuffer.pc);
+	if (!fbuffer.event)
 		return;
 
-	entry = ring_buffer_event_data(event);
+	fbuffer.regs = regs;
+	entry = fbuffer.entry = ring_buffer_event_data(fbuffer.event);
 	entry->func = (unsigned long)tk->rp.kp.addr;
 	entry->ret_ip = (unsigned long)ri->ret_addr;
 	store_trace_args(&entry[1], &tk->tp, regs, sizeof(*entry), dsize);
 
-	event_trigger_unlock_commit_regs(trace_file, buffer, event,
-					 entry, irq_flags, pc, regs);
+	trace_event_buffer_commit(&fbuffer);
 }
 
 static void


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 03/15] tracing: Expose EXPORT_SYMBOL_GPL symbol
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
  2019-07-15  5:11 ` [RFC PATCH v2 01/15] tracing: Apply soft-disabled and filter to tracepoints printk Masami Hiramatsu
  2019-07-15  5:11 ` [RFC PATCH v2 02/15] tracing: kprobes: Output kprobe event to printk buffer Masami Hiramatsu
@ 2019-07-15  5:11 ` Masami Hiramatsu
  2019-07-15  5:11 ` [RFC PATCH v2 04/15] tracing: kprobes: Register to dynevent earlier stage Masami Hiramatsu
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:11 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Since ftrace_set_clr_event is already exported by EXPORT_SYMBOL_GPL,
it should not be static.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/trace_events.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 22cf08bd2317..c2d38048edae 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -788,7 +788,7 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
 	return ret;
 }
 
-static int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
+int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
 {
 	char *event = NULL, *sub = NULL, *match;
 	int ret;


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 04/15] tracing: kprobes: Register to dynevent earlier stage
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
                   ` (2 preceding siblings ...)
  2019-07-15  5:11 ` [RFC PATCH v2 03/15] tracing: Expose EXPORT_SYMBOL_GPL symbol Masami Hiramatsu
@ 2019-07-15  5:11 ` Masami Hiramatsu
  2019-07-15  5:12 ` [RFC PATCH v2 05/15] tracing: Accept different type for synthetic event fields Masami Hiramatsu
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:11 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Register kprobe event to dynevent in subsys_initcall level.
This will allow kernel to register new kprobe events in
fs_initcall level via trace_run_command.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/trace_kprobe.c |   19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 6c5145525f90..5135c07b6557 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1484,11 +1484,12 @@ static __init void setup_boot_kprobe_events(void)
 	enable_boot_kprobe_events();
 }
 
-/* Make a tracefs interface for controlling probe points */
-static __init int init_kprobe_trace(void)
+/*
+ * Register dynevent at subsys_initcall. This allows kernel to setup kprobe
+ * events in fs_initcall without tracefs.
+ */
+static __init int init_kprobe_trace_early(void)
 {
-	struct dentry *d_tracer;
-	struct dentry *entry;
 	int ret;
 
 	ret = dyn_event_register(&trace_kprobe_ops);
@@ -1498,6 +1499,16 @@ static __init int init_kprobe_trace(void)
 	if (register_module_notifier(&trace_kprobe_module_nb))
 		return -EINVAL;
 
+	return 0;
+}
+subsys_initcall(init_kprobe_trace_early);
+
+/* Make a tracefs interface for controlling probe points */
+static __init int init_kprobe_trace(void)
+{
+	struct dentry *d_tracer;
+	struct dentry *entry;
+
 	d_tracer = tracing_init_dentry();
 	if (IS_ERR(d_tracer))
 		return 0;


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 05/15] tracing: Accept different type for synthetic event fields
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
                   ` (3 preceding siblings ...)
  2019-07-15  5:11 ` [RFC PATCH v2 04/15] tracing: kprobes: Register to dynevent earlier stage Masami Hiramatsu
@ 2019-07-15  5:12 ` Masami Hiramatsu
  2019-07-15  5:12 ` [RFC PATCH v2 06/15] tracing: Add NULL trace-array check in print_synth_event() Masami Hiramatsu
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:12 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Make the synthetic event accepts a different type field to record.
However, the size and signed flag must be same.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/trace_events_hist.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index ca6b0dff60c5..a7f447195143 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -4063,8 +4063,11 @@ static int check_synth_field(struct synth_event *event,
 
 	field = event->fields[field_pos];
 
-	if (strcmp(field->type, hist_field->type) != 0)
-		return -EINVAL;
+	if (strcmp(field->type, hist_field->type) != 0) {
+		if (field->size != hist_field->size ||
+		    field->is_signed != hist_field->is_signed)
+			return -EINVAL;
+	}
 
 	return 0;
 }


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 06/15] tracing: Add NULL trace-array check in print_synth_event()
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
                   ` (4 preceding siblings ...)
  2019-07-15  5:12 ` [RFC PATCH v2 05/15] tracing: Accept different type for synthetic event fields Masami Hiramatsu
@ 2019-07-15  5:12 ` Masami Hiramatsu
  2019-07-15  5:12 ` [RFC PATCH v2 07/15] dt-bindings: tracing: Add ftrace binding document Masami Hiramatsu
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:12 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Add NULL trace-array check in print_synth_event(), because
if we enable tp_printk option, iter->tr can be NULL.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/trace_events_hist.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index a7f447195143..db973928e580 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -822,7 +822,7 @@ static enum print_line_t print_synth_event(struct trace_iterator *iter,
 		fmt = synth_field_fmt(se->fields[i]->type);
 
 		/* parameter types */
-		if (tr->trace_flags & TRACE_ITER_VERBOSE)
+		if (tr && tr->trace_flags & TRACE_ITER_VERBOSE)
 			trace_seq_printf(s, "%s ", fmt);
 
 		snprintf(print_fmt, sizeof(print_fmt), "%%s=%s%%s", fmt);


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 07/15] dt-bindings: tracing: Add ftrace binding document
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
                   ` (5 preceding siblings ...)
  2019-07-15  5:12 ` [RFC PATCH v2 06/15] tracing: Add NULL trace-array check in print_synth_event() Masami Hiramatsu
@ 2019-07-15  5:12 ` Masami Hiramatsu
  2019-07-15  5:12 ` [RFC PATCH v2 08/15] tracing: of: Add setup tracing by devicetree support Masami Hiramatsu
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:12 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Add a devicetree binding document for ftrace node.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
  Changes in v2:
    - Add cpumask, ftrace-filters, ftrace-notraces, fgraph-filters,
      fgraph-notraces, fgraph-max-depth and instance node.
    - Remove compatible and move file to bindings/chosen/linux,ftrace.yaml
---
 .../devicetree/bindings/chosen/linux,ftrace.yaml   |  306 ++++++++++++++++++++
 1 file changed, 306 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/chosen/linux,ftrace.yaml

diff --git a/Documentation/devicetree/bindings/chosen/linux,ftrace.yaml b/Documentation/devicetree/bindings/chosen/linux,ftrace.yaml
new file mode 100644
index 000000000000..a5c60ac2f66d
--- /dev/null
+++ b/Documentation/devicetree/bindings/chosen/linux,ftrace.yaml
@@ -0,0 +1,306 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019 Linaro Ltd.
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/tracing/ftrace.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Ftrace setting devicetree binding
+
+maintainers:
+  - Masami Hiramatsu <mhiramat@kernel.org>
+
+description: |
+  Boot-time ftrace tracing setting via devicetree. Users can use devicetree node
+  for programming ftrace boot-time tracing. This node must be placed at
+  /chosen/linux,ftrace.
+
+properties:
+  dump-on-oops:
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+      - enum: [0, 1, 2]
+    description: |
+      A neumerical flag to enable ftrace dump on Kernel Oops. 0 means no dump,
+      1 means dump on the origin cpu of the oops, and means dump on all cpus.
+
+  traceoff-on-warning:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: A flag to stop tracing on warning.
+
+  tp-printk:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: A flag to send tracing output to printk buffer too.
+
+  alloc-snapshot:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: |
+      A flag to allocate snapshot buffer at boot. This will be required if you
+      use "snapshot" action on some events.
+
+  trace-clock:
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/string
+      - enum: [ global, local, counter, uptime, perf, mono, mono_raw, boot, ppc-tb, x86-tsc ]
+    description: Specify which clock method is used for trace-clock.
+
+  buffer-size-kb:
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+      - minimum: 1
+    description: |
+      The size of per-cpu tracing buffer in KByte. Note that the size must be
+      larger than page size, and total size of buffers depends on the number
+      of CPUs.
+
+  events:
+    minItems: 1
+    $ref: /schemas/types.yaml#/definitions/string-array
+    description: |
+      A string array of enabling events (including wildcard patterns).
+      See Documentation/trace/events.rst for detail.
+
+  options:
+    minItems: 1
+    $ref: /schemas/types.yaml#/definitions/string-array
+    description: |
+      A string array of trace options for ftrace to control what gets printed
+      in the trace output or manipulate the tracers.
+      See Documentation/trace/ftrace.rst#trace_options for detail.
+
+  tracer:
+    default: nop
+    $ref: /schemas/types.yaml#/definitions/string
+    description: A string of the tracer to start up.
+
+  cpumask:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: |
+      A hexadecimal number of the cpu-mask value which is given as a string.
+      This is because the number of cores can be bigger than 64.
+
+  ftrace-filters:
+    minItems: 1
+    $ref: /schemas/types.yaml#/definitions/string-array
+    description: |
+        A string array of the functions traced by the function tracer at boot
+        up. The function can be given with wildcards. This list can be
+        specified in each instance.
+
+  ftrace-notraces:
+    minItems: 1
+    $ref: /schemas/types.yaml#/definitions/string-array
+    description: |
+        A string array of the functions NOT traced by the function tracer at
+        boot up. The function can be given with wildcards. This list can be
+        specified in each instance.
+
+  fgraph-filters:
+    minItems: 1
+    $ref: /schemas/types.yaml#/definitions/string-array
+    description: |
+        A string array of the top level callers functions traced by the
+        function graph tracer at boot up. The function can be given with
+        wildcards. This list is not available in instance node.
+
+  fgraph-notraces:
+    minItems: 1
+    $ref: /schemas/types.yaml#/definitions/string-array
+    description: |
+        A string array of the top level callers functions NOT traced by the
+        function graph tracer at boot up. The function can be given with
+        wildcards. This list is not available in instance node.
+
+  fgraph-max-depth:
+    default: 0
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+        This is the max depth, the function graph tracer will trace into
+        a function. 0 means no limit for the trace depth.
+
+patternProperties:
+   "event[0-9a-fA-F]+$":
+     type: object
+
+     description: |
+       event* properties are child nodes for per-event settings. It is also
+       able to define new kprobe events and synthetic events. Note that you
+       can not define both "probes" and "fields" properties on same event.
+
+     properties:
+       event:
+         $ref: /schemas/types.yaml#/definitions/string
+         description: |
+           Event name string formatted as "GROUP:EVENT". For synthetic event,
+           you must use "synthetic" for group name. For kprobe and synthetic
+           event, you can ommit the group name.
+
+       probes:
+         minItems: 1
+         $ref: /schemas/types.yaml#/definitions/string-array
+         description: |
+           A string array of kprobe event definitions, including location
+           (symbol+offset) and event arguments.
+           See Documentation/trace/kprobetrace.rst for detail.
+
+       fields:
+         minItems: 1
+         $ref: /schemas/types.yaml#/definitions/string-array
+         description: |
+           A string of synthetic event's fields definition. Note that you
+           don't need to repeat event name.
+
+       filter:
+         $ref: /schemas/types.yaml#/definitions/string
+         description: A string of event filter rule
+
+       actions:
+         minItems: 1
+         $ref: /schemas/types.yaml#/definitions/string-array
+         description: A string array of event trigger actions.
+
+       enable:
+         type: boolean
+         description: |
+           A flag to enable event. Note that the event is not enabled by
+           default. (But actions will set the event soft-disabled)
+
+     oneOf:
+       - required:
+         - event
+       - required:
+         - event
+         - probes
+       - required:
+         - event
+         - fields
+
+   "instance[0-9a-fA-F]+$":
+     type: object
+
+     description: |
+       instance* properties are child nodes for per-instance settings.
+       It is also able to have event nodes as linux,ftrace node does.
+
+     properties:
+       instance:
+         $ref: /schemas/types.yaml#/definitions/string
+         description: The name of new instance.
+
+       trace-clock:
+         allOf:
+           - $ref: /schemas/types.yaml#/definitions/string
+           - enum: [ global, local, counter, uptime, perf, mono, mono_raw, boot, ppc-tb, x86-tsc ]
+         description: Specify which clock method is used for trace-clock.
+
+       buffer-size-kb:
+         allOf:
+           - $ref: /schemas/types.yaml#/definitions/uint32
+           - minimum: 1
+         description: |
+           The size of per-cpu tracing buffer in KByte. Note that the size must be
+           larger than page size, and total size of buffers depends on the number
+           of CPUs.
+
+       events:
+         minItems: 1
+         $ref: /schemas/types.yaml#/definitions/string-array
+         description: |
+           A string array of enabling events (including wildcard patterns).
+           See Documentation/trace/events.rst for detail.
+
+       options:
+         minItems: 1
+         $ref: /schemas/types.yaml#/definitions/string-array
+         description: |
+           A string array of trace options for ftrace to control what gets printed
+           in the trace output or manipulate the tracers.
+           See Documentation/trace/ftrace.rst#trace_options for detail.
+
+       tracer:
+         default: nop
+         $ref: /schemas/types.yaml#/definitions/string
+         description: A string of the tracer to start up.
+
+       cpumask:
+         $ref: /schemas/types.yaml#/definitions/string
+         description: |
+           A hexadecimal number of the cpu-mask value which is given as a string.
+           This is because the number of cores can be bigger than 64.
+
+       ftrace-filters:
+         minItems: 1
+         $ref: /schemas/types.yaml#/definitions/string-array
+         description: |
+             A string array of the functions traced by the function tracer at boot
+             up. The function can be given with wildcards. This list can be
+             specified in each instance.
+
+       ftrace-notraces:
+         minItems: 1
+         $ref: /schemas/types.yaml#/definitions/string-array
+         description: |
+             A string array of the functions NOT traced by the function tracer at
+             boot up. The function can be given with wildcards. This list can be
+             specified in each instance.
+
+       fgraph-filters:
+         minItems: 1
+         $ref: /schemas/types.yaml#/definitions/string-array
+         description: |
+             A string array of the top level callers functions traced by the
+             function graph tracer at boot up. The function can be given with
+             wildcards. This list is not available in instance node.
+
+       fgraph-notraces:
+         minItems: 1
+         $ref: /schemas/types.yaml#/definitions/string-array
+         description: |
+             A string array of the top level callers functions NOT traced by the
+             function graph tracer at boot up. The function can be given with
+             wildcards. This list is not available in instance node.
+
+       fgraph-max-depth:
+         default: 0
+         $ref: /schemas/types.yaml#/definitions/uint32
+         description: |
+             This is the max depth, the function graph tracer will trace into
+             a function. 0 means no limit for the trace depth.
+
+     required:
+      - instance
+
+examples:
+  - |
+    ftrace {
+          events = "initcall:*";
+          tp-printk;
+          buffer-size-kb = <0x400>;
+          event1 {
+                event = "kprobes:vfs_read";
+                probes = "vfs_read $arg1 $arg2";
+                filter = "common_pid < 200";
+          };
+          event2 {
+                event = "initcall_latency";
+                fields = "unsigned long func", "u64 lat";
+                actions = "hist:keys=func.sym,lat:vals=lat:sort=lat";
+          };
+          event3 {
+                event = "initcall:initcall_start";
+                actions = "hist:keys=func:ts0=common_timestamp.usecs";
+          };
+          event4 {
+                event = "initcall:initcall_finish";
+                actions = "hist:keys=func:lat=common_timestamp.usecs-$ts0:onmatch(initcall.initcall_start).initcall_latency(func,$lat)";
+          };
+          instance0 {
+                buffer-size-kb = <0x100>;
+                event5 {
+                      event = "task:task_newtask";
+                      filter = "pid < 128";
+                      enable;
+                };
+          };
+
+    };


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 08/15] tracing: of: Add setup tracing by devicetree support
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
                   ` (6 preceding siblings ...)
  2019-07-15  5:12 ` [RFC PATCH v2 07/15] dt-bindings: tracing: Add ftrace binding document Masami Hiramatsu
@ 2019-07-15  5:12 ` Masami Hiramatsu
  2019-07-15  5:12 ` [RFC PATCH v2 09/15] tracing: of: Add trace event settings Masami Hiramatsu
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:12 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Setup tracing options by devicetree instead of kernel parameters.

Since the kernel parameter is limited length, sometimes there is
no space to setup the tracing options. This will read the tracing
options from devicetree "ftrace" node and setup tracers at boot.

Note that this is not replacing the kernel parameters, because
this devicetree based setting is later than that. If you want to
trace earlier boot events, you still need kernel parameters.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
  Changes in v2:
    - Move ftrace node under chosen node.
    - Make events property available only when CONFIG_EVENT_TRACING=y
---
 kernel/trace/Kconfig    |   10 +++
 kernel/trace/Makefile   |    1 
 kernel/trace/trace.c    |   38 ++++++++----
 kernel/trace/trace_of.c |  150 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 186 insertions(+), 13 deletions(-)
 create mode 100644 kernel/trace/trace_of.c

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 07d22c61b634..025198bb2764 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -796,6 +796,16 @@ config GCOV_PROFILE_FTRACE
 	  Note that on a kernel compiled with this config, ftrace will
 	  run significantly slower.
 
+config OF_TRACING
+	bool "Boot Trace Programming by Devicetree"
+	depends on OF && TRACING
+	select OF_EARLY_FLATTREE
+	default y
+	help
+	  Enable developer to setup ftrace subsystem via devicetree
+	  at boot time for debugging (tracing) driver initialization
+	  and boot process.
+
 endif # FTRACE
 
 endif # TRACING_SUPPORT
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index c2b2148bb1d2..6f68271f5c56 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -82,6 +82,7 @@ endif
 obj-$(CONFIG_DYNAMIC_EVENTS) += trace_dynevent.o
 obj-$(CONFIG_PROBE_EVENTS) += trace_probe.o
 obj-$(CONFIG_UPROBE_EVENTS) += trace_uprobe.o
+obj-$(CONFIG_OF_TRACING) += trace_of.o
 
 obj-$(CONFIG_TRACEPOINT_BENCHMARK) += trace_benchmark.o
 
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 30f09c058f79..6f50c58a3ad4 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -158,7 +158,7 @@ union trace_eval_map_item {
 static union trace_eval_map_item *trace_eval_maps;
 #endif /* CONFIG_TRACE_EVAL_MAP_FILE */
 
-static int tracing_set_tracer(struct trace_array *tr, const char *buf);
+int tracing_set_tracer(struct trace_array *tr, const char *buf);
 static void ftrace_trace_userstack(struct ring_buffer *buffer,
 				   unsigned long flags, int pc);
 
@@ -178,6 +178,11 @@ static int __init set_cmdline_ftrace(char *str)
 }
 __setup("ftrace=", set_cmdline_ftrace);
 
+void __init trace_init_dump_on_oops(int mode)
+{
+	ftrace_dump_on_oops = mode;
+}
+
 static int __init set_ftrace_dump_on_oops(char *str)
 {
 	if (*str++ != '=' || !*str) {
@@ -4614,7 +4619,7 @@ int set_tracer_flag(struct trace_array *tr, unsigned int mask, int enabled)
 	return 0;
 }
 
-static int trace_set_options(struct trace_array *tr, char *option)
+int trace_set_options(struct trace_array *tr, char *option)
 {
 	char *cmp;
 	int neg = 0;
@@ -5505,8 +5510,8 @@ static int __tracing_resize_ring_buffer(struct trace_array *tr,
 	return ret;
 }
 
-static ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
-					  unsigned long size, int cpu_id)
+ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
+				  unsigned long size, int cpu_id)
 {
 	int ret = size;
 
@@ -5585,7 +5590,7 @@ static void add_tracer_options(struct trace_array *tr, struct tracer *t)
 	create_trace_option_files(tr, t);
 }
 
-static int tracing_set_tracer(struct trace_array *tr, const char *buf)
+int tracing_set_tracer(struct trace_array *tr, const char *buf)
 {
 	struct tracer *t;
 #ifdef CONFIG_TRACER_MAX_TRACE
@@ -9160,16 +9165,23 @@ __init static int tracer_alloc_buffers(void)
 	return ret;
 }
 
+void __init trace_init_tracepoint_printk(void)
+{
+	tracepoint_printk = 1;
+
+	tracepoint_print_iter =
+		kmalloc(sizeof(*tracepoint_print_iter), GFP_KERNEL);
+	if (WARN_ON(!tracepoint_print_iter))
+		tracepoint_printk = 0;
+	else
+		static_key_enable(&tracepoint_printk_key.key);
+}
+
 void __init early_trace_init(void)
 {
-	if (tracepoint_printk) {
-		tracepoint_print_iter =
-			kmalloc(sizeof(*tracepoint_print_iter), GFP_KERNEL);
-		if (WARN_ON(!tracepoint_print_iter))
-			tracepoint_printk = 0;
-		else
-			static_key_enable(&tracepoint_printk_key.key);
-	}
+	if (tracepoint_printk)
+		trace_init_tracepoint_printk();
+
 	tracer_alloc_buffers();
 }
 
diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
new file mode 100644
index 000000000000..5e108a1d5bb3
--- /dev/null
+++ b/kernel/trace/trace_of.c
@@ -0,0 +1,150 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * trace_of.c
+ * devicetree tracing programming APIs
+ */
+
+#define pr_fmt(fmt)	"trace_of: " fmt
+
+#include <linux/ftrace.h>
+#include <linux/init.h>
+#include <linux/of.h>
+
+#include "trace.h"
+
+#define MAX_BUF_LEN 256
+
+extern int trace_set_options(struct trace_array *tr, char *option);
+extern enum ftrace_dump_mode ftrace_dump_on_oops;
+extern int __disable_trace_on_warning;
+extern int tracing_set_tracer(struct trace_array *tr, const char *buf);
+extern void __init trace_init_tracepoint_printk(void);
+extern ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
+					  unsigned long size, int cpu_id);
+
+static void __init
+trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
+{
+	struct property *prop;
+	const char *p;
+	char buf[MAX_BUF_LEN];
+	u32 v = 0;
+	int err;
+
+	/* Common ftrace options */
+	of_property_for_each_string(node, "options", prop, p) {
+		if (strlcpy(buf, p, ARRAY_SIZE(buf)) >= ARRAY_SIZE(buf)) {
+			pr_err("String is too long: %s\n", p);
+			continue;
+		}
+
+		if (trace_set_options(tr, buf) < 0)
+			pr_err("Failed to set option: %s\n", buf);
+	}
+
+	err = of_property_read_string(node, "trace-clock", &p);
+	if (!err) {
+		if (tracing_set_clock(tr, p) < 0)
+			pr_err("Failed to set trace clock: %s\n", p);
+	}
+
+	/* Command line boot options */
+	if (of_find_property(node, "dump-on-oops", NULL)) {
+		err = of_property_read_u32_index(node, "dump-on-oops", 0, &v);
+		if (err || v == 1)
+			ftrace_dump_on_oops = DUMP_ALL;
+		else if (!err && v == 2)
+			ftrace_dump_on_oops = DUMP_ORIG;
+	}
+
+	if (of_find_property(node, "traceoff-on-warning", NULL))
+		__disable_trace_on_warning = 1;
+
+	if (of_find_property(node, "tp-printk", NULL))
+		trace_init_tracepoint_printk();
+
+	/* This accepts per-cpu buffer size in KB */
+	err = of_property_read_u32_index(node, "buffer-size-kb", 0, &v);
+	if (!err) {
+		v <<= 10;	/* KB to Byte */
+		if (v < PAGE_SIZE)
+			pr_err("Buffer size is too small: %d KB\n", v >> 10);
+		if (tracing_resize_ring_buffer(tr, v, RING_BUFFER_ALL_CPUS) < 0)
+			pr_err("Failed to resize trace buffer to %d KB\n",
+				v >> 10);
+	}
+
+	if (of_find_property(node, "alloc-snapshot", NULL))
+		if (tracing_alloc_snapshot() < 0)
+			pr_err("Failed to allocate snapshot buffer\n");
+}
+
+#ifdef CONFIG_EVENT_TRACING
+extern int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set);
+
+static void __init
+trace_of_enable_events(struct trace_array *tr, struct device_node *node)
+{
+	struct property *prop;
+	char buf[MAX_BUF_LEN];
+	const char *p;
+
+	of_property_for_each_string(node, "events", prop, p) {
+		if (strlcpy(buf, p, ARRAY_SIZE(buf)) >= ARRAY_SIZE(buf)) {
+			pr_err("String is too long: %s\n", p);
+			continue;
+		}
+
+		if (ftrace_set_clr_event(tr, buf, 1) < 0)
+			pr_err("Failed to enable event: %s\n", p);
+	}
+}
+#else
+#define trace_of_enable_events(tr, node) do {} while (0)
+#endif
+
+static void __init
+trace_of_enable_tracer(struct trace_array *tr, struct device_node *node)
+{
+	const char *p;
+
+	if (!of_property_read_string(node, "tracer", &p)) {
+		if (tracing_set_tracer(tr, p) < 0)
+			pr_err("Failed to set given tracer: %s\n", p);
+
+	}
+}
+
+static struct device_node * __init trace_of_find_ftrace_node(void)
+{
+	if (!of_chosen)
+		return NULL;
+
+	return of_find_node_by_name(of_chosen, "linux,ftrace");
+}
+
+static int __init trace_of_init(void)
+{
+	struct device_node *trace_node;
+	struct trace_array *tr;
+
+	trace_node = trace_of_find_ftrace_node();
+	if (!trace_node)
+		return 0;
+
+	trace_node = of_node_get(trace_node);
+
+	tr = top_trace_array();
+	if (!tr)
+		goto end;
+
+	trace_of_set_ftrace_options(tr, trace_node);
+	trace_of_enable_events(tr, trace_node);
+	trace_of_enable_tracer(tr, trace_node);
+
+end:
+	of_node_put(trace_node);
+	return 0;
+}
+
+fs_initcall(trace_of_init);


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 09/15] tracing: of: Add trace event settings
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
                   ` (7 preceding siblings ...)
  2019-07-15  5:12 ` [RFC PATCH v2 08/15] tracing: of: Add setup tracing by devicetree support Masami Hiramatsu
@ 2019-07-15  5:12 ` Masami Hiramatsu
  2019-07-15  5:12 ` [RFC PATCH v2 10/15] tracing: of: Add kprobe event support Masami Hiramatsu
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:12 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Add per-event settings, which includes filter and actions.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
  Changes in v2:
   - Make event node available only if CONFIG_EVENT_TRACING=y
---
 kernel/trace/trace_events_trigger.c |    2 -
 kernel/trace/trace_of.c             |   82 +++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 2a2912cb4533..74a19c18219f 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -208,7 +208,7 @@ static int event_trigger_regex_open(struct inode *inode, struct file *file)
 	return ret;
 }
 
-static int trigger_process_regex(struct trace_event_file *file, char *buff)
+int trigger_process_regex(struct trace_event_file *file, char *buff)
 {
 	char *command, *next = buff;
 	struct event_command *p;
diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
index 5e108a1d5bb3..3719f711144a 100644
--- a/kernel/trace/trace_of.c
+++ b/kernel/trace/trace_of.c
@@ -81,6 +81,7 @@ trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
 
 #ifdef CONFIG_EVENT_TRACING
 extern int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set);
+extern int trigger_process_regex(struct trace_event_file *file, char *buff);
 
 static void __init
 trace_of_enable_events(struct trace_array *tr, struct device_node *node)
@@ -99,8 +100,88 @@ trace_of_enable_events(struct trace_array *tr, struct device_node *node)
 			pr_err("Failed to enable event: %s\n", p);
 	}
 }
+
+static void __init
+trace_of_init_one_event(struct trace_array *tr, struct device_node *node)
+{
+	struct trace_event_file *file;
+	struct property *prop;
+	char buf[MAX_BUF_LEN];
+	char *bufp;
+	const char *p;
+	int err;
+
+	if (!of_node_name_prefix(node, "event"))
+		return;
+
+	err = of_property_read_string(node, "event", &p);
+	if (err) {
+		pr_err("Failed to find event on %s\n", of_node_full_name(node));
+		return;
+	}
+
+	err = strlcpy(buf, p, ARRAY_SIZE(buf));
+	if (err >= ARRAY_SIZE(buf)) {
+		pr_err("Event name is too long: %s\n", p);
+		return;
+	}
+	bufp = buf;
+
+	p = strsep(&bufp, ":");
+	if (!bufp) {
+		pr_err("%s has no group name\n", buf);
+		return;
+	}
+
+	mutex_lock(&event_mutex);
+	file = find_event_file(tr, buf, bufp);
+	if (!file) {
+		pr_err("Failed to find event: %s\n", buf);
+		return;
+	}
+
+	err = of_property_read_string(node, "filter", &p);
+	if (!err) {
+		if (strlcpy(buf, p, ARRAY_SIZE(buf)) >= ARRAY_SIZE(buf)) {
+			pr_err("filter string is too long: %s\n", p);
+			return;
+		}
+
+		if (apply_event_filter(file, buf) < 0) {
+			pr_err("Failed to apply filter: %s\n", buf);
+			return;
+		}
+	}
+
+	of_property_for_each_string(node, "actions", prop, p) {
+		if (strlcpy(buf, p, ARRAY_SIZE(buf)) >= ARRAY_SIZE(buf)) {
+			pr_err("action string is too long: %s\n", p);
+			continue;
+		}
+
+		if (trigger_process_regex(file, buf) < 0)
+			pr_err("Failed to apply an action: %s\n", buf);
+	}
+
+	if (of_property_read_bool(node, "enable")) {
+		if (trace_event_enable_disable(file, 1, 0) < 0)
+			pr_err("Failed to enable event node: %s\n",
+				of_node_full_name(node));
+	}
+	mutex_unlock(&event_mutex);
+}
+
+static void __init
+trace_of_init_events(struct trace_array *tr, struct device_node *node)
+{
+	struct device_node *enode;
+
+	for_each_child_of_node(node, enode)
+		trace_of_init_one_event(tr, enode);
+}
 #else
 #define trace_of_enable_events(tr, node) do {} while (0)
+#define trace_of_init_events(tr, node) do {} while (0)
 #endif
 
 static void __init
@@ -139,6 +220,7 @@ static int __init trace_of_init(void)
 		goto end;
 
 	trace_of_set_ftrace_options(tr, trace_node);
+	trace_of_init_events(tr, trace_node);
 	trace_of_enable_events(tr, trace_node);
 	trace_of_enable_tracer(tr, trace_node);
 


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 10/15] tracing: of: Add kprobe event support
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
                   ` (8 preceding siblings ...)
  2019-07-15  5:12 ` [RFC PATCH v2 09/15] tracing: of: Add trace event settings Masami Hiramatsu
@ 2019-07-15  5:12 ` Masami Hiramatsu
  2019-07-15  5:13 ` [RFC PATCH v2 11/15] tracing: of: Add synthetic " Masami Hiramatsu
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:12 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Add kprobe event support in event node. User can add probe definitions
by "probes" property as a string array.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
  Changes in v2:
   - Make probes property avaiable if CONFIG_KPROBE_EVENTS=y.
---
 kernel/trace/trace_kprobe.c |    5 +++
 kernel/trace/trace_of.c     |   76 +++++++++++++++++++++++++++++++++++++------
 2 files changed, 71 insertions(+), 10 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 5135c07b6557..03ce60928c18 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -728,6 +728,11 @@ static int create_or_delete_trace_kprobe(int argc, char **argv)
 	return ret == -ECANCELED ? -EINVAL : ret;
 }
 
+int trace_kprobe_run_command(const char *command)
+{
+	return trace_run_command(command, create_or_delete_trace_kprobe);
+}
+
 static int trace_kprobe_release(struct dyn_event *ev)
 {
 	struct trace_kprobe *tk = to_trace_kprobe(ev);
diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
index 3719f711144a..56c5deb45f54 100644
--- a/kernel/trace/trace_of.c
+++ b/kernel/trace/trace_of.c
@@ -101,14 +101,59 @@ trace_of_enable_events(struct trace_array *tr, struct device_node *node)
 	}
 }
 
+#ifdef CONFIG_KPROBE_EVENTS
+extern int trace_kprobe_run_command(const char *command);
+
+static int __init
+trace_of_add_kprobe_event(struct device_node *node,
+			  const char *group, const char *event)
+{
+	struct property *prop;
+	char buf[MAX_BUF_LEN];
+	const char *p;
+	char *q;
+	int len, ret;
+
+	len = snprintf(buf, ARRAY_SIZE(buf) - 1, "p:%s/%s ", group, event);
+	if (len >= ARRAY_SIZE(buf)) {
+		pr_err("Event name is too long: %s\n", event);
+		return -E2BIG;
+	}
+	q = buf + len;
+	len = ARRAY_SIZE(buf) - len;
+
+	of_property_for_each_string(node, "probes", prop, p) {
+		if (strlcpy(q, p, len) >= len) {
+			pr_err("Probe definition is too long: %s\n", p);
+			return -E2BIG;
+		}
+		ret = trace_kprobe_run_command(buf);
+		if (ret < 0) {
+			pr_err("Failed to add probe: %s\n", buf);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+#else
+static inline int __init
+trace_of_add_kprobe_event(struct device_node *node,
+			  const char *group, const char *event)
+{
+	pr_err("Kprobe event is not supported.\n");
+	return -ENOTSUPP;
+}
+#endif
+
 static void __init
 trace_of_init_one_event(struct trace_array *tr, struct device_node *node)
 {
 	struct trace_event_file *file;
 	struct property *prop;
 	char buf[MAX_BUF_LEN];
-	char *bufp;
-	const char *p;
+	const char *p, *group;
+	char *event;
 	int err;
 
 	if (!of_node_name_prefix(node, "event"))
@@ -125,18 +170,29 @@ trace_of_init_one_event(struct trace_array *tr, struct device_node *node)
 		pr_err("Event name is too long: %s\n", p);
 		return;
 	}
-	bufp = buf;
-
-	p = strsep(&bufp, ":");
-	if (!bufp) {
-		pr_err("%s has no group name\n", buf);
-		return;
+	event = buf;
+
+	group = strsep(&event, ":");
+	/* For a kprobe event, we have to generates an event at first */
+	if (of_find_property(node, "probes", NULL)) {
+		if (!event) {
+			event = buf;
+			group = "kprobes";
+		}
+		err = trace_of_add_kprobe_event(node, group, event);
+		if (err < 0)
+			return;
+	} else {
+		if (!event) {
+			pr_err("%s has no group name\n", buf);
+			return;
+		}
 	}
 
 	mutex_lock(&event_mutex);
-	file = find_event_file(tr, buf, bufp);
+	file = find_event_file(tr, group, event);
 	if (!file) {
-		pr_err("Failed to find event: %s\n", buf);
+		pr_err("Failed to find event: %s:%s\n", group, event);
 		return;
 	}
 


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 11/15] tracing: of: Add synthetic event support
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
                   ` (9 preceding siblings ...)
  2019-07-15  5:12 ` [RFC PATCH v2 10/15] tracing: of: Add kprobe event support Masami Hiramatsu
@ 2019-07-15  5:13 ` Masami Hiramatsu
  2019-07-15  5:13 ` [RFC PATCH v2 12/15] tracing: of: Add instance node support Masami Hiramatsu
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:13 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Add synthetic event node support. The synthetic event node must be
a child node of ftrace node, and the node must start with "synth@"
prefix. The synth node requires fields string (not string array),
which defines the fields as same as tracing/synth_events interface.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 Changes in v2:
  - Make synth property available only if CONFIG_HIST_TRIGGERS=y
---
 kernel/trace/trace_events_hist.c |    5 +++
 kernel/trace/trace_of.c          |   64 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index db973928e580..e7f5d0a353e2 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1343,6 +1343,11 @@ static int create_or_delete_synth_event(int argc, char **argv)
 	return ret == -ECANCELED ? -EINVAL : ret;
 }
 
+int synth_event_run_command(const char *command)
+{
+	return trace_run_command(command, create_or_delete_synth_event);
+}
+
 static int synth_event_create(int argc, const char **argv)
 {
 	const char *name = argv[0];
diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
index 56c5deb45f54..e9142c63ece1 100644
--- a/kernel/trace/trace_of.c
+++ b/kernel/trace/trace_of.c
@@ -146,6 +146,49 @@ trace_of_add_kprobe_event(struct device_node *node,
 }
 #endif
 
+#ifdef CONFIG_HIST_TRIGGERS
+extern int synth_event_run_command(const char *command);
+
+static int __init
+trace_of_add_synth_event(struct device_node *node, const char *event)
+{
+	struct property *prop;
+	char buf[MAX_BUF_LEN], *q;
+	const char *p;
+	int len, delta, ret;
+
+	len = ARRAY_SIZE(buf);
+	delta = snprintf(buf, len, "%s", event);
+	if (delta >= len) {
+		pr_err("Event name is too long: %s\n", event);
+		return -E2BIG;
+	}
+	len -= delta; q = buf + delta;
+
+	of_property_for_each_string(node, "fields", prop, p) {
+		delta = snprintf(q, len, " %s;", p);
+		if (delta >= len) {
+			pr_err("fields string is too long: %s\n", p);
+			return -E2BIG;
+		}
+		len -= delta; q += delta;
+	}
+
+	ret = synth_event_run_command(buf);
+	if (ret < 0)
+		pr_err("Failed to add synthetic event: %s\n", buf);
+
+	return ret;
+}
+#else
+static inline int __init
+trace_of_add_synth_event(struct device_node *node, const char *event)
+{
+	pr_err("Synthetic event is not supported.\n");
+	return -ENOTSUPP;
+}
+#endif
+
 static void __init
 trace_of_init_one_event(struct trace_array *tr, struct device_node *node)
 {
@@ -173,15 +216,30 @@ trace_of_init_one_event(struct trace_array *tr, struct device_node *node)
 	event = buf;
 
 	group = strsep(&event, ":");
-	/* For a kprobe event, we have to generates an event at first */
+
+	/* Generates kprobe/synth event at first */
 	if (of_find_property(node, "probes", NULL)) {
+		if (of_find_property(node, "fields", NULL)) {
+			pr_err("Error: %s node has both probes and fields\n",
+				of_node_full_name(node));
+			return;
+		}
 		if (!event) {
 			event = buf;
 			group = "kprobes";
 		}
-		err = trace_of_add_kprobe_event(node, group, event);
-		if (err < 0)
+		if (trace_of_add_kprobe_event(node, group, event) < 0)
+			return;
+	} else if (of_find_property(node, "fields", NULL)) {
+		if (!event)
+			event = buf;
+		else if (strcmp(group, "synthetic") != 0) {
+			pr_err("Synthetic event must be in synthetic group\n");
+			return;
+		}
+		if (trace_of_add_synth_event(node, event) < 0)
 			return;
+		group = "synthetic";
 	} else {
 		if (!event) {
 			pr_err("%s has no group name\n", buf);


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 12/15] tracing: of: Add instance node support
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
                   ` (10 preceding siblings ...)
  2019-07-15  5:13 ` [RFC PATCH v2 11/15] tracing: of: Add synthetic " Masami Hiramatsu
@ 2019-07-15  5:13 ` Masami Hiramatsu
  2019-07-15  5:13 ` [RFC PATCH v2 13/15] tracing: of: Add cpumask property support Masami Hiramatsu
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:13 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Add instance node support to devicetree ftrace binding.
User can set some options and event nodes in instance node.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/trace_of.c |   67 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 55 insertions(+), 12 deletions(-)

diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
index e9142c63ece1..7fe81c25dc59 100644
--- a/kernel/trace/trace_of.c
+++ b/kernel/trace/trace_of.c
@@ -21,9 +21,10 @@ extern int tracing_set_tracer(struct trace_array *tr, const char *buf);
 extern void __init trace_init_tracepoint_printk(void);
 extern ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
 					  unsigned long size, int cpu_id);
+extern struct trace_array *trace_array_create(const char *name);
 
 static void __init
-trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
+trace_of_set_instance_options(struct trace_array *tr, struct device_node *node)
 {
 	struct property *prop;
 	const char *p;
@@ -48,6 +49,24 @@ trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
 			pr_err("Failed to set trace clock: %s\n", p);
 	}
 
+	/* This accepts per-cpu buffer size in KB */
+	err = of_property_read_u32_index(node, "buffer-size-kb", 0, &v);
+	if (!err) {
+		v <<= 10;	/* KB to Byte */
+		if (v < PAGE_SIZE)
+			pr_err("Buffer size is too small: %d KB\n", v >> 10);
+		if (tracing_resize_ring_buffer(tr, v, RING_BUFFER_ALL_CPUS) < 0)
+			pr_err("Failed to resize trace buffer to %d KB\n",
+				v >> 10);
+	}
+}
+
+static void __init
+trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
+{
+	u32 v = 0;
+	int err;
+
 	/* Command line boot options */
 	if (of_find_property(node, "dump-on-oops", NULL)) {
 		err = of_property_read_u32_index(node, "dump-on-oops", 0, &v);
@@ -63,20 +82,11 @@ trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
 	if (of_find_property(node, "tp-printk", NULL))
 		trace_init_tracepoint_printk();
 
-	/* This accepts per-cpu buffer size in KB */
-	err = of_property_read_u32_index(node, "buffer-size-kb", 0, &v);
-	if (!err) {
-		v <<= 10;	/* KB to Byte */
-		if (v < PAGE_SIZE)
-			pr_err("Buffer size is too small: %d KB\n", v >> 10);
-		if (tracing_resize_ring_buffer(tr, v, RING_BUFFER_ALL_CPUS) < 0)
-			pr_err("Failed to resize trace buffer to %d KB\n",
-				v >> 10);
-	}
-
 	if (of_find_property(node, "alloc-snapshot", NULL))
 		if (tracing_alloc_snapshot() < 0)
 			pr_err("Failed to allocate snapshot buffer\n");
+
+	trace_of_set_instance_options(tr, node);
 }
 
 #ifdef CONFIG_EVENT_TRACING
@@ -310,6 +320,38 @@ trace_of_enable_tracer(struct trace_array *tr, struct device_node *node)
 	}
 }
 
+static void __init
+trace_of_init_instances(struct device_node *__node)
+{
+	struct device_node *node;
+	struct trace_array *tr;
+	const char *p;
+	int err;
+
+	for_each_child_of_node(__node, node) {
+		if (!of_node_name_prefix(node, "instance"))
+			continue;
+
+		err = of_property_read_string(node, "instance", &p);
+		if (err) {
+			pr_err("Failed to get instance name on %s\n",
+				of_node_full_name(node));
+			continue;
+		}
+
+		tr = trace_array_create(p);
+		if (IS_ERR(tr)) {
+			pr_err("Failed to create instance %s\n", p);
+			continue;
+		}
+
+		trace_of_set_instance_options(tr, node);
+		trace_of_init_events(tr, node);
+		trace_of_enable_events(tr, node);
+		trace_of_enable_tracer(tr, node);
+	}
+}
+
 static struct device_node * __init trace_of_find_ftrace_node(void)
 {
 	if (!of_chosen)
@@ -337,6 +379,7 @@ static int __init trace_of_init(void)
 	trace_of_init_events(tr, trace_node);
 	trace_of_enable_events(tr, trace_node);
 	trace_of_enable_tracer(tr, trace_node);
+	trace_of_init_instances(trace_node);
 
 end:
 	of_node_put(trace_node);


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 13/15] tracing: of: Add cpumask property support
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
                   ` (11 preceding siblings ...)
  2019-07-15  5:13 ` [RFC PATCH v2 12/15] tracing: of: Add instance node support Masami Hiramatsu
@ 2019-07-15  5:13 ` Masami Hiramatsu
  2019-07-15  5:13 ` [RFC PATCH v2 14/15] tracing: of: Add function tracer filter properties Masami Hiramatsu
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:13 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Add cpumask property for tracing. Note that this property accepts
a string cpumask, not a digit number.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/trace.c    |   41 ++++++++++++++++++++++++++++-------------
 kernel/trace/trace_of.c |   14 ++++++++++++++
 2 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6f50c58a3ad4..251d9de14b12 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4437,20 +4437,12 @@ tracing_cpumask_read(struct file *filp, char __user *ubuf,
 	return count;
 }
 
-static ssize_t
-tracing_cpumask_write(struct file *filp, const char __user *ubuf,
-		      size_t count, loff_t *ppos)
+int tracing_set_cpumask(struct trace_array *tr, cpumask_var_t tracing_cpumask_new)
 {
-	struct trace_array *tr = file_inode(filp)->i_private;
-	cpumask_var_t tracing_cpumask_new;
-	int err, cpu;
-
-	if (!alloc_cpumask_var(&tracing_cpumask_new, GFP_KERNEL))
-		return -ENOMEM;
+	int cpu;
 
-	err = cpumask_parse_user(ubuf, count, tracing_cpumask_new);
-	if (err)
-		goto err_unlock;
+	if (!tr)
+		return -EINVAL;
 
 	local_irq_disable();
 	arch_spin_lock(&tr->max_lock);
@@ -4474,11 +4466,34 @@ tracing_cpumask_write(struct file *filp, const char __user *ubuf,
 	local_irq_enable();
 
 	cpumask_copy(tr->tracing_cpumask, tracing_cpumask_new);
+
+	return 0;
+}
+
+static ssize_t
+tracing_cpumask_write(struct file *filp, const char __user *ubuf,
+		      size_t count, loff_t *ppos)
+{
+	struct trace_array *tr = file_inode(filp)->i_private;
+	cpumask_var_t tracing_cpumask_new;
+	int err;
+
+	if (!alloc_cpumask_var(&tracing_cpumask_new, GFP_KERNEL))
+		return -ENOMEM;
+
+	err = cpumask_parse_user(ubuf, count, tracing_cpumask_new);
+	if (err)
+		goto err_free;
+
+	err = tracing_set_cpumask(tr, tracing_cpumask_new);
+	if (err)
+		goto err_free;
+
 	free_cpumask_var(tracing_cpumask_new);
 
 	return count;
 
-err_unlock:
+err_free:
 	free_cpumask_var(tracing_cpumask_new);
 
 	return err;
diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
index 7fe81c25dc59..14d6c9a8fd81 100644
--- a/kernel/trace/trace_of.c
+++ b/kernel/trace/trace_of.c
@@ -22,6 +22,8 @@ extern void __init trace_init_tracepoint_printk(void);
 extern ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
 					  unsigned long size, int cpu_id);
 extern struct trace_array *trace_array_create(const char *name);
+extern int tracing_set_cpumask(struct trace_array *tr,
+			       cpumask_var_t tracing_cpumask_new);
 
 static void __init
 trace_of_set_instance_options(struct trace_array *tr, struct device_node *node)
@@ -59,6 +61,18 @@ trace_of_set_instance_options(struct trace_array *tr, struct device_node *node)
 			pr_err("Failed to resize trace buffer to %d KB\n",
 				v >> 10);
 	}
+
+	err = of_property_read_string(node, "cpumask", &p);
+	if (!err) {
+		cpumask_var_t new_mask;
+
+		if (alloc_cpumask_var(&new_mask, GFP_KERNEL)) {
+			if (cpumask_parse(p, new_mask) < 0 ||
+			    tracing_set_cpumask(tr, new_mask) < 0)
+				pr_err("Failed to set new CPU mask %s\n", p);
+			free_cpumask_var(new_mask);
+		}
+	}
 }
 
 static void __init


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 14/15] tracing: of: Add function tracer filter properties
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
                   ` (12 preceding siblings ...)
  2019-07-15  5:13 ` [RFC PATCH v2 13/15] tracing: of: Add cpumask property support Masami Hiramatsu
@ 2019-07-15  5:13 ` Masami Hiramatsu
  2019-07-15  5:13 ` [RFC PATCH v2 15/15] tracing: of: Add function-graph tracer option properties Masami Hiramatsu
  2019-07-15 14:21 ` [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Frank Rowand
  15 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:13 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Add function tracer filter properties which includes
 - ftrace-filters : string array of filter rules
 - ftrace-notraces : string array of notrace rules
These properties are available on ftrace root node and
instance node.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/trace_of.c |   36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
index 14d6c9a8fd81..1ee6fab918f5 100644
--- a/kernel/trace/trace_of.c
+++ b/kernel/trace/trace_of.c
@@ -322,11 +322,47 @@ trace_of_init_events(struct trace_array *tr, struct device_node *node)
 #define trace_of_init_events(tr, node) do {} while (0)
 #endif
 
+#ifdef CONFIG_FUNCTION_TRACER
+extern bool ftrace_filter_param __initdata;
+extern int ftrace_set_filter(struct ftrace_ops *ops, unsigned char *buf,
+			     int len, int reset);
+extern int ftrace_set_notrace(struct ftrace_ops *ops, unsigned char *buf,
+			     int len, int reset);
+
+static void __init
+trace_of_set_ftrace_filter(struct ftrace_ops *ops, const char *property,
+			   struct device_node *node)
+{
+	struct property *prop;
+	const char *p;
+	char buf[MAX_BUF_LEN];
+	int err;
+
+	of_property_for_each_string(node, property, prop, p) {
+		if (strlcpy(buf, p, ARRAY_SIZE(buf)) >= ARRAY_SIZE(buf)) {
+			pr_err("filter string is too long: %s\n", p);
+			return;
+		}
+		err = ftrace_set_filter(ops, buf, strlen(buf), 0);
+		if (err) {
+			pr_err("Failed to add %s: %s\n", property, buf);
+			return;
+		}
+		ftrace_filter_param = true;
+	}
+}
+#else
+#define trace_of_set_ftrace_filter(ops, prop, node) do {} while (0)
+#endif
+
 static void __init
 trace_of_enable_tracer(struct trace_array *tr, struct device_node *node)
 {
 	const char *p;
 
+	trace_of_set_ftrace_filter(tr->ops, "ftrace-filters", node);
+	trace_of_set_ftrace_filter(tr->ops, "ftrace-notraces", node);
+
 	if (!of_property_read_string(node, "tracer", &p)) {
 		if (tracing_set_tracer(tr, p) < 0)
 			pr_err("Failed to set given tracer: %s\n", p);


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 15/15] tracing: of: Add function-graph tracer option properties
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
                   ` (13 preceding siblings ...)
  2019-07-15  5:13 ` [RFC PATCH v2 14/15] tracing: of: Add function tracer filter properties Masami Hiramatsu
@ 2019-07-15  5:13 ` Masami Hiramatsu
  2019-07-15 14:21 ` [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Frank Rowand
  15 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-15  5:13 UTC (permalink / raw)
  To: Steven Rostedt, Rob Herring, Frank Rowand, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Add following function-graph tracer related options
 - fgraph-filters : string array of filter
 - fgraph-notraces : string array of notrace-filter
 - fgraph-max-depth : u32 value of max depth

Note that these properties are available on ftrace root node
only, because these filters are globally applied.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/ftrace.c   |   85 ++++++++++++++++++++++++++++++-----------------
 kernel/trace/trace_of.c |   68 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 123 insertions(+), 30 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 5c3eadb143ed..7bc60ac4583e 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1241,7 +1241,7 @@ static void clear_ftrace_mod_list(struct list_head *head)
 	mutex_unlock(&ftrace_lock);
 }
 
-static void free_ftrace_hash(struct ftrace_hash *hash)
+void free_ftrace_hash(struct ftrace_hash *hash)
 {
 	if (!hash || hash == EMPTY_HASH)
 		return;
@@ -4895,7 +4895,7 @@ __setup("ftrace_filter=", set_ftrace_filter);
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 static char ftrace_graph_buf[FTRACE_FILTER_SIZE] __initdata;
 static char ftrace_graph_notrace_buf[FTRACE_FILTER_SIZE] __initdata;
-static int ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer);
+int ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer);
 
 static int __init set_graph_function(char *str)
 {
@@ -5224,6 +5224,26 @@ __ftrace_graph_open(struct inode *inode, struct file *file,
 	return ret;
 }
 
+struct ftrace_hash *ftrace_graph_copy_hash(bool enable)
+{
+	struct ftrace_hash *hash;
+
+	mutex_lock(&graph_lock);
+
+	if (enable)
+		hash = rcu_dereference_protected(ftrace_graph_hash,
+					lockdep_is_held(&graph_lock));
+	else
+		hash = rcu_dereference_protected(ftrace_graph_notrace_hash,
+					lockdep_is_held(&graph_lock));
+
+	hash = alloc_and_copy_ftrace_hash(FTRACE_HASH_DEFAULT_BITS, hash);
+
+	mutex_unlock(&graph_lock);
+
+	return hash;
+}
+
 static int
 ftrace_graph_open(struct inode *inode, struct file *file)
 {
@@ -5280,11 +5300,40 @@ ftrace_graph_notrace_open(struct inode *inode, struct file *file)
 	return ret;
 }
 
+int ftrace_graph_apply_hash(struct ftrace_hash *hash, bool enable)
+{
+	struct ftrace_hash *old_hash, *new_hash;
+
+	new_hash = __ftrace_hash_move(hash);
+	if (!new_hash)
+		return -ENOMEM;
+
+	mutex_lock(&graph_lock);
+
+	if (enable) {
+		old_hash = rcu_dereference_protected(ftrace_graph_hash,
+				lockdep_is_held(&graph_lock));
+		rcu_assign_pointer(ftrace_graph_hash, new_hash);
+	} else {
+		old_hash = rcu_dereference_protected(ftrace_graph_notrace_hash,
+				lockdep_is_held(&graph_lock));
+		rcu_assign_pointer(ftrace_graph_notrace_hash, new_hash);
+	}
+
+	mutex_unlock(&graph_lock);
+
+	/* Wait till all users are no longer using the old hash */
+	synchronize_rcu();
+
+	free_ftrace_hash(old_hash);
+
+	return 0;
+}
+
 static int
 ftrace_graph_release(struct inode *inode, struct file *file)
 {
 	struct ftrace_graph_data *fgd;
-	struct ftrace_hash *old_hash, *new_hash;
 	struct trace_parser *parser;
 	int ret = 0;
 
@@ -5309,41 +5358,17 @@ ftrace_graph_release(struct inode *inode, struct file *file)
 
 		trace_parser_put(parser);
 
-		new_hash = __ftrace_hash_move(fgd->new_hash);
-		if (!new_hash) {
-			ret = -ENOMEM;
-			goto out;
-		}
-
-		mutex_lock(&graph_lock);
-
-		if (fgd->type == GRAPH_FILTER_FUNCTION) {
-			old_hash = rcu_dereference_protected(ftrace_graph_hash,
-					lockdep_is_held(&graph_lock));
-			rcu_assign_pointer(ftrace_graph_hash, new_hash);
-		} else {
-			old_hash = rcu_dereference_protected(ftrace_graph_notrace_hash,
-					lockdep_is_held(&graph_lock));
-			rcu_assign_pointer(ftrace_graph_notrace_hash, new_hash);
-		}
-
-		mutex_unlock(&graph_lock);
-
-		/* Wait till all users are no longer using the old hash */
-		synchronize_rcu();
-
-		free_ftrace_hash(old_hash);
+		ret = ftrace_graph_apply_hash(fgd->new_hash,
+					fgd->type == GRAPH_FILTER_FUNCTION);
 	}
 
- out:
 	free_ftrace_hash(fgd->new_hash);
 	kfree(fgd);
 
 	return ret;
 }
 
-static int
-ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer)
+int ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer)
 {
 	struct ftrace_glob func_g;
 	struct dyn_ftrace *rec;
diff --git a/kernel/trace/trace_of.c b/kernel/trace/trace_of.c
index 1ee6fab918f5..d78a082b1752 100644
--- a/kernel/trace/trace_of.c
+++ b/kernel/trace/trace_of.c
@@ -75,6 +75,71 @@ trace_of_set_instance_options(struct trace_array *tr, struct device_node *node)
 	}
 }
 
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+extern unsigned int fgraph_max_depth;
+extern struct ftrace_hash *ftrace_graph_copy_hash(bool enable);
+extern int ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer);
+extern int ftrace_graph_apply_hash(struct ftrace_hash *hash, bool enable);
+extern void free_ftrace_hash(struct ftrace_hash *hash);
+
+static void __init
+trace_of_set_fgraph_filter(struct device_node *node, const char *property,
+			   bool enable)
+{
+	struct ftrace_hash *hash;
+	struct property *prop;
+	const char *p;
+	char buf[MAX_BUF_LEN];
+	int err;
+
+	if (!of_find_property(node, property, NULL))
+		return;
+
+	/* Get current filter hash */
+	hash = ftrace_graph_copy_hash(enable);
+	if (!hash) {
+		pr_err("Failed to copy hash\n");
+		return;
+	}
+
+	of_property_for_each_string(node, property, prop, p) {
+		if (strlcpy(buf, p, ARRAY_SIZE(buf)) >= ARRAY_SIZE(buf)) {
+			pr_err("filter string is too long: %s\n", p);
+			goto free_hash;
+		}
+		err = ftrace_graph_set_hash(hash, buf);
+		if (err) {
+			pr_err("Failed to add %s: %s\n", property, buf);
+			goto free_hash;
+		}
+	}
+
+	if (ftrace_graph_apply_hash(hash, enable) < 0) {
+		pr_err("Failed to apply new hash\n");
+		goto free_hash;
+	}
+
+	return;
+
+free_hash:
+	free_ftrace_hash(hash);
+}
+
+static void __init
+trace_of_set_fgraph_options(struct device_node *node)
+{
+	u32 v = 0;
+
+	trace_of_set_fgraph_filter(node, "fgraph-filters", true);
+	trace_of_set_fgraph_filter(node, "fgraph-notraces", false);
+
+	if (!of_property_read_u32_index(node, "fgraph-max-depth", 0, &v))
+		fgraph_max_depth = (unsigned int)v;
+}
+#else
+#define trace_of_set_fgraph_options(node) do {} while (0)
+#endif
+
 static void __init
 trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
 {
@@ -100,6 +165,9 @@ trace_of_set_ftrace_options(struct trace_array *tr, struct device_node *node)
 		if (tracing_alloc_snapshot() < 0)
 			pr_err("Failed to allocate snapshot buffer\n");
 
+	/* function graph filters are global settings. */
+	trace_of_set_fgraph_options(node);
+
 	trace_of_set_instance_options(tr, node);
 }
 


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree
  2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
                   ` (14 preceding siblings ...)
  2019-07-15  5:13 ` [RFC PATCH v2 15/15] tracing: of: Add function-graph tracer option properties Masami Hiramatsu
@ 2019-07-15 14:21 ` Frank Rowand
  2019-07-16 15:02   ` Masami Hiramatsu
  15 siblings, 1 reply; 19+ messages in thread
From: Frank Rowand @ 2019-07-15 14:21 UTC (permalink / raw)
  To: Masami Hiramatsu, Steven Rostedt, Rob Herring, Tim Bird
  Cc: Ingo Molnar, Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo,
	Tom Zanussi, linux-kernel, devicetree

Hi Masami,

After receiving this email, I replied to one email on the v1 thread,
so there will be a little bit of overlap in the ordering of the two
threads.  Feel free to reply to my comments in the v1 thread in this
thread instead.

More comments below.

On 7/14/19 10:11 PM, Masami Hiramatsu wrote:> Hello,
> 
> Here is the 2nd version of RFC series to add boot-time tracing using
> devicetree. Previous thread is here.
> 
> https://lkml.kernel.org/r/156113387975.28344.16009584175308192243.stgit@devnote2
> 
> In this version, I moved the ftrace node under /chosen/linux,ftrace
> and remove compatible property, because it must be in fixed place.
> Also this version has following features;
> 
>  - Introduce "instance" node, which can have events nodes for setting
>    events filters and actions for the trace instance.
>  - Introduce "cpumask" property
>  - Introduce "ftrace-filters" and "ftrace-notraces"
>  - Introduce "fgraph-filters", "fgraph-notraces" and "fgraph-max-depth"
> 
> At this moment, this feature is only available on the architecutre which
> supports devicetree. For x86, we can use it on qemu with --dtb option,
> or apply below patch on grub to add devicetree support on grub-x86.
> 
> https://github.com/mhiramat/grub/commit/644c35bfd2d18c772cc353b74215344f8264923a
> 
> Note that the devicetree for x86 must contain the nodes only under
> /chosen/, or it may cause a problem if it conflicts with ACPI.
> (Maybe we need to disable the FDT nodes except for nodes under /chosen
>  on boot if ACPI exists.)
> 
> This series can be applied on Steve's tracing tree (ftrace/core) or
> available on below
> 
> https://github.com/mhiramat/linux.git ftrace-devicetree-v2
> 
> Usage
> ======
> With this series, we can setup new kprobe and synthetic events, more
> complicated event filters and trigger actions including histogram
> via devicetree.
> 
> For example, following kernel parameters
> 
> trace_options=sym-addr trace_event=initcall:* tp_printk trace_buf_size=1M ftrace=function ftrace_filter="vfs*"
> 
> it can be written in devicetree like below.
> 
> /{
> chosen {
> 	...
> 	ftrace {
> 		options = "sym-addr";
> 		events = "initcall:*";
> 		tp-printk;
> 		buffer-size-kb = <0x400>;	// 1024KB == 1MB
> 		ftrace-filters = "vfs*";
> 	};
> 
> Moreover, now we can expand it to add filters for events, kprobe events,
> and synthetic events with histogram like below.
> 
> 	ftrace {
> 		...
> 		event0 {
> 			event = "task:task_newtask";
> 			filter = "pid < 128";	// adding filters
> 			enable;
> 		};
> 		event1 {
> 			event = "kprobes:vfs_read";
> 			probes = "vfs_read $arg1 $arg2"; // add kprobes
> 			filter = "common_pid < 200";
> 			enable;
> 		};
> 		event2 {
> 			event = "initcall_latency";	// add synth event
> 			fields = "unsigned long func", "u64 lat";
> 			// with histogram
> 			actions = "hist:keys=func.sym,lat:vals=lat:sort=lat";
> 		};
> 		// and synthetic event callers
> 		event3 {
> 			event = "initcall:initcall_start";
> 			actions = "hist:keys=func:ts0=common_timestamp.usecs";
> 		};
> 		event4 {
> 			event = "initcall:initcall_finish";
> 			actions = "hist:keys=func:lat=common_timestamp.usecs-$ts0:onmatch(initcall.initcall_start).initcall_latency(func,$lat)";
> 		};
> 	};
> 
> Also, this version supports "instance" node, which allows us to
> run several tracers for different purpose at once. For example,
> one tracer is for tracing functions in module alpha, and others
> tracing module beta, you can write followings.
> 
> 	ftrace {
> 		instance0 {
> 			instance = "foo";
> 			tracer = "function";
> 			ftrace-filters = "*:mod:alpha";
> 		};
> 		instance1 {
> 			instance = "bar";
> 			tracer = "function";
> 			ftrace-filters = "*:mod:beta";
> 			
> 		};
> 	};
> 
> The instance node also accepts event nodes so that each instance
> can customize its event tracing.
> 
> Discussion
> =====
> On the previous thread, we discussed that the this devicetree usage
> itself was acceptable or not. Fortunately, I had a chance to discuss
> it in a F2F meeting with Frank and Tim last week.

Thanks for writing up some of what we discussed.

Let me add a problem statement and use case.  I'll probably get it at least
a little bit wrong, so please update as needed.

(1) You feel the ftrace kernel command line syntax is not sufficiently user
    friendly.

(2) The kernel command line is too small to contain the full set of desired
    ftrace commands and options.

(3) There is a desire to change the boot time ftrace commands and options
    without re-compiling or re-linking the Linux kernel.


> 
> I think the advantages of using devicetree are,
> 
> - reuse devicetree's structured syntax for complicated tracefs settings
> - reuse OF-APIs in linux kernel to accept and parse it
> - reuse dtc complier to compile it and validate syntax. (with yaml schema,
>   we can enhance it)
> - reuse current bootloader (and qemu) to load it

Devicetree is not a universal data structure and communication channel.

Devicetree is a description of the hardware and also conveys bootloader
specific information that is need by the kernel to boot.


> And we talked about some other ideas to avoid using devicetree.
> 
> - expand kernel command line (ascii command strings)
> - expand kernel command line with base64 encoded comressed ascii command 
>    strings

Base64 being one of possibly many ways to convert arbitrary binary data to
ascii safe data _if_ you want to transfer the ftrace options and commands
in a binary format.


> - load (compressed) ascii command strings to somewhere on memory and pass
>    the address via kernel cmdline

Similar to the way initrd is handled, if I understand correctly.  (I am not
up to date on how initrd location is passed to the kernel for a non-devicetree
kernel.)

Compressed or not compressed would be an ftrace design choice.


> - load (compressed) ascii command strings to somewhere on memory and pass
>    the address via /chosen node (as same as initrd)

Compressed or not compressed would be an ftrace design choice.


> - load binary C data and point it from kernel cmdline
> - load binary C data and point it from /chosen node (as same as initrd)
> - load binary C data as a section of kernel image

For the three options above:

Binary data if ftrace prefers structured data.
A list of strings if ftrace wants to use the existing kernel command line
syntax.

For the third of the above three options, the linker would provide the start
and end address of the ftrace options and commands section.


> The first 2 ideas expand the kernel's cmdline to pass some "magic" command
> to setup ftrace. In both case, the problems are the maximal size of cmdline
> and the issues related to the complexity of commands.

Not a "magic" command.  Either continue using the existing ftrace syntax or
add something like: ftrace_cmd="whatever format ftrace desires".

Why can the maximum size of the cmdline not be increased?


> My example showed that the ftrace settings becomes long even if making one
> histogram, which can be longer than 256 bytes. The long and complex data
> can easily lead mis-typing, but cmdline has no syntax validator, it just
> ignores the mis-typed commands.

Hand typing a kernel command line is already not a fun exercise, even
before adding ftrace commands.  If you are hand typing kernel command
lines then I suggest you improve your tools (eg bootloader or whatever
is not allowing you to edit and store command lines).


> (Of course even with the devicetree, it must be smaller than 2 pages)
> 
> Next 2 ideas are similar, but load the commands on some other memory area
> and pass only address via cmdline. This solves the size limitation issue,
> but still no syntax validation. Of course we can make a new structured
> syntax validator similar to (or just forked from) dt-validate.
> The problem (or disadvantage) of these (and following) ideas, is to change
> the kernel and boot loaders to load another binary blobs on memory.
> 
> Maybe if we introduce a generic structured kernel boot arguments, which is
> a kind of /chosen node of devicetree. (But if there is already such hook,
> why we make another one...?)

I got lost in the next sentence, so for my benefit:
GSKBA == generic structured kernel boot arguments

> Also, this "GSKBA" may introduce a parser and access APIs which will be
> very similar to OF-APIs. This also seems redundant to me.
 

> So the last 3 ideas will avoid introducing new parser and APIs, we just
> compile the data as C data and point it from cmdline or somewhere else.

Or if in a kernel data section then the linker can provide the begin and
end address of the blob.  This is already implemented for some other data
structures.

> With these ideas, we still need to expand boot loaders to support
> loading new binary blobs. (And the last one requires to add elf header
> parser/modifier to boot loader too)

Why would the boot loader need to access the elf header?  The linker
can provide the location of the new kernel data section via kernel
variables.

> 
>>From the above reasons, I think using devicetree's /chosen node is 
> the least intrusive way to introduce this boot-time tracing feature.

This is still mis-use of the devicetree data structure.  This data
does not belong in the devicetree.


> 
> Any suggestions, thoughts?
> 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (15):
>       tracing: Apply soft-disabled and filter to tracepoints printk
>       tracing: kprobes: Output kprobe event to printk buffer
>       tracing: Expose EXPORT_SYMBOL_GPL symbol
>       tracing: kprobes: Register to dynevent earlier stage
>       tracing: Accept different type for synthetic event fields
>       tracing: Add NULL trace-array check in print_synth_event()
>       dt-bindings: tracing: Add ftrace binding document
>       tracing: of: Add setup tracing by devicetree support
>       tracing: of: Add trace event settings
>       tracing: of: Add kprobe event support
>       tracing: of: Add synthetic event support
>       tracing: of: Add instance node support
>       tracing: of: Add cpumask property support
>       tracing: of: Add function tracer filter properties
>       tracing: of: Add function-graph tracer option properties
> 
> 
>  .../devicetree/bindings/chosen/linux,ftrace.yaml   |  306 ++++++++++++
>  include/linux/trace_events.h                       |    1 
>  kernel/trace/Kconfig                               |   10 
>  kernel/trace/Makefile                              |    1 
>  kernel/trace/ftrace.c                              |   85 ++-
>  kernel/trace/trace.c                               |   90 ++--
>  kernel/trace/trace_events.c                        |    3 
>  kernel/trace/trace_events_hist.c                   |   14 -
>  kernel/trace/trace_events_trigger.c                |    2 
>  kernel/trace/trace_kprobe.c                        |   81 ++-
>  kernel/trace/trace_of.c                            |  507 ++++++++++++++++++++
>  11 files changed, 1004 insertions(+), 96 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/chosen/linux,ftrace.yaml
>  create mode 100644 kernel/trace/trace_of.c
> 
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree
  2019-07-15 14:21 ` [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Frank Rowand
@ 2019-07-16 15:02   ` Masami Hiramatsu
  2019-07-22 14:38     ` Masami Hiramatsu
  0 siblings, 1 reply; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-16 15:02 UTC (permalink / raw)
  To: Frank Rowand
  Cc: Steven Rostedt, Rob Herring, Tim Bird, Ingo Molnar, Namhyung Kim,
	Jiri Olsa, Arnaldo Carvalho de Melo, Tom Zanussi, linux-kernel,
	devicetree

Hi Frank,

On Mon, 15 Jul 2019 07:21:27 -0700
Frank Rowand <frowand.list@gmail.com> wrote:

> Hi Masami,
> 
> After receiving this email, I replied to one email on the v1 thread,
> so there will be a little bit of overlap in the ordering of the two
> threads.  Feel free to reply to my comments in the v1 thread in this
> thread instead.

OK, thanks for the notice :)

> 
> More comments below.
> 
> On 7/14/19 10:11 PM, Masami Hiramatsu wrote:> Hello,
[...]
> > 
> > Discussion
> > =====
> > On the previous thread, we discussed that the this devicetree usage
> > itself was acceptable or not. Fortunately, I had a chance to discuss
> > it in a F2F meeting with Frank and Tim last week.
> 
> Thanks for writing up some of what we discussed.
> 
> Let me add a problem statement and use case.  I'll probably get it at least
> a little bit wrong, so please update as needed.
> 
> (1) You feel the ftrace kernel command line syntax is not sufficiently user
>     friendly.
> 
> (2) The kernel command line is too small to contain the full set of desired
>     ftrace commands and options.
> 
> (3) There is a desire to change the boot time ftrace commands and options
>     without re-compiling or re-linking the Linux kernel.

Thank you for covering these items :) Yes, these are what I'm thinking.

> > 
> > I think the advantages of using devicetree are,
> > 
> > - reuse devicetree's structured syntax for complicated tracefs settings
> > - reuse OF-APIs in linux kernel to accept and parse it
> > - reuse dtc complier to compile it and validate syntax. (with yaml schema,
> >   we can enhance it)
> > - reuse current bootloader (and qemu) to load it
> 
> Devicetree is not a universal data structure and communication channel.
> 
> Devicetree is a description of the hardware and also conveys bootloader
> specific information that is need by the kernel to boot.

Yes, I see. But I think there is a room to contain a small communication
channel under /chosen, from bootloader.

> 
> > And we talked about some other ideas to avoid using devicetree.
> > 
> > - expand kernel command line (ascii command strings)
> > - expand kernel command line with base64 encoded comressed ascii command 
> >    strings
> 
> Base64 being one of possibly many ways to convert arbitrary binary data to
> ascii safe data _if_ you want to transfer the ftrace options and commands
> in a binary format.

I actually don't want it :( but if the ascii commands can be compressed
(maybe not so efficient), it is a possible way. 

> > - load (compressed) ascii command strings to somewhere on memory and pass
> >    the address via kernel cmdline
> 
> Similar to the way initrd is handled, if I understand correctly.  (I am not
> up to date on how initrd location is passed to the kernel for a non-devicetree
> kernel.)

Initrd is not passed via kernel cmdline, it is loaded and passed via architecture
dependent way. As far as I know, x86 and arm (without DT) uses own data structure,
arm64 (and arm with DT) uses devicetree /chosen node.

> Compressed or not compressed would be an ftrace design choice.
> 
> 
> > - load (compressed) ascii command strings to somewhere on memory and pass
> >    the address via /chosen node (as same as initrd)
> 
> Compressed or not compressed would be an ftrace design choice.
> 

Yes, it is optional.

> 
> > - load binary C data and point it from kernel cmdline
> > - load binary C data and point it from /chosen node (as same as initrd)
> > - load binary C data as a section of kernel image
> 
> For the three options above:
> 
> Binary data if ftrace prefers structured data.
> A list of strings if ftrace wants to use the existing kernel command line
> syntax.

Yes, any data which doesn't need complex parser is OK.

> 
> For the third of the above three options, the linker would provide the start
> and end address of the ftrace options and commands section.

But that means we need to fill the data structure when we build the kernel,
isn't it?

> > The first 2 ideas expand the kernel's cmdline to pass some "magic" command
> > to setup ftrace. In both case, the problems are the maximal size of cmdline
> > and the issues related to the complexity of commands.
> 
> Not a "magic" command.  Either continue using the existing ftrace syntax or
> add something like: ftrace_cmd="whatever format ftrace desires".
> 
> Why can the maximum size of the cmdline not be increased?

We can, but we also has to change bootloaders.

> > My example showed that the ftrace settings becomes long even if making one
> > histogram, which can be longer than 256 bytes. The long and complex data
> > can easily lead mis-typing, but cmdline has no syntax validator, it just
> > ignores the mis-typed commands.
> 
> Hand typing a kernel command line is already not a fun exercise, even
> before adding ftrace commands.  If you are hand typing kernel command
> lines then I suggest you improve your tools (eg bootloader or whatever
> is not allowing you to edit and store command lines).

Indeed, if we extend kernel cmdline to support it, such tool we have to
introduce (like dtc)

> > (Of course even with the devicetree, it must be smaller than 2 pages)
> > 
> > Next 2 ideas are similar, but load the commands on some other memory area
> > and pass only address via cmdline. This solves the size limitation issue,
> > but still no syntax validation. Of course we can make a new structured
> > syntax validator similar to (or just forked from) dt-validate.
> > The problem (or disadvantage) of these (and following) ideas, is to change
> > the kernel and boot loaders to load another binary blobs on memory.
> > 
> > Maybe if we introduce a generic structured kernel boot arguments, which is
> > a kind of /chosen node of devicetree. (But if there is already such hook,
> > why we make another one...?)
> 
> I got lost in the next sentence, so for my benefit:
> GSKBA == generic structured kernel boot arguments

Oh, sorry, that's my bad.

> > Also, this "GSKBA" may introduce a parser and access APIs which will be
> > very similar to OF-APIs. This also seems redundant to me.
>  
> 
> > So the last 3 ideas will avoid introducing new parser and APIs, we just
> > compile the data as C data and point it from cmdline or somewhere else.
> 
> Or if in a kernel data section then the linker can provide the begin and
> end address of the blob.  This is already implemented for some other data
> structures.

Yeah, but does that mean we have to rebuild kernel image?
In some cases, (e.g. debugging distro kernel) we can not modify kernel image
also, I don't like to replace entire image. I would like to choose a tracing
command file from boot loader.

> > With these ideas, we still need to expand boot loaders to support
> > loading new binary blobs. (And the last one requires to add elf header
> > parser/modifier to boot loader too)
> 
> Why would the boot loader need to access the elf header?  The linker
> can provide the location of the new kernel data section via kernel
> variables.

Oh, I thought you meant that the new data was added boot time by
boot loader.

> >>From the above reasons, I think using devicetree's /chosen node is 
> > the least intrusive way to introduce this boot-time tracing feature.
> 
> This is still mis-use of the devicetree data structure.  This data
> does not belong in the devicetree.

I think if the boot loader supports overlay file, we can choose
the overlay file when booting for /chosen node. That can be a
part of boot loader choice, isn't it? :)

Thank you,


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree
  2019-07-16 15:02   ` Masami Hiramatsu
@ 2019-07-22 14:38     ` Masami Hiramatsu
  0 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-07-22 14:38 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Frank Rowand, Steven Rostedt, Rob Herring, Tim Bird, Ingo Molnar,
	Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo, Tom Zanussi,
	linux-kernel, devicetree

Hello,

I discussed with Frank and other kernel developers last week at OSSJ 2019.
Eventually, I decided to leave from devicetree, because it can unstabilize
current devicetree desgin and policy. Instead, aim to introduce a new
generic structured kernel cmdline, something like "configtree".

I thought JSON or other generic data format, but they look a bit bloated
for my purpose. I just need something like "extended hierarchical kernel
cmdline". For example,

ftrace {
  options = "sym-addr"
  events = "initcall:*"
  tp-printk
  event.0 {
    name = "tasl:task_newtask"
    filter = "pid < 128"
  } 
}

Which can be written as

ftrace.options="sym-addr" ftrace.events="initcall:*" ftrace.tp-printk ftrace.event.0.name="tasl:task_newtask" ftrace.event.0.filter="pid < 128"

on current kernel cmdline.

So, the parameters are linearly extended from current kernel cmdline.
Kernel internal APIs must be able to handle both of current cmdline
key-values and configtree key-values.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2019-07-22 14:38 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-15  5:11 [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
2019-07-15  5:11 ` [RFC PATCH v2 01/15] tracing: Apply soft-disabled and filter to tracepoints printk Masami Hiramatsu
2019-07-15  5:11 ` [RFC PATCH v2 02/15] tracing: kprobes: Output kprobe event to printk buffer Masami Hiramatsu
2019-07-15  5:11 ` [RFC PATCH v2 03/15] tracing: Expose EXPORT_SYMBOL_GPL symbol Masami Hiramatsu
2019-07-15  5:11 ` [RFC PATCH v2 04/15] tracing: kprobes: Register to dynevent earlier stage Masami Hiramatsu
2019-07-15  5:12 ` [RFC PATCH v2 05/15] tracing: Accept different type for synthetic event fields Masami Hiramatsu
2019-07-15  5:12 ` [RFC PATCH v2 06/15] tracing: Add NULL trace-array check in print_synth_event() Masami Hiramatsu
2019-07-15  5:12 ` [RFC PATCH v2 07/15] dt-bindings: tracing: Add ftrace binding document Masami Hiramatsu
2019-07-15  5:12 ` [RFC PATCH v2 08/15] tracing: of: Add setup tracing by devicetree support Masami Hiramatsu
2019-07-15  5:12 ` [RFC PATCH v2 09/15] tracing: of: Add trace event settings Masami Hiramatsu
2019-07-15  5:12 ` [RFC PATCH v2 10/15] tracing: of: Add kprobe event support Masami Hiramatsu
2019-07-15  5:13 ` [RFC PATCH v2 11/15] tracing: of: Add synthetic " Masami Hiramatsu
2019-07-15  5:13 ` [RFC PATCH v2 12/15] tracing: of: Add instance node support Masami Hiramatsu
2019-07-15  5:13 ` [RFC PATCH v2 13/15] tracing: of: Add cpumask property support Masami Hiramatsu
2019-07-15  5:13 ` [RFC PATCH v2 14/15] tracing: of: Add function tracer filter properties Masami Hiramatsu
2019-07-15  5:13 ` [RFC PATCH v2 15/15] tracing: of: Add function-graph tracer option properties Masami Hiramatsu
2019-07-15 14:21 ` [RFC PATCH v2 00/15] tracing: of: Boot time tracing using devicetree Frank Rowand
2019-07-16 15:02   ` Masami Hiramatsu
2019-07-22 14:38     ` Masami Hiramatsu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.