linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/20 v2] tracing: Dynamically created function based events
@ 2018-02-07 20:24 Steven Rostedt
  2018-02-07 20:24 ` [PATCH 01/20 v2] tracing: Add " Steven Rostedt
                   ` (19 more replies)
  0 siblings, 20 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

At Kernel Summit back in October, we tried to bring up trace markers, which
would be nops within the kernel proper, that would allow modules to hook
arbitrary trace events to them. The reaction to this proposal was less than
favorable. We were told that we were trying to make a work around for a
problem, and not solving it. The problem in our minds is the notion of a
"stable trace event".

There are maintainers that do not want trace events, or more trace events in
their subsystems. This is due to the fact that trace events post an
interface to user space, and this interface could become required by some
tool. This may cause the trace event to become stable where it must not
break the tool, and thus prevent the code from changing.

Or, the trace event may just have to add padding for fields that tools
may require. The "success" field of the sched_wakeup trace event is one such
instance. There is no more "success" variable, but tools may fail if it were
to go away, so a "1" is simply added to the trace event wasting ring buffer
real estate.

I talked with Linus about this, and he told me that we already have these
markers in the kernel. They are from the mcount/__fentry__ used by function
tracing. Have the trace events be created by these, and see if this will
satisfy most areas that want trace events.

I decided to implement this idea, and here's the patch set.

Introducing "function based events". These are created dynamically by a
tracefs file called "function_events". By writing a pseudo prototype into
this file, you create an event.

 # mount -t tracefs nodev /sys/kernel/tracing
 # cd /sys/kernel/tracing
 # echo 'do_IRQ(symbol ip[16] | x64[6] irq_stack[16])' > function_events
 # cat events/functions/do_IRQ/format
name: do_IRQ
ID: 1399
format:
	field:unsigned short common_type;	offset:0;	size:2;	signed:0;
	field:unsigned char common_flags;	offset:2;	size:1;	signed:0;
	field:unsigned char common_preempt_count;	offset:3;	size:1;	signed:0;
	field:int common_pid;	offset:4;	size:4;	signed:1;

	field:unsigned long __parent_ip;	offset:8;	size:8;	signed:0;
	field:unsigned long __ip;	offset:16;	size:8;	signed:0;
	field:symbol ip;	offset:24;	size:8;	signed:0;
	field:x64 irq_stack[6];	offset:32;	size:48;	signed:0;

print fmt: "%pS->%pS(ip=%pS, irq_stack=%llx:%llx:%llx:%llx:%llx:%llx)", REC->__ip, REC->__parent_ip,
REC->ip, REC->irq_stack[0], REC->irq_stack[1], REC->irq_stack[2], REC->irq_stack[3], REC->irq_stack[4],
REC->irq_stack[5]

 # echo 1 > events/functions/do_IRQ/enable
 # cat trace
          <idle>-0     [003] d..3  3647.049344: ret_from_intr->do_IRQ(ip=cpuidle_enter_state+0xb1/0x330, irq_stack=ffffffff81665db1,10,246,ffffc900006c3e80,18,ffff88011eae9b40)
          <idle>-0     [003] d..3  3647.049433: ret_from_intr->do_IRQ(ip=cpuidle_enter_state+0xb1/0x330, irq_stack=ffffffff81665db1,10,246,ffffc900006c3e80,18,ffff88011eae9b40)
          <idle>-0     [003] d..3  3647.049672: ret_from_intr->do_IRQ(ip=cpuidle_enter_state+0xb1/0x330, irq_stack=ffffffff81665db1,10,246,ffffc900006c3e80,18,ffff88011eae9b40)
          <idle>-0     [003] d..3  3647.325709: ret_from_intr->do_IRQ(ip=cpuidle_enter_state+0xb1/0x330, irq_stack=ffffffff81665db1,10,246,ffffc900006c3e80,18,ffff88011eae9b40)
          <idle>-0     [003] d..3  3647.325929: ret_from_intr->do_IRQ(ip=cpuidle_enter_state+0xb1/0x330, irq_stack=ffffffff81665db1,10,246,ffffc900006c3e80,18,ffff88011eae9b40)
          <idle>-0     [003] d..3  3647.325993: ret_from_intr->do_IRQ(ip=cpuidle_enter_state+0xb1/0x330, irq_stack=ffffffff81665db1,10,246,ffffc900006c3e80,18,ffff88011eae9b40)
          <idle>-0     [003] d..3  3647.387571: ret_from_intr->do_IRQ(ip=cpuidle_enter_state+0xb1/0x330, irq_stack=ffffffff81665db1,10,246,ffffc900006c3e80,18,ffff88011eae9b40)
          <idle>-0     [003] d..3  3647.387791: ret_from_intr->do_IRQ(ip=cpuidle_enter_state+0xb1/0x330, irq_stack=ffffffff81665db1,10,246,ffffc900006c3e80,18,ffff88011eae9b40)
          <idle>-0     [003] d..3  3647.387874: ret_from_intr->do_IRQ(ip=cpuidle_enter_state+0xb1/0x330, irq_stack=ffffffff81665db1,10,246,ffffc900006c3e80,18,ffff88011eae9b40)

And this is much more powerful than just this. We can show strings, and
index off of structures into other structures.

  # echo '__vfs_read(symbol read+40[0]+16)' > function_events

  # echo 1 > events/functions/__vfs_read/enable
  # cat trace
         sshd-1343  [005] ...2   199.734752: vfs_read->__vfs_read(read=tty_read+0x0/0xf0)
         bash-1344  [003] ...2   199.734822: vfs_read->__vfs_read(read=tty_read+0x0/0xf0)
         sshd-1343  [005] ...2   199.734835: vfs_read->__vfs_read(read=tty_read+0x0/0xf0)
 avahi-daemon-910   [003] ...2   200.136740: vfs_read->__vfs_read(read=          (null))
 avahi-daemon-910   [003] ...2   200.136750: vfs_read->__vfs_read(read=          (null))

And even read user space:

  # echo 'SyS_openat(int dfd, string path, x32 flags, x16 mode)' > function_events
  # echo 1 > events/functions/enable
  # grep task_fork /proc/kallsyms 
ffffffff810d5a60 t task_fork_fair
ffffffff810dfc30 t task_fork_dl
  # cat trace
            grep-1820  [000] ...2  3926.107603: entry_SYSCALL_64_fastpath->SyS_openat(dfd=-100, path=/proc/kallsyms, flags=100, mode=0)

These are fully functional events. That is, they work with ftrace,
trace-cmd, perf, histograms, triggers, and eBPF.

What's next? I need to rewrite the function graph tracer, and be able to add
dynamic events on function return.

I made this work with x86 with a simple function that only returns
6 function parameters for x86_64 and 3 for x86_32. But this could easily
be extended.

Cheers!

  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
ftrace/dynamic-ftrace-events

Head SHA1: acf9c4dc5a0662e4ee1fe60e08099b11ae3cffc5


Steven Rostedt (VMware) (20):
      tracing: Add function based events
      tracing: Add documentation for function based events
      tracing: Add simple arguments to function based events
      tracing/x86: Add arch_get_func_args() function
      tracing: Add hex print for dynamic ftrace based events
      tracing: Add indirect offset to args of ftrace based events
      tracing: Add dereferencing multiple fields per arg
      tracing: Add "unsigned" to function based events
      tracing: Add indexing of arguments for function based events
      tracing: Make func_type enums for easier comparing of arg types
      tracing: Add symbol type to function based events
      tracing: Add accessing direct address from function based events
      tracing: Add array type to function based events
      tracing: Have char arrays be strings for function based events
      tracing: Add string type for dynamic strings in function based events
      tracing: Add NULL to skip args for function based events
      tracing: Add indirect to indirect access for function based events
      tracing/perf: Allow perf to use function based events
      tracing: Add error messages for failed writes to function_events
      tracing: Add argument error message too many args for function based events

----
 Documentation/trace/function-based-events.rst |  426 +++++++
 arch/x86/kernel/ftrace.c                      |   28 +
 include/linux/trace_events.h                  |    2 +
 kernel/trace/Kconfig                          |   12 +
 kernel/trace/Makefile                         |    1 +
 kernel/trace/trace.h                          |   11 +
 kernel/trace/trace_event_ftrace.c             | 1653 +++++++++++++++++++++++++
 kernel/trace/trace_probe.h                    |   11 -
 8 files changed, 2133 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/trace/function-based-events.rst
 create mode 100644 kernel/trace/trace_event_ftrace.c
----

Changes since v1:

 The below diff, as well as two patches to supply error messages feedback.

 (Note, I still need to add that quick guide)


diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index b145639eac45..303a56c3339a 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -150,6 +150,8 @@ struct func_string {
 static struct func_string __percpu *str_buffer;
 static int nr_strings;
 
+static int max_args __read_mostly = -1;
+
 /**
  * arch_get_func_args - retrieve function arguments via pt_regs
  * @regs: The registers at the moment the function is called
@@ -269,7 +271,7 @@ static int add_arg(struct func_event *fevent, int ftype, int unsign)
 	struct func_arg *arg;
 
 	/* Make sure the arch can support this many args */
-	if (fevent->arg_cnt >= arch_get_func_args(NULL, 0, 0, NULL))
+	if (fevent->arg_cnt >= max_args)
 		return -EINVAL;
 
 	arg = kzalloc(sizeof(*arg), GFP_KERNEL);
@@ -332,7 +334,7 @@ static int update_arg_arg(struct func_event *fevent)
 		return -EINVAL;
 
 	/* Make sure the arch can support this many args */
-	if (fevent->arg_cnt >= arch_get_func_args(NULL, 0, 0, NULL))
+	if (fevent->arg_cnt >= max_args)
 		return -EINVAL;
 
 	arg->arg = fevent->arg_cnt;
@@ -879,14 +881,18 @@ func_event_call(unsigned long ip, unsigned long parent_ip,
 
 	func_event = container_of(op, struct func_event, ops);
 
-	rcu_read_lock_sched();
+	if (WARN_ON_ONCE(rcu_irq_enter_disabled()))
+		return;
+	rcu_irq_enter_irqson();
+	rcu_read_lock_sched_notrace();
 	list_for_each_entry_rcu(ff, &func_event->files, list) {
 		if (ff->file)
 			func_event_trace(ff->file, func_event, ip, parent_ip, pt_regs);
 		else
 			func_event_perf(func_event, ip, parent_ip, pt_regs);
 	}
-	rcu_read_unlock_sched();
+	rcu_read_unlock_sched_notrace();
+	rcu_irq_exit_irqson();
 }
 
 #define FMT_SIZE	8
@@ -1245,7 +1251,7 @@ static int func_event_create(struct func_event *func_event)
 
 static int create_function_event(int argc, char **argv)
 {
-	struct func_event *func_event, *fe;
+	struct func_event *func_event;
 	enum func_states state = FUNC_STATE_INIT;
 	char *token;
 	char *ptr;
@@ -1275,12 +1281,6 @@ static int create_function_event(int argc, char **argv)
 	if (state != FUNC_STATE_END)
 		goto fail;
 
-	ret = -EALREADY;
-	list_for_each_entry(fe, &func_events, list) {
-		if (strcmp(fe->func, func_event->func) == 0)
-			goto fail;
-	}
-
 	ret = ftrace_set_filter(&func_event->ops, func_event->func,
 				strlen(func_event->func), 0);
 	if (ret < 0)
@@ -1290,7 +1290,9 @@ static int create_function_event(int argc, char **argv)
 	if (ret < 0)
 		goto fail;
 
+	mutex_lock(&func_event_mutex);
 	list_add_tail(&func_event->list, &func_events);
+	mutex_unlock(&func_event_mutex);
 	return 0;
  fail:
 	free_func_event(func_event);
@@ -1349,7 +1351,7 @@ static int func_event_seq_show(struct seq_file *m, void *v)
 			if (redirect->index)
 				seq_printf(m, "+%ld", redirect->index);
 			if (redirect->indirect)
-				seq_printf(m, "[%d]",
+				seq_printf(m, "[%ld]",
 					   (redirect->indirect ^ INDIRECT_FLAG) / arg->size);
 		}
 	}
@@ -1368,22 +1370,27 @@ static const struct seq_operations func_event_seq_op = {
 static int release_all_func_events(void)
 {
 	struct func_event *func_event, *n;
-	int ret;
+	int ret = 0;
 
+	mutex_lock(&func_event_mutex);
 	list_for_each_entry_safe(func_event, n, &func_events, list) {
 		ret = trace_remove_event_call(&func_event->call);
 		if (ret < 0)
-			return ret;
+			break;
 		list_del(&func_event->list);
 		free_func_event(func_event);
 	}
-	return 0;
+	mutex_unlock(&func_event_mutex);
+	return ret;
 }
 
 static int func_event_open(struct inode *inode, struct file *file)
 {
 	int ret;
 
+	if (max_args < 0)
+		max_args = arch_get_func_args(NULL, 0, 0, NULL);
+
 	if ((file->f_mode & FMODE_WRITE) && (file->f_flags & O_TRUNC)) {
 		ret = release_all_func_events();
 		if (ret < 0)

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

* [PATCH 01/20 v2] tracing: Add function based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-08 11:20   ` Jiri Olsa
  2018-02-07 20:24 ` [PATCH 02/20 v2] tracing: Add documentation for " Steven Rostedt
                   ` (18 subsequent siblings)
  19 siblings, 1 reply; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0001-tracing-Add-function-based-events.patch --]
[-- Type: text/plain, Size: 15234 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Add an interface that hooks to the ftrace function tracing to create events
based on functions. A new file is created in the tracefs file system called
function_events. Writing a function name followed by an empty set of
parenthesis will create an event that uses ftrace function callbacks to
record the data. Currently the function takes no arguments, but that will
soon change. The next step is to have arguments within those parenthesis,
and an event created to show it.

Thanks to Jiri Olsa for pointing out the missing mutex protection in
create_function_event().

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/linux/trace_events.h      |   2 +
 kernel/trace/Kconfig              |  12 +
 kernel/trace/Makefile             |   1 +
 kernel/trace/trace.h              |  11 +
 kernel/trace/trace_event_ftrace.c | 473 ++++++++++++++++++++++++++++++++++++++
 kernel/trace/trace_probe.h        |  11 -
 6 files changed, 499 insertions(+), 11 deletions(-)
 create mode 100644 kernel/trace/trace_event_ftrace.c

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index af44e7c2d577..6a3600009c48 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -226,6 +226,7 @@ enum {
 	TRACE_EVENT_FL_TRACEPOINT_BIT,
 	TRACE_EVENT_FL_KPROBE_BIT,
 	TRACE_EVENT_FL_UPROBE_BIT,
+	TRACE_EVENT_FL_FUNC_BIT,
 };
 
 /*
@@ -246,6 +247,7 @@ enum {
 	TRACE_EVENT_FL_TRACEPOINT	= (1 << TRACE_EVENT_FL_TRACEPOINT_BIT),
 	TRACE_EVENT_FL_KPROBE		= (1 << TRACE_EVENT_FL_KPROBE_BIT),
 	TRACE_EVENT_FL_UPROBE		= (1 << TRACE_EVENT_FL_UPROBE_BIT),
+	TRACE_EVENT_FL_FUNC		= (1 << TRACE_EVENT_FL_FUNC_BIT),
 };
 
 #define TRACE_EVENT_FL_UKPROBE (TRACE_EVENT_FL_KPROBE | TRACE_EVENT_FL_UPROBE)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index f54dc62b599c..2118838946cd 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -442,6 +442,18 @@ config BLK_DEV_IO_TRACE
 
 	  If unsure, say N.
 
+config FUNCTION_EVENTS
+	depends on DYNAMIC_FTRACE_WITH_REGS
+	bool "Enable function based events"
+	select TRACING
+	help
+	 This creates a file function_events in the tracefs file system.
+	 Writing function names into this file will create an event
+	 that can be enabled like any other event, but will be at the
+	 location of the specified function.
+
+	 See Documentation/trace/function_events.txt for more details.
+
 config KPROBE_EVENTS
 	depends on KPROBES
 	depends on HAVE_REGS_AND_STACK_ACCESS_API
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index e2538c7638d4..00f6d69652c0 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_FTRACE_SYSCALLS) += trace_syscalls.o
 ifeq ($(CONFIG_PERF_EVENTS),y)
 obj-$(CONFIG_EVENT_TRACING) += trace_event_perf.o
 endif
+obj-$(CONFIG_FUNCTION_EVENTS) += trace_event_ftrace.o
 obj-$(CONFIG_EVENT_TRACING) += trace_events_filter.o
 obj-$(CONFIG_EVENT_TRACING) += trace_events_trigger.o
 obj-$(CONFIG_HIST_TRIGGERS) += trace_events_hist.o
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 2a6d0325a761..67928b53dc06 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1814,4 +1814,15 @@ static inline void trace_event_eval_update(struct trace_eval_map **map, int len)
 
 extern struct trace_iterator *tracepoint_print_iter;
 
+#undef DEFINE_FIELD
+#define DEFINE_FIELD(type, item, name, is_signed)			\
+	do {								\
+		ret = trace_define_field(event_call, #type, name,	\
+					 offsetof(typeof(field), item),	\
+					 sizeof(field.item), is_signed, \
+					 FILTER_OTHER);			\
+		if (ret)						\
+			return ret;					\
+	} while (0)
+
 #endif /* _LINUX_KERNEL_TRACE_H */
diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
new file mode 100644
index 000000000000..fb8e03e663e8
--- /dev/null
+++ b/kernel/trace/trace_event_ftrace.c
@@ -0,0 +1,473 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * trace events created on top of ftrace (functions).
+ *
+ * Copyright (C) 2018 VMware Inc, Steven Rostedt.
+ */
+
+#include <linux/ctype.h>
+#include <linux/slab.h>
+
+#include "trace.h"
+
+#define FUNC_EVENT_SYSTEM "functions"
+#define WRITE_BUFSIZE  4096
+
+struct func_event {
+	struct list_head		list;
+	char				*func;
+	struct trace_event_class	class;
+	struct trace_event_call		call;
+	struct ftrace_ops		ops;
+	struct list_head		files;
+};
+
+struct func_file {
+	struct list_head		list;
+	struct trace_event_file		*file;
+};
+
+struct func_event_hdr {
+	struct trace_entry	ent;
+	unsigned long		ip;
+	unsigned long		parent_ip;
+};
+
+static DEFINE_MUTEX(func_event_mutex);
+static LIST_HEAD(func_events);
+
+enum func_states {
+	FUNC_STATE_INIT,
+	FUNC_STATE_FUNC,
+	FUNC_STATE_PARAM,
+	FUNC_STATE_END,
+	FUNC_STATE_ERROR,
+};
+
+static void free_func_event(struct func_event *func_event)
+{
+	if (!func_event)
+		return;
+
+	ftrace_free_filter(&func_event->ops);
+	kfree(func_event->call.print_fmt);
+	kfree(func_event->func);
+	kfree(func_event);
+}
+
+static char *next_token(char **ptr, char *last)
+{
+	char *arg;
+	char *str;
+
+	if (!*ptr)
+		return NULL;
+
+	arg = *ptr;
+
+	if (*last)
+		*arg = *last;
+
+	if (!*arg)
+		return NULL;
+
+	for (str = arg; *str; str++) {
+		if (*str == '(' ||
+		    *str == ')')
+			break;
+	}
+	if (*str) {
+		if (str == arg)
+			str++;
+		*last = *str;
+		*str = 0;
+		*ptr = str;
+		return arg;
+	}
+
+	*last = 0;
+	*ptr = NULL;
+	return arg;
+}
+
+static enum func_states
+process_event(struct func_event *fevent, const char *token, enum func_states state)
+{
+	switch (state) {
+	case FUNC_STATE_INIT:
+		if (!isalpha(token[0]))
+			return FUNC_STATE_ERROR;
+		/* Do not allow wild cards */
+		if (strstr(token, "*") || strstr(token, "?"))
+			return FUNC_STATE_ERROR;
+		fevent->func = kstrdup(token, GFP_KERNEL);
+		if (!fevent->func)
+			return FUNC_STATE_ERROR;
+		return FUNC_STATE_FUNC;
+
+	case FUNC_STATE_FUNC:
+		if (token[0] != '(')
+			return FUNC_STATE_ERROR;
+		return FUNC_STATE_PARAM;
+
+	case FUNC_STATE_PARAM:
+		if (token[0] != ')')
+			return FUNC_STATE_ERROR;
+		return FUNC_STATE_END;
+
+	default:
+		return FUNC_STATE_ERROR;
+	}
+}
+
+static void func_event_trace(struct trace_event_file *trace_file,
+			     struct func_event *func_event,
+			     unsigned long ip, unsigned long parent_ip,
+			     struct pt_regs *pt_regs)
+{
+	struct func_event_hdr *entry;
+	struct trace_event_call *call = &func_event->call;
+	struct ring_buffer_event *event;
+	struct ring_buffer *buffer;
+	unsigned long irq_flags;
+	int size;
+	int pc;
+
+	if (trace_trigger_soft_disabled(trace_file))
+		return;
+
+	local_save_flags(irq_flags);
+	pc = preempt_count();
+
+	size = sizeof(*entry);
+
+	event = trace_event_buffer_lock_reserve(&buffer, trace_file,
+						call->event.type,
+						size, irq_flags, pc);
+	if (!event)
+		return;
+
+	entry = ring_buffer_event_data(event);
+	entry->ip = ip;
+	entry->parent_ip = parent_ip;
+
+	event_trigger_unlock_commit_regs(trace_file, buffer, event,
+					 entry, irq_flags, pc, pt_regs);
+}
+
+static void
+func_event_call(unsigned long ip, unsigned long parent_ip,
+		    struct ftrace_ops *op, struct pt_regs *pt_regs)
+{
+	struct func_event *func_event;
+	struct func_file *ff;
+
+	func_event = container_of(op, struct func_event, ops);
+
+	if (WARN_ON_ONCE(rcu_irq_enter_disabled()))
+		return;
+	rcu_irq_enter_irqson();
+	rcu_read_lock_sched_notrace();
+	list_for_each_entry_rcu(ff, &func_event->files, list) {
+		func_event_trace(ff->file, func_event, ip, parent_ip, pt_regs);
+	}
+	rcu_read_unlock_sched_notrace();
+	rcu_irq_exit_irqson();
+}
+
+
+
+static enum print_line_t
+func_event_print(struct trace_iterator *iter, int flags,
+		 struct trace_event *event)
+{
+	struct func_event_hdr *entry;
+	struct trace_seq *s = &iter->seq;
+
+	entry = (struct func_event_hdr *)iter->ent;
+
+	trace_seq_printf(s, "%ps->%ps()",
+			 (void *)entry->parent_ip, (void *)entry->ip);
+	trace_seq_putc(s, '\n');
+	return trace_handle_return(s);
+}
+
+static struct trace_event_functions func_event_funcs = {
+	.trace		= func_event_print,
+};
+
+static int func_event_define_fields(struct trace_event_call *event_call)
+{
+	struct func_event_hdr field;
+	int ret;
+
+	DEFINE_FIELD(unsigned long, ip, "__parent_ip", 0);
+	DEFINE_FIELD(unsigned long, parent_ip, "__ip", 0);
+
+	return 0;
+}
+
+static int enable_func_event(struct func_event *func_event,
+			     struct trace_event_file *file)
+{
+	struct func_file *ff;
+	int ret;
+
+	ff = kmalloc(sizeof(*ff), GFP_KERNEL);
+	if (!ff)
+		return -ENOMEM;
+
+	if (list_empty(&func_event->files)) {
+		ret = register_ftrace_function(&func_event->ops);
+		if (ret < 0)
+			return ret;
+	}
+
+	ff->file = file;
+	/* Make sure file is visible before adding to the list */
+	smp_wmb();
+	list_add_rcu(&ff->list, &func_event->files);
+	return 0;
+}
+
+static int disable_func_event(struct func_event *func_event,
+			      struct trace_event_file *file)
+{
+	struct list_head *p, *n;
+	struct func_file *ff;
+
+
+	list_for_each_safe(p, n, &func_event->files) {
+		ff = container_of(p, struct func_file, list);
+		if (ff->file == file) {
+			list_del_rcu(&ff->list);
+			break;
+		}
+		ff = NULL;
+	}
+
+	if (!ff)
+		return -ENODEV;
+
+	if (list_empty(&func_event->files))
+		unregister_ftrace_function(&func_event->ops);
+
+	synchronize_sched();
+	kfree(ff);
+
+	return 0;
+}
+
+static int func_event_register(struct trace_event_call *event,
+			       enum trace_reg type, void *data)
+{
+	struct func_event *func_event = event->data;
+	struct trace_event_file *file = data;
+
+	switch (type) {
+	case TRACE_REG_REGISTER:
+		return enable_func_event(func_event, file);
+	case TRACE_REG_UNREGISTER:
+		return disable_func_event(func_event, file);
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int set_print_fmt(struct func_event *func_event)
+{
+	const char *fmt = "\"%pS->%pS()\", REC->__ip, REC->__parent_ip";
+
+	func_event->call.print_fmt = kstrdup(fmt, GFP_KERNEL);
+	if (!func_event->call.print_fmt)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int func_event_create(struct func_event *func_event)
+{
+	struct trace_event_call *call = &func_event->call;
+	int ret;
+
+	func_event->class.system = FUNC_EVENT_SYSTEM;
+	call->class = &func_event->class;
+	INIT_LIST_HEAD(&call->class->fields);
+	call->event.funcs = &func_event_funcs;
+	call->name = func_event->func;
+	call->class->define_fields = func_event_define_fields;
+	ret = set_print_fmt(func_event);
+	if (ret < 0)
+		return ret;
+	ret = register_trace_event(&call->event);
+	if (ret < 0)
+		return ret;
+	call->flags = TRACE_EVENT_FL_FUNC;
+	call->class->reg = func_event_register;
+	call->data = func_event;
+	ret = trace_add_event_call(call);
+	if (ret) {
+		pr_info("Failed to register func event: %s\n", func_event->func);
+		unregister_trace_event(&call->event);
+	}
+	return ret;
+}
+
+static int create_function_event(int argc, char **argv)
+{
+	struct func_event *func_event;
+	enum func_states state = FUNC_STATE_INIT;
+	char *token;
+	char *ptr;
+	char last;
+	int ret = -EINVAL;
+	int i;
+
+	func_event = kzalloc(sizeof(*func_event), GFP_KERNEL);
+	if (!func_event)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&func_event->files);
+	func_event->ops.func = func_event_call;
+	func_event->ops.flags = FTRACE_OPS_FL_SAVE_REGS;
+
+	for (i = 0; i < argc; i++) {
+		ptr = argv[i];
+		last = 0;
+		for (token = next_token(&ptr, &last); token;
+		     token = next_token(&ptr, &last)) {
+			state = process_event(func_event, token, state);
+			if (state == FUNC_STATE_ERROR)
+				goto fail;
+		}
+	}
+	if (state != FUNC_STATE_END)
+		goto fail;
+
+	ret = ftrace_set_filter(&func_event->ops, func_event->func,
+				strlen(func_event->func), 0);
+	if (ret < 0)
+		goto fail;
+
+	ret = func_event_create(func_event);
+	if (ret < 0)
+		goto fail;
+
+	mutex_lock(&func_event_mutex);
+	list_add_tail(&func_event->list, &func_events);
+	mutex_unlock(&func_event_mutex);
+	return 0;
+ fail:
+	free_func_event(func_event);
+	return ret;
+}
+
+static void *func_event_seq_start(struct seq_file *m, loff_t *pos)
+{
+	mutex_lock(&func_event_mutex);
+	return seq_list_start(&func_events, *pos);
+}
+
+static void *func_event_seq_next(struct seq_file *m, void *v, loff_t *pos)
+{
+	return seq_list_next(v, &func_events, pos);
+}
+
+static void func_event_seq_stop(struct seq_file *m, void *v)
+{
+	mutex_unlock(&func_event_mutex);
+}
+
+static int func_event_seq_show(struct seq_file *m, void *v)
+{
+	struct func_event *func_event = v;
+
+	seq_printf(m, "%s()\n", func_event->func);
+
+	return 0;
+}
+
+static const struct seq_operations func_event_seq_op = {
+	.start  = func_event_seq_start,
+	.next   = func_event_seq_next,
+	.stop   = func_event_seq_stop,
+	.show   = func_event_seq_show
+};
+
+static int release_all_func_events(void)
+{
+	struct func_event *func_event, *n;
+	int ret = 0;
+
+	mutex_lock(&func_event_mutex);
+	list_for_each_entry_safe(func_event, n, &func_events, list) {
+		ret = trace_remove_event_call(&func_event->call);
+		if (ret < 0)
+			break;
+		list_del(&func_event->list);
+		free_func_event(func_event);
+	}
+	mutex_unlock(&func_event_mutex);
+	return ret;
+}
+
+static int func_event_open(struct inode *inode, struct file *file)
+{
+	int ret;
+
+	if ((file->f_mode & FMODE_WRITE) && (file->f_flags & O_TRUNC)) {
+		ret = release_all_func_events();
+		if (ret < 0)
+			return ret;
+	}
+
+	return seq_open(file, &func_event_seq_op);
+}
+
+static ssize_t
+func_event_write(struct file *filp, const char __user *ubuf,
+		 size_t cnt, loff_t *ppos)
+{
+	return trace_parse_run_command(filp, ubuf, cnt, ppos,
+				       create_function_event);
+}
+
+static const struct file_operations func_event_fops = {
+	.open		= func_event_open,
+	.read           = seq_read,
+	.llseek         = seq_lseek,
+	.release        = seq_release,
+	.write		= func_event_write,
+};
+
+void create_function_event_file(struct dentry *d_tracer)
+{
+	struct dentry *d;
+
+	d = trace_create_file("function_events", 0644, d_tracer, NULL,
+			      &func_event_fops);
+	WARN(!d, "Failed to create function_events file");
+}
+
+/* Make a tracefs interface for controlling probe points */
+static __init int init_func_events(void)
+{
+	struct dentry *d_tracer;
+	struct dentry *entry;
+
+	d_tracer = tracing_init_dentry();
+	if (IS_ERR(d_tracer))
+		return 0;
+
+	entry = trace_create_file("function_events", 0644, d_tracer, NULL,
+				  &func_event_fops);
+
+	/* Event list interface */
+	if (!entry)
+		pr_warn("Could not create tracefs 'function-events' entry\n");
+
+	return 0;
+}
+fs_initcall(init_func_events);
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index fb66e3eaa192..a51caafd993b 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -49,17 +49,6 @@
 #define FIELD_STRING_RETIP	"__probe_ret_ip"
 #define FIELD_STRING_FUNC	"__probe_func"
 
-#undef DEFINE_FIELD
-#define DEFINE_FIELD(type, item, name, is_signed)			\
-	do {								\
-		ret = trace_define_field(event_call, #type, name,	\
-					 offsetof(typeof(field), item),	\
-					 sizeof(field.item), is_signed, \
-					 FILTER_OTHER);			\
-		if (ret)						\
-			return ret;					\
-	} while (0)
-
 
 /* Flags for trace_probe */
 #define TP_FLAG_TRACE		1
-- 
2.15.1

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

* [PATCH 02/20 v2] tracing: Add documentation for function based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
  2018-02-07 20:24 ` [PATCH 01/20 v2] tracing: Add " Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 03/20 v2] tracing: Add simple arguments to " Steven Rostedt
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0002-tracing-Add-documentation-for-function-based-events.patch --]
[-- Type: text/plain, Size: 3380 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Start documenting the usage of function based events. This only gives an
introduction for the function based events. As new features are added to
them, those features will be documented in this document.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/function-based-events.rst | 69 +++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/trace/function-based-events.rst

diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
new file mode 100644
index 000000000000..843a1bf76459
--- /dev/null
+++ b/Documentation/trace/function-based-events.rst
@@ -0,0 +1,69 @@
+=====================
+Function based events
+=====================
+
+.. Copyright 2018 VMware Inc.
+..   Author:   Steven Rostedt <srostedt@goodmis.org>
+..  License:   The GNU Free Documentation License, Version 1.2
+..               (dual licensed under the GPL v2)
+
+
+Introduction
+============
+
+Static events are extremely useful for analyzing the happenings of
+inside the Linux kernel. But there are times where events are not
+available, either due to not being in control of the kernel, or simply
+because a maintainer refuses to have them in their subsystem.
+
+The function tracer is a way trace within a subsystem without trace events.
+But it only provides information of when a function was hit and who
+called it. Combining trace events with the function tracer allows
+for dynamically creating trace events where they do not exist at
+function entry. They provide more information than the function
+tracer can provide, as they can read the parameters of a function
+or simply read an address. This makes it possible to create a
+trace point at any function that the function tracer can trace, and
+read the parameters of the function.
+
+
+Usage
+=====
+
+Simply writing an ASCII string into a file called "function_events"
+in the tracefs file system will create the function based events.
+Note, this file is only writable by root.
+
+ # mount -t tracefs nodev /sys/kernel/tracing
+ # cd /sys/kernel/tracing
+ # echo 'do_IRQ()' > function_events
+
+The above will create a trace event on the do_IRQ function call.
+As no parameters were specified, it will not trace anything other
+than the function and the parent. This is the minimum function
+based event.
+
+ # ls events/functions/do_IRQ
+enable  filter  format  hist  id  trigger
+
+Even though the above function based event does not record much more
+than the function tracer does, it does become a full fledge event.
+This can be used by the histogram infrastructure, and triggers.
+
+ # cat events/functions/do_IRQ/format
+name: do_IRQ
+ID: 1304
+format:
+	field:unsigned short common_type;	offset:0;	size:2;	signed:0;
+	field:unsigned char common_flags;	offset:2;	size:1;	signed:0;
+	field:unsigned char common_preempt_count;	offset:3;	size:1;	signed:0;
+	field:int common_pid;	offset:4;	size:4;	signed:1;
+
+	field:unsigned long __parent_ip;	offset:8;	size:8;	signed:0;
+	field:unsigned long __ip;	offset:16;	size:8;	signed:0;
+
+print fmt: "%pS->%pS()", REC->__ip, REC->__parent_ip
+
+The above shows that the format is very close to the function trace
+except that it displays the parent function followed by the called
+function.
-- 
2.15.1

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

* [PATCH 03/20 v2] tracing: Add simple arguments to function based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
  2018-02-07 20:24 ` [PATCH 01/20 v2] tracing: Add " Steven Rostedt
  2018-02-07 20:24 ` [PATCH 02/20 v2] tracing: Add documentation for " Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 04/20 v2] tracing/x86: Add arch_get_func_args() function Steven Rostedt
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0003-tracing-Add-simple-arguments-to-function-based-event.patch --]
[-- Type: text/plain, Size: 15561 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

The function based events can now have arguments passed in. A weak function
arch_get_func_args() is created so that archs can fill in the arguments
based on pt_regs. Currently no arch implements this function, so no
arguments are returned. Passing NULL for pt_regs into this function returns
the number of supported args that can be processed, and the format will
only allow the user to add valid args. Which currently are all arguments
until an arch implements the arg_get_func_args() function.

[ missing 'static' found by Fengguang Wu's kbuild test robot ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/function-based-events.rst |  57 +++++
 kernel/trace/trace_event_ftrace.c             | 324 ++++++++++++++++++++++++--
 2 files changed, 367 insertions(+), 14 deletions(-)

diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
index 843a1bf76459..94c2c975295a 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -67,3 +67,60 @@ print fmt: "%pS->%pS()", REC->__ip, REC->__parent_ip
 The above shows that the format is very close to the function trace
 except that it displays the parent function followed by the called
 function.
+
+
+Number of arguments
+===================
+
+The number of arguments that can be specified is dependent on the
+architecture. An architecture may not allow any arguments, or it
+may limit to just three or six. If more arguments are used than
+supported, it will fail with -EINVAL.
+
+Parameters
+==========
+
+Adding parameters creates fields within the events. The format is
+as follows:
+
+ # echo EVENT > function_events
+
+ EVENT := <function> '(' ARGS ')'
+
+ Where <function> is any function that the function tracer can trace.
+
+ ARGS := ARG | ARG ',' ARGS | ''
+
+ ARG := TYPE FIELD
+
+ TYPE := ATOM
+
+ ATOM := 'u8' | 'u16' | 'u32' | 'u64' |
+         's8' | 's16' | 's32' | 's64' |
+         'char' | 'short' | 'int' | 'long' | 'size_t'
+
+ FIELD := <name>
+
+ Where <name> is a unique string starting with an alphabetic character
+ and consists only of letters and numbers and underscores.
+
+
+Simple arguments
+================
+
+Looking at kernel code, we can see something like:
+
+ v4.15: net/ipv4/ip_input.c:
+
+int ip_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+
+If we are only interested in the first argument (skb):
+
+ # echo 'ip_rcv(u64 skb, u64 dev)' > function_events
+
+ # echo 1 > events/functions/ip_rcv/enable
+ # cat trace
+     <idle>-0     [003] ..s3  2119.041935: __netif_receive_skb_core->ip_rcv(skb=18446612136982403072, dev=18446612136968273920)
+     <idle>-0     [003] ..s3  2119.041944: __netif_receive_skb_core->ip_rcv(skb=18446612136982403072, dev=18446612136968273920)
+     <idle>-0     [003] ..s3  2119.288337: __netif_receive_skb_core->ip_rcv(skb=18446612136982403072, dev=18446612136968273920)
+     <idle>-0     [003] ..s3  2119.288960: __netif_receive_skb_core->ip_rcv(skb=18446612136982403072, dev=18446612136968273920)
diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index fb8e03e663e8..e915f70e2fd5 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -13,6 +13,16 @@
 #define FUNC_EVENT_SYSTEM "functions"
 #define WRITE_BUFSIZE  4096
 
+struct func_arg {
+	struct list_head		list;
+	char				*type;
+	char				*name;
+	short				offset;
+	short				size;
+	char				arg;
+	char				sign;
+};
+
 struct func_event {
 	struct list_head		list;
 	char				*func;
@@ -20,6 +30,10 @@ struct func_event {
 	struct trace_event_call		call;
 	struct ftrace_ops		ops;
 	struct list_head		files;
+	struct list_head		args;
+	struct func_arg			*last_arg;
+	int				arg_cnt;
+	int				arg_offset;
 };
 
 struct func_file {
@@ -31,6 +45,7 @@ struct func_event_hdr {
 	struct trace_entry	ent;
 	unsigned long		ip;
 	unsigned long		parent_ip;
+	char			data[0];
 };
 
 static DEFINE_MUTEX(func_event_mutex);
@@ -40,15 +55,91 @@ enum func_states {
 	FUNC_STATE_INIT,
 	FUNC_STATE_FUNC,
 	FUNC_STATE_PARAM,
+	FUNC_STATE_TYPE,
+	FUNC_STATE_VAR,
+	FUNC_STATE_COMMA,
 	FUNC_STATE_END,
 	FUNC_STATE_ERROR,
 };
 
+#define TYPE_TUPLE(type)			\
+	{ #type, sizeof(type), is_signed_type(type) }
+
+static struct func_type {
+	char		*name;
+	int		size;
+	int		sign;
+} func_types[] = {
+	TYPE_TUPLE(long),
+	TYPE_TUPLE(int),
+	TYPE_TUPLE(short),
+	TYPE_TUPLE(char),
+	TYPE_TUPLE(size_t),
+	TYPE_TUPLE(u64),
+	TYPE_TUPLE(s64),
+	TYPE_TUPLE(u32),
+	TYPE_TUPLE(s32),
+	TYPE_TUPLE(u16),
+	TYPE_TUPLE(s16),
+	TYPE_TUPLE(u8),
+	TYPE_TUPLE(s8),
+	{ NULL,		0,	0 }
+};
+
+static int max_args __read_mostly = -1;
+
+/**
+ * arch_get_func_args - retrieve function arguments via pt_regs
+ * @regs: The registers at the moment the function is called
+ * @start: The first argument to retrieve (usually zero)
+ * @end: The last argument to retrive (end - start arguments to get)
+ * @args: The array to store the arguments in
+ *
+ * This is to be implemented by architecture code.
+ *
+ * If @regs is NULL, return the number of supported arguments that
+ * can be retrieved (this default function supports no arguments,
+ * and returns zero). The other parameters are ignored when @regs
+ * is NULL.
+ *
+ * If the function can support 6 arguments, then it should return
+ * 6 if @regs is NULL. If @regs is not NULL and it should start
+ * loading the arguments into @args. If @start is 2 and @end is 4,
+ * @args[0] would get the third argument (0 is the first argument)
+ * and @args[1] would get the forth argument. The function would
+ * return 2 (@end - @start).
+ *
+ * If @start is 5 and @end is 7, as @end is greater than the number
+ * of supported arguments, @args[0] would get the sixth argument,
+ * and 1 would be returned. The function does not error if more
+ * than the supported arguments is asked for. It only load what it
+ * can into @args, and return the number of arguments copied.
+ *
+ * Returns:
+ *  If @regs is NULL, the number of supported arguments it can handle.
+ *
+ *  Otherwise, it returns the number of arguments copied to @args.
+ */
+int __weak arch_get_func_args(struct pt_regs *regs,
+			      int start, int end,
+			      long *args)
+{
+	return 0;
+}
+
 static void free_func_event(struct func_event *func_event)
 {
+	struct func_arg *arg, *n;
+
 	if (!func_event)
 		return;
 
+	list_for_each_entry_safe(arg, n, &func_event->args, list) {
+		list_del(&arg->list);
+		kfree(arg->name);
+		kfree(arg->type);
+		kfree(arg);
+	}
 	ftrace_free_filter(&func_event->ops);
 	kfree(func_event->call.print_fmt);
 	kfree(func_event->func);
@@ -73,6 +164,7 @@ static char *next_token(char **ptr, char *last)
 
 	for (str = arg; *str; str++) {
 		if (*str == '(' ||
+		    *str == ',' ||
 		    *str == ')')
 			break;
 	}
@@ -90,34 +182,99 @@ static char *next_token(char **ptr, char *last)
 	return arg;
 }
 
+static int add_arg(struct func_event *fevent, int ftype)
+{
+	struct func_type *func_type = &func_types[ftype];
+	struct func_arg *arg;
+
+	/* Make sure the arch can support this many args */
+	if (fevent->arg_cnt >= max_args)
+		return -EINVAL;
+
+	arg = kzalloc(sizeof(*arg), GFP_KERNEL);
+	if (!arg)
+		return -ENOMEM;
+
+	arg->type = kstrdup(func_type->name, GFP_KERNEL);
+	if (!arg->type) {
+		kfree(arg);
+		return -ENOMEM;
+	}
+	arg->size = func_type->size;
+	arg->sign = func_type->sign;
+	arg->offset = ALIGN(fevent->arg_offset, arg->size);
+	arg->arg = fevent->arg_cnt;
+	fevent->arg_offset = arg->offset + arg->size;
+
+	list_add_tail(&arg->list, &fevent->args);
+	fevent->last_arg = arg;
+	fevent->arg_cnt++;
+
+	return 0;
+}
+
 static enum func_states
 process_event(struct func_event *fevent, const char *token, enum func_states state)
 {
+	int ret;
+	int i;
+
 	switch (state) {
 	case FUNC_STATE_INIT:
 		if (!isalpha(token[0]))
-			return FUNC_STATE_ERROR;
+			break;
 		/* Do not allow wild cards */
 		if (strstr(token, "*") || strstr(token, "?"))
-			return FUNC_STATE_ERROR;
+			break;
 		fevent->func = kstrdup(token, GFP_KERNEL);
 		if (!fevent->func)
-			return FUNC_STATE_ERROR;
+			break;
 		return FUNC_STATE_FUNC;
 
 	case FUNC_STATE_FUNC:
 		if (token[0] != '(')
-			return FUNC_STATE_ERROR;
+			break;
 		return FUNC_STATE_PARAM;
 
 	case FUNC_STATE_PARAM:
-		if (token[0] != ')')
-			return FUNC_STATE_ERROR;
-		return FUNC_STATE_END;
+		if (token[0] == ')')
+			return FUNC_STATE_END;
+		/* Fall through */
+	case FUNC_STATE_COMMA:
+		for (i = 0; func_types[i].size; i++) {
+			if (strcmp(token, func_types[i].name) == 0)
+				break;
+		}
+		if (!func_types[i].size)
+			break;
+		ret = add_arg(fevent, i);
+		if (ret < 0)
+			break;
+		return FUNC_STATE_TYPE;
+
+	case FUNC_STATE_TYPE:
+		if (!isalpha(token[0]))
+			break;
+		if (WARN_ON(!fevent->last_arg))
+			break;
+		fevent->last_arg->name = kstrdup(token, GFP_KERNEL);
+		if (!fevent->last_arg->name)
+			break;
+		return FUNC_STATE_VAR;
+
+	case FUNC_STATE_VAR:
+		switch (token[0]) {
+		case ')':
+			return FUNC_STATE_END;
+		case ',':
+			return FUNC_STATE_COMMA;
+		}
+		break;
 
 	default:
-		return FUNC_STATE_ERROR;
+		break;
 	}
+	return FUNC_STATE_ERROR;
 }
 
 static void func_event_trace(struct trace_event_file *trace_file,
@@ -129,9 +286,14 @@ static void func_event_trace(struct trace_event_file *trace_file,
 	struct trace_event_call *call = &func_event->call;
 	struct ring_buffer_event *event;
 	struct ring_buffer *buffer;
+	struct func_arg *arg;
+	long args[func_event->arg_cnt];
+	long long val = 1;
 	unsigned long irq_flags;
+	int nr_args;
 	int size;
 	int pc;
+	int i = 0;
 
 	if (trace_trigger_soft_disabled(trace_file))
 		return;
@@ -139,7 +301,7 @@ static void func_event_trace(struct trace_event_file *trace_file,
 	local_save_flags(irq_flags);
 	pc = preempt_count();
 
-	size = sizeof(*entry);
+	size = func_event->arg_offset + sizeof(*entry);
 
 	event = trace_event_buffer_lock_reserve(&buffer, trace_file,
 						call->event.type,
@@ -150,6 +312,15 @@ static void func_event_trace(struct trace_event_file *trace_file,
 	entry = ring_buffer_event_data(event);
 	entry->ip = ip;
 	entry->parent_ip = parent_ip;
+	nr_args = arch_get_func_args(pt_regs, 0, func_event->arg_cnt, args);
+
+	list_for_each_entry(arg, &func_event->args, list) {
+		if (i < nr_args)
+			val = args[i];
+		else
+			val = 0;
+		memcpy(&entry->data[arg->offset], &val, arg->size);
+	}
 
 	event_trigger_unlock_commit_regs(trace_file, buffer, event,
 					 entry, irq_flags, pc, pt_regs);
@@ -175,7 +346,26 @@ func_event_call(unsigned long ip, unsigned long parent_ip,
 	rcu_irq_exit_irqson();
 }
 
+#define FMT_SIZE	8
+
+static void make_fmt(struct func_arg *arg, char *fmt)
+{
+	int c = 0;
 
+	fmt[c++] = '%';
+
+	if (arg->size == 8) {
+		fmt[c++] = 'l';
+		fmt[c++] = 'l';
+	}
+
+	if (arg->sign)
+		fmt[c++] = 'd';
+	else
+		fmt[c++] = 'u';
+
+	fmt[c++] = '\0';
+}
 
 static enum print_line_t
 func_event_print(struct trace_iterator *iter, int flags,
@@ -183,12 +373,43 @@ func_event_print(struct trace_iterator *iter, int flags,
 {
 	struct func_event_hdr *entry;
 	struct trace_seq *s = &iter->seq;
+	struct func_event *func_event;
+	struct func_arg *arg;
+	char fmt[FMT_SIZE];
+	void *data;
+	bool comma = false;
 
 	entry = (struct func_event_hdr *)iter->ent;
 
-	trace_seq_printf(s, "%ps->%ps()",
+	func_event = container_of(event, struct func_event, call.event);
+
+	trace_seq_printf(s, "%ps->%ps(",
 			 (void *)entry->parent_ip, (void *)entry->ip);
-	trace_seq_putc(s, '\n');
+	list_for_each_entry(arg, &func_event->args, list) {
+		if (comma)
+			trace_seq_puts(s, ", ");
+		comma = true;
+		trace_seq_printf(s, "%s=", arg->name);
+		data = &entry->data[arg->offset];
+
+		make_fmt(arg, fmt);
+
+		switch (arg->size) {
+		case 8:
+			trace_seq_printf(s, fmt, *(unsigned long long *)data);
+			break;
+		case 4:
+			trace_seq_printf(s, fmt, *(unsigned *)data);
+			break;
+		case 2:
+			trace_seq_printf(s, fmt, *(unsigned short *)data);
+			break;
+		case 1:
+			trace_seq_printf(s, fmt, *(unsigned char *)data);
+			break;
+		}
+	}
+	trace_seq_puts(s, ")\n");
 	return trace_handle_return(s);
 }
 
@@ -198,12 +419,25 @@ static struct trace_event_functions func_event_funcs = {
 
 static int func_event_define_fields(struct trace_event_call *event_call)
 {
+	struct func_event *fevent;
 	struct func_event_hdr field;
+	struct func_arg *arg;
 	int ret;
 
+	fevent = (struct func_event *)event_call->data;
+
 	DEFINE_FIELD(unsigned long, ip, "__parent_ip", 0);
 	DEFINE_FIELD(unsigned long, parent_ip, "__ip", 0);
 
+	list_for_each_entry(arg, &fevent->args, list) {
+		ret = trace_define_field(event_call, arg->type,
+					 arg->name,
+					 sizeof(field) + arg->offset,
+					 arg->size, arg->sign,
+					 FILTER_OTHER);
+		if (ret < 0)
+			return ret;
+	}
 	return 0;
 }
 
@@ -276,13 +510,61 @@ static int func_event_register(struct trace_event_call *event,
 	return 0;
 }
 
+static int update_len(int len, int i)
+{
+	len -= i;
+	if (len < 0)
+		return 0;
+	return len;
+}
+
+static int __set_print_fmt(struct func_event *func_event,
+			   char *buf, int len)
+{
+	struct func_arg *arg;
+	const char *fmt_start = "\"%pS->%pS(";
+	const char *fmt_end = ")\", REC->__ip, REC->__parent_ip";
+	char fmt[FMT_SIZE];
+	int r, i;
+	bool comma = false;
+
+	r = snprintf(buf, len, "%s", fmt_start);
+	len = update_len(len, r);
+	list_for_each_entry(arg, &func_event->args, list) {
+		if (comma) {
+			i = snprintf(buf + r, len, ", ");
+			r += i;
+			len = update_len(len, i);
+		}
+		comma = true;
+		make_fmt(arg, fmt);
+		i = snprintf(buf + r, len, "%s=%s", arg->name, fmt);
+		r += i;
+		len = update_len(len, i);
+	}
+	i = snprintf(buf + r, len, "%s", fmt_end);
+	r += i;
+	len = update_len(len, i);
+
+	list_for_each_entry(arg, &func_event->args, list) {
+		i = snprintf(buf + r, len, ", REC->%s", arg->name);
+		r += i;
+		len = update_len(len, i);
+	}
+
+	return r;
+}
+
 static int set_print_fmt(struct func_event *func_event)
 {
-	const char *fmt = "\"%pS->%pS()\", REC->__ip, REC->__parent_ip";
+	int len;
 
-	func_event->call.print_fmt = kstrdup(fmt, GFP_KERNEL);
+	/* Get required length */
+	len = __set_print_fmt(func_event, NULL, 0) + 1;
+	func_event->call.print_fmt = kmalloc(len, GFP_KERNEL);
 	if (!func_event->call.print_fmt)
 		return -ENOMEM;
+	__set_print_fmt(func_event, func_event->call.print_fmt, len);
 
 	return 0;
 }
@@ -330,6 +612,7 @@ static int create_function_event(int argc, char **argv)
 		return -ENOMEM;
 
 	INIT_LIST_HEAD(&func_event->files);
+	INIT_LIST_HEAD(&func_event->args);
 	func_event->ops.func = func_event_call;
 	func_event->ops.flags = FTRACE_OPS_FL_SAVE_REGS;
 
@@ -383,8 +666,18 @@ static void func_event_seq_stop(struct seq_file *m, void *v)
 static int func_event_seq_show(struct seq_file *m, void *v)
 {
 	struct func_event *func_event = v;
+	struct func_arg *arg;
+	bool comma = false;
 
-	seq_printf(m, "%s()\n", func_event->func);
+	seq_printf(m, "%s(", func_event->func);
+
+	list_for_each_entry(arg, &func_event->args, list) {
+		if (comma)
+			seq_puts(m, ", ");
+		comma = true;
+		seq_printf(m, "%s %s", arg->type, arg->name);
+	}
+	seq_puts(m, ")\n");
 
 	return 0;
 }
@@ -417,6 +710,9 @@ static int func_event_open(struct inode *inode, struct file *file)
 {
 	int ret;
 
+	if (max_args < 0)
+		max_args = arch_get_func_args(NULL, 0, 0, NULL);
+
 	if ((file->f_mode & FMODE_WRITE) && (file->f_flags & O_TRUNC)) {
 		ret = release_all_func_events();
 		if (ret < 0)
-- 
2.15.1

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

* [PATCH 04/20 v2] tracing/x86: Add arch_get_func_args() function
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (2 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 03/20 v2] tracing: Add simple arguments to " Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 05/20 v2] tracing: Add hex print for dynamic ftrace based events Steven Rostedt
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0004-tracing-x86-Add-arch_get_func_args-function.patch --]
[-- Type: text/plain, Size: 1130 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Add function to get the function arguments from pt_regs.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 arch/x86/kernel/ftrace.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 01ebcb6f263e..5e845c8cf89d 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -46,6 +46,34 @@ int ftrace_arch_code_modify_post_process(void)
 	return 0;
 }
 
+int arch_get_func_args(struct pt_regs *regs,
+		       int start, int end, long *args)
+{
+#ifdef CONFIG_X86_64
+# define MAX_ARGS 6
+# define INIT_REGS				\
+	{	regs->di, regs->si, regs->dx,	\
+		regs->cx, regs->r8, regs->r9	\
+	}
+#else
+# define MAX_ARGS 3
+# define INIT_REGS				\
+	{	regs->ax, regs->dx, regs->cx	}
+#endif
+	if (!regs)
+		return MAX_ARGS;
+
+	{
+		long pt_args[] = INIT_REGS;
+		int i;
+
+		for (i = start; i <= end && i < MAX_ARGS; i++)
+			args[i - start] = pt_args[i];
+
+		return i - start;
+	}
+}
+
 union ftrace_code_union {
 	char code[MCOUNT_INSN_SIZE];
 	struct {
-- 
2.15.1

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

* [PATCH 05/20 v2] tracing: Add hex print for dynamic ftrace based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (3 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 04/20 v2] tracing/x86: Add arch_get_func_args() function Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 06/20 v2] tracing: Add indirect offset to args of " Steven Rostedt
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0005-tracing-Add-hex-print-for-dynamic-ftrace-based-event.patch --]
[-- Type: text/plain, Size: 3370 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Add x64, x32, x16 and x8 to represent numbers of the same size in hex.
Similar to u64, u32, u16, and u8 but uses %x instead of %u.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/function-based-events.rst | 14 +++++++++-----
 kernel/trace/trace_event_ftrace.c             | 13 ++++++++++++-
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
index 94c2c975295a..f27a0c4e829c 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -97,6 +97,7 @@ as follows:
 
  ATOM := 'u8' | 'u16' | 'u32' | 'u64' |
          's8' | 's16' | 's32' | 's64' |
+         'x8' | 'x16' | 'x32' | 'x64' |
          'char' | 'short' | 'int' | 'long' | 'size_t'
 
  FIELD := <name>
@@ -116,11 +117,14 @@ int ip_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt,
 
 If we are only interested in the first argument (skb):
 
- # echo 'ip_rcv(u64 skb, u64 dev)' > function_events
+ # echo 'ip_rcv(x64 skb, x86 dev)' > function_events
 
  # echo 1 > events/functions/ip_rcv/enable
  # cat trace
-     <idle>-0     [003] ..s3  2119.041935: __netif_receive_skb_core->ip_rcv(skb=18446612136982403072, dev=18446612136968273920)
-     <idle>-0     [003] ..s3  2119.041944: __netif_receive_skb_core->ip_rcv(skb=18446612136982403072, dev=18446612136968273920)
-     <idle>-0     [003] ..s3  2119.288337: __netif_receive_skb_core->ip_rcv(skb=18446612136982403072, dev=18446612136968273920)
-     <idle>-0     [003] ..s3  2119.288960: __netif_receive_skb_core->ip_rcv(skb=18446612136982403072, dev=18446612136968273920)
+     <idle>-0     [003] ..s3  5543.133460: __netif_receive_skb_core->ip_rcv(skb=ffff88007f960700, net=ffff880114250000)
+     <idle>-0     [003] ..s3  5543.133475: __netif_receive_skb_core->ip_rcv(skb=ffff88007f960700, net=ffff880114250000)
+     <idle>-0     [003] ..s3  5543.312592: __netif_receive_skb_core->ip_rcv(skb=ffff88007f960700, net=ffff880114250000)
+     <idle>-0     [003] ..s3  5543.313150: __netif_receive_skb_core->ip_rcv(skb=ffff88007f960700, net=ffff880114250000)
+
+We use "x64" in order to make sure that the data is displayed in hex.
+This is on a x86_64 machine, and we know the pointer sizes are 8 bytes.
diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index e915f70e2fd5..be7f5bc416cc 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -62,6 +62,11 @@ enum func_states {
 	FUNC_STATE_ERROR,
 };
 
+typedef u64 x64;
+typedef u32 x32;
+typedef u16 x16;
+typedef u8 x8;
+
 #define TYPE_TUPLE(type)			\
 	{ #type, sizeof(type), is_signed_type(type) }
 
@@ -77,12 +82,16 @@ static struct func_type {
 	TYPE_TUPLE(size_t),
 	TYPE_TUPLE(u64),
 	TYPE_TUPLE(s64),
+	TYPE_TUPLE(x64),
 	TYPE_TUPLE(u32),
 	TYPE_TUPLE(s32),
+	TYPE_TUPLE(x32),
 	TYPE_TUPLE(u16),
 	TYPE_TUPLE(s16),
+	TYPE_TUPLE(x16),
 	TYPE_TUPLE(u8),
 	TYPE_TUPLE(s8),
+	TYPE_TUPLE(x8),
 	{ NULL,		0,	0 }
 };
 
@@ -359,7 +368,9 @@ static void make_fmt(struct func_arg *arg, char *fmt)
 		fmt[c++] = 'l';
 	}
 
-	if (arg->sign)
+	if (arg->type[0] == 'x')
+		fmt[c++] = 'x';
+	else if (arg->sign)
 		fmt[c++] = 'd';
 	else
 		fmt[c++] = 'u';
-- 
2.15.1

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

* [PATCH 06/20 v2] tracing: Add indirect offset to args of ftrace based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (4 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 05/20 v2] tracing: Add hex print for dynamic ftrace based events Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 07/20 v2] tracing: Add dereferencing multiple fields per arg Steven Rostedt
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0006-tracing-Add-indirect-offset-to-args-of-ftrace-based-.patch --]
[-- Type: text/plain, Size: 5984 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Add '[' ']' syntex to allow to get values indirectly from the arguments.
For example:

 echo replenish_dl_entity(s64 dl_se[4]) > function_events

Will get the 4th long long word from the first parameter like an array.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/function-based-events.rst | 32 +++++++++++-
 kernel/trace/trace_event_ftrace.c             | 73 +++++++++++++++++++++++++--
 2 files changed, 101 insertions(+), 4 deletions(-)

diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
index f27a0c4e829c..7d67229e8e88 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -100,11 +100,15 @@ as follows:
          'x8' | 'x16' | 'x32' | 'x64' |
          'char' | 'short' | 'int' | 'long' | 'size_t'
 
- FIELD := <name>
+ FIELD := <name> | <name> INDEX
+
+ INDEX := '[' <number> ']'
 
  Where <name> is a unique string starting with an alphabetic character
  and consists only of letters and numbers and underscores.
 
+ Where <number> is a number that can be read by kstrtol() (hex, decimal, etc).
+
 
 Simple arguments
 ================
@@ -128,3 +132,29 @@ If we are only interested in the first argument (skb):
 
 We use "x64" in order to make sure that the data is displayed in hex.
 This is on a x86_64 machine, and we know the pointer sizes are 8 bytes.
+
+
+Indexing
+========
+
+The pointers of the skb and the dev isn't that interesting. But if we want the
+length "len" field of skb, we could index it with an index operator '[' and ']'.
+
+Using gdb, we can find the offset of 'len' from the sk_buff type:
+
+ $ gdb vmlinux
+ (gdb) printf "%d\n", &((struct sk_buff *)0)->len
+128
+
+As 128 / 4 (length of int) is 32, we can see the length of the skb with:
+
+ # echo 'ip_rcv(int skb[32], x64 dev)' > function_events
+
+ # echo 1 > events/functions/ip_rcv/enable
+ # cat trace
+    <idle>-0     [003] ..s3   280.167137: __netif_receive_skb_core->ip_rcv(skb=52, dev=ffff8801092f9400)
+    <idle>-0     [003] ..s3   280.167152: __netif_receive_skb_core->ip_rcv(skb=52, dev=ffff8801092f9400)
+    <idle>-0     [003] ..s3   280.806629: __netif_receive_skb_core->ip_rcv(skb=88, dev=ffff8801092f9400)
+    <idle>-0     [003] ..s3   280.807023: __netif_receive_skb_core->ip_rcv(skb=52, dev=ffff8801092f9400)
+
+Now we see the length of the sk_buff per event.
diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index be7f5bc416cc..b622634062db 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -10,13 +10,15 @@
 
 #include "trace.h"
 
-#define FUNC_EVENT_SYSTEM "functions"
-#define WRITE_BUFSIZE  4096
+#define FUNC_EVENT_SYSTEM	"functions"
+#define WRITE_BUFSIZE		4096
+#define INDIRECT_FLAG		0x10000000
 
 struct func_arg {
 	struct list_head		list;
 	char				*type;
 	char				*name;
+	long				indirect;
 	short				offset;
 	short				size;
 	char				arg;
@@ -55,6 +57,9 @@ enum func_states {
 	FUNC_STATE_INIT,
 	FUNC_STATE_FUNC,
 	FUNC_STATE_PARAM,
+	FUNC_STATE_BRACKET,
+	FUNC_STATE_BRACKET_END,
+	FUNC_STATE_INDIRECT,
 	FUNC_STATE_TYPE,
 	FUNC_STATE_VAR,
 	FUNC_STATE_COMMA,
@@ -173,6 +178,8 @@ static char *next_token(char **ptr, char *last)
 
 	for (str = arg; *str; str++) {
 		if (*str == '(' ||
+		    *str == '[' ||
+		    *str == ']' ||
 		    *str == ',' ||
 		    *str == ')')
 			break;
@@ -225,6 +232,7 @@ static int add_arg(struct func_event *fevent, int ftype)
 static enum func_states
 process_event(struct func_event *fevent, const char *token, enum func_states state)
 {
+	long val;
 	int ret;
 	int i;
 
@@ -271,12 +279,37 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 			break;
 		return FUNC_STATE_VAR;
 
+	case FUNC_STATE_BRACKET:
+		WARN_ON(!fevent->last_arg);
+		ret = kstrtol(token, 0, &val);
+		if (ret)
+			break;
+		val *= fevent->last_arg->size;
+		fevent->last_arg->indirect = val ^ INDIRECT_FLAG;
+		return FUNC_STATE_INDIRECT;
+
+	case FUNC_STATE_INDIRECT:
+		if (token[0] != ']')
+			break;
+		return FUNC_STATE_BRACKET_END;
+
+	case FUNC_STATE_BRACKET_END:
+		switch (token[0]) {
+		case ')':
+			return FUNC_STATE_END;
+		case ',':
+			return FUNC_STATE_COMMA;
+		}
+		break;
+
 	case FUNC_STATE_VAR:
 		switch (token[0]) {
 		case ')':
 			return FUNC_STATE_END;
 		case ',':
 			return FUNC_STATE_COMMA;
+		case '[':
+			return FUNC_STATE_BRACKET;
 		}
 		break;
 
@@ -286,6 +319,37 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 	return FUNC_STATE_ERROR;
 }
 
+static long long get_arg(struct func_arg *arg, unsigned long val)
+{
+	char buf[8];
+	int ret;
+
+	if (!arg->indirect)
+		return val;
+
+	val = val + (arg->indirect ^ INDIRECT_FLAG);
+
+	ret = probe_kernel_read(buf, (void *)val, arg->size);
+	if (ret)
+		return 0;
+
+	switch (arg->size) {
+		case 8:
+			val = *(unsigned long long *)buf;
+			break;
+		case 4:
+			val = *(unsigned int *)buf;
+			break;
+		case 2:
+			val = *(unsigned short *)buf;
+			break;
+		case 1:
+			val = *(unsigned char *)buf;
+			break;
+	}
+	return val;
+}
+
 static void func_event_trace(struct trace_event_file *trace_file,
 			     struct func_event *func_event,
 			     unsigned long ip, unsigned long parent_ip,
@@ -325,7 +389,7 @@ static void func_event_trace(struct trace_event_file *trace_file,
 
 	list_for_each_entry(arg, &func_event->args, list) {
 		if (i < nr_args)
-			val = args[i];
+			val = get_arg(arg, args[i]);
 		else
 			val = 0;
 		memcpy(&entry->data[arg->offset], &val, arg->size);
@@ -687,6 +751,9 @@ static int func_event_seq_show(struct seq_file *m, void *v)
 			seq_puts(m, ", ");
 		comma = true;
 		seq_printf(m, "%s %s", arg->type, arg->name);
+		if (arg->indirect && arg->size)
+			seq_printf(m, "[%ld]",
+				   (arg->indirect ^ INDIRECT_FLAG) / arg->size);
 	}
 	seq_puts(m, ")\n");
 
-- 
2.15.1

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

* [PATCH 07/20 v2] tracing: Add dereferencing multiple fields per arg
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (5 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 06/20 v2] tracing: Add indirect offset to args of " Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 08/20 v2] tracing: Add "unsigned" to function based events Steven Rostedt
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0007-tracing-Add-dereferencing-multiple-fields-per-arg.patch --]
[-- Type: text/plain, Size: 5005 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

As an argument may be a structure or an array, we may want to dereference
more than one field per argument. Create a pipe '|' token to the parsing
that allows to reference multipe dereference fields per function argument.

Change func_arg fields from char to s8 or u8 to allow them to be
subscripts to arrays.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/function-based-events.rst | 20 +++++++++++++++++-
 kernel/trace/trace_event_ftrace.c             | 29 ++++++++++++++++++++-------
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
index 7d67229e8e88..2a002c8a500b 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -91,7 +91,7 @@ as follows:
 
  ARGS := ARG | ARG ',' ARGS | ''
 
- ARG := TYPE FIELD
+ ARG := TYPE FIELD | ARG '|' ARG
 
  TYPE := ATOM
 
@@ -158,3 +158,21 @@ As 128 / 4 (length of int) is 32, we can see the length of the skb with:
     <idle>-0     [003] ..s3   280.807023: __netif_receive_skb_core->ip_rcv(skb=52, dev=ffff8801092f9400)
 
 Now we see the length of the sk_buff per event.
+
+
+Multiple fields per argument
+============================
+
+
+If we still want to see the skb pointer value along with the length of the
+skb, then using the '|' option allows us to add more than one option to
+an argument:
+
+ # echo 'ip_rcv(x64 skb | int skb[32], x64 dev)' > function_events
+
+ # echo 1 > events/functions/ip_rcv/enable
+ # cat trace
+    <idle>-0     [003] ..s3   904.075838: __netif_receive_skb_core->ip_rcv(skb=ffff88011396e800, skb=52, dev=ffff880115204000)
+    <idle>-0     [003] ..s3   904.075848: __netif_receive_skb_core->ip_rcv(skb=ffff88011396e800, skb=52, dev=ffff880115204000)
+    <idle>-0     [003] ..s3   904.725486: __netif_receive_skb_core->ip_rcv(skb=ffff88011396e800, skb=194, dev=ffff880115204000)
+    <idle>-0     [003] ..s3   905.152537: __netif_receive_skb_core->ip_rcv(skb=ffff88011396f200, skb=88, dev=ffff880115204000)
diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index b622634062db..666edd76b70f 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -21,8 +21,8 @@ struct func_arg {
 	long				indirect;
 	short				offset;
 	short				size;
-	char				arg;
-	char				sign;
+	s8				arg;
+	u8				sign;
 };
 
 struct func_event {
@@ -60,6 +60,7 @@ enum func_states {
 	FUNC_STATE_BRACKET,
 	FUNC_STATE_BRACKET_END,
 	FUNC_STATE_INDIRECT,
+	FUNC_STATE_PIPE,
 	FUNC_STATE_TYPE,
 	FUNC_STATE_VAR,
 	FUNC_STATE_COMMA,
@@ -181,6 +182,7 @@ static char *next_token(char **ptr, char *last)
 		    *str == '[' ||
 		    *str == ']' ||
 		    *str == ',' ||
+		    *str == '|' ||
 		    *str == ')')
 			break;
 	}
@@ -253,11 +255,15 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 			break;
 		return FUNC_STATE_PARAM;
 
+	case FUNC_STATE_PIPE:
+		fevent->arg_cnt--;
+		goto comma;
 	case FUNC_STATE_PARAM:
 		if (token[0] == ')')
 			return FUNC_STATE_END;
 		/* Fall through */
 	case FUNC_STATE_COMMA:
+ comma:
 		for (i = 0; func_types[i].size; i++) {
 			if (strcmp(token, func_types[i].name) == 0)
 				break;
@@ -299,6 +305,8 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 			return FUNC_STATE_END;
 		case ',':
 			return FUNC_STATE_COMMA;
+		case '|':
+			return FUNC_STATE_PIPE;
 		}
 		break;
 
@@ -308,6 +316,8 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 			return FUNC_STATE_END;
 		case ',':
 			return FUNC_STATE_COMMA;
+		case '|':
+			return FUNC_STATE_PIPE;
 		case '[':
 			return FUNC_STATE_BRACKET;
 		}
@@ -366,7 +376,6 @@ static void func_event_trace(struct trace_event_file *trace_file,
 	int nr_args;
 	int size;
 	int pc;
-	int i = 0;
 
 	if (trace_trigger_soft_disabled(trace_file))
 		return;
@@ -388,8 +397,8 @@ static void func_event_trace(struct trace_event_file *trace_file,
 	nr_args = arch_get_func_args(pt_regs, 0, func_event->arg_cnt, args);
 
 	list_for_each_entry(arg, &func_event->args, list) {
-		if (i < nr_args)
-			val = get_arg(arg, args[i]);
+		if (arg->arg < nr_args)
+			val = get_arg(arg, args[arg->arg]);
 		else
 			val = 0;
 		memcpy(&entry->data[arg->offset], &val, arg->size);
@@ -743,12 +752,18 @@ static int func_event_seq_show(struct seq_file *m, void *v)
 	struct func_event *func_event = v;
 	struct func_arg *arg;
 	bool comma = false;
+	int last_arg = 0;
 
 	seq_printf(m, "%s(", func_event->func);
 
 	list_for_each_entry(arg, &func_event->args, list) {
-		if (comma)
-			seq_puts(m, ", ");
+		if (comma) {
+			if (last_arg == arg->arg)
+				seq_puts(m, " | ");
+			else
+				seq_puts(m, ", ");
+		}
+		last_arg = arg->arg;
 		comma = true;
 		seq_printf(m, "%s %s", arg->type, arg->name);
 		if (arg->indirect && arg->size)
-- 
2.15.1

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

* [PATCH 08/20 v2] tracing: Add "unsigned" to function based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (6 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 07/20 v2] tracing: Add dereferencing multiple fields per arg Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 09/20 v2] tracing: Add indexing of arguments for " Steven Rostedt
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0008-tracing-Add-unsigned-to-function-based-events.patch --]
[-- Type: text/plain, Size: 5151 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Add "unsigned" to the format processing to creating dynamic function based
events. For example: "unsigned long" now works.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/function-based-events.rst | 47 ++++++++++++++++++++++++++-
 kernel/trace/trace_event_ftrace.c             | 23 ++++++++++---
 2 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
index 2a002c8a500b..72e3e7730d63 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -93,7 +93,7 @@ as follows:
 
  ARG := TYPE FIELD | ARG '|' ARG
 
- TYPE := ATOM
+ TYPE := ATOM | 'unsigned' ATOM
 
  ATOM := 'u8' | 'u16' | 'u32' | 'u64' |
          's8' | 's16' | 's32' | 's64' |
@@ -176,3 +176,48 @@ an argument:
     <idle>-0     [003] ..s3   904.075848: __netif_receive_skb_core->ip_rcv(skb=ffff88011396e800, skb=52, dev=ffff880115204000)
     <idle>-0     [003] ..s3   904.725486: __netif_receive_skb_core->ip_rcv(skb=ffff88011396e800, skb=194, dev=ffff880115204000)
     <idle>-0     [003] ..s3   905.152537: __netif_receive_skb_core->ip_rcv(skb=ffff88011396f200, skb=88, dev=ffff880115204000)
+
+
+Unsigned usage
+==============
+
+One can also use "unsigned" to make some types unsigned. It works against
+"long", "int", "short" and "char". It doesn't error against other types but
+may not make any sense.
+
+ # echo 'ip_rcv(int skb[32])' > function_events
+ # cat events/functions/ip_rcv/format
+name: ip_rcv
+ID: 1397
+format:
+	field:unsigned short common_type;	offset:0;	size:2;	signed:0;
+	field:unsigned char common_flags;	offset:2;	size:1;	signed:0;
+	field:unsigned char common_preempt_count;	offset:3;	size:1;	signed:0;
+	field:int common_pid;	offset:4;	size:4;	signed:1;
+
+	field:unsigned long __parent_ip;	offset:8;	size:8;	signed:0;
+	field:unsigned long __ip;	offset:16;	size:8;	signed:0;
+	field:int skb;	offset:24;	size:4;	signed:1;
+
+print fmt: "%pS->%pS(skb=%d)", REC->__ip, REC->__parent_ip, REC->skb
+
+
+Notice that REC->skb is printed with "%d". By adding "unsigned"
+
+ # echo 'ip_rcv(unsigned int skb[32])' > function_events
+ # cat events/functions/ip_rcv/format
+name: ip_rcv
+ID: 1398
+format:
+	field:unsigned short common_type;	offset:0;	size:2;	signed:0;
+	field:unsigned char common_flags;	offset:2;	size:1;	signed:0;
+	field:unsigned char common_preempt_count;	offset:3;	size:1;	signed:0;
+	field:int common_pid;	offset:4;	size:4;	signed:1;
+
+	field:unsigned long __parent_ip;	offset:8;	size:8;	signed:0;
+	field:unsigned long __ip;	offset:16;	size:8;	signed:0;
+	field:unsigned int skb;	offset:24;	size:4;	signed:0;
+
+print fmt: "%pS->%pS(skb=%u)", REC->__ip, REC->__parent_ip, REC->skb
+
+It is now printed with a "%u".
diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index 666edd76b70f..8327346799eb 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -60,6 +60,7 @@ enum func_states {
 	FUNC_STATE_BRACKET,
 	FUNC_STATE_BRACKET_END,
 	FUNC_STATE_INDIRECT,
+	FUNC_STATE_UNSIGNED,
 	FUNC_STATE_PIPE,
 	FUNC_STATE_TYPE,
 	FUNC_STATE_VAR,
@@ -200,7 +201,7 @@ static char *next_token(char **ptr, char *last)
 	return arg;
 }
 
-static int add_arg(struct func_event *fevent, int ftype)
+static int add_arg(struct func_event *fevent, int ftype, int unsign)
 {
 	struct func_type *func_type = &func_types[ftype];
 	struct func_arg *arg;
@@ -213,13 +214,18 @@ static int add_arg(struct func_event *fevent, int ftype)
 	if (!arg)
 		return -ENOMEM;
 
-	arg->type = kstrdup(func_type->name, GFP_KERNEL);
+	if (unsign)
+		arg->type = kasprintf(GFP_KERNEL, "unsigned %s",
+				      func_type->name);
+	else
+		arg->type = kstrdup(func_type->name, GFP_KERNEL);
 	if (!arg->type) {
 		kfree(arg);
 		return -ENOMEM;
 	}
 	arg->size = func_type->size;
-	arg->sign = func_type->sign;
+	if (!unsign)
+		arg->sign = func_type->sign;
 	arg->offset = ALIGN(fevent->arg_offset, arg->size);
 	arg->arg = fevent->arg_cnt;
 	fevent->arg_offset = arg->offset + arg->size;
@@ -234,12 +240,14 @@ static int add_arg(struct func_event *fevent, int ftype)
 static enum func_states
 process_event(struct func_event *fevent, const char *token, enum func_states state)
 {
+	static int unsign;
 	long val;
 	int ret;
 	int i;
 
 	switch (state) {
 	case FUNC_STATE_INIT:
+		unsign = 0;
 		if (!isalpha(token[0]))
 			break;
 		/* Do not allow wild cards */
@@ -264,13 +272,20 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 		/* Fall through */
 	case FUNC_STATE_COMMA:
  comma:
+		if (strcmp(token, "unsigned") == 0) {
+			unsign = 2;
+			return FUNC_STATE_UNSIGNED;
+		}
+		/* Fall through */
+	case FUNC_STATE_UNSIGNED:
 		for (i = 0; func_types[i].size; i++) {
 			if (strcmp(token, func_types[i].name) == 0)
 				break;
 		}
 		if (!func_types[i].size)
 			break;
-		ret = add_arg(fevent, i);
+		ret = add_arg(fevent, i, unsign);
+		unsign = 0;
 		if (ret < 0)
 			break;
 		return FUNC_STATE_TYPE;
-- 
2.15.1

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

* [PATCH 09/20 v2] tracing: Add indexing of arguments for function based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (7 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 08/20 v2] tracing: Add "unsigned" to function based events Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 10/20 v2] tracing: Make func_type enums for easier comparing of arg types Steven Rostedt
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0009-tracing-Add-indexing-of-arguments-for-function-based.patch --]
[-- Type: text/plain, Size: 4009 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Currently reading of 8 byte words can only happen 8 bytes aligned from the
argument. But there may be cases that they are 4 bytes aligned. To make the
capturing of arguments more flexible, add a plus '+' operator that can index
the variable at arbitrary indexes to get any location.

 u64 arg+4[3]

Will get an 8 byte word at index 28 (3 * 8 + 4)

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/function-based-events.rst | 24 +++++++++++++++++++++++-
 kernel/trace/trace_event_ftrace.c             | 18 ++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
index 72e3e7730d63..bdb28f433bfb 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -100,10 +100,12 @@ as follows:
          'x8' | 'x16' | 'x32' | 'x64' |
          'char' | 'short' | 'int' | 'long' | 'size_t'
 
- FIELD := <name> | <name> INDEX
+ FIELD := <name> | <name> INDEX | <name> OFFSET | <name> OFFSET INDEX
 
  INDEX := '[' <number> ']'
 
+ OFFSET := '+' <number>
+
  Where <name> is a unique string starting with an alphabetic character
  and consists only of letters and numbers and underscores.
 
@@ -221,3 +223,23 @@ format:
 print fmt: "%pS->%pS(skb=%u)", REC->__ip, REC->__parent_ip, REC->skb
 
 It is now printed with a "%u".
+
+
+Offsets
+=======
+
+After the name of the variable, brackets '[' number ']' will index the value of
+the argument by the number given times the size of the field.
+
+ int field[5] will dereference the value of the argument 20 bytes away (4 * 5)
+  as sizeof(int) is 4.
+
+If there's a case where the type is of 8 bytes in size but is not 8 bytes
+alligned in the structure, an offset may be required.
+
+  For example: x64 param+4[2]
+
+The above will take the parameter value, add it by 4, then index it by two
+8 byte words. It's the same in C as: (u64 *)((void *)param + 4)[2]
+
+ Note: "int skb[32]" is the same as "int skb+4[31]".
diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index 8327346799eb..1fa281386fa0 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -19,6 +19,7 @@ struct func_arg {
 	char				*type;
 	char				*name;
 	long				indirect;
+	long				index;
 	short				offset;
 	short				size;
 	s8				arg;
@@ -62,6 +63,7 @@ enum func_states {
 	FUNC_STATE_INDIRECT,
 	FUNC_STATE_UNSIGNED,
 	FUNC_STATE_PIPE,
+	FUNC_STATE_PLUS,
 	FUNC_STATE_TYPE,
 	FUNC_STATE_VAR,
 	FUNC_STATE_COMMA,
@@ -184,6 +186,7 @@ static char *next_token(char **ptr, char *last)
 		    *str == ']' ||
 		    *str == ',' ||
 		    *str == '|' ||
+		    *str == '+' ||
 		    *str == ')')
 			break;
 	}
@@ -325,6 +328,15 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 		}
 		break;
 
+	case FUNC_STATE_PLUS:
+		if (WARN_ON(!fevent->last_arg))
+			break;
+		ret = kstrtol(token, 0, &val);
+		if (ret)
+			break;
+		fevent->last_arg->index += val;
+		return FUNC_STATE_VAR;
+
 	case FUNC_STATE_VAR:
 		switch (token[0]) {
 		case ')':
@@ -333,6 +345,8 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 			return FUNC_STATE_COMMA;
 		case '|':
 			return FUNC_STATE_PIPE;
+		case '+':
+			return FUNC_STATE_PLUS;
 		case '[':
 			return FUNC_STATE_BRACKET;
 		}
@@ -349,6 +363,8 @@ static long long get_arg(struct func_arg *arg, unsigned long val)
 	char buf[8];
 	int ret;
 
+	val += arg->index;
+
 	if (!arg->indirect)
 		return val;
 
@@ -781,6 +797,8 @@ static int func_event_seq_show(struct seq_file *m, void *v)
 		last_arg = arg->arg;
 		comma = true;
 		seq_printf(m, "%s %s", arg->type, arg->name);
+		if (arg->index)
+			seq_printf(m, "+%ld", arg->index);
 		if (arg->indirect && arg->size)
 			seq_printf(m, "[%ld]",
 				   (arg->indirect ^ INDIRECT_FLAG) / arg->size);
-- 
2.15.1

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

* [PATCH 10/20 v2] tracing: Make func_type enums for easier comparing of arg types
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (8 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 09/20 v2] tracing: Add indexing of arguments for " Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 11/20 v2] tracing: Add symbol type to function based events Steven Rostedt
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0010-tracing-Make-func_type-enums-for-easier-comparing-of.patch --]
[-- Type: text/plain, Size: 2338 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

For the function based event args, knowing quickly what type they are is
advantageous, as decisions can be made quickly based on them. Having an
enum for the types is useful for this purpose.

Use macros to create both the func_type array as well as enums that
match the type to the index into that array.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_event_ftrace.c | 47 +++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index 1fa281386fa0..6e48361643c0 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -24,6 +24,7 @@ struct func_arg {
 	short				size;
 	s8				arg;
 	u8				sign;
+	u8				func_type;
 };
 
 struct func_event {
@@ -79,31 +80,42 @@ typedef u8 x8;
 #define TYPE_TUPLE(type)			\
 	{ #type, sizeof(type), is_signed_type(type) }
 
+#define FUNC_TYPES				\
+	TYPE_TUPLE(long),			\
+	TYPE_TUPLE(int),			\
+	TYPE_TUPLE(short),			\
+	TYPE_TUPLE(char),			\
+	TYPE_TUPLE(size_t),			\
+	TYPE_TUPLE(u64),			\
+	TYPE_TUPLE(s64),			\
+	TYPE_TUPLE(x64),			\
+	TYPE_TUPLE(u32),			\
+	TYPE_TUPLE(s32),			\
+	TYPE_TUPLE(x32),			\
+	TYPE_TUPLE(u16),			\
+	TYPE_TUPLE(s16),			\
+	TYPE_TUPLE(x16),			\
+	TYPE_TUPLE(u8),				\
+	TYPE_TUPLE(s8),				\
+	TYPE_TUPLE(x8)
+
 static struct func_type {
 	char		*name;
 	int		size;
 	int		sign;
 } func_types[] = {
-	TYPE_TUPLE(long),
-	TYPE_TUPLE(int),
-	TYPE_TUPLE(short),
-	TYPE_TUPLE(char),
-	TYPE_TUPLE(size_t),
-	TYPE_TUPLE(u64),
-	TYPE_TUPLE(s64),
-	TYPE_TUPLE(x64),
-	TYPE_TUPLE(u32),
-	TYPE_TUPLE(s32),
-	TYPE_TUPLE(x32),
-	TYPE_TUPLE(u16),
-	TYPE_TUPLE(s16),
-	TYPE_TUPLE(x16),
-	TYPE_TUPLE(u8),
-	TYPE_TUPLE(s8),
-	TYPE_TUPLE(x8),
+	FUNC_TYPES,
 	{ NULL,		0,	0 }
 };
 
+#undef TYPE_TUPLE
+#define TYPE_TUPLE(type)	FUNC_TYPE_##type
+
+enum {
+	FUNC_TYPES,
+	FUNC_TYPE_MAX
+};
+
 static int max_args __read_mostly = -1;
 
 /**
@@ -230,6 +242,7 @@ static int add_arg(struct func_event *fevent, int ftype, int unsign)
 	if (!unsign)
 		arg->sign = func_type->sign;
 	arg->offset = ALIGN(fevent->arg_offset, arg->size);
+	arg->func_type = ftype;
 	arg->arg = fevent->arg_cnt;
 	fevent->arg_offset = arg->offset + arg->size;
 
-- 
2.15.1

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

* [PATCH 11/20 v2] tracing: Add symbol type to function based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (9 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 10/20 v2] tracing: Make func_type enums for easier comparing of arg types Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-08 11:20   ` Jiri Olsa
  2018-02-07 20:24 ` [PATCH 12/20 v2] tracing: Add accessing direct address from " Steven Rostedt
                   ` (8 subsequent siblings)
  19 siblings, 1 reply; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0011-tracing-Add-symbol-type-to-function-based-events.patch --]
[-- Type: text/plain, Size: 3834 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Add a special type "symbol" that will use %pS to display the field of a
function based event.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/function-based-events.rst | 26 +++++++++++++++++++++++++-
 kernel/trace/trace_event_ftrace.c             | 13 ++++++++++---
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
index bdb28f433bfb..f18c8f3ef330 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -98,7 +98,8 @@ as follows:
  ATOM := 'u8' | 'u16' | 'u32' | 'u64' |
          's8' | 's16' | 's32' | 's64' |
          'x8' | 'x16' | 'x32' | 'x64' |
-         'char' | 'short' | 'int' | 'long' | 'size_t'
+         'char' | 'short' | 'int' | 'long' | 'size_t' |
+	 'symbol'
 
  FIELD := <name> | <name> INDEX | <name> OFFSET | <name> OFFSET INDEX
 
@@ -243,3 +244,26 @@ The above will take the parameter value, add it by 4, then index it by two
 8 byte words. It's the same in C as: (u64 *)((void *)param + 4)[2]
 
  Note: "int skb[32]" is the same as "int skb+4[31]".
+
+
+Symbols (function names)
+========================
+
+To display kallsyms "%pS" type of output, use the special type "symbol".
+
+Again, using gdb to find the offset of the "func" field of struct work_struct
+
+(gdb) printf "%d\n", &((struct work_struct *)0)->func
+24
+
+ Both "symbol func[3]" and "symbol func+24[0]" will work.
+
+ # echo '__queue_work(int cpu, x64 wq, symbol func[3])' > function_events
+
+ # echo 1 > events/functions/__queue_work/enable
+ # cat trace
+       bash-1641  [007] d..2  6241.171332: queue_work_on->__queue_work(cpu=128, wq=ffff88011a010e00, func=flush_to_ldisc+0x0/0xa0)
+       bash-1641  [007] d..2  6241.171460: queue_work_on->__queue_work(cpu=128, wq=ffff88011a010e00, func=flush_to_ldisc+0x0/0xa0)
+     <idle>-0     [000] dNs3  6241.172004: delayed_work_timer_fn->__queue_work(cpu=128, wq=ffff88011a010800, func=vmstat_shepherd+0x0/0xb0)
+ worker/0:2-1689  [000] d..2  6241.172026: __queue_delayed_work->__queue_work(cpu=7, wq=ffff88011a11da00, func=vmstat_update+0x0/0x70)
+     <idle>-0     [005] d.s3  6241.347996: queue_work_on->__queue_work(cpu=128, wq=ffff88011a011200, func=fb_flashcursor+0x0/0x110 [fb])
diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index 6e48361643c0..1359f714a1b1 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -76,6 +76,7 @@ typedef u64 x64;
 typedef u32 x32;
 typedef u16 x16;
 typedef u8 x8;
+typedef void * symbol;
 
 #define TYPE_TUPLE(type)			\
 	{ #type, sizeof(type), is_signed_type(type) }
@@ -97,7 +98,8 @@ typedef u8 x8;
 	TYPE_TUPLE(x16),			\
 	TYPE_TUPLE(u8),				\
 	TYPE_TUPLE(s8),				\
-	TYPE_TUPLE(x8)
+	TYPE_TUPLE(x8),				\
+	TYPE_TUPLE(symbol)
 
 static struct func_type {
 	char		*name;
@@ -264,7 +266,7 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 	switch (state) {
 	case FUNC_STATE_INIT:
 		unsign = 0;
-		if (!isalpha(token[0]))
+		if (!isalpha(token[0]) && token[0] != '_')
 			break;
 		/* Do not allow wild cards */
 		if (strstr(token, "*") || strstr(token, "?"))
@@ -307,7 +309,7 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 		return FUNC_STATE_TYPE;
 
 	case FUNC_STATE_TYPE:
-		if (!isalpha(token[0]))
+		if (!isalpha(token[0]) || token[0] == '_')
 			break;
 		if (WARN_ON(!fevent->last_arg))
 			break;
@@ -478,6 +480,11 @@ static void make_fmt(struct func_arg *arg, char *fmt)
 {
 	int c = 0;
 
+	if (arg->func_type == FUNC_TYPE_symbol) {
+		strcpy(fmt, "%pS");
+		return;
+	}
+
 	fmt[c++] = '%';
 
 	if (arg->size == 8) {
-- 
2.15.1

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

* [PATCH 12/20 v2] tracing: Add accessing direct address from function based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (10 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 11/20 v2] tracing: Add symbol type to function based events Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 13/20 v2] tracing: Add array type to " Steven Rostedt
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0012-tracing-Add-accessing-direct-address-from-function-b.patch --]
[-- Type: text/plain, Size: 10276 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Allow referencing any address during the function based event. The syntax is
to use <type> <name>=<addr> For example:

 # echo 'do_IRQ(long total_forks=0xffffffffa2a4b4c0)' > function_events
 # echo 1 > events/function/enable
 # cat trace
            sshd-832   [000] d... 221639.210845: ret_from_intr->do_IRQ(total_forks=855)
            sshd-832   [000] d... 221639.211114: ret_from_intr->do_IRQ(total_forks=855)
          <idle>-0     [000] d... 221639.211198: ret_from_intr->do_IRQ(total_forks=855)

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/function-based-events.rst |  40 +++++++-
 kernel/trace/trace_event_ftrace.c             | 129 +++++++++++++++++++++-----
 2 files changed, 143 insertions(+), 26 deletions(-)

diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
index f18c8f3ef330..b0e6725f3032 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -91,7 +91,7 @@ as follows:
 
  ARGS := ARG | ARG ',' ARGS | ''
 
- ARG := TYPE FIELD | ARG '|' ARG
+ ARG := TYPE FIELD | TYPE <name> '=' ADDR | TYPE ADDR | ARG '|' ARG
 
  TYPE := ATOM | 'unsigned' ATOM
 
@@ -107,6 +107,8 @@ as follows:
 
  OFFSET := '+' <number>
 
+ ADDR := A hexidecimal address starting with '0x'
+
  Where <name> is a unique string starting with an alphabetic character
  and consists only of letters and numbers and underscores.
 
@@ -267,3 +269,39 @@ Again, using gdb to find the offset of the "func" field of struct work_struct
      <idle>-0     [000] dNs3  6241.172004: delayed_work_timer_fn->__queue_work(cpu=128, wq=ffff88011a010800, func=vmstat_shepherd+0x0/0xb0)
  worker/0:2-1689  [000] d..2  6241.172026: __queue_delayed_work->__queue_work(cpu=7, wq=ffff88011a11da00, func=vmstat_update+0x0/0x70)
      <idle>-0     [005] d.s3  6241.347996: queue_work_on->__queue_work(cpu=128, wq=ffff88011a011200, func=fb_flashcursor+0x0/0x110 [fb])
+
+
+Direct memory access
+====================
+
+Function arguments are not the only thing that can be recorded from a function
+based event. Memory addresses can also be examined. If there's a global variable
+that you want to monitor via an interrupt, you can put in the address directly.
+
+  # grep total_forks /proc/kallsyms
+ffffffff82354c18 B total_forks
+
+  # echo 'do_IRQ(int total_forks=0xffffffff82354c18)' > function_events
+
+  # echo 1 events/functions/do_IRQ/enable
+  # cat trace
+    <idle>-0     [003] d..3   337.076709: ret_from_intr->do_IRQ(total_forks=1419)
+    <idle>-0     [003] d..3   337.077046: ret_from_intr->do_IRQ(total_forks=1419)
+    <idle>-0     [003] d..3   337.077076: ret_from_intr->do_IRQ(total_forks=1420)
+
+Note, address notations do not affect the argument count. For instance, with
+
+__visible unsigned int __irq_entry do_IRQ(struct pt_regs *regs)
+
+  # echo 'do_IRQ(int total_forks=0xffffffff82354c18, symbol regs[16])' > function_events
+
+Is the same as
+
+  # echo 'do_IRQ(int total_forks=0xffffffff82354c18 | symbol regs[16])' > function_events
+
+  # cat trace
+    <idle>-0     [003] d..3   653.839546: ret_from_intr->do_IRQ(total_forks=1504, regs=cpuidle_enter_state+0xb1/0x330)
+    <idle>-0     [003] d..3   653.906011: ret_from_intr->do_IRQ(total_forks=1504, regs=cpuidle_enter_state+0xb1/0x330)
+    <idle>-0     [003] d..3   655.823498: ret_from_intr->do_IRQ(total_forks=1504, regs=tick_nohz_idle_enter+0x4c/0x50)
+    <idle>-0     [003] d..3   655.954096: ret_from_intr->do_IRQ(total_forks=1504, regs=cpuidle_enter_state+0xb1/0x330)
+
diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index 1359f714a1b1..97748edc8bcb 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -63,6 +63,8 @@ enum func_states {
 	FUNC_STATE_BRACKET_END,
 	FUNC_STATE_INDIRECT,
 	FUNC_STATE_UNSIGNED,
+	FUNC_STATE_ADDR,
+	FUNC_STATE_EQUAL,
 	FUNC_STATE_PIPE,
 	FUNC_STATE_PLUS,
 	FUNC_STATE_TYPE,
@@ -201,6 +203,7 @@ static char *next_token(char **ptr, char *last)
 		    *str == ',' ||
 		    *str == '|' ||
 		    *str == '+' ||
+		    *str == '=' ||
 		    *str == ')')
 			break;
 	}
@@ -245,12 +248,39 @@ static int add_arg(struct func_event *fevent, int ftype, int unsign)
 		arg->sign = func_type->sign;
 	arg->offset = ALIGN(fevent->arg_offset, arg->size);
 	arg->func_type = ftype;
-	arg->arg = fevent->arg_cnt;
 	fevent->arg_offset = arg->offset + arg->size;
 
 	list_add_tail(&arg->list, &fevent->args);
 	fevent->last_arg = arg;
-	fevent->arg_cnt++;
+
+	return 0;
+}
+
+static int update_arg_name(struct func_event *fevent, const char *name)
+{
+	struct func_arg *arg = fevent->last_arg;
+
+	if (WARN_ON(!arg))
+		return -EINVAL;
+
+	arg->name = kstrdup(name, GFP_KERNEL);
+	if (!arg->name)
+		return -ENOMEM;
+	return 0;
+}
+
+static int update_arg_arg(struct func_event *fevent)
+{
+	struct func_arg *arg = fevent->last_arg;
+
+	if (WARN_ON(!arg))
+		return -EINVAL;
+
+	/* Make sure the arch can support this many args */
+	if (fevent->arg_cnt >= max_args)
+		return -EINVAL;
+
+	arg->arg = fevent->arg_cnt;
 
 	return 0;
 }
@@ -258,14 +288,16 @@ static int add_arg(struct func_event *fevent, int ftype, int unsign)
 static enum func_states
 process_event(struct func_event *fevent, const char *token, enum func_states state)
 {
+	static bool update_arg;
 	static int unsign;
-	long val;
+	unsigned long val;
 	int ret;
 	int i;
 
 	switch (state) {
 	case FUNC_STATE_INIT:
 		unsign = 0;
+		update_arg = false;
 		if (!isalpha(token[0]) && token[0] != '_')
 			break;
 		/* Do not allow wild cards */
@@ -281,15 +313,15 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 			break;
 		return FUNC_STATE_PARAM;
 
-	case FUNC_STATE_PIPE:
-		fevent->arg_cnt--;
-		goto comma;
 	case FUNC_STATE_PARAM:
 		if (token[0] == ')')
 			return FUNC_STATE_END;
 		/* Fall through */
 	case FUNC_STATE_COMMA:
- comma:
+		if (update_arg)
+			fevent->arg_cnt++;
+		update_arg = false;
+	case FUNC_STATE_PIPE:
 		if (strcmp(token, "unsigned") == 0) {
 			unsign = 2;
 			return FUNC_STATE_UNSIGNED;
@@ -309,18 +341,20 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 		return FUNC_STATE_TYPE;
 
 	case FUNC_STATE_TYPE:
-		if (!isalpha(token[0]) || token[0] == '_')
-			break;
 		if (WARN_ON(!fevent->last_arg))
 			break;
-		fevent->last_arg->name = kstrdup(token, GFP_KERNEL);
-		if (!fevent->last_arg->name)
+		if (update_arg_name(fevent, token) < 0)
+			break;
+		if (strncmp(token, "0x", 2) == 0)
+			goto equal;
+		if (!isalpha(token[0]) && token[0] != '_')
 			break;
+		update_arg = true;
 		return FUNC_STATE_VAR;
 
 	case FUNC_STATE_BRACKET:
 		WARN_ON(!fevent->last_arg);
-		ret = kstrtol(token, 0, &val);
+		ret = kstrtoul(token, 0, &val);
 		if (ret)
 			break;
 		val *= fevent->last_arg->size;
@@ -335,7 +369,7 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 	case FUNC_STATE_BRACKET_END:
 		switch (token[0]) {
 		case ')':
-			return FUNC_STATE_END;
+			goto end;
 		case ',':
 			return FUNC_STATE_COMMA;
 		case '|':
@@ -346,16 +380,33 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 	case FUNC_STATE_PLUS:
 		if (WARN_ON(!fevent->last_arg))
 			break;
-		ret = kstrtol(token, 0, &val);
+		ret = kstrtoul(token, 0, &val);
 		if (ret)
 			break;
 		fevent->last_arg->index += val;
 		return FUNC_STATE_VAR;
 
+	case FUNC_STATE_ADDR:
+		switch (token[0]) {
+		case ')':
+			goto end;
+		case ',':
+			return FUNC_STATE_COMMA;
+		case '|':
+			return FUNC_STATE_PIPE;
+		}
+		break;
+
 	case FUNC_STATE_VAR:
+		if (token[0] == '=')
+			return FUNC_STATE_EQUAL;
+		if (WARN_ON(!fevent->last_arg))
+			break;
+		update_arg_arg(fevent);
+		update_arg = true;
 		switch (token[0]) {
 		case ')':
-			return FUNC_STATE_END;
+			goto end;
 		case ',':
 			return FUNC_STATE_COMMA;
 		case '|':
@@ -367,10 +418,29 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 		}
 		break;
 
+	case FUNC_STATE_EQUAL:
+		if (strncmp(token, "0x", 2) != 0)
+			break;
+ equal:
+		if (WARN_ON(!fevent->last_arg))
+			break;
+		ret = kstrtoul(token, 0, &val);
+		if (ret < 0)
+			break;
+		update_arg = false;
+		fevent->last_arg->index = val;
+		fevent->last_arg->arg = -1;
+		fevent->last_arg->indirect = INDIRECT_FLAG;
+		return FUNC_STATE_ADDR;
+
 	default:
 		break;
 	}
 	return FUNC_STATE_ERROR;
+ end:
+	if (update_arg)
+		fevent->arg_cnt++;
+	return FUNC_STATE_END;
 }
 
 static long long get_arg(struct func_arg *arg, unsigned long val)
@@ -419,7 +489,7 @@ static void func_event_trace(struct trace_event_file *trace_file,
 	long args[func_event->arg_cnt];
 	long long val = 1;
 	unsigned long irq_flags;
-	int nr_args;
+	int nr_args = 0;
 	int size;
 	int pc;
 
@@ -440,12 +510,17 @@ static void func_event_trace(struct trace_event_file *trace_file,
 	entry = ring_buffer_event_data(event);
 	entry->ip = ip;
 	entry->parent_ip = parent_ip;
-	nr_args = arch_get_func_args(pt_regs, 0, func_event->arg_cnt, args);
+	if (func_event->arg_cnt)
+		nr_args = arch_get_func_args(pt_regs, 0, func_event->arg_cnt, args);
 
 	list_for_each_entry(arg, &func_event->args, list) {
-		if (arg->arg < nr_args)
-			val = get_arg(arg, args[arg->arg]);
-		else
+		if (arg->arg < nr_args) {
+			/* Is arg an address and not a parameter? */
+			if (arg->arg < 0)
+				val = get_arg(arg, 0);
+			else
+				val = get_arg(arg, args[arg->arg]);
+		} else
 			val = 0;
 		memcpy(&entry->data[arg->offset], &val, arg->size);
 	}
@@ -817,11 +892,15 @@ static int func_event_seq_show(struct seq_file *m, void *v)
 		last_arg = arg->arg;
 		comma = true;
 		seq_printf(m, "%s %s", arg->type, arg->name);
-		if (arg->index)
-			seq_printf(m, "+%ld", arg->index);
-		if (arg->indirect && arg->size)
-			seq_printf(m, "[%ld]",
-				   (arg->indirect ^ INDIRECT_FLAG) / arg->size);
+		if (arg->arg < 0) {
+			seq_printf(m, "=0x%lx", arg->index);
+		} else {
+			if (arg->index)
+				seq_printf(m, "+%ld", arg->index);
+			if (arg->indirect && arg->size)
+				seq_printf(m, "[%ld]",
+					   (arg->indirect ^ INDIRECT_FLAG) / arg->size);
+		}
 	}
 	seq_puts(m, ")\n");
 
-- 
2.15.1

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

* [PATCH 13/20 v2] tracing: Add array type to function based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (11 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 12/20 v2] tracing: Add accessing direct address from " Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 14/20 v2] tracing: Have char arrays be strings for " Steven Rostedt
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0013-tracing-Add-array-type-to-function-based-events.patch --]
[-- Type: text/plain, Size: 9928 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Add syntex to allow the user to create an array type. Brackets after the
type field will denote that this is an array type. For example:

 # echo 'SyS_open(x8[32] buf, x32 flags, x32 mode)' > function_events

Will make the first argument of the sys_open function call an array of
32 bytes.

The array type can also be used in conjunction with the indirect offset
brackets as well. For example to get the interrupt stack of regs in do_IRQ()
for x86_64.

 # echo 'do_IRQ(x64[5] regs[16])' > function_events

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/function-based-events.rst |  22 +++-
 kernel/trace/trace_event_ftrace.c             | 157 +++++++++++++++++++++-----
 2 files changed, 151 insertions(+), 28 deletions(-)

diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
index b0e6725f3032..4a8a6fb16a0a 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -93,7 +93,7 @@ as follows:
 
  ARG := TYPE FIELD | TYPE <name> '=' ADDR | TYPE ADDR | ARG '|' ARG
 
- TYPE := ATOM | 'unsigned' ATOM
+ TYPE := ATOM | ATOM '[' <number> ']' | 'unsigned' TYPE
 
  ATOM := 'u8' | 'u16' | 'u32' | 'u64' |
          's8' | 's16' | 's32' | 's64' |
@@ -305,3 +305,23 @@ Is the same as
     <idle>-0     [003] d..3   655.823498: ret_from_intr->do_IRQ(total_forks=1504, regs=tick_nohz_idle_enter+0x4c/0x50)
     <idle>-0     [003] d..3   655.954096: ret_from_intr->do_IRQ(total_forks=1504, regs=cpuidle_enter_state+0xb1/0x330)
 
+
+Array types
+===========
+
+If there's a case where you want to see an array of a type, then you can
+declare a type as an array by adding '[' number ']' after the type.
+
+To get the net_device perm_addr, from the dev parameter.
+
+ (gdb) printf "%d\n", &((struct net_device *)0)->perm_addr
+558
+
+ # echo 'ip_rcv(x64 skb, x8[6] perm_addr+558)' > function_events
+
+ # echo 1 > events/functions/ip_rcv/enable
+ # cat trace
+    <idle>-0     [003] ..s3   219.813582: __netif_receive_skb_core->ip_rcv(skb=ffff880118195e00, perm_addr=b4,b5,2f,ce,18,65)
+    <idle>-0     [003] ..s3   219.813595: __netif_receive_skb_core->ip_rcv(skb=ffff880118195e00, perm_addr=b4,b5,2f,ce,18,65)
+    <idle>-0     [003] ..s3   220.115053: __netif_receive_skb_core->ip_rcv(skb=ffff880118195c00, perm_addr=b4,b5,2f,ce,18,65)
+    <idle>-0     [003] ..s3   220.115293: __netif_receive_skb_core->ip_rcv(skb=ffff880118195c00, perm_addr=b4,b5,2f,ce,18,65)
diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index 97748edc8bcb..f62bd6a2ea87 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -20,6 +20,7 @@ struct func_arg {
 	char				*name;
 	long				indirect;
 	long				index;
+	short				array;
 	short				offset;
 	short				size;
 	s8				arg;
@@ -68,6 +69,9 @@ enum func_states {
 	FUNC_STATE_PIPE,
 	FUNC_STATE_PLUS,
 	FUNC_STATE_TYPE,
+	FUNC_STATE_ARRAY,
+	FUNC_STATE_ARRAY_SIZE,
+	FUNC_STATE_ARRAY_END,
 	FUNC_STATE_VAR,
 	FUNC_STATE_COMMA,
 	FUNC_STATE_END,
@@ -291,6 +295,7 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 	static bool update_arg;
 	static int unsign;
 	unsigned long val;
+	char *type;
 	int ret;
 	int i;
 
@@ -341,6 +346,10 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 		return FUNC_STATE_TYPE;
 
 	case FUNC_STATE_TYPE:
+		if (token[0] == '[')
+			return FUNC_STATE_ARRAY;
+		/* Fall through */
+	case FUNC_STATE_ARRAY_END:
 		if (WARN_ON(!fevent->last_arg))
 			break;
 		if (update_arg_name(fevent, token) < 0)
@@ -352,14 +361,37 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 		update_arg = true;
 		return FUNC_STATE_VAR;
 
+	case FUNC_STATE_ARRAY:
 	case FUNC_STATE_BRACKET:
-		WARN_ON(!fevent->last_arg);
+		if (WARN_ON(!fevent->last_arg))
+			break;
 		ret = kstrtoul(token, 0, &val);
 		if (ret)
 			break;
-		val *= fevent->last_arg->size;
-		fevent->last_arg->indirect = val ^ INDIRECT_FLAG;
-		return FUNC_STATE_INDIRECT;
+		if (state == FUNC_STATE_BRACKET) {
+			val *= fevent->last_arg->size;
+			fevent->last_arg->indirect = val ^ INDIRECT_FLAG;
+			return FUNC_STATE_INDIRECT;
+		}
+		if (val <= 0)
+			break;
+		fevent->last_arg->array = val;
+		type = kasprintf(GFP_KERNEL, "%s[%d]", fevent->last_arg->type, (unsigned)val);
+		if (!type)
+			break;
+		kfree(fevent->last_arg->type);
+		fevent->last_arg->type = type;
+		/*
+		 * arg_offset has already been updated once by size.
+		 * This update needs to account for that (hence the "- 1").
+		 */
+		fevent->arg_offset += fevent->last_arg->size * (fevent->last_arg->array - 1);
+		return FUNC_STATE_ARRAY_SIZE;
+
+	case FUNC_STATE_ARRAY_SIZE:
+		if (token[0] != ']')
+			break;
+		return FUNC_STATE_ARRAY_END;
 
 	case FUNC_STATE_INDIRECT:
 		if (token[0] != ']')
@@ -455,6 +487,10 @@ static long long get_arg(struct func_arg *arg, unsigned long val)
 
 	val = val + (arg->indirect ^ INDIRECT_FLAG);
 
+	/* Arrays do their own indirect reads */
+	if (arg->array)
+		return val;
+
 	ret = probe_kernel_read(buf, (void *)val, arg->size);
 	if (ret)
 		return 0;
@@ -476,6 +512,21 @@ static long long get_arg(struct func_arg *arg, unsigned long val)
 	return val;
 }
 
+static void get_array(void *dst, struct func_arg *arg, unsigned long val)
+{
+	void *ptr = (void *)val;
+	int ret;
+	int i;
+
+	for (i = 0; i < arg->array; i++) {
+		ret = probe_kernel_read(dst, ptr, arg->size);
+		if (ret)
+			memset(dst, 0, arg->size);
+		ptr += arg->size;
+		dst += arg->size;
+	}
+}
+
 static void func_event_trace(struct trace_event_file *trace_file,
 			     struct func_event *func_event,
 			     unsigned long ip, unsigned long parent_ip,
@@ -522,7 +573,10 @@ static void func_event_trace(struct trace_event_file *trace_file,
 				val = get_arg(arg, args[arg->arg]);
 		} else
 			val = 0;
-		memcpy(&entry->data[arg->offset], &val, arg->size);
+		if (arg->array)
+			get_array(&entry->data[arg->offset], arg, val);
+		else
+			memcpy(&entry->data[arg->offset], &val, arg->size);
 	}
 
 	event_trigger_unlock_commit_regs(trace_file, buffer, event,
@@ -577,6 +631,25 @@ static void make_fmt(struct func_arg *arg, char *fmt)
 	fmt[c++] = '\0';
 }
 
+static void write_data(struct trace_seq *s, const struct func_arg *arg, const char *fmt,
+		       const void *data)
+{
+	switch (arg->size) {
+	case 8:
+		trace_seq_printf(s, fmt, *(unsigned long long *)data);
+		break;
+	case 4:
+		trace_seq_printf(s, fmt, *(unsigned *)data);
+		break;
+	case 2:
+		trace_seq_printf(s, fmt, *(unsigned short *)data);
+		break;
+	case 1:
+		trace_seq_printf(s, fmt, *(unsigned char *)data);
+		break;
+	}
+}
+
 static enum print_line_t
 func_event_print(struct trace_iterator *iter, int flags,
 		 struct trace_event *event)
@@ -588,6 +661,7 @@ func_event_print(struct trace_iterator *iter, int flags,
 	char fmt[FMT_SIZE];
 	void *data;
 	bool comma = false;
+	int a;
 
 	entry = (struct func_event_hdr *)iter->ent;
 
@@ -604,20 +678,16 @@ func_event_print(struct trace_iterator *iter, int flags,
 
 		make_fmt(arg, fmt);
 
-		switch (arg->size) {
-		case 8:
-			trace_seq_printf(s, fmt, *(unsigned long long *)data);
-			break;
-		case 4:
-			trace_seq_printf(s, fmt, *(unsigned *)data);
-			break;
-		case 2:
-			trace_seq_printf(s, fmt, *(unsigned short *)data);
-			break;
-		case 1:
-			trace_seq_printf(s, fmt, *(unsigned char *)data);
-			break;
-		}
+		if (arg->array) {
+			comma = false;
+			for (a = 0; a < arg->array; a++, data += arg->size) {
+				if (comma)
+					trace_seq_putc(s, ',');
+				comma = true;
+				write_data(s, arg, fmt, data);
+			}
+		} else
+			write_data(s, arg, fmt, data);
 	}
 	trace_seq_puts(s, ")\n");
 	return trace_handle_return(s);
@@ -640,11 +710,14 @@ static int func_event_define_fields(struct trace_event_call *event_call)
 	DEFINE_FIELD(unsigned long, parent_ip, "__ip", 0);
 
 	list_for_each_entry(arg, &fevent->args, list) {
+		int size = arg->size;
+
+		if (arg->array)
+			size *= arg->array;
 		ret = trace_define_field(event_call, arg->type,
 					 arg->name,
 					 sizeof(field) + arg->offset,
-					 arg->size, arg->sign,
-					 FILTER_OTHER);
+					 size, arg->sign, FILTER_OTHER);
 		if (ret < 0)
 			return ret;
 	}
@@ -735,7 +808,7 @@ static int __set_print_fmt(struct func_event *func_event,
 	const char *fmt_start = "\"%pS->%pS(";
 	const char *fmt_end = ")\", REC->__ip, REC->__parent_ip";
 	char fmt[FMT_SIZE];
-	int r, i;
+	int r, i, a;
 	bool comma = false;
 
 	r = snprintf(buf, len, "%s", fmt_start);
@@ -747,19 +820,49 @@ static int __set_print_fmt(struct func_event *func_event,
 			len = update_len(len, i);
 		}
 		comma = true;
-		make_fmt(arg, fmt);
-		i = snprintf(buf + r, len, "%s=%s", arg->name, fmt);
+
+		i = snprintf(buf + r, len, "%s=", arg->name);
 		r += i;
 		len = update_len(len, i);
+
+		make_fmt(arg, fmt);
+
+		if (arg->array) {
+			bool colon = false;
+
+			for (a = 0; a < arg->array; a++) {
+				if (colon) {
+					i = snprintf(buf + r, len, ":");
+					r += i;
+					len = update_len(len, i);
+				}
+				colon = true;
+				i = snprintf(buf + r, len, "%s", fmt);
+				r += i;
+				len = update_len(len, i);
+			}
+		} else {
+			i = snprintf(buf + r, len, "%s", fmt);
+			r += i;
+			len = update_len(len, i);
+		}
 	}
 	i = snprintf(buf + r, len, "%s", fmt_end);
 	r += i;
 	len = update_len(len, i);
 
 	list_for_each_entry(arg, &func_event->args, list) {
-		i = snprintf(buf + r, len, ", REC->%s", arg->name);
-		r += i;
-		len = update_len(len, i);
+		if (arg->array) {
+			for (a = 0; a < arg->array; a++) {
+				i = snprintf(buf + r, len, ", REC->%s[%d]", arg->name, a);
+				r += i;
+				len = update_len(len, i);
+			}
+		} else {
+			i = snprintf(buf + r, len, ", REC->%s", arg->name);
+			r += i;
+			len = update_len(len, i);
+		}
 	}
 
 	return r;
-- 
2.15.1

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

* [PATCH 14/20 v2] tracing: Have char arrays be strings for function based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (12 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 13/20 v2] tracing: Add array type to " Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 15/20 v2] tracing: Add string type for dynamic strings in " Steven Rostedt
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0014-tracing-Have-char-arrays-be-strings-for-function-bas.patch --]
[-- Type: text/plain, Size: 5070 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

If a field in a function based event is defined with type "char[##]" then it
will be considered a static string. If a user wants an actual byte array
they should use one of u8, s8, or x8.

Now we can get strings from events:

 # echo 'SyS_openat(int dfd, char[64] buf, x32 flags, x32 mode)' > function_events
 # grep xxx /etc/*
 # cat trace
  grep-1745  [001] .... 346135.431364: entry_SYSCALL_64_fastpath->SyS_openat(dfd=-100, buf=/etc/adjtime, flags=100, mode=0)
  grep-1745  [001] .... 346135.431734: entry_SYSCALL_64_fastpath->SyS_openat(dfd=-100, buf=/etc/aliases, flags=100, mode=0)
  grep-1745  [001] .... 346135.618765: entry_SYSCALL_64_fastpath->SyS_openat(dfd=-100, buf=/etc/alternatives, flags=100, mode=0)
  grep-1745  [001] .... 346135.619063: entry_SYSCALL_64_fastpath->SyS_openat(dfd=-100, buf=/etc/anacrontab, flags=100, mode=0)
  grep-1745  [001] .... 346135.619134: entry_SYSCALL_64_fastpath->SyS_openat(dfd=-100, buf=/etc/asciidoc, flags=100, mode=0)
  grep-1745  [001] .... 346135.619390: entry_SYSCALL_64_fastpath->SyS_openat(dfd=-100, buf=/etc/asound.conf, flags=100, mode=0)
  grep-1745  [001] .... 346135.624350: entry_SYSCALL_64_fastpath->SyS_openat(dfd=-100, buf=/etc/audisp, flags=100, mode=0)
  grep-1745  [001] .... 346135.624565: entry_SYSCALL_64_fastpath->SyS_openat(dfd=-100, buf=/etc/audit, flags=100, mode=0)

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/function-based-events.rst | 17 +++++++++++++++++
 kernel/trace/trace_event_ftrace.c             | 21 +++++++++++++++++----
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
index 4a8a6fb16a0a..99ae77cd59e6 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -325,3 +325,20 @@ To get the net_device perm_addr, from the dev parameter.
     <idle>-0     [003] ..s3   219.813595: __netif_receive_skb_core->ip_rcv(skb=ffff880118195e00, perm_addr=b4,b5,2f,ce,18,65)
     <idle>-0     [003] ..s3   220.115053: __netif_receive_skb_core->ip_rcv(skb=ffff880118195c00, perm_addr=b4,b5,2f,ce,18,65)
     <idle>-0     [003] ..s3   220.115293: __netif_receive_skb_core->ip_rcv(skb=ffff880118195c00, perm_addr=b4,b5,2f,ce,18,65)
+
+
+Static strings
+==============
+
+An array of type 'char' or 'unsigned char' will be processed as a string using
+the format "%s". If a nul is found, the output will stop. Use another type
+(x8, u8, s8) if this is not desired.
+
+  # echo 'link_path_walk(char[64] name)' > function_events
+
+  # echo 1 > events/functions/link_path_walk/enable
+  # cat trace
+      bash-1470  [003] ...2   980.678664: path_openat->link_path_walk(name=/usr/bin/cat)
+      bash-1470  [003] ...2   980.678715: path_openat->link_path_walk(name=/lib64/ld-linux-x86-64.so.2)
+      bash-1470  [003] ...2   980.678721: path_openat->link_path_walk(name=ld-2.24.so)
+      bash-1470  [003] ...2   980.678978: path_lookupat->link_path_walk(name=/etc/ld.so.preload)
diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index f62bd6a2ea87..aa390339b571 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -616,6 +616,14 @@ static void make_fmt(struct func_arg *arg, char *fmt)
 
 	fmt[c++] = '%';
 
+	if (arg->func_type == FUNC_TYPE_char) {
+		if (arg->array)
+			fmt[c++] = 's';
+		else
+			fmt[c++] = 'c';
+		goto out;
+	}
+
 	if (arg->size == 8) {
 		fmt[c++] = 'l';
 		fmt[c++] = 'l';
@@ -628,6 +636,7 @@ static void make_fmt(struct func_arg *arg, char *fmt)
 	else
 		fmt[c++] = 'u';
 
+ out:
 	fmt[c++] = '\0';
 }
 
@@ -645,7 +654,10 @@ static void write_data(struct trace_seq *s, const struct func_arg *arg, const ch
 		trace_seq_printf(s, fmt, *(unsigned short *)data);
 		break;
 	case 1:
-		trace_seq_printf(s, fmt, *(unsigned char *)data);
+		if (arg->array && arg->func_type == FUNC_TYPE_char)
+			trace_seq_printf(s, fmt, (char *)data);
+		else
+			trace_seq_printf(s, fmt, *(unsigned char *)data);
 		break;
 	}
 }
@@ -678,7 +690,7 @@ func_event_print(struct trace_iterator *iter, int flags,
 
 		make_fmt(arg, fmt);
 
-		if (arg->array) {
+		if (arg->array && arg->func_type != FUNC_TYPE_char) {
 			comma = false;
 			for (a = 0; a < arg->array; a++, data += arg->size) {
 				if (comma)
@@ -827,7 +839,7 @@ static int __set_print_fmt(struct func_event *func_event,
 
 		make_fmt(arg, fmt);
 
-		if (arg->array) {
+		if (arg->array && arg->func_type != FUNC_TYPE_char) {
 			bool colon = false;
 
 			for (a = 0; a < arg->array; a++) {
@@ -852,7 +864,8 @@ static int __set_print_fmt(struct func_event *func_event,
 	len = update_len(len, i);
 
 	list_for_each_entry(arg, &func_event->args, list) {
-		if (arg->array) {
+		/* Don't iterate for strings */
+		if (arg->array && arg->func_type != FUNC_TYPE_char) {
 			for (a = 0; a < arg->array; a++) {
 				i = snprintf(buf + r, len, ", REC->%s[%d]", arg->name, a);
 				r += i;
-- 
2.15.1

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

* [PATCH 15/20 v2] tracing: Add string type for dynamic strings in function based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (13 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 14/20 v2] tracing: Have char arrays be strings for " Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 16/20 v2] tracing: Add NULL to skip args for " Steven Rostedt
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0015-tracing-Add-string-type-for-dynamic-strings-in-funct.patch --]
[-- Type: text/plain, Size: 10665 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Add a "string" type that will create a dynamic length string for the
event, this is the same as the __string() field in normal TRACE_EVENTS.

[ missing 'static' found by Fengguang Wu's kbuild test robot ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/function-based-events.rst |  19 ++-
 kernel/trace/trace_event_ftrace.c             | 183 +++++++++++++++++++++++---
 2 files changed, 181 insertions(+), 21 deletions(-)

diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
index 99ae77cd59e6..6c643ea749e7 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -99,7 +99,7 @@ as follows:
          's8' | 's16' | 's32' | 's64' |
          'x8' | 'x16' | 'x32' | 'x64' |
          'char' | 'short' | 'int' | 'long' | 'size_t' |
-	 'symbol'
+	 'symbol' | 'string'
 
  FIELD := <name> | <name> INDEX | <name> OFFSET | <name> OFFSET INDEX
 
@@ -342,3 +342,20 @@ the format "%s". If a nul is found, the output will stop. Use another type
       bash-1470  [003] ...2   980.678715: path_openat->link_path_walk(name=/lib64/ld-linux-x86-64.so.2)
       bash-1470  [003] ...2   980.678721: path_openat->link_path_walk(name=ld-2.24.so)
       bash-1470  [003] ...2   980.678978: path_lookupat->link_path_walk(name=/etc/ld.so.preload)
+
+
+Dynamic strings
+===============
+
+Static strings are fine, but they can waste a lot of memory in the ring buffer.
+The above allocated 64 bytes for a character array, but most of the output was
+less than 20 characters. Not wanting to truncate strings or waste space on
+the ring buffer, the dynamic string can help.
+
+Use the "string" type for strings that have a large range in size. The max
+size that will be recorded is 512 bytes. If a string is larger than that, then
+it will be truncated.
+
+ # echo 'link_path_walk(string name)' > function_events
+
+Gives the same result as above, but does not waste buffer space.
diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index aa390339b571..50d0c4d32596 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -39,6 +39,7 @@ struct func_event {
 	struct func_arg			*last_arg;
 	int				arg_cnt;
 	int				arg_offset;
+	int				has_strings;
 };
 
 struct func_file {
@@ -83,6 +84,8 @@ typedef u32 x32;
 typedef u16 x16;
 typedef u8 x8;
 typedef void * symbol;
+/* 2 byte offset, 2 byte length */
+typedef u32 string;
 
 #define TYPE_TUPLE(type)			\
 	{ #type, sizeof(type), is_signed_type(type) }
@@ -105,7 +108,8 @@ typedef void * symbol;
 	TYPE_TUPLE(u8),				\
 	TYPE_TUPLE(s8),				\
 	TYPE_TUPLE(x8),				\
-	TYPE_TUPLE(symbol)
+	TYPE_TUPLE(symbol),			\
+	TYPE_TUPLE(string)
 
 static struct func_type {
 	char		*name;
@@ -124,6 +128,16 @@ enum {
 	FUNC_TYPE_MAX
 };
 
+#define MAX_STR		512
+
+/* Two contexts, normal and NMI, hence the " * 2" */
+struct func_string {
+	char		buf[MAX_STR * 2];
+};
+
+static struct func_string __percpu *str_buffer;
+static int nr_strings;
+
 static int max_args __read_mostly = -1;
 
 /**
@@ -165,6 +179,23 @@ int __weak arch_get_func_args(struct pt_regs *regs,
 	return 0;
 }
 
+static void free_arg(struct func_arg *arg)
+{
+	list_del(&arg->list);
+	if (arg->func_type == FUNC_TYPE_string) {
+		nr_strings--;
+		if (WARN_ON(nr_strings < 0))
+			nr_strings = 0;
+		if (!nr_strings) {
+			free_percpu(str_buffer);
+			str_buffer = NULL;
+		}
+	}
+	kfree(arg->name);
+	kfree(arg->type);
+	kfree(arg);
+}
+
 static void free_func_event(struct func_event *func_event)
 {
 	struct func_arg *arg, *n;
@@ -173,10 +204,7 @@ static void free_func_event(struct func_event *func_event)
 		return;
 
 	list_for_each_entry_safe(arg, n, &func_event->args, list) {
-		list_del(&arg->list);
-		kfree(arg->name);
-		kfree(arg->type);
-		kfree(arg);
+		free_arg(arg);
 	}
 	ftrace_free_filter(&func_event->ops);
 	kfree(func_event->call.print_fmt);
@@ -257,6 +285,17 @@ static int add_arg(struct func_event *fevent, int ftype, int unsign)
 	list_add_tail(&arg->list, &fevent->args);
 	fevent->last_arg = arg;
 
+	if (ftype == FUNC_TYPE_string) {
+		fevent->has_strings++;
+		nr_strings++;
+		if (nr_strings == 1) {
+			str_buffer = alloc_percpu(struct func_string);
+			if (!str_buffer) {
+				free_arg(arg);
+				return -ENOMEM;
+			}
+		}
+	}
 	return 0;
 }
 
@@ -346,8 +385,19 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 		return FUNC_STATE_TYPE;
 
 	case FUNC_STATE_TYPE:
-		if (token[0] == '[')
+		if (token[0] == '[') {
+			/* Strings are already arrays */
+			if (fevent->last_arg->func_type == FUNC_TYPE_string)
+				break;
 			return FUNC_STATE_ARRAY;
+		}
+		if (fevent->last_arg->func_type == FUNC_TYPE_string) {
+			type = kstrdup("__data_loc char[]", GFP_KERNEL);
+			if (!type)
+				break;
+			kfree(fevent->last_arg->type);
+			fevent->last_arg->type = type;
+		}
 		/* Fall through */
 	case FUNC_STATE_ARRAY_END:
 		if (WARN_ON(!fevent->last_arg))
@@ -475,7 +525,7 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 	return FUNC_STATE_END;
 }
 
-static long long get_arg(struct func_arg *arg, unsigned long val)
+static long long __get_arg(struct func_arg *arg, unsigned long val)
 {
 	char buf[8];
 	int ret;
@@ -487,8 +537,8 @@ static long long get_arg(struct func_arg *arg, unsigned long val)
 
 	val = val + (arg->indirect ^ INDIRECT_FLAG);
 
-	/* Arrays do their own indirect reads */
-	if (arg->array)
+	/* Arrays and strings do their own indirect reads */
+	if (arg->array || arg->func_type == FUNC_TYPE_string)
 		return val;
 
 	ret = probe_kernel_read(buf, (void *)val, arg->size);
@@ -512,6 +562,15 @@ static long long get_arg(struct func_arg *arg, unsigned long val)
 	return val;
 }
 
+static long long get_arg(struct func_arg *arg, long *args)
+{
+	/* Is arg an address and not a parameter? */
+	if (arg->arg < 0)
+		return __get_arg(arg, 0);
+	else
+		return __get_arg(arg, args[arg->arg]);
+}
+
 static void get_array(void *dst, struct func_arg *arg, unsigned long val)
 {
 	void *ptr = (void *)val;
@@ -527,6 +586,67 @@ static void get_array(void *dst, struct func_arg *arg, unsigned long val)
 	}
 }
 
+static int read_string(char *str, unsigned long addr)
+{
+	unsigned long flags;
+	struct func_string *strbuf;
+	char *ptr = (void *)addr;
+	char *buf;
+	int ret;
+
+	if (!str_buffer)
+		return 0;
+
+	strbuf = this_cpu_ptr(str_buffer);
+	buf = &strbuf->buf[0];
+
+	if (in_nmi())
+		buf += MAX_STR;
+
+	local_irq_save(flags);
+	ret = strncpy_from_unsafe(buf, ptr, MAX_STR);
+	if (ret < 0)
+		ret = 0;
+	if (ret > 0 && str)
+		memcpy(str, buf, ret);
+	local_irq_restore(flags);
+
+	return ret;
+}
+
+static int calculate_strings(struct func_event *func_event, int nr_args, long *args)
+{
+	struct func_arg *arg;
+	unsigned long val;
+	int str_count = 0;
+	int size = 0;
+
+	list_for_each_entry(arg, &func_event->args, list) {
+		if (arg->func_type != FUNC_TYPE_string)
+			continue;
+		if (arg->arg < nr_args)
+			val = get_arg(arg, args);
+		else
+			goto skip;
+		size += read_string(NULL, val);
+ skip:
+		if (++str_count >= func_event->has_strings)
+			return size;
+	}
+	return size;
+}
+
+static int get_string(unsigned long addr, unsigned int idx,
+		      unsigned int *info, char *data)
+{
+	int len;
+
+	len = read_string(data, addr);
+	*info = len << 16 | idx;
+
+	return len;
+}
+
 static void func_event_trace(struct trace_event_file *trace_file,
 			     struct func_event *func_event,
 			     unsigned long ip, unsigned long parent_ip,
@@ -540,6 +660,8 @@ static void func_event_trace(struct trace_event_file *trace_file,
 	long args[func_event->arg_cnt];
 	long long val = 1;
 	unsigned long irq_flags;
+	int str_offset;
+	int str_idx = 0;
 	int nr_args = 0;
 	int size;
 	int pc;
@@ -552,6 +674,12 @@ static void func_event_trace(struct trace_event_file *trace_file,
 
 	size = func_event->arg_offset + sizeof(*entry);
 
+	if (func_event->arg_cnt)
+		nr_args = arch_get_func_args(pt_regs, 0, func_event->arg_cnt, args);
+
+	if (func_event->has_strings)
+		size += calculate_strings(func_event, nr_args, args);
+
 	event = trace_event_buffer_lock_reserve(&buffer, trace_file,
 						call->event.type,
 						size, irq_flags, pc);
@@ -561,21 +689,22 @@ static void func_event_trace(struct trace_event_file *trace_file,
 	entry = ring_buffer_event_data(event);
 	entry->ip = ip;
 	entry->parent_ip = parent_ip;
-	if (func_event->arg_cnt)
-		nr_args = arch_get_func_args(pt_regs, 0, func_event->arg_cnt, args);
 
 	list_for_each_entry(arg, &func_event->args, list) {
-		if (arg->arg < nr_args) {
-			/* Is arg an address and not a parameter? */
-			if (arg->arg < 0)
-				val = get_arg(arg, 0);
-			else
-				val = get_arg(arg, args[arg->arg]);
-		} else
+		if (arg->arg < nr_args)
+			val = get_arg(arg, args);
+		else
 			val = 0;
 		if (arg->array)
 			get_array(&entry->data[arg->offset], arg, val);
-		else
+		else if (arg->func_type == FUNC_TYPE_string) {
+			str_offset = sizeof(struct func_event_hdr) +
+				func_event->arg_offset;
+
+			str_idx += get_string(val, str_offset + str_idx,
+					      (unsigned int *)&entry->data[arg->offset],
+					      &entry->data[func_event->arg_offset + str_idx]);
+		} else
 			memcpy(&entry->data[arg->offset], &val, arg->size);
 	}
 
@@ -616,6 +745,11 @@ static void make_fmt(struct func_arg *arg, char *fmt)
 
 	fmt[c++] = '%';
 
+	if (arg->func_type == FUNC_TYPE_string) {
+		fmt[c++] = 's';
+		goto out;
+	}
+
 	if (arg->func_type == FUNC_TYPE_char) {
 		if (arg->array)
 			fmt[c++] = 's';
@@ -673,6 +807,7 @@ func_event_print(struct trace_iterator *iter, int flags,
 	char fmt[FMT_SIZE];
 	void *data;
 	bool comma = false;
+	int info;
 	int a;
 
 	entry = (struct func_event_hdr *)iter->ent;
@@ -698,6 +833,11 @@ func_event_print(struct trace_iterator *iter, int flags,
 				comma = true;
 				write_data(s, arg, fmt, data);
 			}
+		} else if (arg->func_type == FUNC_TYPE_string) {
+			info = *(unsigned int *)data;
+			info = (info & 0xffff) - sizeof(struct func_event_hdr);
+			data = &entry->data[info];
+			trace_seq_printf(s, fmt, data);
 		} else
 			write_data(s, arg, fmt, data);
 	}
@@ -872,7 +1012,10 @@ static int __set_print_fmt(struct func_event *func_event,
 				len = update_len(len, i);
 			}
 		} else {
-			i = snprintf(buf + r, len, ", REC->%s", arg->name);
+			if (arg->func_type == FUNC_TYPE_string)
+				i = snprintf(buf + r, len, ", __get_str(%s)", arg->name);
+			else
+				i = snprintf(buf + r, len, ", REC->%s", arg->name);
 			r += i;
 			len = update_len(len, i);
 		}
-- 
2.15.1

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

* [PATCH 16/20 v2] tracing: Add NULL to skip args for function based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (14 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 15/20 v2] tracing: Add string type for dynamic strings in " Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 17/20 v2] tracing: Add indirect to indirect access " Steven Rostedt
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0016-tracing-Add-NULL-to-skip-args-for-function-based-eve.patch --]
[-- Type: text/plain, Size: 5890 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

If args are to be skipped (only care about second, third or later arguments)
then add a NULL to ignore them. For example, if one only wants to record the
third argument of a function, they can perform:

 echo foo(NULL, NULL, u32 arg3) > function_events

Then only the third argument is saved in the function based event.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/function-based-events.rst | 28 +++++++++++++++++++++-
 kernel/trace/trace_event_ftrace.c             | 34 ++++++++++++++++++++++++++-
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
index 6c643ea749e7..b90b52b7061d 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -91,7 +91,7 @@ as follows:
 
  ARGS := ARG | ARG ',' ARGS | ''
 
- ARG := TYPE FIELD | TYPE <name> '=' ADDR | TYPE ADDR | ARG '|' ARG
+ ARG := TYPE FIELD | TYPE <name> '=' ADDR | TYPE ADDR | ARG '|' ARG | 'NULL'
 
  TYPE := ATOM | ATOM '[' <number> ']' | 'unsigned' TYPE
 
@@ -359,3 +359,29 @@ it will be truncated.
  # echo 'link_path_walk(string name)' > function_events
 
 Gives the same result as above, but does not waste buffer space.
+
+
+NULL arguments
+==============
+
+If you are only interested in the second, or later parameter of a function,
+you do not have to record the previous parameters. Just set them as NULL and
+they will not be recorded.
+
+If we only wanted the perm_addr of the net_device of ip_rcv() and not the
+sk_buff, we put a NULL into the first parameter when created the function
+based event.
+
+  # echo 'ip_rcv(NULL, x8[6] perm_addr+558)' > function_events
+
+  # echo 1 > events/functions/ip_rcv/enable
+  # cat trace
+    <idle>-0     [003] ..s3   165.617114: __netif_receive_skb_core->ip_rcv(perm_addr=b4,b5,2f,ce,18,65)
+    <idle>-0     [003] ..s3   165.617133: __netif_receive_skb_core->ip_rcv(perm_addr=b4,b5,2f,ce,18,65)
+    <idle>-0     [003] ..s3   166.412277: __netif_receive_skb_core->ip_rcv(perm_addr=b4,b5,2f,ce,18,65)
+    <idle>-0     [003] ..s3   166.412797: __netif_receive_skb_core->ip_rcv(perm_addr=b4,b5,2f,ce,18,65)
+
+
+NULL can appear in any argument, to have them ignored. Note, skipping arguments
+does not give you access to later arguments if they are not supported by the
+architecture. The architecture only supplies the first set of arguments.
diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index 50d0c4d32596..673e2d963319 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -75,6 +75,7 @@ enum func_states {
 	FUNC_STATE_ARRAY_END,
 	FUNC_STATE_VAR,
 	FUNC_STATE_COMMA,
+	FUNC_STATE_NULL,
 	FUNC_STATE_END,
 	FUNC_STATE_ERROR,
 };
@@ -117,6 +118,7 @@ static struct func_type {
 	int		sign;
 } func_types[] = {
 	FUNC_TYPES,
+	{ "NULL",	0,	0 },
 	{ NULL,		0,	0 }
 };
 
@@ -125,6 +127,7 @@ static struct func_type {
 
 enum {
 	FUNC_TYPES,
+	FUNC_TYPE_NULL,
 	FUNC_TYPE_MAX
 };
 
@@ -366,6 +369,8 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 			fevent->arg_cnt++;
 		update_arg = false;
 	case FUNC_STATE_PIPE:
+		if (strcmp(token, "NULL") == 0)
+			return FUNC_STATE_NULL;
 		if (strcmp(token, "unsigned") == 0) {
 			unsign = 2;
 			return FUNC_STATE_UNSIGNED;
@@ -515,6 +520,19 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 		fevent->last_arg->indirect = INDIRECT_FLAG;
 		return FUNC_STATE_ADDR;
 
+	case FUNC_STATE_NULL:
+		ret = add_arg(fevent, FUNC_TYPE_NULL, 0);
+		if (ret < 0)
+			break;
+		switch (token[0]) {
+		case ')':
+			goto end;
+		case ',':
+			update_arg = true;
+			return FUNC_STATE_COMMA;
+		}
+		break;
+
 	default:
 		break;
 	}
@@ -691,6 +709,8 @@ static void func_event_trace(struct trace_event_file *trace_file,
 	entry->parent_ip = parent_ip;
 
 	list_for_each_entry(arg, &func_event->args, list) {
+		if (arg->func_type == FUNC_TYPE_NULL)
+			continue;
 		if (arg->arg < nr_args)
 			val = get_arg(arg, args);
 		else
@@ -817,6 +837,8 @@ func_event_print(struct trace_iterator *iter, int flags,
 	trace_seq_printf(s, "%ps->%ps(",
 			 (void *)entry->parent_ip, (void *)entry->ip);
 	list_for_each_entry(arg, &func_event->args, list) {
+		if (arg->func_type == FUNC_TYPE_NULL)
+			continue;
 		if (comma)
 			trace_seq_puts(s, ", ");
 		comma = true;
@@ -864,6 +886,9 @@ static int func_event_define_fields(struct trace_event_call *event_call)
 	list_for_each_entry(arg, &fevent->args, list) {
 		int size = arg->size;
 
+		if (arg->func_type == FUNC_TYPE_NULL)
+			continue;
+
 		if (arg->array)
 			size *= arg->array;
 		ret = trace_define_field(event_call, arg->type,
@@ -966,6 +991,8 @@ static int __set_print_fmt(struct func_event *func_event,
 	r = snprintf(buf, len, "%s", fmt_start);
 	len = update_len(len, r);
 	list_for_each_entry(arg, &func_event->args, list) {
+		if (arg->func_type == FUNC_TYPE_NULL)
+			continue;
 		if (comma) {
 			i = snprintf(buf + r, len, ", ");
 			r += i;
@@ -1004,6 +1031,8 @@ static int __set_print_fmt(struct func_event *func_event,
 	len = update_len(len, i);
 
 	list_for_each_entry(arg, &func_event->args, list) {
+		if (arg->func_type == FUNC_TYPE_NULL)
+			continue;
 		/* Don't iterate for strings */
 		if (arg->array && arg->func_type != FUNC_TYPE_char) {
 			for (a = 0; a < arg->array; a++) {
@@ -1150,7 +1179,10 @@ static int func_event_seq_show(struct seq_file *m, void *v)
 		}
 		last_arg = arg->arg;
 		comma = true;
-		seq_printf(m, "%s %s", arg->type, arg->name);
+		if (arg->func_type == FUNC_TYPE_NULL)
+			seq_puts(m, "NULL");
+		else
+			seq_printf(m, "%s %s", arg->type, arg->name);
 		if (arg->arg < 0) {
 			seq_printf(m, "=0x%lx", arg->index);
 		} else {
-- 
2.15.1

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

* [PATCH 17/20 v2] tracing: Add indirect to indirect access for function based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (15 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 16/20 v2] tracing: Add NULL to skip args for " Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 18/20 v2] tracing/perf: Allow perf to use " Steven Rostedt
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0017-tracing-Add-indirect-to-indirect-access-for-function.patch --]
[-- Type: text/plain, Size: 8773 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Allow the function based events to retrieve not only the parameters offsets,
but also get data from a pointer within a parameter structure. Something
like:

 # echo 'ip_rcv(string skdev+16[0][0] | x8[6] skperm+16[0]+558)' > function_events

 # echo 1 > events/functions/ip_rcv/enable
 # cat trace
    <idle>-0     [003] ..s3   310.626391: __netif_receive_skb_core->ip_rcv(skdev=em1, skperm=b4,b5,2f,ce,18,65)
    <idle>-0     [003] ..s3   310.626400: __netif_receive_skb_core->ip_rcv(skdev=em1, skperm=b4,b5,2f,ce,18,65)
    <idle>-0     [003] ..s3   312.183775: __netif_receive_skb_core->ip_rcv(skdev=em1, skperm=b4,b5,2f,ce,18,65)
    <idle>-0     [003] ..s3   312.184329: __netif_receive_skb_core->ip_rcv(skdev=em1, skperm=b4,b5,2f,ce,18,65)
    <idle>-0     [003] ..s3   312.303895: __netif_receive_skb_core->ip_rcv(skdev=em1, skperm=b4,b5,2f,ce,18,65)
    <idle>-0     [003] ..s3   312.304610: __netif_receive_skb_core->ip_rcv(skdev=em1, skperm=b4,b5,2f,ce,18,65)
    <idle>-0     [003] ..s3   312.471980: __netif_receive_skb_core->ip_rcv(skdev=em1, skperm=b4,b5,2f,ce,18,65)
    <idle>-0     [003] ..s3   312.472908: __netif_receive_skb_core->ip_rcv(skdev=em1, skperm=b4,b5,2f,ce,18,65)
    <idle>-0     [003] ..s3   313.135804: __netif_receive_skb_core->ip_rcv(skdev=em1, skperm=b4,b5,2f,ce,18,65)

That is, we retrieved the net_device of the sk_buff and displayed its name
and perm_addr info.

  sk->dev->name, sk->dev->perm_addr

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/function-based-events.rst |  40 +++++++++-
 kernel/trace/trace_event_ftrace.c             | 102 ++++++++++++++++++++++++--
 2 files changed, 136 insertions(+), 6 deletions(-)

diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
index b90b52b7061d..3b341992b93d 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -101,12 +101,15 @@ as follows:
          'char' | 'short' | 'int' | 'long' | 'size_t' |
 	 'symbol' | 'string'
 
- FIELD := <name> | <name> INDEX | <name> OFFSET | <name> OFFSET INDEX
+ FIELD := <name> | <name> INDEX | <name> OFFSET | <name> OFFSET INDEX |
+	 FIELD INDIRECT
 
  INDEX := '[' <number> ']'
 
  OFFSET := '+' <number>
 
+ INDIRECT := INDEX | OFFSET | INDIRECT INDIRECT | ''
+
  ADDR := A hexidecimal address starting with '0x'
 
  Where <name> is a unique string starting with an alphabetic character
@@ -385,3 +388,38 @@ based event.
 NULL can appear in any argument, to have them ignored. Note, skipping arguments
 does not give you access to later arguments if they are not supported by the
 architecture. The architecture only supplies the first set of arguments.
+
+
+The chain of indirects
+======================
+
+When a parameter is a structure, and that structure points to another structure,
+the data of that structure can still be found.
+
+ssize_t __vfs_read(struct file *file, char __user *buf, size_t count,
+		   loff_t *pos)
+
+has the following code.
+
+	if (file->f_op->read)
+		return file->f_op->read(file, buf, count, pos);
+
+To trace all the functions that are called by f_op->read(), that information
+can be obtained from the file pointer.
+
+Using gdb again:
+
+   (gdb) printf "%d\n", &((struct file *)0)->f_op
+40
+   (gdb) printf "%d\n", &((struct file_operations *)0)->read
+16
+
+    # echo '__vfs_read(symbol read+40[0]+16)' > function_events
+
+  # echo 1 > events/functions/__vfs_read/enable
+  # cat trace
+         sshd-1343  [005] ...2   199.734752: vfs_read->__vfs_read(read=tty_read+0x0/0xf0)
+         bash-1344  [003] ...2   199.734822: vfs_read->__vfs_read(read=tty_read+0x0/0xf0)
+         sshd-1343  [005] ...2   199.734835: vfs_read->__vfs_read(read=tty_read+0x0/0xf0)
+ avahi-daemon-910   [003] ...2   200.136740: vfs_read->__vfs_read(read=          (null))
+ avahi-daemon-910   [003] ...2   200.136750: vfs_read->__vfs_read(read=          (null))
diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index 673e2d963319..732ba570466b 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -14,8 +14,15 @@
 #define WRITE_BUFSIZE		4096
 #define INDIRECT_FLAG		0x10000000
 
+struct func_arg_redirect {
+	struct list_head		list;
+	long				index;
+	long				indirect;
+};
+
 struct func_arg {
 	struct list_head		list;
+	struct list_head		redirects;
 	char				*type;
 	char				*name;
 	long				indirect;
@@ -73,6 +80,8 @@ enum func_states {
 	FUNC_STATE_ARRAY,
 	FUNC_STATE_ARRAY_SIZE,
 	FUNC_STATE_ARRAY_END,
+	FUNC_STATE_REDIRECT_PLUS,
+	FUNC_STATE_REDIRECT_BRACKET,
 	FUNC_STATE_VAR,
 	FUNC_STATE_COMMA,
 	FUNC_STATE_NULL,
@@ -269,6 +278,8 @@ static int add_arg(struct func_event *fevent, int ftype, int unsign)
 	if (!arg)
 		return -ENOMEM;
 
+	INIT_LIST_HEAD(&arg->redirects);
+
 	if (unsign)
 		arg->type = kasprintf(GFP_KERNEL, "unsigned %s",
 				      func_type->name);
@@ -331,6 +342,22 @@ static int update_arg_arg(struct func_event *fevent)
 	return 0;
 }
 
+static int add_arg_redirect(struct func_arg *arg, long index, long indirect)
+{
+	struct func_arg_redirect *redirect;
+
+	redirect = kzalloc(sizeof(*redirect), GFP_KERNEL);
+	if (!redirect)
+		return -ENOMEM;
+
+	redirect->index = index;
+	redirect->indirect = indirect;
+
+	list_add_tail(&redirect->list, &arg->redirects);
+
+	return 0;
+}
+
 static enum func_states
 process_event(struct func_event *fevent, const char *token, enum func_states state)
 {
@@ -461,6 +488,10 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 			return FUNC_STATE_COMMA;
 		case '|':
 			return FUNC_STATE_PIPE;
+		case '+':
+			return FUNC_STATE_REDIRECT_PLUS;
+		case '[':
+			return FUNC_STATE_REDIRECT_BRACKET;
 		}
 		break;
 
@@ -484,6 +515,30 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 		}
 		break;
 
+	case FUNC_STATE_REDIRECT_PLUS:
+		if (WARN_ON(!fevent->last_arg))
+			break;
+		ret = kstrtoul(token, 0, &val);
+		if (ret)
+			break;
+		ret = add_arg_redirect(fevent->last_arg, val, 0);
+		if (ret)
+			break;
+		return FUNC_STATE_VAR;
+
+	case FUNC_STATE_REDIRECT_BRACKET:
+		if (WARN_ON(!fevent->last_arg))
+			break;
+		ret = kstrtoul(token, 0, &val);
+		if (ret)
+			break;
+		val *= fevent->last_arg->size;
+		val ^= INDIRECT_FLAG;
+		ret = add_arg_redirect(fevent->last_arg, 0, val);
+		if (ret)
+			break;
+		return FUNC_STATE_INDIRECT;
+
 	case FUNC_STATE_VAR:
 		if (token[0] == '=')
 			return FUNC_STATE_EQUAL;
@@ -543,20 +598,49 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 	return FUNC_STATE_END;
 }
 
-static long long __get_arg(struct func_arg *arg, unsigned long val)
+static unsigned long process_redirects(struct func_arg *arg, unsigned long val,
+				       char *buf)
+{
+	struct func_arg_redirect *redirect;
+	int ret;
+
+	if (arg->indirect) {
+		ret = probe_kernel_read(buf, (void *)val, sizeof(long));
+		if (ret)
+			return 0;
+		val = *(unsigned long *)buf;
+	}
+
+	list_for_each_entry(redirect, &arg->redirects, list) {
+		val += redirect->index;
+		if (redirect->indirect) {
+			val += (redirect->indirect ^ INDIRECT_FLAG);
+			ret = probe_kernel_read(buf, (void *)val, sizeof(long));
+			if (ret)
+				return 0;
+		}
+	}
+	return val;
+}
+
+static long long __get_arg(struct func_arg *arg, unsigned long long val)
 {
 	char buf[8];
 	int ret;
 
 	val += arg->index;
 
-	if (!arg->indirect)
-		return val;
+	if (arg->indirect)
+		val += (arg->indirect ^ INDIRECT_FLAG);
+
+	if (!list_empty(&arg->redirects))
+		val = process_redirects(arg, val, buf);
 
-	val = val + (arg->indirect ^ INDIRECT_FLAG);
+	if (!val)
+		return 0;
 
 	/* Arrays and strings do their own indirect reads */
-	if (arg->array || arg->func_type == FUNC_TYPE_string)
+	if (!arg->indirect || arg->array || arg->func_type == FUNC_TYPE_string)
 		return val;
 
 	ret = probe_kernel_read(buf, (void *)val, arg->size);
@@ -1164,6 +1248,7 @@ static void func_event_seq_stop(struct seq_file *m, void *v)
 static int func_event_seq_show(struct seq_file *m, void *v)
 {
 	struct func_event *func_event = v;
+	struct func_arg_redirect *redirect;
 	struct func_arg *arg;
 	bool comma = false;
 	int last_arg = 0;
@@ -1192,6 +1277,13 @@ static int func_event_seq_show(struct seq_file *m, void *v)
 				seq_printf(m, "[%ld]",
 					   (arg->indirect ^ INDIRECT_FLAG) / arg->size);
 		}
+		list_for_each_entry(redirect, &arg->redirects, list) {
+			if (redirect->index)
+				seq_printf(m, "+%ld", redirect->index);
+			if (redirect->indirect)
+				seq_printf(m, "[%ld]",
+					   (redirect->indirect ^ INDIRECT_FLAG) / arg->size);
+		}
 	}
 	seq_puts(m, ")\n");
 
-- 
2.15.1

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

* [PATCH 18/20 v2] tracing/perf: Allow perf to use function based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (16 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 17/20 v2] tracing: Add indirect to indirect access " Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 19/20 v2] tracing: Add error messages for failed writes to function_events Steven Rostedt
  2018-02-07 20:24 ` [PATCH 20/20 v2] tracing: Add argument error message too many args for function based events Steven Rostedt
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0018-tracing-perf-Allow-perf-to-use-function-based-events.patch --]
[-- Type: text/plain, Size: 6744 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Have perf use function based events.

 # echo 'SyS_openat(int dfd, string buf, x32 flags, x32 mode)' > /sys/kernel/tracing/function_events
 # perf record -e functions:SyS_openat grep task_forks /proc/kallsyms
 # perf script
    grep   913 [002]  5713.413239: functions:SyS_openat: entry_SYSCALL_64_fastpath->sys_openat(dfd=-100, buf=/proc/kallsyms, flags=100, mode=0)

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/function-based-events.rst |   3 +-
 kernel/trace/trace_event_ftrace.c             | 134 ++++++++++++++++++++------
 2 files changed, 104 insertions(+), 33 deletions(-)

diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
index 3b341992b93d..6effde96d3d6 100644
--- a/Documentation/trace/function-based-events.rst
+++ b/Documentation/trace/function-based-events.rst
@@ -48,7 +48,8 @@ enable  filter  format  hist  id  trigger
 
 Even though the above function based event does not record much more
 than the function tracer does, it does become a full fledge event.
-This can be used by the histogram infrastructure, and triggers.
+This can be used by the histogram infrastructure, triggers, and perf
+where one can attach eBPF programs to.
 
  # cat events/functions/do_IRQ/format
 name: do_IRQ
diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index 732ba570466b..303a56c3339a 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -749,46 +749,33 @@ static int get_string(unsigned long addr, unsigned int idx,
 	return len;
 }
 
-static void func_event_trace(struct trace_event_file *trace_file,
-			     struct func_event *func_event,
-			     unsigned long ip, unsigned long parent_ip,
-			     struct pt_regs *pt_regs)
+static int get_event_size(struct func_event *func_event, struct pt_regs *pt_regs,
+			  long *args, int *nr_args)
 {
-	struct func_event_hdr *entry;
-	struct trace_event_call *call = &func_event->call;
-	struct ring_buffer_event *event;
-	struct ring_buffer *buffer;
-	struct func_arg *arg;
-	long args[func_event->arg_cnt];
-	long long val = 1;
-	unsigned long irq_flags;
-	int str_offset;
-	int str_idx = 0;
-	int nr_args = 0;
 	int size;
-	int pc;
-
-	if (trace_trigger_soft_disabled(trace_file))
-		return;
-
-	local_save_flags(irq_flags);
-	pc = preempt_count();
 
-	size = func_event->arg_offset + sizeof(*entry);
+	size = func_event->arg_offset + sizeof(struct func_event_hdr);
 
 	if (func_event->arg_cnt)
-		nr_args = arch_get_func_args(pt_regs, 0, func_event->arg_cnt, args);
+		*nr_args = arch_get_func_args(pt_regs, 0, func_event->arg_cnt, args);
+	else
+		*nr_args = 0;
 
 	if (func_event->has_strings)
-		size += calculate_strings(func_event, nr_args, args);
+		size += calculate_strings(func_event, *nr_args, args);
 
-	event = trace_event_buffer_lock_reserve(&buffer, trace_file,
-						call->event.type,
-						size, irq_flags, pc);
-	if (!event)
-		return;
+	return size;
+}
+
+static void
+record_entry(struct func_event_hdr *entry, struct func_event *func_event,
+	     unsigned long ip, unsigned long parent_ip, int nr_args, long *args)
+{
+	struct func_arg *arg;
+	long long val;
+	int str_offset;
+	int str_idx = 0;
 
-	entry = ring_buffer_event_data(event);
 	entry->ip = ip;
 	entry->parent_ip = parent_ip;
 
@@ -811,11 +798,80 @@ static void func_event_trace(struct trace_event_file *trace_file,
 		} else
 			memcpy(&entry->data[arg->offset], &val, arg->size);
 	}
+}
+
+static void func_event_trace(struct trace_event_file *trace_file,
+			     struct func_event *func_event,
+			     unsigned long ip, unsigned long parent_ip,
+			     struct pt_regs *pt_regs)
+{
+	struct func_event_hdr *entry;
+	struct trace_event_call *call = &func_event->call;
+	struct ring_buffer_event *event;
+	struct ring_buffer *buffer;
+	long args[func_event->arg_cnt];
+	unsigned long irq_flags;
+	int nr_args;
+	int size;
+	int pc;
+
+	if (trace_trigger_soft_disabled(trace_file))
+		return;
+
+	local_save_flags(irq_flags);
+	pc = preempt_count();
+
+	size = get_event_size(func_event, pt_regs, args, &nr_args);
+
+	event = trace_event_buffer_lock_reserve(&buffer, trace_file,
+						call->event.type,
+						size, irq_flags, pc);
+	if (!event)
+		return;
 
+	entry = ring_buffer_event_data(event);
+	record_entry(entry, func_event, ip, parent_ip, nr_args, args);
 	event_trigger_unlock_commit_regs(trace_file, buffer, event,
 					 entry, irq_flags, pc, pt_regs);
 }
 
+#ifdef CONFIG_PERF_EVENTS
+/* Kprobe profile handler */
+static void func_event_perf(struct func_event *func_event,
+			    unsigned long ip, unsigned long parent_ip,
+			    struct pt_regs *pt_regs)
+{
+	struct trace_event_call *call = &func_event->call;
+	struct func_event_hdr *entry;
+	struct hlist_head *head;
+	long args[func_event->arg_cnt];
+	int nr_args = 0;
+	int rctx;
+	int size;
+
+	if (bpf_prog_array_valid(call) && !trace_call_bpf(call, pt_regs))
+		return;
+
+	head = this_cpu_ptr(call->perf_events);
+	if (hlist_empty(head))
+		return;
+
+	size = get_event_size(func_event, pt_regs, args, &nr_args);
+
+	entry = perf_trace_buf_alloc(size, NULL, &rctx);
+	if (!entry)
+		return;
+
+	record_entry(entry, func_event, ip, parent_ip, nr_args, args);
+	perf_trace_buf_submit(entry, size, rctx, call->event.type, 1, pt_regs,
+			      head, NULL);
+}
+#else
+static inline void func_event_perf(struct func_event *func_event,
+				   unsigned long ip, unsigned long parent_ip,
+				   struct pt_regs *pt_regs) { }
+#endif
+
 static void
 func_event_call(unsigned long ip, unsigned long parent_ip,
 		    struct ftrace_ops *op, struct pt_regs *pt_regs)
@@ -830,7 +886,10 @@ func_event_call(unsigned long ip, unsigned long parent_ip,
 	rcu_irq_enter_irqson();
 	rcu_read_lock_sched_notrace();
 	list_for_each_entry_rcu(ff, &func_event->files, list) {
-		func_event_trace(ff->file, func_event, ip, parent_ip, pt_regs);
+		if (ff->file)
+			func_event_trace(ff->file, func_event, ip, parent_ip, pt_regs);
+		else
+			func_event_perf(func_event, ip, parent_ip, pt_regs);
 	}
 	rcu_read_unlock_sched_notrace();
 	rcu_irq_exit_irqson();
@@ -1047,6 +1106,17 @@ static int func_event_register(struct trace_event_call *event,
 		return enable_func_event(func_event, file);
 	case TRACE_REG_UNREGISTER:
 		return disable_func_event(func_event, file);
+#ifdef CONFIG_PERF_EVENTS
+	case TRACE_REG_PERF_REGISTER:
+		return enable_func_event(func_event, NULL);
+	case TRACE_REG_PERF_UNREGISTER:
+		return disable_func_event(func_event, NULL);
+	case TRACE_REG_PERF_OPEN:
+	case TRACE_REG_PERF_CLOSE:
+	case TRACE_REG_PERF_ADD:
+	case TRACE_REG_PERF_DEL:
+		return 0;
+#endif
 	default:
 		break;
 	}
-- 
2.15.1

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

* [PATCH 19/20 v2] tracing: Add error messages for failed writes to function_events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (17 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 18/20 v2] tracing/perf: Allow perf to use " Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  2018-02-07 20:24 ` [PATCH 20/20 v2] tracing: Add argument error message too many args for function based events Steven Rostedt
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0019-tracing-Add-error-messages-for-failed-writes-to-func.patch --]
[-- Type: text/plain, Size: 10902 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

When a write to function_events fails to parse, produce an error message to
help the user know why it failed. The error message will display at the end
of reading the function_events file the next time.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_event_ftrace.c | 288 ++++++++++++++++++++++++++++++++------
 1 file changed, 244 insertions(+), 44 deletions(-)

diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index 303a56c3339a..314d025dc676 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -38,6 +38,7 @@ struct func_arg {
 struct func_event {
 	struct list_head		list;
 	char				*func;
+	/* The above must match func_event_err below */
 	struct trace_event_class	class;
 	struct trace_event_call		call;
 	struct ftrace_ops		ops;
@@ -49,6 +50,15 @@ struct func_event {
 	int				has_strings;
 };
 
+#define ERR_SIZE	(256 - (sizeof(struct list_head) + sizeof(char *)))
+
+struct func_event_err {
+	struct list_head		list;
+	char				*func;
+	/* The above must match func_event above */
+	char				err_str[ERR_SIZE];
+};
+
 struct func_file {
 	struct list_head		list;
 	struct trace_event_file		*file;
@@ -64,29 +74,42 @@ struct func_event_hdr {
 static DEFINE_MUTEX(func_event_mutex);
 static LIST_HEAD(func_events);
 
+#define FUNC_STATES				\
+	C(INIT),				\
+	C(FUNC),				\
+	C(PARAM),				\
+	C(BRACKET),				\
+	C(BRACKET_END),				\
+	C(INDIRECT),				\
+	C(UNSIGNED),				\
+	C(ADDR),				\
+	C(EQUAL),				\
+	C(PIPE),				\
+	C(PLUS),				\
+	C(TYPE),				\
+	C(ARRAY),				\
+	C(ARRAY_SIZE),				\
+	C(ARRAY_END),				\
+	C(REDIRECT_PLUS),			\
+	C(REDIRECT_BRACKET),			\
+	C(VAR),					\
+	C(COMMA),				\
+	C(NULL),				\
+	C(END),					\
+	C(ERROR)
+
+#undef C
+#define C(x)	FUNC_STATE_##x
+
 enum func_states {
-	FUNC_STATE_INIT,
-	FUNC_STATE_FUNC,
-	FUNC_STATE_PARAM,
-	FUNC_STATE_BRACKET,
-	FUNC_STATE_BRACKET_END,
-	FUNC_STATE_INDIRECT,
-	FUNC_STATE_UNSIGNED,
-	FUNC_STATE_ADDR,
-	FUNC_STATE_EQUAL,
-	FUNC_STATE_PIPE,
-	FUNC_STATE_PLUS,
-	FUNC_STATE_TYPE,
-	FUNC_STATE_ARRAY,
-	FUNC_STATE_ARRAY_SIZE,
-	FUNC_STATE_ARRAY_END,
-	FUNC_STATE_REDIRECT_PLUS,
-	FUNC_STATE_REDIRECT_BRACKET,
-	FUNC_STATE_VAR,
-	FUNC_STATE_COMMA,
-	FUNC_STATE_NULL,
-	FUNC_STATE_END,
-	FUNC_STATE_ERROR,
+	FUNC_STATES
+};
+
+#undef C
+#define C(x)	#x
+
+static char *func_state_names[] = {
+	FUNC_STATES
 };
 
 typedef u64 x64;
@@ -215,6 +238,16 @@ static void free_func_event(struct func_event *func_event)
 	if (!func_event)
 		return;
 
+	/*
+	 * If func is NULL then this is a func_event_err, or
+	 * nothing else has been allocated for the func_event.
+	 * In either case, it is safe just to free the func_event.
+	 */
+	if (!func_event->func) {
+		kfree(func_event);
+		return;
+	}
+
 	list_for_each_entry_safe(arg, n, &func_event->args, list) {
 		free_arg(arg);
 	}
@@ -438,8 +471,11 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
 			break;
 		if (strncmp(token, "0x", 2) == 0)
 			goto equal;
-		if (!isalpha(token[0]) && token[0] != '_')
+		if (!isalpha(token[0]) && token[0] != '_') {
+			kfree(fevent->last_arg->name);
+			fevent->last_arg->name = NULL;
 			break;
+		}
 		update_arg = true;
 		return FUNC_STATE_VAR;
 
@@ -1249,10 +1285,121 @@ static int func_event_create(struct func_event *func_event)
 	return ret;
 }
 
+static void show_func_event(struct trace_seq *s, struct func_event *func_event);
+
+static void add_failure(struct func_event *func_event, char *token,
+			enum func_states state, char *ptr, char last,
+			int i, int argc, char **argv)
+{
+	struct func_event_err *func_err;
+	struct trace_seq *s;
+	char *save_token = NULL;
+	int len;
+
+	/* Don't do anything if we were not able to get the first field */
+	if (!func_event->func)
+		return;
+
+	func_err = kzalloc(sizeof(*func_err), GFP_KERNEL);
+	if (!func_err)
+		return;
+
+	s = kmalloc(sizeof(*s), GFP_KERNEL);
+	if (!s) {
+		kfree(func_err);
+		return;
+	}
+	trace_seq_init(s);
+	show_func_event(s, func_event);
+
+	/*
+	 * show_func_event() doesn't print some tokens if it crashed
+	 * at a certain state.
+	 */
+	switch (state) {
+	case FUNC_STATE_PIPE:
+		trace_seq_puts(s, " | ");
+		break;
+	case FUNC_STATE_COMMA:
+		trace_seq_puts(s, ", ");
+		break;
+	case FUNC_STATE_PLUS:
+	case FUNC_STATE_REDIRECT_PLUS:
+		trace_seq_putc(s, '+');
+		break;
+	case FUNC_STATE_BRACKET:
+	case FUNC_STATE_ARRAY:
+		trace_seq_putc(s, '[');
+		break;
+	case FUNC_STATE_UNSIGNED:
+		trace_seq_puts(s, "unsigned ");
+		break;
+	case FUNC_STATE_INDIRECT:
+	case FUNC_STATE_ARRAY_SIZE:
+		/* show_func_event() adds a ']' for these */
+		s->seq.len--;
+		break;
+	default:
+		break;
+	}
+	trace_seq_putc(s, ' ');
+	len = s->seq.len + 1;
+
+	if (!token) {
+		/* Parser didn't end properly */
+		trace_seq_printf(s, "\n%*s\nUnexpected ending",
+				 len, "^");
+		goto finish;
+	}
+
+	save_token = kstrdup(token, GFP_KERNEL);
+	if (!save_token) {
+		kfree(func_err);
+		kfree(s);
+		return;
+	}
+
+	trace_seq_puts(s, token);
+	trace_seq_putc(s, ' ');
+
+	/* Finish parsing the tokens */
+	for (token = next_token(&ptr, &last); token;
+	     token = next_token(&ptr, &last)) {
+		if (token[0] == '|')
+			trace_seq_putc(s, ' ');
+		trace_seq_puts(s, token);
+		if (token[0] == ',' || token[0] == '|')
+			trace_seq_putc(s, ' ');
+	}
+
+	/* Add the rest of the line */
+	for (i++; i < argc; i++) {
+		trace_seq_puts(s, argv[i]);
+		trace_seq_putc(s, ' ');
+	}
+
+	trace_seq_printf(s, "\n%*s\n", len, "^");
+
+	trace_seq_printf(s, "Unexpected token '%s' for %s state",
+			 save_token, func_state_names[state]);
+
+ finish:
+	len = min(ERR_SIZE-1, s->seq.len);
+	strncpy(func_err->err_str, s->buffer, len);
+	func_err->err_str[len] = 0;
+
+	mutex_lock(&func_event_mutex);
+	list_add_tail(&func_err->list, &func_events);
+	mutex_unlock(&func_event_mutex);
+
+	kfree(save_token);
+	kfree(s);
+}
+
 static int create_function_event(int argc, char **argv)
 {
 	struct func_event *func_event;
-	enum func_states state = FUNC_STATE_INIT;
+	enum func_states last_state, state = FUNC_STATE_INIT;
 	char *token;
 	char *ptr;
 	char last;
@@ -1268,11 +1415,13 @@ static int create_function_event(int argc, char **argv)
 	func_event->ops.func = func_event_call;
 	func_event->ops.flags = FTRACE_OPS_FL_SAVE_REGS;
 
+	last_state = state;
 	for (i = 0; i < argc; i++) {
 		ptr = argv[i];
 		last = 0;
 		for (token = next_token(&ptr, &last); token;
 		     token = next_token(&ptr, &last)) {
+			last_state = state;
 			state = process_event(func_event, token, state);
 			if (state == FUNC_STATE_ERROR)
 				goto fail;
@@ -1295,6 +1444,9 @@ static int create_function_event(int argc, char **argv)
 	mutex_unlock(&func_event_mutex);
 	return 0;
  fail:
+	if (state != FUNC_STATE_END)
+		add_failure(func_event, token, last_state, ptr,
+			    last, i, argc, argv);
 	free_func_event(func_event);
 	return ret;
 }
@@ -1315,46 +1467,71 @@ static void func_event_seq_stop(struct seq_file *m, void *v)
 	mutex_unlock(&func_event_mutex);
 }
 
-static int func_event_seq_show(struct seq_file *m, void *v)
+static int show_error (struct seq_file *m, struct func_event *func_event)
+{
+	struct func_event_err *func_err = (struct func_event_err *)func_event;
+
+	seq_puts(m, func_err->err_str);
+	seq_putc(m, '\n');
+	return 0;
+}
+
+static void show_func_event(struct trace_seq *s, struct func_event *func_event)
 {
-	struct func_event *func_event = v;
 	struct func_arg_redirect *redirect;
 	struct func_arg *arg;
 	bool comma = false;
 	int last_arg = 0;
 
-	seq_printf(m, "%s(", func_event->func);
+	trace_seq_printf(s, "%s(", func_event->func);
 
 	list_for_each_entry(arg, &func_event->args, list) {
 		if (comma) {
 			if (last_arg == arg->arg)
-				seq_puts(m, " | ");
+				trace_seq_puts(s, " | ");
 			else
-				seq_puts(m, ", ");
+				trace_seq_puts(s, ", ");
 		}
 		last_arg = arg->arg;
 		comma = true;
 		if (arg->func_type == FUNC_TYPE_NULL)
-			seq_puts(m, "NULL");
-		else
-			seq_printf(m, "%s %s", arg->type, arg->name);
+			trace_seq_puts(s, "NULL");
+		else {
+			if (arg->type)
+				trace_seq_puts(s, arg->type);
+			if (arg->name)
+				trace_seq_printf(s, " %s", arg->name);
+		}
 		if (arg->arg < 0) {
-			seq_printf(m, "=0x%lx", arg->index);
+			trace_seq_printf(s, "=0x%lx", arg->index);
 		} else {
 			if (arg->index)
-				seq_printf(m, "+%ld", arg->index);
+				trace_seq_printf(s, "+%ld", arg->index);
 			if (arg->indirect && arg->size)
-				seq_printf(m, "[%ld]",
+				trace_seq_printf(s, "[%ld]",
 					   (arg->indirect ^ INDIRECT_FLAG) / arg->size);
 		}
 		list_for_each_entry(redirect, &arg->redirects, list) {
 			if (redirect->index)
-				seq_printf(m, "+%ld", redirect->index);
+				trace_seq_printf(s, "+%ld", redirect->index);
 			if (redirect->indirect)
-				seq_printf(m, "[%ld]",
+				trace_seq_printf(s, "[%ld]",
 					   (redirect->indirect ^ INDIRECT_FLAG) / arg->size);
 		}
 	}
+}
+
+static int func_event_seq_show(struct seq_file *m, void *v)
+{
+	static struct trace_seq s;
+	struct func_event *func_event = v;
+
+	if (!func_event->func)
+		return show_error(m, func_event);
+
+	trace_seq_init(&s);
+	show_func_event(&s, func_event);
+	trace_print_seq(m, &s);
 	seq_puts(m, ")\n");
 
 	return 0;
@@ -1374,9 +1551,12 @@ static int release_all_func_events(void)
 
 	mutex_lock(&func_event_mutex);
 	list_for_each_entry_safe(func_event, n, &func_events, list) {
-		ret = trace_remove_event_call(&func_event->call);
-		if (ret < 0)
-			break;
+		/* NULL func means it is a func_event_err message */
+		if (func_event->func) {
+			ret = trace_remove_event_call(&func_event->call);
+			if (ret < 0)
+				return ret;
+		}
 		list_del(&func_event->list);
 		free_func_event(func_event);
 	}
@@ -1384,6 +1564,21 @@ static int release_all_func_events(void)
 	return ret;
 }
 
+static void remove_func_errors(void)
+{
+	struct func_event *func_event, *n;
+
+	mutex_lock(&func_event_mutex);
+	list_for_each_entry_safe(func_event, n, &func_events, list) {
+		/* NULL func means it is a func_event_err message */
+		if (func_event->func)
+			continue;
+		list_del(&func_event->list);
+		free_func_event(func_event);
+	}
+	mutex_unlock(&func_event_mutex);
+}
+
 static int func_event_open(struct inode *inode, struct file *file)
 {
 	int ret;
@@ -1391,10 +1586,15 @@ static int func_event_open(struct inode *inode, struct file *file)
 	if (max_args < 0)
 		max_args = arch_get_func_args(NULL, 0, 0, NULL);
 
-	if ((file->f_mode & FMODE_WRITE) && (file->f_flags & O_TRUNC)) {
-		ret = release_all_func_events();
-		if (ret < 0)
-			return ret;
+	if ((file->f_mode & FMODE_WRITE)) {
+		if (file->f_flags & O_TRUNC) {
+			ret = release_all_func_events();
+			if (ret < 0)
+				return ret;
+		} else {
+			/* Only keep one error per write */
+			remove_func_errors();
+		}
 	}
 
 	return seq_open(file, &func_event_seq_op);
-- 
2.15.1

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

* [PATCH 20/20 v2] tracing: Add argument error message too many args for function based events
  2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
                   ` (18 preceding siblings ...)
  2018-02-07 20:24 ` [PATCH 19/20 v2] tracing: Add error messages for failed writes to function_events Steven Rostedt
@ 2018-02-07 20:24 ` Steven Rostedt
  19 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-07 20:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Peter Zijlstra, Masami Hiramatsu, Tom Zanussi, linux-rt-users,
	linux-trace-users, Arnaldo Carvalho de Melo, Clark Williams,
	Jiri Olsa, Daniel Bristot de Oliveira, Juri Lelli

[-- Attachment #1: 0020-tracing-Add-argument-error-message-too-many-args-for.patch --]
[-- Type: text/plain, Size: 1266 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

If too many arguments are added to the function_events, have the error
message state that instead of saying the type of argument that passed the
allowed amount is incorrect.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_event_ftrace.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
index 314d025dc676..e612b1c4fc40 100644
--- a/kernel/trace/trace_event_ftrace.c
+++ b/kernel/trace/trace_event_ftrace.c
@@ -1380,8 +1380,14 @@ static void add_failure(struct func_event *func_event, char *token,
 
 	trace_seq_printf(s, "\n%*s\n", len, "^");
 
-	trace_seq_printf(s, "Unexpected token '%s' for %s state",
-			 save_token, func_state_names[state]);
+	/* for COMMA or PARAM state, the error could be too many args */
+	if ((state == FUNC_STATE_COMMA || state == FUNC_STATE_PARAM) &&
+	    func_event->arg_cnt >= max_args)
+		trace_seq_printf(s, "Error: Too many arguments (max of %d)",
+				 max_args);
+	else
+		trace_seq_printf(s, "Unexpected token '%s' for %s state",
+				 save_token, func_state_names[state]);
 
  finish:
 	len = min(ERR_SIZE-1, s->seq.len);
-- 
2.15.1

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

* Re: [PATCH 01/20 v2] tracing: Add function based events
  2018-02-07 20:24 ` [PATCH 01/20 v2] tracing: Add " Steven Rostedt
@ 2018-02-08 11:20   ` Jiri Olsa
  2018-02-08 15:53     ` Steven Rostedt
  0 siblings, 1 reply; 27+ messages in thread
From: Jiri Olsa @ 2018-02-08 11:20 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Linus Torvalds, Ingo Molnar, Andrew Morton,
	Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu, Tom Zanussi,
	linux-rt-users, linux-trace-users, Arnaldo Carvalho de Melo,
	Clark Williams, Daniel Bristot de Oliveira, Juri Lelli

On Wed, Feb 07, 2018 at 03:24:03PM -0500, Steven Rostedt wrote:

SNIP

> +
> +void create_function_event_file(struct dentry *d_tracer)
> +{
> +	struct dentry *d;
> +
> +	d = trace_create_file("function_events", 0644, d_tracer, NULL,
> +			      &func_event_fops);
> +	WARN(!d, "Failed to create function_events file");
> +}

this fucntion seems like leftover.. can't see any code using it

jirka

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

* Re: [PATCH 11/20 v2] tracing: Add symbol type to function based events
  2018-02-07 20:24 ` [PATCH 11/20 v2] tracing: Add symbol type to function based events Steven Rostedt
@ 2018-02-08 11:20   ` Jiri Olsa
  2018-02-08 15:59     ` Steven Rostedt
  2018-02-09 15:03     ` Masami Hiramatsu
  0 siblings, 2 replies; 27+ messages in thread
From: Jiri Olsa @ 2018-02-08 11:20 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Linus Torvalds, Ingo Molnar, Andrew Morton,
	Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu, Tom Zanussi,
	linux-rt-users, linux-trace-users, Arnaldo Carvalho de Melo,
	Clark Williams, Daniel Bristot de Oliveira, Juri Lelli

On Wed, Feb 07, 2018 at 03:24:13PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> Add a special type "symbol" that will use %pS to display the field of a
> function based event.
> 
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  Documentation/trace/function-based-events.rst | 26 +++++++++++++++++++++++++-
>  kernel/trace/trace_event_ftrace.c             | 13 ++++++++++---
>  2 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
> index bdb28f433bfb..f18c8f3ef330 100644
> --- a/Documentation/trace/function-based-events.rst
> +++ b/Documentation/trace/function-based-events.rst
> @@ -98,7 +98,8 @@ as follows:
>   ATOM := 'u8' | 'u16' | 'u32' | 'u64' |
>           's8' | 's16' | 's32' | 's64' |
>           'x8' | 'x16' | 'x32' | 'x64' |
> -         'char' | 'short' | 'int' | 'long' | 'size_t'
> +         'char' | 'short' | 'int' | 'long' | 'size_t' |
> +	 'symbol'
>  
>   FIELD := <name> | <name> INDEX | <name> OFFSET | <name> OFFSET INDEX
>  
> @@ -243,3 +244,26 @@ The above will take the parameter value, add it by 4, then index it by two
>  8 byte words. It's the same in C as: (u64 *)((void *)param + 4)[2]
>  
>   Note: "int skb[32]" is the same as "int skb+4[31]".
> +
> +
> +Symbols (function names)
> +========================
> +
> +To display kallsyms "%pS" type of output, use the special type "symbol".
> +
> +Again, using gdb to find the offset of the "func" field of struct work_struct
> +
> +(gdb) printf "%d\n", &((struct work_struct *)0)->func
> +24

you could also use Arnaldo's pahole for this, seems like less typing:

  $ pahole ./vmlinux -C work_struct
  die__process_function: tag not supported (INVALID)!
  struct work_struct {
          atomic_long_t              data;                 /*     0     8 */
          struct list_head           entry;                /*     8    16 */
          work_func_t                func;                 /*    24     8 */

it's in 'dwarves' package

jirka

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

* Re: [PATCH 01/20 v2] tracing: Add function based events
  2018-02-08 11:20   ` Jiri Olsa
@ 2018-02-08 15:53     ` Steven Rostedt
  0 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2018-02-08 15:53 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Linus Torvalds, Ingo Molnar, Andrew Morton,
	Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu, Tom Zanussi,
	linux-rt-users, linux-trace-users, Arnaldo Carvalho de Melo,
	Clark Williams, Daniel Bristot de Oliveira, Juri Lelli

On Thu, 8 Feb 2018 12:20:14 +0100
Jiri Olsa <jolsa@redhat.com> wrote:

> On Wed, Feb 07, 2018 at 03:24:03PM -0500, Steven Rostedt wrote:
> 
> SNIP
> 
> > +
> > +void create_function_event_file(struct dentry *d_tracer)
> > +{
> > +	struct dentry *d;
> > +
> > +	d = trace_create_file("function_events", 0644, d_tracer, NULL,
> > +			      &func_event_fops);
> > +	WARN(!d, "Failed to create function_events file");
> > +}  
> 
> this fucntion seems like leftover.. can't see any code using it
> 

You are right. My original version had it created via instance, but
then I realized that the events are for all instances. I'll nuke this.

Thanks!

-- Steve

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

* Re: [PATCH 11/20 v2] tracing: Add symbol type to function based events
  2018-02-08 11:20   ` Jiri Olsa
@ 2018-02-08 15:59     ` Steven Rostedt
  2018-02-08 16:22       ` Arnaldo Carvalho de Melo
  2018-02-09 15:03     ` Masami Hiramatsu
  1 sibling, 1 reply; 27+ messages in thread
From: Steven Rostedt @ 2018-02-08 15:59 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Linus Torvalds, Ingo Molnar, Andrew Morton,
	Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu, Tom Zanussi,
	linux-rt-users, linux-trace-users, Arnaldo Carvalho de Melo,
	Clark Williams, Daniel Bristot de Oliveira, Juri Lelli

On Thu, 8 Feb 2018 12:20:31 +0100
Jiri Olsa <jolsa@redhat.com> wrote:

> > +Symbols (function names)
> > +========================
> > +
> > +To display kallsyms "%pS" type of output, use the special type "symbol".
> > +
> > +Again, using gdb to find the offset of the "func" field of struct work_struct
> > +
> > +(gdb) printf "%d\n", &((struct work_struct *)0)->func
> > +24  
> 
> you could also use Arnaldo's pahole for this, seems like less typing:
> 
>   $ pahole ./vmlinux -C work_struct
>   die__process_function: tag not supported (INVALID)!
>   struct work_struct {
>           atomic_long_t              data;                 /*     0     8 */
>           struct list_head           entry;                /*     8    16 */
>           work_func_t                func;                 /*    24     8 */
> 
> it's in 'dwarves' package

Nice, I'll have to document that:

 $ pahole ./vmlinux -C net_device |grep perm
        unsigned char              perm_addr[32];        /*   558    32 */


-- Steve

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

* Re: [PATCH 11/20 v2] tracing: Add symbol type to function based events
  2018-02-08 15:59     ` Steven Rostedt
@ 2018-02-08 16:22       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-02-08 16:22 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Jiri Olsa, linux-kernel, Linus Torvalds, Ingo Molnar,
	Andrew Morton, Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu,
	Tom Zanussi, linux-rt-users, linux-trace-users, Clark Williams,
	Daniel Bristot de Oliveira, Juri Lelli

Em Thu, Feb 08, 2018 at 10:59:00AM -0500, Steven Rostedt escreveu:
> On Thu, 8 Feb 2018 12:20:31 +0100
> Jiri Olsa <jolsa@redhat.com> wrote:
> 
> > > +Symbols (function names)
> > > +========================
> > > +
> > > +To display kallsyms "%pS" type of output, use the special type "symbol".
> > > +
> > > +Again, using gdb to find the offset of the "func" field of struct work_struct
> > > +
> > > +(gdb) printf "%d\n", &((struct work_struct *)0)->func
> > > +24  
> > 
> > you could also use Arnaldo's pahole for this, seems like less typing:
> > 
> >   $ pahole ./vmlinux -C work_struct
> >   die__process_function: tag not supported (INVALID)!
> >   struct work_struct {
> >           atomic_long_t              data;                 /*     0     8 */
> >           struct list_head           entry;                /*     8    16 */
> >           work_func_t                func;                 /*    24     8 */
> > 
> > it's in 'dwarves' package
> 
> Nice, I'll have to document that:
> 
>  $ pahole ./vmlinux -C net_device |grep perm
>         unsigned char              perm_addr[32];        /*   558    32 */

You can also use just one .o file to speed things up instead of using
vmlinux, where it ill go on parsing stuff till it finds a .o with that
struct definition, i.e.:

[acme@jouet linux]$ pahole -C work_struct ../build/v4.15.0-rc9+/kernel/sys.o 
struct work_struct {
	atomic_long_t              data;                 /*     0     8 */
	struct list_head           entry;                /*     8    16 */
	work_func_t                func;                 /*    24     8 */

	/* size: 32, cachelines: 1, members: 3 */
	/* last cacheline: 32 bytes */
};

One may prefer hexadecimal numbers:

[acme@jouet linux]$ pahole --hex -C work_struct ../build/v4.15.0-rc9+/kernel/sys.o 
struct work_struct {
	atomic_long_t              data;                 /*     0   0x8 */
	struct list_head           entry;                /*   0x8  0x10 */
	work_func_t                func;                 /*  0x18   0x8 */

	/* size: 32, cachelines: 1, members: 3 */
	/* last cacheline: 32 bytes */
};
[acme@jouet linux]$

Or even expand everything and get offsets from that work_struct to
inside the list_head or other embedded types:

[acme@jouet linux]$ pahole -E --hex -C work_struct ../build/v4.15.0-rc9+/kernel/sys.o 
struct work_struct {
	/* typedef atomic_long_t -> atomic64_t */ struct {
		long int           counter;                                              /*     0   0x8 */
	} data; /*     0   0x8 */
	struct list_head {
		struct list_head * next;                                                 /*   0x8   0x8 */
		struct list_head * prev;                                                 /*  0x10   0x8 */
	} entry; /*   0x8  0x10 */
	/* typedef work_func_t */ void                       (*func)(struct work_struct *); /*  0x18   0x8 */

	/* size: 32, cachelines: 1, members: 3 */
	/* last cacheline: 32 bytes */
};
[acme@jouet linux]$ 

A long time ago I put the expansion of task_struct somewhere... yeah,
almost a decade ago, 2008, but it was not -E, was -p (i.e. expand
pointer types):

http://vger.kernel.org/~acme/vmlinux-expand_pointers-task_struct.txt

	/* size: 1680, cachelines: 27, members: 141 */
	/* sum members: 1641, holes: 10, sum holes: 39 */
	/* bit holes: 1, sum bit holes: 31 bits */
	/* paddings: 1, sum paddings: 4 */
	/* last cacheline: 16 bytes */
};

Anyway, now we have:

[acme@jouet linux]$ pahole -C task_struct ../build/v4.15.0-rc9+/kernel/sys.o  | tail -6
	struct thread_struct       thread;               /*  6592  4352 */

	/* size: 10944, cachelines: 171, members: 4 */
	/* sum members: 10896, holes: 1, sum holes: 48 */
	/* paddings: 1, sum paddings: 48 */
};
[acme@jouet linux]$

We have way more memory and features now :-)

The number of members is because now we have that randomize_struct stuff
that creates an unamed struct inside task_struct, so we have:

struct task_struct {
        struct thread_info         thread_info;          /*     0     8 */
        volatile long int          state;                /*     8     8 */

        /* XXX 48 bytes hole, try to pack */

        /* --- cacheline 1 boundary (64 bytes) --- */
        struct {
                void *             stack;                /*    64     8 */
                atomic_t           usage;                /*    72     4 */
<SNIP>
                int                pagefault_disabled;   /*  6504     4 */

                /* XXX 4 bytes hole, try to pack */

                struct task_struct * oom_reaper_list;    /*  6512     8 */
                struct vm_struct * stack_vm_area;        /*  6520     8 */
                /* --- cacheline 102 boundary (6528 bytes) --- */
                atomic_t           stack_refcount;       /*  6528     4 */

                /* XXX 4 bytes hole, try to pack */

                void *             security;             /*  6536     8 */
        };                                               /*    64  6528 */

        /* XXX last struct has 48 bytes of padding */

        /* --- cacheline 103 boundary (6592 bytes) --- */
        struct thread_struct       thread;               /*  6592  4352 */

        /* size: 10944, cachelines: 171, members: 4 */
        /* sum members: 10896, holes: 1, sum holes: 48 */
        /* paddings: 1, sum paddings: 48 */
};

So indeed, 'just' 4 members.

I guess I'll have to add some new mode where one can ask for unnamed
structs to be removed for the sake of --reorganize and the summary
information at the end, erm...

Anyway, digressed too much :-)

- Arnaldo

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

* Re: [PATCH 11/20 v2] tracing: Add symbol type to function based events
  2018-02-08 11:20   ` Jiri Olsa
  2018-02-08 15:59     ` Steven Rostedt
@ 2018-02-09 15:03     ` Masami Hiramatsu
  1 sibling, 0 replies; 27+ messages in thread
From: Masami Hiramatsu @ 2018-02-09 15:03 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Steven Rostedt, linux-kernel, Linus Torvalds, Ingo Molnar,
	Andrew Morton, Thomas Gleixner, Peter Zijlstra, Masami Hiramatsu,
	Tom Zanussi, linux-rt-users, linux-trace-users,
	Arnaldo Carvalho de Melo, Clark Williams,
	Daniel Bristot de Oliveira, Juri Lelli

On Thu, 8 Feb 2018 12:20:31 +0100
Jiri Olsa <jolsa@redhat.com> wrote:

> On Wed, Feb 07, 2018 at 03:24:13PM -0500, Steven Rostedt wrote:
> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> > 
> > Add a special type "symbol" that will use %pS to display the field of a
> > function based event.
> > 
> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > ---
> >  Documentation/trace/function-based-events.rst | 26 +++++++++++++++++++++++++-
> >  kernel/trace/trace_event_ftrace.c             | 13 ++++++++++---
> >  2 files changed, 35 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
> > index bdb28f433bfb..f18c8f3ef330 100644
> > --- a/Documentation/trace/function-based-events.rst
> > +++ b/Documentation/trace/function-based-events.rst
> > @@ -98,7 +98,8 @@ as follows:
> >   ATOM := 'u8' | 'u16' | 'u32' | 'u64' |
> >           's8' | 's16' | 's32' | 's64' |
> >           'x8' | 'x16' | 'x32' | 'x64' |
> > -         'char' | 'short' | 'int' | 'long' | 'size_t'
> > +         'char' | 'short' | 'int' | 'long' | 'size_t' |
> > +	 'symbol'
> >  
> >   FIELD := <name> | <name> INDEX | <name> OFFSET | <name> OFFSET INDEX
> >  
> > @@ -243,3 +244,26 @@ The above will take the parameter value, add it by 4, then index it by two
> >  8 byte words. It's the same in C as: (u64 *)((void *)param + 4)[2]
> >  
> >   Note: "int skb[32]" is the same as "int skb+4[31]".
> > +
> > +
> > +Symbols (function names)
> > +========================
> > +
> > +To display kallsyms "%pS" type of output, use the special type "symbol".
> > +
> > +Again, using gdb to find the offset of the "func" field of struct work_struct
> > +
> > +(gdb) printf "%d\n", &((struct work_struct *)0)->func
> > +24
> 
> you could also use Arnaldo's pahole for this, seems like less typing:
> 
>   $ pahole ./vmlinux -C work_struct
>   die__process_function: tag not supported (INVALID)!
>   struct work_struct {
>           atomic_long_t              data;                 /*     0     8 */
>           struct list_head           entry;                /*     8    16 */
>           work_func_t                func;                 /*    24     8 */
> 
> it's in 'dwarves' package
> 

Just FYI, perf probe gives you the offset too :)

# perf probe -D "work_busy work->func"
p:probe/work_busy _text+591936 func=+24(%di):x64

Anyway, this "symbol" type is also interesting. I'll pick it.

Thanks,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

end of thread, other threads:[~2018-02-09 15:03 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-07 20:24 [PATCH 00/20 v2] tracing: Dynamically created function based events Steven Rostedt
2018-02-07 20:24 ` [PATCH 01/20 v2] tracing: Add " Steven Rostedt
2018-02-08 11:20   ` Jiri Olsa
2018-02-08 15:53     ` Steven Rostedt
2018-02-07 20:24 ` [PATCH 02/20 v2] tracing: Add documentation for " Steven Rostedt
2018-02-07 20:24 ` [PATCH 03/20 v2] tracing: Add simple arguments to " Steven Rostedt
2018-02-07 20:24 ` [PATCH 04/20 v2] tracing/x86: Add arch_get_func_args() function Steven Rostedt
2018-02-07 20:24 ` [PATCH 05/20 v2] tracing: Add hex print for dynamic ftrace based events Steven Rostedt
2018-02-07 20:24 ` [PATCH 06/20 v2] tracing: Add indirect offset to args of " Steven Rostedt
2018-02-07 20:24 ` [PATCH 07/20 v2] tracing: Add dereferencing multiple fields per arg Steven Rostedt
2018-02-07 20:24 ` [PATCH 08/20 v2] tracing: Add "unsigned" to function based events Steven Rostedt
2018-02-07 20:24 ` [PATCH 09/20 v2] tracing: Add indexing of arguments for " Steven Rostedt
2018-02-07 20:24 ` [PATCH 10/20 v2] tracing: Make func_type enums for easier comparing of arg types Steven Rostedt
2018-02-07 20:24 ` [PATCH 11/20 v2] tracing: Add symbol type to function based events Steven Rostedt
2018-02-08 11:20   ` Jiri Olsa
2018-02-08 15:59     ` Steven Rostedt
2018-02-08 16:22       ` Arnaldo Carvalho de Melo
2018-02-09 15:03     ` Masami Hiramatsu
2018-02-07 20:24 ` [PATCH 12/20 v2] tracing: Add accessing direct address from " Steven Rostedt
2018-02-07 20:24 ` [PATCH 13/20 v2] tracing: Add array type to " Steven Rostedt
2018-02-07 20:24 ` [PATCH 14/20 v2] tracing: Have char arrays be strings for " Steven Rostedt
2018-02-07 20:24 ` [PATCH 15/20 v2] tracing: Add string type for dynamic strings in " Steven Rostedt
2018-02-07 20:24 ` [PATCH 16/20 v2] tracing: Add NULL to skip args for " Steven Rostedt
2018-02-07 20:24 ` [PATCH 17/20 v2] tracing: Add indirect to indirect access " Steven Rostedt
2018-02-07 20:24 ` [PATCH 18/20 v2] tracing/perf: Allow perf to use " Steven Rostedt
2018-02-07 20:24 ` [PATCH 19/20 v2] tracing: Add error messages for failed writes to function_events Steven Rostedt
2018-02-07 20:24 ` [PATCH 20/20 v2] tracing: Add argument error message too many args for function based events Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).