linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo
@ 2020-07-02 18:53 Steven Rostedt
  2020-07-02 18:53 ` [PATCH v2 01/15] tools lib traceevent: Add API to read time information from kbuffer Steven Rostedt
                   ` (15 more replies)
  0 siblings, 16 replies; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton

Hi Arnaldo,

I was missing a few patches, here's take two. I also changed some of the
patches to state that they are coming from trace-cmd.git.

-- Steve

We noticed that the libtraceevent in trace-cmd.git is a bit out of sync with
what is in the kernel. These patches can help bring it by in sync again.


Jan Kiszka (2):
      tools lib traceevent: Add more SVM exit reasons
      tools lib traceevent: Fix reporting of unknown SVM exit reasons

Julia Cartwright (1):
      tools lib traceevent: Add plugin for decoding syscalls/sys_enter_futex

Steven Rostedt (Red Hat) (2):
      tools lib traceevent: Add API to read time information from kbuffer
      tools lib traceevent: Add plugin for tlb_flush

Steven Rostedt (VMware) (3):
      tools lib traceevent: Add offset option for function plugin
      tools lib traceevent: Add builtin handler for trace_marker_raw
      tools lib traceevent: Change to SPDX License format

Tom Zanussi (1):
      tools lib traceevent: Add proper KBUFFER_TYPE_TIME_STAMP handling

Tzvetomir Stoyanov (VMware) (6):
      tools lib traceevent: Add tep_load_plugins_hook() API
      tools lib traceevent: Add interface for options to plugins
      tools lib traceevent: Introduced new traceevent API, for adding new plugins directories.
      tools lib traceevent: Add support for more printk format specifiers
      tools lib traceevent: Optimize pretty_print() function
      tools lib traceevent: Move kernel_stack event handler to "function" plugin.

----
 tools/lib/traceevent/event-parse-local.h           |   22 +-
 tools/lib/traceevent/event-parse.c                 | 1004 +++++++++++++++-----
 tools/lib/traceevent/event-parse.h                 |   34 +-
 tools/lib/traceevent/event-plugin.c                |  261 ++++-
 tools/lib/traceevent/kbuffer-parse.c               |   43 +-
 tools/lib/traceevent/kbuffer.h                     |   19 +-
 tools/lib/traceevent/plugins/Build                 |    2 +
 tools/lib/traceevent/plugins/Makefile              |    2 +
 tools/lib/traceevent/plugins/plugin_function.c     |  123 ++-
 tools/lib/traceevent/plugins/plugin_futex.c        |  123 +++
 tools/lib/traceevent/plugins/plugin_hrtimer.c      |   17 +-
 tools/lib/traceevent/plugins/plugin_jbd2.c         |   17 +-
 tools/lib/traceevent/plugins/plugin_kmem.c         |   17 +-
 tools/lib/traceevent/plugins/plugin_kvm.c          |   42 +-
 tools/lib/traceevent/plugins/plugin_mac80211.c     |   17 +-
 tools/lib/traceevent/plugins/plugin_sched_switch.c |   17 +-
 tools/lib/traceevent/plugins/plugin_tlb.c          |   66 ++
 17 files changed, 1406 insertions(+), 420 deletions(-)
 create mode 100644 tools/lib/traceevent/plugins/plugin_futex.c
 create mode 100644 tools/lib/traceevent/plugins/plugin_tlb.c

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

* [PATCH v2 01/15] tools lib traceevent: Add API to read time information from kbuffer
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
@ 2020-07-02 18:53 ` Steven Rostedt
  2020-07-03 11:31   ` Arnaldo Carvalho de Melo
  2020-07-02 18:53 ` [PATCH v2 02/15] tools lib traceevent: Add proper KBUFFER_TYPE_TIME_STAMP handling Steven Rostedt
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton, Tzvetomir Stoyanov (VMware)

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

Add the functions kbuffer_subbuf_timestamp() and kbuffer_ptr_delta() to get
the timing data stored in the ring buffer that is used to produced the time
stamps of the records.

This is useful for tools like trace-cmd to be able to display the content of
the read data to understand why the records show the time stamps that they
do.

Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-2-tz.stoyanov@gmail.com

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
[ Ported from trace-cmd.git ]
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/kbuffer-parse.c | 28 ++++++++++++++++++++++++++++
 tools/lib/traceevent/kbuffer.h       |  2 ++
 2 files changed, 30 insertions(+)

diff --git a/tools/lib/traceevent/kbuffer-parse.c b/tools/lib/traceevent/kbuffer-parse.c
index 27f3b07fdae8..583db99aee94 100644
--- a/tools/lib/traceevent/kbuffer-parse.c
+++ b/tools/lib/traceevent/kbuffer-parse.c
@@ -546,6 +546,34 @@ int kbuffer_load_subbuffer(struct kbuffer *kbuf, void *subbuffer)
 	return 0;
 }
 
+/**
+ * kbuffer_subbuf_timestamp - read the timestamp from a sub buffer
+ * @kbuf:      The kbuffer to load
+ * @subbuf:    The subbuffer to read from.
+ *
+ * Return the timestamp from a subbuffer.
+ */
+unsigned long long kbuffer_subbuf_timestamp(struct kbuffer *kbuf, void *subbuf)
+{
+	return kbuf->read_8(subbuf);
+}
+
+/**
+ * kbuffer_ptr_delta - read the delta field from a record
+ * @kbuf:      The kbuffer to load
+ * @ptr:       The record in the buffe.
+ *
+ * Return the timestamp delta from a record
+ */
+unsigned int kbuffer_ptr_delta(struct kbuffer *kbuf, void *ptr)
+{
+	unsigned int type_len_ts;
+
+	type_len_ts = read_4(kbuf, ptr);
+	return ts4host(kbuf, type_len_ts);
+}
+
+
 /**
  * kbuffer_read_event - read the next event in the kbuffer subbuffer
  * @kbuf:	The kbuffer to read from
diff --git a/tools/lib/traceevent/kbuffer.h b/tools/lib/traceevent/kbuffer.h
index ed4d697fc137..5fa8292e341b 100644
--- a/tools/lib/traceevent/kbuffer.h
+++ b/tools/lib/traceevent/kbuffer.h
@@ -49,6 +49,8 @@ int kbuffer_load_subbuffer(struct kbuffer *kbuf, void *subbuffer);
 void *kbuffer_read_event(struct kbuffer *kbuf, unsigned long long *ts);
 void *kbuffer_next_event(struct kbuffer *kbuf, unsigned long long *ts);
 unsigned long long kbuffer_timestamp(struct kbuffer *kbuf);
+unsigned long long kbuffer_subbuf_timestamp(struct kbuffer *kbuf, void *subbuf);
+unsigned int kbuffer_ptr_delta(struct kbuffer *kbuf, void *ptr);
 
 void *kbuffer_translate_data(int swap, void *data, unsigned int *size);
 
-- 
2.26.2



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

* [PATCH v2 02/15] tools lib traceevent: Add proper KBUFFER_TYPE_TIME_STAMP handling
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
  2020-07-02 18:53 ` [PATCH v2 01/15] tools lib traceevent: Add API to read time information from kbuffer Steven Rostedt
@ 2020-07-02 18:53 ` Steven Rostedt
  2020-07-02 18:53 ` [PATCH v2 03/15] tools lib traceevent: Add tep_load_plugins_hook() API Steven Rostedt
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton, Tom Zanussi, Tzvetomir Stoyanov (VMware)

From: Tom Zanussi <zanussi@kernel.org>

Kernel commit dc4e2801d400 (ring-buffer: Redefine the unimplemented
RINGBUF_TYPE_TIME_STAMP) changed the way the ring buffer timestamps
work - after that commit the previously unimplemented
RINGBUF_TYPE_TIME_STAMP type causes the time delta to be used as a
timestamp rather than a delta to be added to the timestamp.

The trace-cmd code didn't get updated to handle this, so misinterprets
the event data for this case, which causes a cascade of errors,
including trace-report not being able to identify synthetic (or any
other) events generated by the histogram code (which uses TIME_STAMP
mode).  For example, the following triggers along with the trace-cmd
shown cause an UNKNOWN_EVENT error and trace-cmd report crash:

  # echo 'wakeup_latency  u64 lat pid_t pid char comm[16]' > /sys/kernel/debug/tracing/synthetic_events

  # echo 'hist:keys=pid:ts0=common_timestamp.usecs if comm=="ping"' > /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger
  # echo 'hist:keys=next_pid:wakeup_lat=common_timestamp.usecs-$ts0:onmatch(sched.sched_wakeup).trace(wakeup_latency,$wakeup_lat,next_pid,next_comm) if next_comm=="ping"' > /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
  # echo 'hist:keys=comm,pid,lat:wakeup_lat=lat:sort=lat' > /sys/kernel/debug/tracing/events/synthetic/wakeup_latency/trigger

  # trace-cmd record -e wakeup_latency -e sched_wakeup -f comm==\"ping\" ping localhost -c 5

  # trace-cmd report
  CPU 0 is empty
  CPU 1 is empty
  CPU 2 is empty
  CPU 3 is empty
  CPU 5 is empty
  CPU 6 is empty
  CPU 7 is empty
  cpus=8
    ug! no event found for type 0
  [UNKNOWN TYPE 0]
    ug! no event found for type 11520
  Segmentation fault (core dumped)

After this patch we get the correct interpretation and the events are
shown properly:

  # trace-cmd report
  CPU 0 is empty
  CPU 1 is empty
  CPU 2 is empty
  CPU 3 is empty
  CPU 5 is empty
  CPU 6 is empty
  CPU 7 is empty
  cpus=8
          <idle>-0     [004] 23284.341392: sched_wakeup:         ping:12031 [120] success=1 CPU:004
          <idle>-0     [004] 23284.341464: wakeup_latency:       lat=58, pid=12031, comm=ping
          <idle>-0     [004] 23285.365303: sched_wakeup:         ping:12031 [120] success=1 CPU:004
          <idle>-0     [004] 23285.365382: wakeup_latency:       lat=64, pid=12031, comm=ping
          <idle>-0     [004] 23286.389290: sched_wakeup:         ping:12031 [120] success=1 CPU:004
          <idle>-0     [004] 23286.389378: wakeup_latency:       lat=72, pid=12031, comm=ping
          <idle>-0     [004] 23287.413213: sched_wakeup:         ping:12031 [120] success=1 CPU:004
          <idle>-0     [004] 23287.413291: wakeup_latency:       lat=64, pid=12031, comm=ping

Link: http://lkml.kernel.org/r/1567628224.13841.4.camel@kernel.org
Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-3-tz.stoyanov@gmail.com

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
[ Ported from trace-cmd.git ]
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/kbuffer-parse.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/lib/traceevent/kbuffer-parse.c b/tools/lib/traceevent/kbuffer-parse.c
index 583db99aee94..f1640d651c8a 100644
--- a/tools/lib/traceevent/kbuffer-parse.c
+++ b/tools/lib/traceevent/kbuffer-parse.c
@@ -361,6 +361,7 @@ translate_data(struct kbuffer *kbuf, void *data, void **rptr,
 		break;
 
 	case KBUFFER_TYPE_TIME_EXTEND:
+	case KBUFFER_TYPE_TIME_STAMP:
 		extend = read_4(kbuf, data);
 		data += 4;
 		extend <<= TS_SHIFT;
@@ -369,10 +370,6 @@ translate_data(struct kbuffer *kbuf, void *data, void **rptr,
 		*length = 0;
 		break;
 
-	case KBUFFER_TYPE_TIME_STAMP:
-		data += 12;
-		*length = 0;
-		break;
 	case 0:
 		*length = read_4(kbuf, data) - 4;
 		*length = (*length + 3) & ~3;
@@ -397,7 +394,11 @@ static unsigned int update_pointers(struct kbuffer *kbuf)
 
 	type_len = translate_data(kbuf, ptr, &ptr, &delta, &length);
 
-	kbuf->timestamp += delta;
+	if (type_len == KBUFFER_TYPE_TIME_STAMP)
+		kbuf->timestamp = delta;
+	else
+		kbuf->timestamp += delta;
+
 	kbuf->index = calc_index(kbuf, ptr);
 	kbuf->next = kbuf->index + length;
 
@@ -454,7 +455,9 @@ static int __next_event(struct kbuffer *kbuf)
 		if (kbuf->next >= kbuf->size)
 			return -1;
 		type = update_pointers(kbuf);
-	} while (type == KBUFFER_TYPE_TIME_EXTEND || type == KBUFFER_TYPE_PADDING);
+	} while (type == KBUFFER_TYPE_TIME_EXTEND ||
+		 type == KBUFFER_TYPE_TIME_STAMP ||
+		 type == KBUFFER_TYPE_PADDING);
 
 	return 0;
 }
-- 
2.26.2



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

* [PATCH v2 03/15] tools lib traceevent: Add tep_load_plugins_hook() API
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
  2020-07-02 18:53 ` [PATCH v2 01/15] tools lib traceevent: Add API to read time information from kbuffer Steven Rostedt
  2020-07-02 18:53 ` [PATCH v2 02/15] tools lib traceevent: Add proper KBUFFER_TYPE_TIME_STAMP handling Steven Rostedt
@ 2020-07-02 18:53 ` Steven Rostedt
  2020-07-07 14:55   ` Namhyung Kim
  2020-07-02 18:53 ` [PATCH v2 04/15] tools lib traceevent: Add interface for options to plugins Steven Rostedt
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton, Tzvetomir Stoyanov (VMware)

From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>

Add the API function tep_load_plugins_hook() to the traceevent API to allow
tools a common method to load in the plugins that are part of the lib
traceevent library.

Link: http://lore.kernel.org/linux-trace-devel/20190802110101.14759-4-tz.stoyanov@gmail.com
Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-4-tz.stoyanov@gmail.com

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse.h  |  6 ++++++
 tools/lib/traceevent/event-plugin.c | 19 +++++++++----------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index b77837f75a0d..776c7c24ee79 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -396,6 +396,12 @@ struct tep_plugin_list;
 struct tep_plugin_list *tep_load_plugins(struct tep_handle *tep);
 void tep_unload_plugins(struct tep_plugin_list *plugin_list,
 			struct tep_handle *tep);
+void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
+			   void (*load_plugin)(struct tep_handle *tep,
+					       const char *path,
+					       const char *name,
+					       void *data),
+			   void *data);
 char **tep_plugin_list_options(void);
 void tep_plugin_free_options_list(char **list);
 int tep_plugin_add_options(const char *name,
diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
index e1f7ddd5a6cf..b53d9a53bcf9 100644
--- a/tools/lib/traceevent/event-plugin.c
+++ b/tools/lib/traceevent/event-plugin.c
@@ -365,20 +365,19 @@ load_plugins_dir(struct tep_handle *tep, const char *suffix,
 	closedir(dir);
 }
 
-static void
-load_plugins(struct tep_handle *tep, const char *suffix,
-	     void (*load_plugin)(struct tep_handle *tep,
-				 const char *path,
-				 const char *name,
-				 void *data),
-	     void *data)
+void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
+			   void (*load_plugin)(struct tep_handle *tep,
+					       const char *path,
+					       const char *name,
+					       void *data),
+			   void *data)
 {
 	char *home;
 	char *path;
 	char *envdir;
 	int ret;
 
-	if (tep->flags & TEP_DISABLE_PLUGINS)
+	if (tep && tep->flags & TEP_DISABLE_PLUGINS)
 		return;
 
 	/*
@@ -386,7 +385,7 @@ load_plugins(struct tep_handle *tep, const char *suffix,
 	 * check that first.
 	 */
 #ifdef PLUGIN_DIR
-	if (!(tep->flags & TEP_DISABLE_SYS_PLUGINS))
+	if (!tep || !(tep->flags & TEP_DISABLE_SYS_PLUGINS))
 		load_plugins_dir(tep, suffix, PLUGIN_DIR,
 				 load_plugin, data);
 #endif
@@ -423,7 +422,7 @@ tep_load_plugins(struct tep_handle *tep)
 {
 	struct tep_plugin_list *list = NULL;
 
-	load_plugins(tep, ".so", load_plugin, &list);
+	tep_load_plugins_hook(tep, ".so", load_plugin, &list);
 	return list;
 }
 
-- 
2.26.2



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

* [PATCH v2 04/15] tools lib traceevent: Add interface for options to plugins
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
                   ` (2 preceding siblings ...)
  2020-07-02 18:53 ` [PATCH v2 03/15] tools lib traceevent: Add tep_load_plugins_hook() API Steven Rostedt
@ 2020-07-02 18:53 ` Steven Rostedt
  2020-07-07 15:03   ` Namhyung Kim
  2020-07-02 18:53 ` [PATCH v2 05/15] tools lib traceevent: Introduced new traceevent API, for adding new plugins directories Steven Rostedt
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton, Tzvetomir Stoyanov (VMware)

From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>

Add tep_plugin_add_option() and tep_plugin_print_options() to lib traceevent
library that allows plugins to have their own options. For example, the
function plugin by default does not print the parent, as it uses the parent
to do the indenting. The "parent" option is created by the function plugin
that will print the parent of the function like it does in the trace file.

The tep_plugin_print_options() will print out the list of options that a
plugin has defined.

Link: http://lore.kernel.org/linux-trace-devel/20190802110101.14759-3-tz.stoyanov@gmail.com
Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-5-tz.stoyanov@gmail.com

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse.h  |   2 +
 tools/lib/traceevent/event-plugin.c | 172 ++++++++++++++++++++++++++++
 2 files changed, 174 insertions(+)

diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 776c7c24ee79..02c0438527de 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -406,7 +406,9 @@ char **tep_plugin_list_options(void);
 void tep_plugin_free_options_list(char **list);
 int tep_plugin_add_options(const char *name,
 			   struct tep_plugin_option *options);
+int tep_plugin_add_option(const char *name, const char *val);
 void tep_plugin_remove_options(struct tep_plugin_option *options);
+void tep_plugin_print_options(struct trace_seq *s);
 void tep_print_plugins(struct trace_seq *s,
 			const char *prefix, const char *suffix,
 			const struct tep_plugin_list *list);
diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
index b53d9a53bcf9..e8f4329ba8e0 100644
--- a/tools/lib/traceevent/event-plugin.c
+++ b/tools/lib/traceevent/event-plugin.c
@@ -13,6 +13,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 #include <dirent.h>
+#include <errno.h>
 #include "event-parse.h"
 #include "event-parse-local.h"
 #include "event-utils.h"
@@ -247,6 +248,166 @@ void tep_plugin_remove_options(struct tep_plugin_option *options)
 	}
 }
 
+static void parse_option_name(char **option, char **plugin)
+{
+	char *p;
+
+	*plugin = NULL;
+
+	if ((p = strstr(*option, ":"))) {
+		*plugin = *option;
+		*p = '\0';
+		*option = strdup(p + 1);
+		if (!*option)
+			return;
+	}
+}
+
+static struct tep_plugin_option *
+find_registered_option(const char *plugin, const char *option)
+{
+	struct registered_plugin_options *reg;
+	struct tep_plugin_option *op;
+	const char *op_plugin;
+
+	for (reg = registered_options; reg; reg = reg->next) {
+		for (op = reg->options; op->name; op++) {
+			if (op->plugin_alias)
+				op_plugin = op->plugin_alias;
+			else
+				op_plugin = op->file;
+
+			if (plugin && strcmp(plugin, op_plugin) != 0)
+				continue;
+			if (strcmp(option, op->name) != 0)
+				continue;
+
+			return op;
+		}
+	}
+
+	return NULL;
+}
+
+static int process_option(const char *plugin, const char *option, const char *val)
+{
+	struct tep_plugin_option *op;
+
+	op = find_registered_option(plugin, option);
+	if (!op)
+		return 0;
+
+	return update_option_value(op, val);
+}
+
+/**
+ * tep_plugin_add_option - add an option/val pair to set plugin options
+ * @name: The name of the option (format: <plugin>:<option> or just <option>)
+ * @val: (optiona) the value for the option
+ *
+ * Modify a plugin option. If @val is given than the value of the option
+ * is set (note, some options just take a boolean, so @val must be either
+ * "1" or "0" or "true" or "false").
+ */
+int tep_plugin_add_option(const char *name, const char *val)
+{
+	struct trace_plugin_options *op;
+	char *option_str;
+	char *plugin;
+
+	option_str = strdup(name);
+	if (!option_str)
+		return -ENOMEM;
+
+	parse_option_name(&option_str, &plugin);
+
+	/* If the option exists, update the val */
+	for (op = trace_plugin_options; op; op = op->next) {
+		/* Both must be NULL or not NULL */
+		if ((!plugin || !op->plugin) && plugin != op->plugin)
+			continue;
+		if (plugin && strcmp(plugin, op->plugin) != 0)
+			continue;
+		if (strcmp(op->option, option_str) != 0)
+			continue;
+
+		/* update option */
+		free(op->value);
+		if (val) {
+			op->value = strdup(val);
+			if (!op->value)
+				goto out_free;
+		} else
+			op->value = NULL;
+
+		/* plugin and option_str don't get freed at the end */
+		free(plugin);
+		free(option_str);
+
+		plugin = op->plugin;
+		option_str = op->option;
+		break;
+	}
+
+	/* If not found, create */
+	if (!op) {
+		op = malloc(sizeof(*op));
+		if (!op)
+			return -ENOMEM;
+		memset(op, 0, sizeof(*op));
+		op->next = trace_plugin_options;
+		trace_plugin_options = op;
+
+		op->plugin = plugin;
+		op->option = option_str;
+
+		if (val) {
+			op->value = strdup(val);
+			if (!op->value)
+				goto out_free;
+		}
+	}
+
+	return process_option(plugin, option_str, val);
+ out_free:
+	free(option_str);
+	return -ENOMEM;
+}
+
+static void print_op_data(struct trace_seq *s, const char *name,
+			  const char *op)
+{
+	if (op)
+		trace_seq_printf(s, "%8s:\t%s\n", name, op);
+}
+
+/**
+ * tep_plugin_print_options - print out the registered plugin options
+ * @s: The trace_seq descriptor to write the plugin options into
+ *
+ * Writes a list of options into trace_seq @s.
+ */
+void tep_plugin_print_options(struct trace_seq *s)
+{
+	struct registered_plugin_options *reg;
+	struct tep_plugin_option *op;
+
+	for (reg = registered_options; reg; reg = reg->next) {
+		if (reg != registered_options)
+			trace_seq_printf(s, "============\n");
+		for (op = reg->options; op->name; op++) {
+			if (op != reg->options)
+				trace_seq_printf(s, "------------\n");
+			print_op_data(s, "file", op->file);
+			print_op_data(s, "plugin", op->plugin_alias);
+			print_op_data(s, "option", op->name);
+			print_op_data(s, "desc", op->description);
+			print_op_data(s, "value", op->value);
+			trace_seq_printf(s, "%8s:\t%d\n", "set", op->set);
+		}
+	}
+}
+
 /**
  * tep_print_plugins - print out the list of plugins loaded
  * @s: the trace_seq descripter to write to
@@ -273,6 +434,7 @@ load_plugin(struct tep_handle *tep, const char *path,
 	    const char *file, void *data)
 {
 	struct tep_plugin_list **plugin_list = data;
+	struct tep_plugin_option *options;
 	tep_plugin_load_func func;
 	struct tep_plugin_list *list;
 	const char *alias;
@@ -297,6 +459,16 @@ load_plugin(struct tep_handle *tep, const char *path,
 	if (!alias)
 		alias = file;
 
+	options = dlsym(handle, TEP_PLUGIN_OPTIONS_NAME);
+	if (options) {
+		while (options->name) {
+			ret = update_option(alias, options);
+			if (ret < 0)
+				goto out_free;
+			options++;
+		}
+	}
+
 	func = dlsym(handle, TEP_PLUGIN_LOADER_NAME);
 	if (!func) {
 		warning("could not find func '%s' in plugin '%s'\n%s\n",
-- 
2.26.2



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

* [PATCH v2 05/15] tools lib traceevent: Introduced new traceevent API, for adding new plugins directories.
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
                   ` (3 preceding siblings ...)
  2020-07-02 18:53 ` [PATCH v2 04/15] tools lib traceevent: Add interface for options to plugins Steven Rostedt
@ 2020-07-02 18:53 ` Steven Rostedt
  2020-07-07 15:06   ` Namhyung Kim
  2020-07-02 18:53 ` [PATCH v2 06/15] tools lib traceevent: Add support for more printk format specifiers Steven Rostedt
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton, Tzvetomir Stoyanov (VMware)

From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>

Implement new traceevent plugin API, which can be used to add new plugins
directories:
enum tep_plugin_load_priority {
	TEP_PLUGIN_FIRST,
	TEP_PLUGIN_LAST,
};
int tep_add_plugin_path(struct tep_handle *tep, char *path,
			enum tep_plugin_load_priority prio);

It adds the "path" as new plugin directory, in the context of
the handler "tep". The tep_load_plugins() API searches for plugins
in this new location. Depending of the priority "prio", the plugins
from this directory are loaded before (TEP_PLUGIN_FIRST) or after
(TEP_PLUGIN_LAST) the ordinary libtraceevent plugin locations.

Link: http://lore.kernel.org/linux-trace-devel/20191007114947.17104-2-tz.stoyanov@gmail.com
Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-6-tz.stoyanov@gmail.com

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse-local.h |  5 +-
 tools/lib/traceevent/event-parse.c       |  1 +
 tools/lib/traceevent/event-parse.h       |  7 +++
 tools/lib/traceevent/event-plugin.c      | 70 ++++++++++++++++++++++++
 4 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
index cee469803a34..96a0b0ca0675 100644
--- a/tools/lib/traceevent/event-parse-local.h
+++ b/tools/lib/traceevent/event-parse-local.h
@@ -13,6 +13,7 @@ struct func_map;
 struct func_list;
 struct event_handler;
 struct func_resolver;
+struct tep_plugins_dir;
 
 struct tep_handle {
 	int ref_count;
@@ -47,7 +48,6 @@ struct tep_handle {
 	struct printk_list *printklist;
 	unsigned int printk_count;
 
-
 	struct tep_event **events;
 	int nr_events;
 	struct tep_event **sort_events;
@@ -81,10 +81,13 @@ struct tep_handle {
 
 	/* cache */
 	struct tep_event *last_event;
+
+	struct tep_plugins_dir *plugins_dir;
 };
 
 void tep_free_event(struct tep_event *event);
 void tep_free_format_field(struct tep_format_field *field);
+void tep_free_plugin_paths(struct tep_handle *tep);
 
 unsigned short tep_data2host2(struct tep_handle *tep, unsigned short data);
 unsigned int tep_data2host4(struct tep_handle *tep, unsigned int data);
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index e1bd2a93c6db..064c100d2d5a 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -7065,6 +7065,7 @@ void tep_free(struct tep_handle *tep)
 	free(tep->events);
 	free(tep->sort_events);
 	free(tep->func_resolver);
+	tep_free_plugin_paths(tep);
 
 	free(tep);
 }
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 02c0438527de..91f462f5a606 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -393,6 +393,13 @@ struct tep_plugin_list;
 
 #define INVALID_PLUGIN_LIST_OPTION	((char **)((unsigned long)-1))
 
+enum tep_plugin_load_priority {
+	TEP_PLUGIN_FIRST,
+	TEP_PLUGIN_LAST,
+};
+
+int tep_add_plugin_path(struct tep_handle *tep, char *path,
+			enum tep_plugin_load_priority prio);
 struct tep_plugin_list *tep_load_plugins(struct tep_handle *tep);
 void tep_unload_plugins(struct tep_plugin_list *plugin_list,
 			struct tep_handle *tep);
diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
index e8f4329ba8e0..1d4f1809cf17 100644
--- a/tools/lib/traceevent/event-plugin.c
+++ b/tools/lib/traceevent/event-plugin.c
@@ -39,6 +39,12 @@ struct tep_plugin_list {
 	void			*handle;
 };
 
+struct tep_plugins_dir {
+	struct tep_plugins_dir		*next;
+	char				*path;
+	enum tep_plugin_load_priority	prio;
+};
+
 static void lower_case(char *str)
 {
 	if (!str)
@@ -544,6 +550,7 @@ void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
 					       void *data),
 			   void *data)
 {
+	struct tep_plugins_dir *dir = NULL;
 	char *home;
 	char *path;
 	char *envdir;
@@ -552,6 +559,15 @@ void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
 	if (tep && tep->flags & TEP_DISABLE_PLUGINS)
 		return;
 
+	if (tep)
+		dir = tep->plugins_dir;
+	while (dir) {
+		if (dir->prio == TEP_PLUGIN_FIRST)
+			load_plugins_dir(tep, suffix, dir->path,
+					 load_plugin, data);
+		dir = dir->next;
+	}
+
 	/*
 	 * If a system plugin directory was defined,
 	 * check that first.
@@ -586,6 +602,15 @@ void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
 
 	load_plugins_dir(tep, suffix, path, load_plugin, data);
 
+	if (tep)
+		dir = tep->plugins_dir;
+	while (dir) {
+		if (dir->prio == TEP_PLUGIN_LAST)
+			load_plugins_dir(tep, suffix, dir->path,
+					 load_plugin, data);
+		dir = dir->next;
+	}
+
 	free(path);
 }
 
@@ -598,6 +623,51 @@ tep_load_plugins(struct tep_handle *tep)
 	return list;
 }
 
+/**
+ * tep_add_plugin_path - Add a new plugin directory.
+ * @tep: Trace event handler.
+ * @path: Path to a directory. All files with extension .so in that
+ *	  directory will be loaded as plugins.
+ *@prio: Load priority of the plugins in that directory.
+ *
+ * Returns -1 in case of an error, 0 otherwise.
+ */
+int tep_add_plugin_path(struct tep_handle *tep, char *path,
+			enum tep_plugin_load_priority prio)
+{
+	struct tep_plugins_dir *dir;
+
+	if (!tep || !path)
+		return -1;
+
+	dir = calloc(1, sizeof(*dir));
+	if (!dir)
+		return -1;
+
+	dir->path = strdup(path);
+	dir->prio = prio;
+	dir->next = tep->plugins_dir;
+	tep->plugins_dir = dir;
+
+	return 0;
+}
+
+void tep_free_plugin_paths(struct tep_handle *tep)
+{
+	struct tep_plugins_dir *dir;
+
+	if (!tep)
+		return;
+
+	dir = tep->plugins_dir;
+	while (dir) {
+		tep->plugins_dir = tep->plugins_dir->next;
+		free(dir->path);
+		free(dir);
+		dir = tep->plugins_dir;
+	}
+}
+
 void
 tep_unload_plugins(struct tep_plugin_list *plugin_list, struct tep_handle *tep)
 {
-- 
2.26.2



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

* [PATCH v2 06/15] tools lib traceevent: Add support for more printk format specifiers
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
                   ` (4 preceding siblings ...)
  2020-07-02 18:53 ` [PATCH v2 05/15] tools lib traceevent: Introduced new traceevent API, for adding new plugins directories Steven Rostedt
@ 2020-07-02 18:53 ` Steven Rostedt
  2020-07-07 15:09   ` Namhyung Kim
  2020-07-02 18:53 ` [PATCH v2 07/15] tools lib traceevent: Optimize pretty_print() function Steven Rostedt
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton, Johannes Berg, Tzvetomir Stoyanov (VMware)

From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>

The printk format specifiers used in event's print format files extend
the standard printf formats. There are a lot of new options related to
printing pointers and kernel specific structures. Currently trace-cmd
does not support many of them.

Support for these new printk specifiers is added to the pretty_print()
function:
 - UUID/GUID address: %pU[bBlL]
 - Raw buffer as a hex string: %*ph[CDN]

These are improved:
 - MAC address: %pMF, %pM and %pmR
 - IPv4 adderss: %p[Ii]4[hnbl]

Function pretty_print() is refactored. The logic for printing pointers
%p[...] is moved to its own function.

Link: https://lore.kernel.org/linux-trace-devel/20200515053754.3695335-1-tz.stoyanov@gmail.com
Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-7-tz.stoyanov@gmail.com

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=207605
Reported-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse.c | 363 +++++++++++++++++++++++------
 1 file changed, 289 insertions(+), 74 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 064c100d2d5a..c28cbba34d39 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4510,43 +4510,93 @@ get_bprint_format(void *data, int size __maybe_unused,
 	return format;
 }
 
-static void print_mac_arg(struct trace_seq *s, int mac, void *data, int size,
-			  struct tep_event *event, struct tep_print_arg *arg)
+static int print_mac_arg(struct trace_seq *s, const char *format,
+			 void *data, int size, struct tep_event *event,
+			 struct tep_print_arg *arg)
 {
-	unsigned char *buf;
 	const char *fmt = "%.2x:%.2x:%.2x:%.2x:%.2x:%.2x";
+	bool reverse = false;
+	unsigned char *buf;
+	int ret = 0;
 
 	if (arg->type == TEP_PRINT_FUNC) {
 		process_defined_func(s, data, size, event, arg);
-		return;
+		return 0;
 	}
 
 	if (arg->type != TEP_PRINT_FIELD) {
 		trace_seq_printf(s, "ARG TYPE NOT FIELD BUT %d",
 				 arg->type);
-		return;
+		return 0;
 	}
 
-	if (mac == 'm')
+	if (format[0] == 'm') {
 		fmt = "%.2x%.2x%.2x%.2x%.2x%.2x";
+	} else if (format[0] == 'M' && format[1] == 'F') {
+		fmt = "%.2x-%.2x-%.2x-%.2x-%.2x-%.2x";
+		ret++;
+	}
+	if (format[1] == 'R') {
+		reverse = true;
+		ret++;
+	}
+
 	if (!arg->field.field) {
 		arg->field.field =
 			tep_find_any_field(event, arg->field.name);
 		if (!arg->field.field) {
 			do_warning_event(event, "%s: field %s not found",
 					 __func__, arg->field.name);
-			return;
+			return ret;
 		}
 	}
 	if (arg->field.field->size != 6) {
 		trace_seq_printf(s, "INVALIDMAC");
-		return;
+		return ret;
 	}
+
 	buf = data + arg->field.field->offset;
-	trace_seq_printf(s, fmt, buf[0], buf[1], buf[2], buf[3], buf[4], buf[5]);
+	if (reverse)
+		trace_seq_printf(s, fmt, buf[5], buf[4], buf[3], buf[2], buf[1], buf[0]);
+	else
+		trace_seq_printf(s, fmt, buf[0], buf[1], buf[2], buf[3], buf[4], buf[5]);
+
+	return ret;
+}
+
+static int parse_ip4_print_args(struct tep_handle *tep,
+				const char *ptr, bool *reverse)
+{
+	int ret = 0;
+
+	*reverse = false;
+
+	/* hnbl */
+	switch (*ptr) {
+	case 'h':
+		if (tep->file_bigendian)
+			*reverse = false;
+		else
+			*reverse = true;
+		ret++;
+	break;
+	case 'l':
+		*reverse = true;
+		ret++;
+	break;
+	case 'n':
+	case 'b':
+		ret++;
+		/* fall through */
+	default:
+		*reverse = false;
+		break;
+	}
+
+	return ret;
 }
 
-static void print_ip4_addr(struct trace_seq *s, char i, unsigned char *buf)
+static void print_ip4_addr(struct trace_seq *s, char i, bool reverse, unsigned char *buf)
 {
 	const char *fmt;
 
@@ -4555,7 +4605,11 @@ static void print_ip4_addr(struct trace_seq *s, char i, unsigned char *buf)
 	else
 		fmt = "%d.%d.%d.%d";
 
-	trace_seq_printf(s, fmt, buf[0], buf[1], buf[2], buf[3]);
+	if (reverse)
+		trace_seq_printf(s, fmt, buf[3], buf[2], buf[1], buf[0]);
+	else
+		trace_seq_printf(s, fmt, buf[0], buf[1], buf[2], buf[3]);
+
 }
 
 static inline bool ipv6_addr_v4mapped(const struct in6_addr *a)
@@ -4638,7 +4692,7 @@ static void print_ip6c_addr(struct trace_seq *s, unsigned char *addr)
 	if (useIPv4) {
 		if (needcolon)
 			trace_seq_printf(s, ":");
-		print_ip4_addr(s, 'I', &in6.s6_addr[12]);
+		print_ip4_addr(s, 'I', false, &in6.s6_addr[12]);
 	}
 
 	return;
@@ -4667,16 +4721,20 @@ static int print_ipv4_arg(struct trace_seq *s, const char *ptr, char i,
 			  void *data, int size, struct tep_event *event,
 			  struct tep_print_arg *arg)
 {
+	bool reverse = false;
 	unsigned char *buf;
+	int ret;
+
+	ret = parse_ip4_print_args(event->tep, ptr, &reverse);
 
 	if (arg->type == TEP_PRINT_FUNC) {
 		process_defined_func(s, data, size, event, arg);
-		return 0;
+		return ret;
 	}
 
 	if (arg->type != TEP_PRINT_FIELD) {
 		trace_seq_printf(s, "ARG TYPE NOT FIELD BUT %d", arg->type);
-		return 0;
+		return ret;
 	}
 
 	if (!arg->field.field) {
@@ -4685,7 +4743,7 @@ static int print_ipv4_arg(struct trace_seq *s, const char *ptr, char i,
 		if (!arg->field.field) {
 			do_warning("%s: field %s not found",
 				   __func__, arg->field.name);
-			return 0;
+			return ret;
 		}
 	}
 
@@ -4693,11 +4751,12 @@ static int print_ipv4_arg(struct trace_seq *s, const char *ptr, char i,
 
 	if (arg->field.field->size != 4) {
 		trace_seq_printf(s, "INVALIDIPv4");
-		return 0;
+		return ret;
 	}
-	print_ip4_addr(s, i, buf);
 
-	return 0;
+	print_ip4_addr(s, i, reverse, buf);
+	return ret;
+
 }
 
 static int print_ipv6_arg(struct trace_seq *s, const char *ptr, char i,
@@ -4757,7 +4816,9 @@ static int print_ipsa_arg(struct trace_seq *s, const char *ptr, char i,
 	char have_c = 0, have_p = 0;
 	unsigned char *buf;
 	struct sockaddr_storage *sa;
+	bool reverse = false;
 	int rc = 0;
+	int ret;
 
 	/* pISpc */
 	if (i == 'I') {
@@ -4772,6 +4833,9 @@ static int print_ipsa_arg(struct trace_seq *s, const char *ptr, char i,
 			rc++;
 		}
 	}
+	ret = parse_ip4_print_args(event->tep, ptr, &reverse);
+	ptr += ret;
+	rc += ret;
 
 	if (arg->type == TEP_PRINT_FUNC) {
 		process_defined_func(s, data, size, event, arg);
@@ -4803,7 +4867,7 @@ static int print_ipsa_arg(struct trace_seq *s, const char *ptr, char i,
 			return rc;
 		}
 
-		print_ip4_addr(s, i, (unsigned char *) &sa4->sin_addr);
+		print_ip4_addr(s, i, reverse, (unsigned char *) &sa4->sin_addr);
 		if (have_p)
 			trace_seq_printf(s, ":%d", ntohs(sa4->sin_port));
 
@@ -4837,25 +4901,20 @@ static int print_ip_arg(struct trace_seq *s, const char *ptr,
 			struct tep_print_arg *arg)
 {
 	char i = *ptr;  /* 'i' or 'I' */
-	char ver;
-	int rc = 0;
-
-	ptr++;
-	rc++;
+	int rc = 1;
 
-	ver = *ptr;
+	/* IP version */
 	ptr++;
-	rc++;
 
-	switch (ver) {
+	switch (*ptr) {
 	case '4':
-		rc += print_ipv4_arg(s, ptr, i, data, size, event, arg);
+		rc += print_ipv4_arg(s, ptr + 1, i, data, size, event, arg);
 		break;
 	case '6':
-		rc += print_ipv6_arg(s, ptr, i, data, size, event, arg);
+		rc += print_ipv6_arg(s, ptr + 1, i, data, size, event, arg);
 		break;
 	case 'S':
-		rc += print_ipsa_arg(s, ptr, i, data, size, event, arg);
+		rc += print_ipsa_arg(s, ptr + 1, i, data, size, event, arg);
 		break;
 	default:
 		return 0;
@@ -4864,6 +4923,133 @@ static int print_ip_arg(struct trace_seq *s, const char *ptr,
 	return rc;
 }
 
+static const int guid_index[16] = {3, 2, 1, 0, 5, 4, 7, 6, 8, 9, 10, 11, 12, 13, 14, 15};
+static const int uuid_index[16] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15};
+
+static int print_uuid_arg(struct trace_seq *s, const char *ptr,
+			void *data, int size, struct tep_event *event,
+			struct tep_print_arg *arg)
+{
+	const int *index = uuid_index;
+	char *format = "%02x";
+	int ret = 0;
+	char *buf;
+	int i;
+
+	switch (*(ptr + 1)) {
+	case 'L':
+		format = "%02X";
+		/* fall through */
+	case 'l':
+		index = guid_index;
+		ret++;
+		break;
+	case 'B':
+		format = "%02X";
+		/* fall through */
+	case 'b':
+		ret++;
+		break;
+	}
+
+	if (arg->type == TEP_PRINT_FUNC) {
+		process_defined_func(s, data, size, event, arg);
+		return ret;
+	}
+
+	if (arg->type != TEP_PRINT_FIELD) {
+		trace_seq_printf(s, "ARG TYPE NOT FIELD BUT %d", arg->type);
+		return ret;
+	}
+
+	if (!arg->field.field) {
+		arg->field.field =
+			tep_find_any_field(event, arg->field.name);
+		if (!arg->field.field) {
+			do_warning("%s: field %s not found",
+				   __func__, arg->field.name);
+			return ret;
+		}
+	}
+
+	if (arg->field.field->size != 16) {
+		trace_seq_printf(s, "INVALIDUUID");
+		return ret;
+	}
+
+	buf = data + arg->field.field->offset;
+
+	for (i = 0; i < 16; i++) {
+		trace_seq_printf(s, format, buf[index[i]] & 0xff);
+		switch (i) {
+		case 3:
+		case 5:
+		case 7:
+		case 9:
+			trace_seq_printf(s, "-");
+			break;
+		}
+	}
+
+	return ret;
+}
+
+static int print_raw_buff_arg(struct trace_seq *s, const char *ptr,
+			      void *data, int size, struct tep_event *event,
+			      struct tep_print_arg *arg, int print_len)
+{
+	int plen = print_len;
+	char *delim = " ";
+	int ret = 0;
+	char *buf;
+	int i;
+	unsigned long offset;
+	int arr_len;
+
+	switch (*(ptr + 1)) {
+	case 'C':
+		delim = ":";
+		ret++;
+		break;
+	case 'D':
+		delim = "-";
+		ret++;
+		break;
+	case 'N':
+		delim = "";
+		ret++;
+		break;
+	}
+
+	if (arg->type == TEP_PRINT_FUNC) {
+		process_defined_func(s, data, size, event, arg);
+		return ret;
+	}
+
+	if (arg->type != TEP_PRINT_DYNAMIC_ARRAY) {
+		trace_seq_printf(s, "ARG TYPE NOT FIELD BUT %d", arg->type);
+		return ret;
+	}
+
+	offset = tep_read_number(event->tep,
+				 data + arg->dynarray.field->offset,
+				 arg->dynarray.field->size);
+	arr_len = (unsigned long long)(offset >> 16);
+	buf = data + (offset & 0xffff);
+
+	if (arr_len < plen)
+		plen = arr_len;
+
+	if (plen < 1)
+		return ret;
+
+	trace_seq_printf(s, "%02x", buf[0] & 0xff);
+	for (i = 1; i < plen; i++)
+		trace_seq_printf(s, "%s%02x", delim, buf[i] & 0xff);
+
+	return ret;
+}
+
 static int is_printable_array(char *p, unsigned int len)
 {
 	unsigned int i;
@@ -4952,6 +5138,68 @@ void tep_print_fields(struct trace_seq *s, void *data,
 	}
 }
 
+static int print_function(struct trace_seq *s, const char *format,
+			  void *data, int size, struct tep_event *event,
+			  struct tep_print_arg *arg)
+{
+	struct func_map *func;
+	unsigned long long val;
+
+	val = eval_num_arg(data, size, event, arg);
+	func = find_func(event->tep, val);
+	if (func) {
+		trace_seq_puts(s, func->func);
+		if (*format == 'F' || *format == 'S')
+			trace_seq_printf(s, "+0x%llx", val - func->addr);
+	} else {
+		if (event->tep->long_size == 4)
+			trace_seq_printf(s, "0x%lx", (long)val);
+		else
+			trace_seq_printf(s, "0x%llx", (long long)val);
+	}
+
+	return 0;
+}
+
+static int print_pointer(struct trace_seq *s, const char *format, int plen,
+			 void *data, int size,
+			 struct tep_event *event, struct tep_print_arg *arg)
+{
+	unsigned long long val;
+	int ret = 1;
+
+	switch (*format) {
+	case 'F':
+	case 'f':
+	case 'S':
+	case 's':
+		ret += print_function(s, format, data, size, event, arg);
+		break;
+	case 'M':
+	case 'm':
+		ret += print_mac_arg(s, format, data, size, event, arg);
+		break;
+	case 'I':
+	case 'i':
+		ret += print_ip_arg(s, format, data, size, event, arg);
+		break;
+	case 'U':
+		ret += print_uuid_arg(s, format, data, size, event, arg);
+		break;
+	case 'h':
+		ret += print_raw_buff_arg(s, format, data, size, event, arg, plen);
+		break;
+	default:
+		ret = 0;
+		val = eval_num_arg(data, size, event, arg);
+		trace_seq_printf(s, "%p", (void *)val);
+		break;
+	}
+
+	return ret;
+
+}
+
 static void pretty_print(struct trace_seq *s, void *data, int size, struct tep_event *event)
 {
 	struct tep_handle *tep = event->tep;
@@ -4960,16 +5208,15 @@ static void pretty_print(struct trace_seq *s, void *data, int size, struct tep_e
 	struct tep_print_arg *args = NULL;
 	const char *ptr = print_fmt->format;
 	unsigned long long val;
-	struct func_map *func;
 	const char *saveptr;
 	struct trace_seq p;
 	char *bprint_fmt = NULL;
 	char format[32];
-	int show_func;
 	int len_as_arg;
 	int len_arg = 0;
 	int len;
 	int ls;
+	int ret;
 
 	if (event->flags & TEP_EVENT_FL_FAILED) {
 		trace_seq_printf(s, "[FAILED TO PARSE]");
@@ -5008,7 +5255,6 @@ static void pretty_print(struct trace_seq *s, void *data, int size, struct tep_e
 
 		} else if (*ptr == '%') {
 			saveptr = ptr;
-			show_func = 0;
 			len_as_arg = 0;
  cont_process:
 			ptr++;
@@ -5046,39 +5292,21 @@ static void pretty_print(struct trace_seq *s, void *data, int size, struct tep_e
 			case '-':
 				goto cont_process;
 			case 'p':
-				if (tep->long_size == 4)
-					ls = 1;
-				else
-					ls = 2;
-
-				if (isalnum(ptr[1]))
-					ptr++;
-
 				if (arg->type == TEP_PRINT_BSTRING) {
+					if (isalnum(ptr[1]))
+						ptr++;
 					trace_seq_puts(s, arg->string.string);
 					arg = arg->next;
 					break;
 				}
-
-				if (*ptr == 'F' || *ptr == 'f' ||
-				    *ptr == 'S' || *ptr == 's') {
-					show_func = *ptr;
-				} else if (*ptr == 'M' || *ptr == 'm') {
-					print_mac_arg(s, *ptr, data, size, event, arg);
-					arg = arg->next;
-					break;
-				} else if (*ptr == 'I' || *ptr == 'i') {
-					int n;
-
-					n = print_ip_arg(s, ptr, data, size, event, arg);
-					if (n > 0) {
-						ptr += n - 1;
-						arg = arg->next;
-						break;
-					}
-				}
-
-				/* fall through */
+				ret = print_pointer(s, ptr + 1,
+						    len_as_arg ? len_arg : 1,
+						    data, size,
+						    event, arg);
+				arg = arg->next;
+				if (ret > 0)
+					ptr += ret;
+				break;
 			case 'd':
 			case 'u':
 			case 'i':
@@ -5107,17 +5335,6 @@ static void pretty_print(struct trace_seq *s, void *data, int size, struct tep_e
 				val = eval_num_arg(data, size, event, arg);
 				arg = arg->next;
 
-				if (show_func) {
-					func = find_func(tep, val);
-					if (func) {
-						trace_seq_puts(s, func->func);
-						if (show_func == 'F')
-							trace_seq_printf(s,
-							       "+0x%llx",
-							       val - func->addr);
-						break;
-					}
-				}
 				if (tep->long_size == 8 && ls == 1 &&
 				    sizeof(long) != 8) {
 					char *p;
@@ -5125,8 +5342,6 @@ static void pretty_print(struct trace_seq *s, void *data, int size, struct tep_e
 					/* make %l into %ll */
 					if (ls == 1 && (p = strchr(format, 'l')))
 						memmove(p+1, p, strlen(p)+1);
-					else if (strcmp(format, "%p") == 0)
-						strcpy(format, "0x%llx");
 					ls = 2;
 				}
 				switch (ls) {
-- 
2.26.2



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

* [PATCH v2 07/15] tools lib traceevent: Optimize pretty_print() function
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
                   ` (5 preceding siblings ...)
  2020-07-02 18:53 ` [PATCH v2 06/15] tools lib traceevent: Add support for more printk format specifiers Steven Rostedt
@ 2020-07-02 18:53 ` Steven Rostedt
  2020-07-07 15:11   ` Namhyung Kim
  2020-07-02 18:53 ` [PATCH v2 08/15] tools lib traceevent: Add plugin for tlb_flush Steven Rostedt
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton, Tzvetomir Stoyanov (VMware)

From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>

Each time the pretty_print() function is called to print an event,
the event's format string is parsed. As this format string does not
change, this parsing can be done only once - when the event struct
is initialized.

Link: https://lore.kernel.org/linux-trace-devel/20200529134929.537110-1-tz.stoyanov@gmail.com
Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-8-tz.stoyanov@gmail.com

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse-local.h |  17 +
 tools/lib/traceevent/event-parse.c       | 672 ++++++++++++++++-------
 tools/lib/traceevent/event-parse.h       |   3 +
 3 files changed, 495 insertions(+), 197 deletions(-)

diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
index 96a0b0ca0675..e71296a62236 100644
--- a/tools/lib/traceevent/event-parse-local.h
+++ b/tools/lib/traceevent/event-parse-local.h
@@ -85,6 +85,23 @@ struct tep_handle {
 	struct tep_plugins_dir *plugins_dir;
 };
 
+enum tep_print_parse_type {
+	PRINT_FMT_STING,
+	PRINT_FMT_ARG_DIGIT,
+	PRINT_FMT_ARG_POINTER,
+	PRINT_FMT_ARG_STRING,
+};
+
+struct tep_print_parse {
+	struct tep_print_parse	*next;
+
+	char				*format;
+	int				ls;
+	enum tep_print_parse_type	type;
+	struct tep_print_arg		*arg;
+	struct tep_print_arg		*len_as_arg;
+};
+
 void tep_free_event(struct tep_event *event);
 void tep_free_format_field(struct tep_format_field *field);
 void tep_free_plugin_paths(struct tep_handle *tep);
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index c28cbba34d39..d8115d7af812 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -5161,13 +5161,25 @@ static int print_function(struct trace_seq *s, const char *format,
 	return 0;
 }
 
-static int print_pointer(struct trace_seq *s, const char *format, int plen,
-			 void *data, int size,
-			 struct tep_event *event, struct tep_print_arg *arg)
+static int print_arg_pointer(struct trace_seq *s, const char *format, int plen,
+			     void *data, int size,
+			     struct tep_event *event, struct tep_print_arg *arg)
 {
 	unsigned long long val;
 	int ret = 1;
 
+	if (arg->type == TEP_PRINT_BSTRING) {
+		trace_seq_puts(s, arg->string.string);
+		return 0;
+	}
+	while (*format) {
+		if (*format == 'p') {
+			format++;
+			break;
+		}
+		format++;
+	}
+
 	switch (*format) {
 	case 'F':
 	case 'f':
@@ -5200,231 +5212,493 @@ static int print_pointer(struct trace_seq *s, const char *format, int plen,
 
 }
 
-static void pretty_print(struct trace_seq *s, void *data, int size, struct tep_event *event)
+static int print_arg_number(struct trace_seq *s, const char *format, int plen,
+			    void *data, int size, int ls,
+			    struct tep_event *event, struct tep_print_arg *arg)
 {
-	struct tep_handle *tep = event->tep;
-	struct tep_print_fmt *print_fmt = &event->print_fmt;
-	struct tep_print_arg *arg = print_fmt->args;
-	struct tep_print_arg *args = NULL;
-	const char *ptr = print_fmt->format;
 	unsigned long long val;
-	const char *saveptr;
+
+	val = eval_num_arg(data, size, event, arg);
+
+	switch (ls) {
+	case -2:
+		if (plen >= 0)
+			trace_seq_printf(s, format, plen, (char)val);
+		else
+			trace_seq_printf(s, format, (char)val);
+		break;
+	case -1:
+		if (plen >= 0)
+			trace_seq_printf(s, format, plen, (short)val);
+		else
+			trace_seq_printf(s, format, (short)val);
+		break;
+	case 0:
+		if (plen >= 0)
+			trace_seq_printf(s, format, plen, (int)val);
+		else
+			trace_seq_printf(s, format, (int)val);
+		break;
+	case 1:
+		if (plen >= 0)
+			trace_seq_printf(s, format, plen, (long)val);
+		else
+			trace_seq_printf(s, format, (long)val);
+		break;
+	case 2:
+		if (plen >= 0)
+			trace_seq_printf(s, format, plen, (long long)val);
+		else
+			trace_seq_printf(s, format, (long long)val);
+		break;
+	default:
+		do_warning_event(event, "bad count (%d)", ls);
+		event->flags |= TEP_EVENT_FL_FAILED;
+	}
+	return 0;
+}
+
+
+static void print_arg_string(struct trace_seq *s, const char *format, int plen,
+			     void *data, int size,
+			     struct tep_event *event, struct tep_print_arg *arg)
+{
 	struct trace_seq p;
-	char *bprint_fmt = NULL;
-	char format[32];
-	int len_as_arg;
-	int len_arg = 0;
-	int len;
-	int ls;
-	int ret;
 
-	if (event->flags & TEP_EVENT_FL_FAILED) {
-		trace_seq_printf(s, "[FAILED TO PARSE]");
-		tep_print_fields(s, data, size, event);
-		return;
+	/* Use helper trace_seq */
+	trace_seq_init(&p);
+	print_str_arg(&p, data, size, event,
+		      format, plen, arg);
+	trace_seq_terminate(&p);
+	trace_seq_puts(s, p.buffer);
+	trace_seq_destroy(&p);
+}
+
+static int parse_arg_format_pointer(const char *format)
+{
+	int ret = 0;
+	int index;
+	int loop;
+
+	switch (*format) {
+	case 'F':
+	case 'S':
+	case 'f':
+	case 's':
+		ret++;
+		break;
+	case 'M':
+	case 'm':
+		/* [mM]R , [mM]F */
+		switch (format[1]) {
+		case 'R':
+		case 'F':
+			ret++;
+			break;
+		}
+		ret++;
+		break;
+	case 'I':
+	case 'i':
+		index = 2;
+		loop = 1;
+		switch (format[1]) {
+		case 'S':
+			/*[S][pfs]*/
+			while (loop) {
+				switch (format[index]) {
+				case 'p':
+				case 'f':
+				case 's':
+					ret++;
+					index++;
+					break;
+				default:
+					loop = 0;
+					break;
+				}
+			}
+			/* fall through */
+		case '4':
+			/* [4S][hnbl] */
+			switch (format[index]) {
+			case 'h':
+			case 'n':
+			case 'l':
+			case 'b':
+				ret++;
+				index++;
+				break;
+			}
+			if (format[1] == '4') {
+				ret++;
+				break;
+			}
+			/* fall through */
+		case '6':
+			/* [6S]c */
+			if (format[index] == 'c')
+				ret++;
+			ret++;
+			break;
+		}
+		ret++;
+		break;
+	case 'U':
+		switch (format[1]) {
+		case 'L':
+		case 'l':
+		case 'B':
+		case 'b':
+			ret++;
+			break;
+		}
+		ret++;
+		break;
+	case 'h':
+		switch (format[1]) {
+		case 'C':
+		case 'D':
+		case 'N':
+			ret++;
+			break;
+		}
+		ret++;
+		break;
+	default:
+		break;
 	}
 
-	if (event->flags & TEP_EVENT_FL_ISBPRINT) {
-		bprint_fmt = get_bprint_format(data, size, event);
-		args = make_bprint_args(bprint_fmt, data, size, event);
-		arg = args;
-		ptr = bprint_fmt;
+	return ret;
+}
+
+static void free_parse_args(struct tep_print_parse *arg)
+{
+	struct tep_print_parse *del;
+
+	while (arg) {
+		del = arg;
+		arg = del->next;
+		free(del->format);
+		free(del);
 	}
+}
 
-	for (; *ptr; ptr++) {
-		ls = 0;
-		if (*ptr == '\\') {
-			ptr++;
-			switch (*ptr) {
+static int parse_arg_add(struct tep_print_parse **parse, char *format,
+			 enum tep_print_parse_type type,
+			 struct tep_print_arg *arg,
+			 struct tep_print_arg *len_as_arg,
+			 int ls)
+{
+	struct tep_print_parse *parg = NULL;
+
+	parg = calloc(1, sizeof(*parg));
+	if (!parg)
+		goto error;
+	parg->format = strdup(format);
+	if (!parg->format)
+		goto error;
+	parg->type = type;
+	parg->arg = arg;
+	parg->len_as_arg = len_as_arg;
+	parg->ls = ls;
+	*parse = parg;
+	return 0;
+error:
+	if (parg) {
+		free(parg->format);
+		free(parg);
+	}
+	return -1;
+}
+
+static int parse_arg_format(struct tep_print_parse **parse,
+			    struct tep_event *event,
+			    const char *format, struct tep_print_arg **arg)
+{
+	struct tep_print_arg *len_arg = NULL;
+	char print_format[32];
+	const char *start = format;
+	int ret = 0;
+	int ls = 0;
+	int res;
+	int len;
+
+	format++;
+	ret++;
+	for (; *format; format++) {
+		switch (*format) {
+		case '#':
+			/* FIXME: need to handle properly */
+			break;
+		case 'h':
+			ls--;
+			break;
+		case 'l':
+			ls++;
+			break;
+		case 'L':
+			ls = 2;
+			break;
+		case '.':
+		case 'z':
+		case 'Z':
+		case '0' ... '9':
+		case '-':
+			break;
+		case '*':
+			/* The argument is the length. */
+			if (!*arg) {
+				do_warning_event(event, "no argument match");
+				event->flags |= TEP_EVENT_FL_FAILED;
+				goto out_failed;
+			}
+			if (len_arg) {
+				do_warning_event(event, "argument already matched");
+				event->flags |= TEP_EVENT_FL_FAILED;
+				goto out_failed;
+			}
+			len_arg = *arg;
+			*arg = (*arg)->next;
+			break;
+		case 'p':
+			if (!*arg) {
+				do_warning_event(event, "no argument match");
+				event->flags |= TEP_EVENT_FL_FAILED;
+				goto out_failed;
+			}
+			res = parse_arg_format_pointer(format + 1);
+			if (res > 0) {
+				format += res;
+				ret += res;
+			}
+			len = ((unsigned long)format + 1) -
+				(unsigned long)start;
+			/* should never happen */
+			if (len > 31) {
+				do_warning_event(event, "bad format!");
+				event->flags |= TEP_EVENT_FL_FAILED;
+				len = 31;
+			}
+			memcpy(print_format, start, len);
+			print_format[len] = 0;
+
+			parse_arg_add(parse, print_format,
+				      PRINT_FMT_ARG_POINTER, *arg, len_arg, ls);
+			*arg = (*arg)->next;
+			ret++;
+			return ret;
+		case 'd':
+		case 'u':
+		case 'i':
+		case 'x':
+		case 'X':
+		case 'o':
+			if (!*arg) {
+				do_warning_event(event, "no argument match");
+				event->flags |= TEP_EVENT_FL_FAILED;
+				goto out_failed;
+			}
+
+			len = ((unsigned long)format + 1) -
+				(unsigned long)start;
+
+			/* should never happen */
+			if (len > 30) {
+				do_warning_event(event, "bad format!");
+				event->flags |= TEP_EVENT_FL_FAILED;
+				len = 31;
+			}
+			memcpy(print_format, start, len);
+			print_format[len] = 0;
+
+			if (event->tep->long_size == 8 && ls == 1 &&
+			    sizeof(long) != 8) {
+				char *p;
+
+				/* make %l into %ll */
+				if (ls == 1 && (p = strchr(print_format, 'l')))
+					memmove(p+1, p, strlen(p)+1);
+				ls = 2;
+			}
+			if (ls < -2 || ls > 2) {
+				do_warning_event(event, "bad count (%d)", ls);
+				event->flags |= TEP_EVENT_FL_FAILED;
+			}
+			parse_arg_add(parse, print_format,
+				      PRINT_FMT_ARG_DIGIT, *arg, len_arg, ls);
+			*arg = (*arg)->next;
+			ret++;
+			return ret;
+		case 's':
+			if (!*arg) {
+				do_warning_event(event, "no matching argument");
+				event->flags |= TEP_EVENT_FL_FAILED;
+				goto out_failed;
+			}
+
+			len = ((unsigned long)format + 1) -
+				(unsigned long)start;
+
+			/* should never happen */
+			if (len > 31) {
+				do_warning_event(event, "bad format!");
+				event->flags |= TEP_EVENT_FL_FAILED;
+				len = 31;
+			}
+
+			memcpy(print_format, start, len);
+			print_format[len] = 0;
+
+			parse_arg_add(parse, print_format,
+					PRINT_FMT_ARG_STRING, *arg, len_arg, 0);
+			*arg = (*arg)->next;
+			ret++;
+			return ret;
+		default:
+			snprintf(print_format, 32, ">%c<", *format);
+			parse_arg_add(parse, print_format,
+					PRINT_FMT_STING, NULL, NULL, 0);
+			ret++;
+			return ret;
+		}
+		ret++;
+	}
+
+out_failed:
+	return ret;
+
+}
+
+static int parse_arg_string(struct tep_print_parse **parse, const char *format)
+{
+	struct trace_seq s;
+	int ret = 0;
+
+	trace_seq_init(&s);
+	for (; *format; format++) {
+		if (*format == '\\') {
+			format++;
+			ret++;
+			switch (*format) {
 			case 'n':
-				trace_seq_putc(s, '\n');
+				trace_seq_putc(&s, '\n');
 				break;
 			case 't':
-				trace_seq_putc(s, '\t');
+				trace_seq_putc(&s, '\t');
 				break;
 			case 'r':
-				trace_seq_putc(s, '\r');
+				trace_seq_putc(&s, '\r');
 				break;
 			case '\\':
-				trace_seq_putc(s, '\\');
+				trace_seq_putc(&s, '\\');
 				break;
 			default:
-				trace_seq_putc(s, *ptr);
+				trace_seq_putc(&s, *format);
 				break;
 			}
-
-		} else if (*ptr == '%') {
-			saveptr = ptr;
-			len_as_arg = 0;
- cont_process:
-			ptr++;
-			switch (*ptr) {
-			case '%':
-				trace_seq_putc(s, '%');
-				break;
-			case '#':
-				/* FIXME: need to handle properly */
-				goto cont_process;
-			case 'h':
-				ls--;
-				goto cont_process;
-			case 'l':
-				ls++;
-				goto cont_process;
-			case 'L':
-				ls = 2;
-				goto cont_process;
-			case '*':
-				/* The argument is the length. */
-				if (!arg) {
-					do_warning_event(event, "no argument match");
-					event->flags |= TEP_EVENT_FL_FAILED;
-					goto out_failed;
-				}
-				len_arg = eval_num_arg(data, size, event, arg);
-				len_as_arg = 1;
-				arg = arg->next;
-				goto cont_process;
-			case '.':
-			case 'z':
-			case 'Z':
-			case '0' ... '9':
-			case '-':
-				goto cont_process;
-			case 'p':
-				if (arg->type == TEP_PRINT_BSTRING) {
-					if (isalnum(ptr[1]))
-						ptr++;
-					trace_seq_puts(s, arg->string.string);
-					arg = arg->next;
-					break;
-				}
-				ret = print_pointer(s, ptr + 1,
-						    len_as_arg ? len_arg : 1,
-						    data, size,
-						    event, arg);
-				arg = arg->next;
-				if (ret > 0)
-					ptr += ret;
+		} else if (*format == '%') {
+			if (*(format + 1) == '%') {
+				trace_seq_putc(&s, '%');
+				format++;
+				ret++;
+			} else
 				break;
-			case 'd':
-			case 'u':
-			case 'i':
-			case 'x':
-			case 'X':
-			case 'o':
-				if (!arg) {
-					do_warning_event(event, "no argument match");
-					event->flags |= TEP_EVENT_FL_FAILED;
-					goto out_failed;
-				}
-
-				len = ((unsigned long)ptr + 1) -
-					(unsigned long)saveptr;
-
-				/* should never happen */
-				if (len > 31) {
-					do_warning_event(event, "bad format!");
-					event->flags |= TEP_EVENT_FL_FAILED;
-					len = 31;
-				}
-
-				memcpy(format, saveptr, len);
-				format[len] = 0;
+		} else
+			trace_seq_putc(&s, *format);
 
-				val = eval_num_arg(data, size, event, arg);
-				arg = arg->next;
+		ret++;
+	}
+	trace_seq_terminate(&s);
+	parse_arg_add(parse, s.buffer, PRINT_FMT_STING, NULL, NULL, 0);
+	trace_seq_destroy(&s);
 
-				if (tep->long_size == 8 && ls == 1 &&
-				    sizeof(long) != 8) {
-					char *p;
+	return ret;
+}
 
-					/* make %l into %ll */
-					if (ls == 1 && (p = strchr(format, 'l')))
-						memmove(p+1, p, strlen(p)+1);
-					ls = 2;
-				}
-				switch (ls) {
-				case -2:
-					if (len_as_arg)
-						trace_seq_printf(s, format, len_arg, (char)val);
-					else
-						trace_seq_printf(s, format, (char)val);
-					break;
-				case -1:
-					if (len_as_arg)
-						trace_seq_printf(s, format, len_arg, (short)val);
-					else
-						trace_seq_printf(s, format, (short)val);
-					break;
-				case 0:
-					if (len_as_arg)
-						trace_seq_printf(s, format, len_arg, (int)val);
-					else
-						trace_seq_printf(s, format, (int)val);
-					break;
-				case 1:
-					if (len_as_arg)
-						trace_seq_printf(s, format, len_arg, (long)val);
-					else
-						trace_seq_printf(s, format, (long)val);
-					break;
-				case 2:
-					if (len_as_arg)
-						trace_seq_printf(s, format, len_arg,
-								 (long long)val);
-					else
-						trace_seq_printf(s, format, (long long)val);
-					break;
-				default:
-					do_warning_event(event, "bad count (%d)", ls);
-					event->flags |= TEP_EVENT_FL_FAILED;
-				}
-				break;
-			case 's':
-				if (!arg) {
-					do_warning_event(event, "no matching argument");
-					event->flags |= TEP_EVENT_FL_FAILED;
-					goto out_failed;
-				}
+static struct tep_print_parse *
+parse_args(struct tep_event *event, const char *format, struct tep_print_arg *arg)
+{
+	struct tep_print_parse *parse_ret = NULL;
+	struct tep_print_parse **parse = NULL;
+	int ret;
+	int len;
 
-				len = ((unsigned long)ptr + 1) -
-					(unsigned long)saveptr;
+	len = strlen(format);
+	while (*format) {
+		if (!parse_ret)
+			parse = &parse_ret;
+		if (*format == '%' && *(format + 1) != '%')
+			ret = parse_arg_format(parse, event, format, &arg);
+		else
+			ret = parse_arg_string(parse, format);
+		if (*parse)
+			parse = &((*parse)->next);
 
-				/* should never happen */
-				if (len > 31) {
-					do_warning_event(event, "bad format!");
-					event->flags |= TEP_EVENT_FL_FAILED;
-					len = 31;
-				}
+		len -= ret;
+		if (len > 0)
+			format += ret;
+		else
+			break;
+	}
+	return parse_ret;
+}
 
-				memcpy(format, saveptr, len);
-				format[len] = 0;
-				if (!len_as_arg)
-					len_arg = -1;
-				/* Use helper trace_seq */
-				trace_seq_init(&p);
-				print_str_arg(&p, data, size, event,
-					      format, len_arg, arg);
-				trace_seq_terminate(&p);
-				trace_seq_puts(s, p.buffer);
-				trace_seq_destroy(&p);
-				arg = arg->next;
-				break;
-			default:
-				trace_seq_printf(s, ">%c<", *ptr);
+static void print_event_cache(struct tep_print_parse *parse, struct trace_seq *s,
+			      void *data, int size, struct tep_event *event)
+{
+	int len_arg;
 
-			}
-		} else
-			trace_seq_putc(s, *ptr);
+	while (parse) {
+		if (parse->len_as_arg)
+			len_arg = eval_num_arg(data, size, event, parse->len_as_arg);
+		switch (parse->type) {
+		case PRINT_FMT_ARG_DIGIT:
+			print_arg_number(s, parse->format,
+					parse->len_as_arg ? len_arg : -1, data,
+					 size, parse->ls, event, parse->arg);
+			break;
+		case PRINT_FMT_ARG_POINTER:
+			print_arg_pointer(s, parse->format,
+					  parse->len_as_arg ? len_arg : 1,
+					  data, size, event, parse->arg);
+			break;
+		case PRINT_FMT_ARG_STRING:
+			print_arg_string(s, parse->format,
+					 parse->len_as_arg ? len_arg : -1,
+					 data, size, event, parse->arg);
+			break;
+		case PRINT_FMT_STING:
+		default:
+			trace_seq_printf(s, "%s", parse->format);
+			break;
+		}
+		parse = parse->next;
 	}
+}
+
+static void pretty_print(struct trace_seq *s, void *data, int size, struct tep_event *event)
+{
+	struct tep_print_parse *parse = event->print_fmt.print_cache;
+	struct tep_print_arg *args = NULL;
+	char *bprint_fmt = NULL;
 
 	if (event->flags & TEP_EVENT_FL_FAILED) {
-out_failed:
 		trace_seq_printf(s, "[FAILED TO PARSE]");
+		tep_print_fields(s, data, size, event);
+		return;
+	}
+
+	if (event->flags & TEP_EVENT_FL_ISBPRINT) {
+		bprint_fmt = get_bprint_format(data, size, event);
+		args = make_bprint_args(bprint_fmt, data, size, event);
+		parse = parse_args(event, bprint_fmt, args);
 	}
 
-	if (args) {
+	print_event_cache(parse, s, data, size, event);
+
+	if (event->flags & TEP_EVENT_FL_ISBPRINT) {
+		free_parse_args(parse);
 		free_args(args);
 		free(bprint_fmt);
 	}
@@ -6523,9 +6797,13 @@ enum tep_errno __tep_parse_format(struct tep_event **eventp,
 			*list = arg;
 			list = &arg->next;
 		}
-		return 0;
 	}
 
+	if (!(event->flags & TEP_EVENT_FL_ISBPRINT))
+		event->print_fmt.print_cache = parse_args(event,
+							  event->print_fmt.format,
+							  event->print_fmt.args);
+
 	return 0;
 
  event_parse_failed:
@@ -7192,7 +7470,7 @@ void tep_free_event(struct tep_event *event)
 
 	free(event->print_fmt.format);
 	free_args(event->print_fmt.args);
-
+	free_parse_args(event->print_fmt.print_cache);
 	free(event);
 }
 
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 91f462f5a606..ac162472268b 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -272,9 +272,12 @@ struct tep_print_arg {
 	};
 };
 
+struct tep_print_parse;
+
 struct tep_print_fmt {
 	char			*format;
 	struct tep_print_arg	*args;
+	struct tep_print_parse	*print_cache;
 };
 
 struct tep_event {
-- 
2.26.2



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

* [PATCH v2 08/15] tools lib traceevent: Add plugin for tlb_flush
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
                   ` (6 preceding siblings ...)
  2020-07-02 18:53 ` [PATCH v2 07/15] tools lib traceevent: Optimize pretty_print() function Steven Rostedt
@ 2020-07-02 18:53 ` Steven Rostedt
  2020-07-02 18:53 ` [PATCH v2 09/15] tools lib traceevent: Add more SVM exit reasons Steven Rostedt
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton, Tzvetomir Stoyanov (VMware)

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

The tlb_flush tracepoints uses enums that are not yet known by
the traceevent library. Add a plugin to handle that.

Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-9-tz.stoyanov@gmail.com

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
[ Ported from trace-cmd.git ]
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/plugins/Build        |  1 +
 tools/lib/traceevent/plugins/Makefile     |  1 +
 tools/lib/traceevent/plugins/plugin_tlb.c | 81 +++++++++++++++++++++++
 3 files changed, 83 insertions(+)
 create mode 100644 tools/lib/traceevent/plugins/plugin_tlb.c

diff --git a/tools/lib/traceevent/plugins/Build b/tools/lib/traceevent/plugins/Build
index 210d26910613..b866f231a536 100644
--- a/tools/lib/traceevent/plugins/Build
+++ b/tools/lib/traceevent/plugins/Build
@@ -8,3 +8,4 @@ plugin_function-y     += plugin_function.o
 plugin_xen-y          += plugin_xen.o
 plugin_scsi-y         += plugin_scsi.o
 plugin_cfg80211-y     += plugin_cfg80211.o
+plugin_tlb-y          += plugin_tlb.o
\ No newline at end of file
diff --git a/tools/lib/traceevent/plugins/Makefile b/tools/lib/traceevent/plugins/Makefile
index 349bb81482ab..8e72707e8630 100644
--- a/tools/lib/traceevent/plugins/Makefile
+++ b/tools/lib/traceevent/plugins/Makefile
@@ -137,6 +137,7 @@ PLUGINS += plugin_function.so
 PLUGINS += plugin_xen.so
 PLUGINS += plugin_scsi.so
 PLUGINS += plugin_cfg80211.so
+PLUGINS += plugin_tlb.so
 
 PLUGINS    := $(addprefix $(OUTPUT),$(PLUGINS))
 PLUGINS_IN := $(PLUGINS:.so=-in.o)
diff --git a/tools/lib/traceevent/plugins/plugin_tlb.c b/tools/lib/traceevent/plugins/plugin_tlb.c
new file mode 100644
index 000000000000..1adade776306
--- /dev/null
+++ b/tools/lib/traceevent/plugins/plugin_tlb.c
@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2015 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License (not later!)
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this program; if not,  see <http://www.gnu.org/licenses>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "event-parse.h"
+
+enum tlb_flush_reason {
+	TLB_FLUSH_ON_TASK_SWITCH,
+	TLB_REMOTE_SHOOTDOWN,
+	TLB_LOCAL_SHOOTDOWN,
+	TLB_LOCAL_MM_SHOOTDOWN,
+	NR_TLB_FLUSH_REASONS,
+};
+
+static int tlb_flush_handler(struct trace_seq *s, struct tep_record *record,
+			     struct tep_event *event, void *context)
+{
+	unsigned long long val;
+
+	trace_seq_printf(s, "pages=");
+
+	tep_print_num_field(s, "%ld", event, "pages", record, 1);
+
+	if (tep_get_field_val(s, event, "reason", record, &val, 1) < 0)
+		return -1;
+
+	trace_seq_puts(s, " reason=");
+
+	switch (val) {
+	case TLB_FLUSH_ON_TASK_SWITCH:
+		trace_seq_puts(s, "flush on task switch");
+		break;
+	case TLB_REMOTE_SHOOTDOWN:
+		trace_seq_puts(s, "remote shootdown");
+		break;
+	case TLB_LOCAL_SHOOTDOWN:
+		trace_seq_puts(s, "local shootdown");
+		break;
+	case TLB_LOCAL_MM_SHOOTDOWN:
+		trace_seq_puts(s, "local mm shootdown");
+		break;
+	}
+
+	trace_seq_printf(s, " (%lld)", val);
+
+	return 0;
+}
+
+int TEP_PLUGIN_LOADER(struct tep_handle *tep)
+{
+	tep_register_event_handler(tep, -1, "tlb", "tlb_flush",
+				   tlb_flush_handler, NULL);
+
+	return 0;
+}
+
+void TEP_PLUGIN_UNLOADER(struct tep_handle *tep)
+{
+	tep_unregister_event_handler(tep, -1,
+				     "tlb", "tlb_flush",
+				     tlb_flush_handler, NULL);
+}
-- 
2.26.2



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

* [PATCH v2 09/15] tools lib traceevent: Add more SVM exit reasons
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
                   ` (7 preceding siblings ...)
  2020-07-02 18:53 ` [PATCH v2 08/15] tools lib traceevent: Add plugin for tlb_flush Steven Rostedt
@ 2020-07-02 18:53 ` Steven Rostedt
  2020-07-02 18:53 ` [PATCH v2 10/15] tools lib traceevent: Add offset option for function plugin Steven Rostedt
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton, Jan Kiszka, Tzvetomir Stoyanov (VMware)

From: Jan Kiszka <jan.kiszka@siemens.com>

Exceptions require individual decoding (only feasible intercepts
listed), XSETBV was missing and the AVIC brought in two new exit codes.

Link: http://lkml.kernel.org/r/5741D822.3030203@web.de
Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-10-tz.stoyanov@gmail.com

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
[ Ported from trace-cmd.git ]
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/plugins/plugin_kvm.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/tools/lib/traceevent/plugins/plugin_kvm.c b/tools/lib/traceevent/plugins/plugin_kvm.c
index c8e623065a7e..768c658f5904 100644
--- a/tools/lib/traceevent/plugins/plugin_kvm.c
+++ b/tools/lib/traceevent/plugins/plugin_kvm.c
@@ -155,7 +155,23 @@ static const char *disassemble(unsigned char *insn, int len, uint64_t rip,
 	_ER(EXIT_WRITE_DR5,	0x035)		\
 	_ER(EXIT_WRITE_DR6,	0x036)		\
 	_ER(EXIT_WRITE_DR7,	0x037)		\
-	_ER(EXIT_EXCP_BASE,     0x040)		\
+	_ER(EXIT_EXCP_DE,	0x040)		\
+	_ER(EXIT_EXCP_DB,	0x041)		\
+	_ER(EXIT_EXCP_BP,	0x043)		\
+	_ER(EXIT_EXCP_OF,	0x044)		\
+	_ER(EXIT_EXCP_BR,	0x045)		\
+	_ER(EXIT_EXCP_UD,	0x046)		\
+	_ER(EXIT_EXCP_NM,	0x047)		\
+	_ER(EXIT_EXCP_DF,	0x048)		\
+	_ER(EXIT_EXCP_TS,	0x04a)		\
+	_ER(EXIT_EXCP_NP,	0x04b)		\
+	_ER(EXIT_EXCP_SS,	0x04c)		\
+	_ER(EXIT_EXCP_GP,	0x04d)		\
+	_ER(EXIT_EXCP_PF,	0x04e)		\
+	_ER(EXIT_EXCP_MF,	0x050)		\
+	_ER(EXIT_EXCP_AC,	0x051)		\
+	_ER(EXIT_EXCP_MC,	0x052)		\
+	_ER(EXIT_EXCP_XF,	0x053)		\
 	_ER(EXIT_INTR,		0x060)		\
 	_ER(EXIT_NMI,		0x061)		\
 	_ER(EXIT_SMI,		0x062)		\
@@ -201,7 +217,10 @@ static const char *disassemble(unsigned char *insn, int len, uint64_t rip,
 	_ER(EXIT_MONITOR,	0x08a)		\
 	_ER(EXIT_MWAIT,		0x08b)		\
 	_ER(EXIT_MWAIT_COND,	0x08c)		\
-	_ER(EXIT_NPF,		0x400)		\
+	_ER(EXIT_XSETBV,	0x08d)		\
+	_ER(EXIT_NPF, 		0x400)		\
+	_ER(EXIT_AVIC_INCOMPLETE_IPI,		0x401)	\
+	_ER(EXIT_AVIC_UNACCELERATED_ACCESS,	0x402)	\
 	_ER(EXIT_ERR,		-1)
 
 #define _ER(reason, val)	{ #reason, val },
-- 
2.26.2



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

* [PATCH v2 10/15] tools lib traceevent: Add offset option for function plugin
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
                   ` (8 preceding siblings ...)
  2020-07-02 18:53 ` [PATCH v2 09/15] tools lib traceevent: Add more SVM exit reasons Steven Rostedt
@ 2020-07-02 18:53 ` Steven Rostedt
  2020-07-02 18:53 ` [PATCH v2 11/15] tools lib traceevent: Add plugin for decoding syscalls/sys_enter_futex Steven Rostedt
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton, Tzvetomir Stoyanov (VMware)

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

When the offset option is set for the function plugin enabled, it will
display the offset of the functions along with their names.  This helps in
finding exactly where a function was called by its parent.

 trace-cmd report -O parent -O offset
[..]
        rcuc/163-1330  [163]   740.653251: function: _raw_spin_lock+0x0  <-- rcu_cpu_kthread+0x4d8

Link: http://lore.kernel.org/linux-trace-devel/20200702174950.123454-2-tz.stoyanov@gmail.com

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
[ Ported from trace-cmd.git ]
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 .../lib/traceevent/plugins/plugin_function.c  | 24 +++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/tools/lib/traceevent/plugins/plugin_function.c b/tools/lib/traceevent/plugins/plugin_function.c
index 7770fcb78e0f..cafddd6be983 100644
--- a/tools/lib/traceevent/plugins/plugin_function.c
+++ b/tools/lib/traceevent/plugins/plugin_function.c
@@ -49,6 +49,13 @@ struct tep_plugin_option plugin_options[] =
 		"Try to show function call indents, based on parents",
 		.set = 1,
 	},
+	{
+		.name = "offset",
+		.plugin_alias = "ftrace",
+		.description =
+		"Show function names as well as their offsets",
+		.set = 0,
+	},
 	{
 		.name = NULL,
 	}
@@ -56,6 +63,7 @@ struct tep_plugin_option plugin_options[] =
 
 static struct tep_plugin_option *ftrace_parent = &plugin_options[0];
 static struct tep_plugin_option *ftrace_indent = &plugin_options[1];
+static struct tep_plugin_option *ftrace_offset = &plugin_options[2];
 
 static void add_child(struct func_stack *stack, const char *child, int pos)
 {
@@ -123,6 +131,18 @@ static int add_and_get_index(const char *parent, const char *child, int cpu)
 	return 0;
 }
 
+static void show_function(struct trace_seq *s, struct tep_handle *tep,
+			  const char *func, unsigned long long function)
+{
+	unsigned long long offset;
+
+	trace_seq_printf(s, "%s", func);
+	if (ftrace_offset->set) {
+		offset = tep_find_function_address(tep, function);
+		trace_seq_printf(s, "+0x%x ", (int)(function - offset));
+	}
+}
+
 static int function_handler(struct trace_seq *s, struct tep_record *record,
 			    struct tep_event *event, void *context)
 {
@@ -149,14 +169,14 @@ static int function_handler(struct trace_seq *s, struct tep_record *record,
 	trace_seq_printf(s, "%*s", index*3, "");
 
 	if (func)
-		trace_seq_printf(s, "%s", func);
+		show_function(s, tep, func, function);
 	else
 		trace_seq_printf(s, "0x%llx", function);
 
 	if (ftrace_parent->set) {
 		trace_seq_printf(s, " <-- ");
 		if (parent)
-			trace_seq_printf(s, "%s", parent);
+			show_function(s, tep, parent, pfunction);
 		else
 			trace_seq_printf(s, "0x%llx", pfunction);
 	}
-- 
2.26.2



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

* [PATCH v2 11/15] tools lib traceevent: Add plugin for decoding syscalls/sys_enter_futex
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
                   ` (9 preceding siblings ...)
  2020-07-02 18:53 ` [PATCH v2 10/15] tools lib traceevent: Add offset option for function plugin Steven Rostedt
@ 2020-07-02 18:53 ` Steven Rostedt
  2020-07-02 18:53 ` [PATCH v2 12/15] tools lib traceevent: Move kernel_stack event handler to "function" plugin Steven Rostedt
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton, Julia Cartwright, Tzvetomir Stoyanov (VMware)

From: Julia Cartwright <julia@ni.com>

The futex syscall is a complicated one.  It supports thirteen
multiplexed operations, each with different semantics and encodings for
the syscalls six arguments.

Manually decoding these arguments is tedious and error prone.

This plugin provides symbolic names for futex operations, futex flags,
and tries to be intelligent about the intent of specific arguments (for
example, waking operations use 'val' as an integer count, not just an
arbitrary value).

It doesn't do a full decode of the FUTEX_WAKE_OP's 'val3' argument,
however, this is a good starting point.

Link: http://lkml.kernel.org/r/20171207025649.12160-1-julia@ni.com
Link: http://lore.kernel.org/linux-trace-devel/20200702174950.123454-3-tz.stoyanov@gmail.com

Signed-off-by: Julia Cartwright <julia@ni.com>
[ Ported from trace-cmd.git ]
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/plugins/Build          |   1 +
 tools/lib/traceevent/plugins/Makefile       |   1 +
 tools/lib/traceevent/plugins/plugin_futex.c | 137 ++++++++++++++++++++
 3 files changed, 139 insertions(+)
 create mode 100644 tools/lib/traceevent/plugins/plugin_futex.c

diff --git a/tools/lib/traceevent/plugins/Build b/tools/lib/traceevent/plugins/Build
index b866f231a536..dd4da823c38f 100644
--- a/tools/lib/traceevent/plugins/Build
+++ b/tools/lib/traceevent/plugins/Build
@@ -5,6 +5,7 @@ plugin_kvm-y          += plugin_kvm.o
 plugin_mac80211-y     += plugin_mac80211.o
 plugin_sched_switch-y += plugin_sched_switch.o
 plugin_function-y     += plugin_function.o
+plugin_futex-y        += plugin_futex.o
 plugin_xen-y          += plugin_xen.o
 plugin_scsi-y         += plugin_scsi.o
 plugin_cfg80211-y     += plugin_cfg80211.o
diff --git a/tools/lib/traceevent/plugins/Makefile b/tools/lib/traceevent/plugins/Makefile
index 8e72707e8630..946a4d31fcaf 100644
--- a/tools/lib/traceevent/plugins/Makefile
+++ b/tools/lib/traceevent/plugins/Makefile
@@ -134,6 +134,7 @@ PLUGINS += plugin_kvm.so
 PLUGINS += plugin_mac80211.so
 PLUGINS += plugin_sched_switch.so
 PLUGINS += plugin_function.so
+PLUGINS += plugin_futex.so
 PLUGINS += plugin_xen.so
 PLUGINS += plugin_scsi.so
 PLUGINS += plugin_cfg80211.so
diff --git a/tools/lib/traceevent/plugins/plugin_futex.c b/tools/lib/traceevent/plugins/plugin_futex.c
new file mode 100644
index 000000000000..5d76df141401
--- /dev/null
+++ b/tools/lib/traceevent/plugins/plugin_futex.c
@@ -0,0 +1,137 @@
+/*
+ * Copyright (C) 2017 National Instruments Corp.
+ *
+ * Author: Julia Cartwright <julia@ni.com>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License (not later!)
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this program; if not,  see <http://www.gnu.org/licenses>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <linux/futex.h>
+
+#include "event-parse.h"
+
+#define ARRAY_SIZE(_a) (sizeof(_a) / sizeof((_a)[0]))
+
+struct futex_args {
+	unsigned long long	uaddr;
+	unsigned long long	op;
+	unsigned long long	val;
+	unsigned long long	utime; /* or val2 */
+	unsigned long long	uaddr2;
+	unsigned long long	val3;
+};
+
+struct futex_op {
+	const char	*name;
+	const char	*fmt_val;
+	const char	*fmt_utime;
+	const char	*fmt_uaddr2;
+	const char	*fmt_val3;
+};
+
+static const struct futex_op futex_op_tbl[] = {
+	{            "FUTEX_WAIT", " val=0x%08llx", " utime=0x%08llx",               NULL,             NULL },
+	{            "FUTEX_WAKE",     " val=%llu",              NULL,               NULL,             NULL },
+	{              "FUTEX_FD",     " val=%llu",              NULL,               NULL,             NULL },
+	{         "FUTEX_REQUEUE",     " val=%llu",      " val2=%llu", " uaddr2=0x%08llx",             NULL },
+	{     "FUTEX_CMP_REQUEUE",     " val=%llu",      " val2=%llu", " uaddr2=0x%08llx", " val3=0x%08llx" },
+	{         "FUTEX_WAKE_OP",     " val=%llu",      " val2=%llu", " uaddr2=0x%08llx", " val3=0x%08llx" },
+	{         "FUTEX_LOCK_PI",            NULL, " utime=0x%08llx",               NULL,             NULL },
+	{       "FUTEX_UNLOCK_PI",            NULL,              NULL,               NULL,             NULL },
+	{      "FUTEX_TRYLOCK_PI",            NULL,              NULL,               NULL,             NULL },
+	{     "FUTEX_WAIT_BITSET", " val=0x%08llx", " utime=0x%08llx",               NULL, " val3=0x%08llx" },
+	{     "FUTEX_WAKE_BITSET",     " val=%llu",              NULL,               NULL, " val3=0x%08llx" },
+	{ "FUTEX_WAIT_REQUEUE_PI", " val=0x%08llx", " utime=0x%08llx", " uaddr2=0x%08llx", " val3=0x%08llx" },
+	{  "FUTEX_CMP_REQUEUE_PI",     " val=%llu",      " val2=%llu", " uaddr2=0x%08llx", " val3=0x%08llx" },
+};
+
+
+static void futex_print(struct trace_seq *s, const struct futex_args *args,
+			const struct futex_op *fop)
+{
+	trace_seq_printf(s, " uaddr=0x%08llx", args->uaddr);
+
+	if (fop->fmt_val)
+		trace_seq_printf(s, fop->fmt_val, args->val);
+
+	if (fop->fmt_utime)
+		trace_seq_printf(s,fop->fmt_utime, args->utime);
+
+	if (fop->fmt_uaddr2)
+		trace_seq_printf(s, fop->fmt_uaddr2, args->uaddr2);
+
+	if (fop->fmt_val3)
+		trace_seq_printf(s, fop->fmt_val3, args->val3);
+}
+
+static int futex_handler(struct trace_seq *s, struct tep_record *record,
+			 struct tep_event *event, void *context)
+{
+	const struct futex_op *fop;
+	struct futex_args args;
+	unsigned long long cmd;
+
+	if (tep_get_field_val(s, event, "uaddr", record, &args.uaddr, 1))
+		return 1;
+
+	if (tep_get_field_val(s, event, "op", record, &args.op, 1))
+		return 1;
+
+	if (tep_get_field_val(s, event, "val", record, &args.val, 1))
+		return 1;
+
+	if (tep_get_field_val(s, event, "utime", record, &args.utime, 1))
+		return 1;
+
+	if (tep_get_field_val(s, event, "uaddr2", record, &args.uaddr2, 1))
+		return 1;
+
+	if (tep_get_field_val(s, event, "val3", record, &args.val3, 1))
+		return 1;
+
+	cmd = args.op & FUTEX_CMD_MASK;
+	if (cmd >= ARRAY_SIZE(futex_op_tbl))
+		return 1;
+
+	fop = &futex_op_tbl[cmd];
+
+	trace_seq_printf(s, "op=%s", fop->name);
+
+	if (args.op & FUTEX_PRIVATE_FLAG)
+		trace_seq_puts(s, "|FUTEX_PRIVATE_FLAG");
+
+	if (args.op & FUTEX_CLOCK_REALTIME)
+		trace_seq_puts(s, "|FUTEX_CLOCK_REALTIME");
+
+	futex_print(s, &args, fop);
+	return 0;
+}
+
+int TEP_PLUGIN_LOADER(struct tep_handle *tep)
+{
+	tep_register_event_handler(tep, -1, "syscalls", "sys_enter_futex",
+				   futex_handler, NULL);
+	return 0;
+}
+
+void TEP_PLUGIN_UNLOADER(struct tep_handle *tep)
+{
+	tep_unregister_event_handler(tep, -1, "syscalls", "sys_enter_futex",
+				     futex_handler, NULL);
+}
-- 
2.26.2



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

* [PATCH v2 12/15] tools lib traceevent: Move kernel_stack event handler to "function" plugin.
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
                   ` (10 preceding siblings ...)
  2020-07-02 18:53 ` [PATCH v2 11/15] tools lib traceevent: Add plugin for decoding syscalls/sys_enter_futex Steven Rostedt
@ 2020-07-02 18:53 ` Steven Rostedt
  2020-07-02 18:53 ` [PATCH v2 13/15] tools lib traceevent: Add builtin handler for trace_marker_raw Steven Rostedt
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton, Tzvetomir Stoyanov (VMware)

From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>

The "kernel_stack" event handler does not depend on any trace-cmd
context, it can be used aside from the application. The code is
moved to libtraceevent "function" plugin.

Link: http://lore.kernel.org/linux-trace-devel/20190726124308.18735-2-tz.stoyanov@gmail.com
Link: http://lore.kernel.org/linux-trace-devel/20200702174950.123454-4-tz.stoyanov@gmail.com

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 .../lib/traceevent/plugins/plugin_function.c  | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/tools/lib/traceevent/plugins/plugin_function.c b/tools/lib/traceevent/plugins/plugin_function.c
index cafddd6be983..c7d321968f39 100644
--- a/tools/lib/traceevent/plugins/plugin_function.c
+++ b/tools/lib/traceevent/plugins/plugin_function.c
@@ -184,11 +184,52 @@ static int function_handler(struct trace_seq *s, struct tep_record *record,
 	return 0;
 }
 
+static int
+trace_stack_handler(struct trace_seq *s, struct tep_record *record,
+		    struct tep_event *event, void *context)
+{
+	struct tep_format_field *field;
+	unsigned long long addr;
+	const char *func;
+	int long_size;
+	void *data = record->data;
+
+	field = tep_find_any_field(event, "caller");
+	if (!field) {
+		trace_seq_printf(s, "<CANT FIND FIELD %s>", "caller");
+		return 0;
+	}
+
+	trace_seq_puts(s, "<stack trace >\n");
+
+	long_size = tep_get_long_size(event->tep);
+
+	for (data += field->offset; data < record->data + record->size;
+	     data += long_size) {
+		addr = tep_read_number(event->tep, data, long_size);
+
+		if ((long_size == 8 && addr == (unsigned long long)-1) ||
+		    ((int)addr == -1))
+			break;
+
+		func = tep_find_function(event->tep, addr);
+		if (func)
+			trace_seq_printf(s, "=> %s (%llx)\n", func, addr);
+		else
+			trace_seq_printf(s, "=> %llx\n", addr);
+	}
+
+	return 0;
+}
+
 int TEP_PLUGIN_LOADER(struct tep_handle *tep)
 {
 	tep_register_event_handler(tep, -1, "ftrace", "function",
 				   function_handler, NULL);
 
+	tep_register_event_handler(tep, -1, "ftrace", "kernel_stack",
+				      trace_stack_handler, NULL);
+
 	tep_plugin_add_options("ftrace", plugin_options);
 
 	return 0;
-- 
2.26.2



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

* [PATCH v2 13/15] tools lib traceevent: Add builtin handler for trace_marker_raw
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
                   ` (11 preceding siblings ...)
  2020-07-02 18:53 ` [PATCH v2 12/15] tools lib traceevent: Move kernel_stack event handler to "function" plugin Steven Rostedt
@ 2020-07-02 18:53 ` Steven Rostedt
  2020-07-02 18:53 ` [PATCH v2 14/15] tools lib traceevent: Change to SPDX License format Steven Rostedt
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton, Tzvetomir Stoyanov (VMware)

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

When something is written into trace_marker_raw, it goes in as a binary. But
the printk_fmt() of the event that is created (raw_data)'s format file only
prints the first byte of data:

  print fmt: "id:%04x %08x", REC->id, (int)REC->buf[0]

This is not very useful if we want to see the full data output.

Implement the processing of the raw_data event like it is in the kernel.

Link: http://lore.kernel.org/linux-trace-devel/20200702174950.123454-5-tz.stoyanov@gmail.com

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
[ Ported from trace-cmd.git ]
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 .../lib/traceevent/plugins/plugin_function.c  | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/tools/lib/traceevent/plugins/plugin_function.c b/tools/lib/traceevent/plugins/plugin_function.c
index c7d321968f39..7e8e29b43d1c 100644
--- a/tools/lib/traceevent/plugins/plugin_function.c
+++ b/tools/lib/traceevent/plugins/plugin_function.c
@@ -222,6 +222,44 @@ trace_stack_handler(struct trace_seq *s, struct tep_record *record,
 	return 0;
 }
 
+static int
+trace_raw_data_handler(struct trace_seq *s, struct tep_record *record,
+		    struct tep_event *event, void *context)
+{
+	struct tep_format_field *field;
+	unsigned long long id;
+	int long_size;
+	void *data = record->data;
+
+	if (tep_get_field_val(s, event, "id", record, &id, 1))
+		return trace_seq_putc(s, '!');
+
+	trace_seq_printf(s, "# %llx", id);
+
+	field = tep_find_any_field(event, "buf");
+	if (!field) {
+		trace_seq_printf(s, "<CANT FIND FIELD %s>", "buf");
+		return 0;
+	}
+
+	long_size = tep_get_long_size(event->tep);
+
+	for (data += field->offset; data < record->data + record->size;
+	     data += long_size) {
+		int size = sizeof(long);
+		int left = (record->data + record->size) - data;
+		int i;
+
+		if (size > left)
+			size = left;
+
+		for (i = 0; i < size; i++)
+			trace_seq_printf(s, " %02x", *(unsigned char *)(data + i));
+	}
+
+	return 0;
+}
+
 int TEP_PLUGIN_LOADER(struct tep_handle *tep)
 {
 	tep_register_event_handler(tep, -1, "ftrace", "function",
@@ -230,6 +268,9 @@ int TEP_PLUGIN_LOADER(struct tep_handle *tep)
 	tep_register_event_handler(tep, -1, "ftrace", "kernel_stack",
 				      trace_stack_handler, NULL);
 
+	tep_register_event_handler(tep, -1, "ftrace", "raw_data",
+				      trace_raw_data_handler, NULL);
+
 	tep_plugin_add_options("ftrace", plugin_options);
 
 	return 0;
-- 
2.26.2



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

* [PATCH v2 14/15] tools lib traceevent: Change to SPDX License format
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
                   ` (12 preceding siblings ...)
  2020-07-02 18:53 ` [PATCH v2 13/15] tools lib traceevent: Add builtin handler for trace_marker_raw Steven Rostedt
@ 2020-07-02 18:53 ` Steven Rostedt
  2020-07-02 18:53 ` [PATCH v2 15/15] tools lib traceevent: Fix reporting of unknown SVM exit reasons Steven Rostedt
  2020-07-08  2:02 ` [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Namhyung Kim
  15 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton, Tzvetomir Stoyanov (VMware)

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

Replaced COPYING with a description of how the SPDX identifiers are used.
Added a GPL-2.0 and LGPL-2.1 license file in the new LICENSES directory.
Then removed all the license templates from the source files and replaced
them with the corresponding SPDX identifier.

Link: http://lore.kernel.org/linux-trace-devel/20200702174950.123454-6-tz.stoyanov@gmail.com

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
[ Ported from trace-cmd.git ]
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse.h              | 16 +---------------
 tools/lib/traceevent/kbuffer.h                  | 17 +----------------
 tools/lib/traceevent/plugins/plugin_function.c  | 17 +----------------
 tools/lib/traceevent/plugins/plugin_futex.c     | 16 +---------------
 tools/lib/traceevent/plugins/plugin_hrtimer.c   | 17 +----------------
 tools/lib/traceevent/plugins/plugin_jbd2.c      | 17 +----------------
 tools/lib/traceevent/plugins/plugin_kmem.c      | 17 +----------------
 tools/lib/traceevent/plugins/plugin_kvm.c       | 17 +----------------
 tools/lib/traceevent/plugins/plugin_mac80211.c  | 17 +----------------
 .../traceevent/plugins/plugin_sched_switch.c    | 17 +----------------
 tools/lib/traceevent/plugins/plugin_tlb.c       | 17 +----------------
 11 files changed, 11 insertions(+), 174 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index ac162472268b..acd6791480a4 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -1,21 +1,7 @@
+/* SPDX-License-Identifier: LGPL-2.1 */
 /*
  * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
  *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License (not later!)
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not,  see <http://www.gnu.org/licenses>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 #ifndef _PARSE_EVENTS_H
 #define _PARSE_EVENTS_H
diff --git a/tools/lib/traceevent/kbuffer.h b/tools/lib/traceevent/kbuffer.h
index 5fa8292e341b..a2b522093cfd 100644
--- a/tools/lib/traceevent/kbuffer.h
+++ b/tools/lib/traceevent/kbuffer.h
@@ -1,22 +1,7 @@
+/* SPDX-License-Identifier: LGPL-2.1 */
 /*
  * Copyright (C) 2012 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
  *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License (not later!)
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 #ifndef _KBUFFER_H
 #define _KBUFFER_H
diff --git a/tools/lib/traceevent/plugins/plugin_function.c b/tools/lib/traceevent/plugins/plugin_function.c
index 7e8e29b43d1c..807b16e1bf0f 100644
--- a/tools/lib/traceevent/plugins/plugin_function.c
+++ b/tools/lib/traceevent/plugins/plugin_function.c
@@ -1,21 +1,6 @@
+// SPDX-License-Identifier: LGPL-2.1
 /*
  * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License (not later!)
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not,  see <http://www.gnu.org/licenses>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 #include <stdio.h>
 #include <stdlib.h>
diff --git a/tools/lib/traceevent/plugins/plugin_futex.c b/tools/lib/traceevent/plugins/plugin_futex.c
index 5d76df141401..eb7c9f8a850a 100644
--- a/tools/lib/traceevent/plugins/plugin_futex.c
+++ b/tools/lib/traceevent/plugins/plugin_futex.c
@@ -1,23 +1,9 @@
+// SPDX-License-Identifier: LGPL-2.1
 /*
  * Copyright (C) 2017 National Instruments Corp.
  *
  * Author: Julia Cartwright <julia@ni.com>
  *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License (not later!)
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not,  see <http://www.gnu.org/licenses>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 #include <stdio.h>
 #include <stdlib.h>
diff --git a/tools/lib/traceevent/plugins/plugin_hrtimer.c b/tools/lib/traceevent/plugins/plugin_hrtimer.c
index bb434e0ed03a..d98466788f14 100644
--- a/tools/lib/traceevent/plugins/plugin_hrtimer.c
+++ b/tools/lib/traceevent/plugins/plugin_hrtimer.c
@@ -1,22 +1,7 @@
+// SPDX-License-Identifier: LGPL-2.1
 /*
  * Copyright (C) 2009 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
  * Copyright (C) 2009 Johannes Berg <johannes@sipsolutions.net>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License (not later!)
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not,  see <http://www.gnu.org/licenses>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 #include <stdio.h>
 #include <stdlib.h>
diff --git a/tools/lib/traceevent/plugins/plugin_jbd2.c b/tools/lib/traceevent/plugins/plugin_jbd2.c
index 04fc125f38cb..69111a68d3cf 100644
--- a/tools/lib/traceevent/plugins/plugin_jbd2.c
+++ b/tools/lib/traceevent/plugins/plugin_jbd2.c
@@ -1,21 +1,6 @@
+// SPDX-License-Identifier: LGPL-2.1
 /*
  * Copyright (C) 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License (not later!)
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not,  see <http://www.gnu.org/licenses>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 #include <stdio.h>
 #include <stdlib.h>
diff --git a/tools/lib/traceevent/plugins/plugin_kmem.c b/tools/lib/traceevent/plugins/plugin_kmem.c
index edaec5d962c3..4b4f7f9616e3 100644
--- a/tools/lib/traceevent/plugins/plugin_kmem.c
+++ b/tools/lib/traceevent/plugins/plugin_kmem.c
@@ -1,21 +1,6 @@
+// SPDX-License-Identifier: LGPL-2.1
 /*
  * Copyright (C) 2009 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License (not later!)
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not,  see <http://www.gnu.org/licenses>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 #include <stdio.h>
 #include <stdlib.h>
diff --git a/tools/lib/traceevent/plugins/plugin_kvm.c b/tools/lib/traceevent/plugins/plugin_kvm.c
index 768c658f5904..de76a1e79d9e 100644
--- a/tools/lib/traceevent/plugins/plugin_kvm.c
+++ b/tools/lib/traceevent/plugins/plugin_kvm.c
@@ -1,21 +1,6 @@
+// SPDX-License-Identifier: LGPL-2.1
 /*
  * Copyright (C) 2009 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License (not later!)
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not,  see <http://www.gnu.org/licenses>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 #include <stdio.h>
 #include <stdlib.h>
diff --git a/tools/lib/traceevent/plugins/plugin_mac80211.c b/tools/lib/traceevent/plugins/plugin_mac80211.c
index 884303c26b5c..f48071e3cfb8 100644
--- a/tools/lib/traceevent/plugins/plugin_mac80211.c
+++ b/tools/lib/traceevent/plugins/plugin_mac80211.c
@@ -1,21 +1,6 @@
+// SPDX-License-Identifier: LGPL-2.1
 /*
  * Copyright (C) 2009 Johannes Berg <johannes@sipsolutions.net>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License (not later!)
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not,  see <http://www.gnu.org/licenses>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 #include <stdio.h>
 #include <stdlib.h>
diff --git a/tools/lib/traceevent/plugins/plugin_sched_switch.c b/tools/lib/traceevent/plugins/plugin_sched_switch.c
index 957389a0ff7a..e12fa103820a 100644
--- a/tools/lib/traceevent/plugins/plugin_sched_switch.c
+++ b/tools/lib/traceevent/plugins/plugin_sched_switch.c
@@ -1,21 +1,6 @@
+// SPDX-License-Identifier: LGPL-2.1
 /*
  * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License (not later!)
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not,  see <http://www.gnu.org/licenses>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 #include <stdio.h>
 #include <stdlib.h>
diff --git a/tools/lib/traceevent/plugins/plugin_tlb.c b/tools/lib/traceevent/plugins/plugin_tlb.c
index 1adade776306..43657fb60504 100644
--- a/tools/lib/traceevent/plugins/plugin_tlb.c
+++ b/tools/lib/traceevent/plugins/plugin_tlb.c
@@ -1,21 +1,6 @@
+// SPDX-License-Identifier: LGPL-2.1
 /*
  * Copyright (C) 2015 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License (not later!)
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not,  see <http://www.gnu.org/licenses>
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 #include <stdio.h>
 #include <stdlib.h>
-- 
2.26.2



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

* [PATCH v2 15/15] tools lib traceevent: Fix reporting of unknown SVM exit reasons
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
                   ` (13 preceding siblings ...)
  2020-07-02 18:53 ` [PATCH v2 14/15] tools lib traceevent: Change to SPDX License format Steven Rostedt
@ 2020-07-02 18:53 ` Steven Rostedt
  2020-07-08  2:02 ` [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Namhyung Kim
  15 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2020-07-02 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-trace-devel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Jiri Olsa, Namhyung Kim,
	Andrew Morton, Jan Kiszka, Tzvetomir Stoyanov (VMware)

From: Jan Kiszka <jan.kiszka@siemens.com>

On AMD, exist code -1 is also a possible value, but we use it for
terminating the list of known exit reasons. This leads to EXIT_ERR
being reported for unkown ones. Fix this by using an NULL string
pointer as terminal.

Link: http://lkml.kernel.org/r/5741D817.3070902@web.de
Link: http://lore.kernel.org/linux-trace-devel/20200702174950.123454-7-tz.stoyanov@gmail.com

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
[ Ported from trace-cmd.git ]
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/plugins/plugin_kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/plugins/plugin_kvm.c b/tools/lib/traceevent/plugins/plugin_kvm.c
index de76a1e79d9e..51ceeb9147eb 100644
--- a/tools/lib/traceevent/plugins/plugin_kvm.c
+++ b/tools/lib/traceevent/plugins/plugin_kvm.c
@@ -245,7 +245,7 @@ static const char *find_exit_reason(unsigned isa, int val)
 		}
 	if (!strings)
 		return "UNKNOWN-ISA";
-	for (i = 0; strings[i].val >= 0; i++)
+	for (i = 0; strings[i].str; i++)
 		if (strings[i].val == val)
 			break;
 
-- 
2.26.2



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

* Re: [PATCH v2 01/15] tools lib traceevent: Add API to read time information from kbuffer
  2020-07-02 18:53 ` [PATCH v2 01/15] tools lib traceevent: Add API to read time information from kbuffer Steven Rostedt
@ 2020-07-03 11:31   ` Arnaldo Carvalho de Melo
  2020-07-03 16:18     ` Steven Rostedt
  0 siblings, 1 reply; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-07-03 11:31 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, linux-trace-devel, Ingo Molnar, Jiri Olsa,
	Namhyung Kim, Andrew Morton, Tzvetomir Stoyanov (VMware)

Em Thu, Jul 02, 2020 at 02:53:45PM -0400, Steven Rostedt escreveu:
> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> 
> Add the functions kbuffer_subbuf_timestamp() and kbuffer_ptr_delta() to get
> the timing data stored in the ring buffer that is used to produced the time
> stamps of the records.
> 
> This is useful for tools like trace-cmd to be able to display the content of
> the read data to understand why the records show the time stamps that they
> do.
> 
> Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-2-tz.stoyanov@gmail.com
> 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> [ Ported from trace-cmd.git ]
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

Humm, is that intentional, i.e. two signed-off-by you?

- Arnaldo

> ---
>  tools/lib/traceevent/kbuffer-parse.c | 28 ++++++++++++++++++++++++++++
>  tools/lib/traceevent/kbuffer.h       |  2 ++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/tools/lib/traceevent/kbuffer-parse.c b/tools/lib/traceevent/kbuffer-parse.c
> index 27f3b07fdae8..583db99aee94 100644
> --- a/tools/lib/traceevent/kbuffer-parse.c
> +++ b/tools/lib/traceevent/kbuffer-parse.c
> @@ -546,6 +546,34 @@ int kbuffer_load_subbuffer(struct kbuffer *kbuf, void *subbuffer)
>  	return 0;
>  }
>  
> +/**
> + * kbuffer_subbuf_timestamp - read the timestamp from a sub buffer
> + * @kbuf:      The kbuffer to load
> + * @subbuf:    The subbuffer to read from.
> + *
> + * Return the timestamp from a subbuffer.
> + */
> +unsigned long long kbuffer_subbuf_timestamp(struct kbuffer *kbuf, void *subbuf)
> +{
> +	return kbuf->read_8(subbuf);
> +}
> +
> +/**
> + * kbuffer_ptr_delta - read the delta field from a record
> + * @kbuf:      The kbuffer to load
> + * @ptr:       The record in the buffe.
> + *
> + * Return the timestamp delta from a record
> + */
> +unsigned int kbuffer_ptr_delta(struct kbuffer *kbuf, void *ptr)
> +{
> +	unsigned int type_len_ts;
> +
> +	type_len_ts = read_4(kbuf, ptr);
> +	return ts4host(kbuf, type_len_ts);
> +}
> +
> +
>  /**
>   * kbuffer_read_event - read the next event in the kbuffer subbuffer
>   * @kbuf:	The kbuffer to read from
> diff --git a/tools/lib/traceevent/kbuffer.h b/tools/lib/traceevent/kbuffer.h
> index ed4d697fc137..5fa8292e341b 100644
> --- a/tools/lib/traceevent/kbuffer.h
> +++ b/tools/lib/traceevent/kbuffer.h
> @@ -49,6 +49,8 @@ int kbuffer_load_subbuffer(struct kbuffer *kbuf, void *subbuffer);
>  void *kbuffer_read_event(struct kbuffer *kbuf, unsigned long long *ts);
>  void *kbuffer_next_event(struct kbuffer *kbuf, unsigned long long *ts);
>  unsigned long long kbuffer_timestamp(struct kbuffer *kbuf);
> +unsigned long long kbuffer_subbuf_timestamp(struct kbuffer *kbuf, void *subbuf);
> +unsigned int kbuffer_ptr_delta(struct kbuffer *kbuf, void *ptr);
>  
>  void *kbuffer_translate_data(int swap, void *data, unsigned int *size);
>  
> -- 
> 2.26.2
> 
> 

-- 

- Arnaldo

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

* Re: [PATCH v2 01/15] tools lib traceevent: Add API to read time information from kbuffer
  2020-07-03 11:31   ` Arnaldo Carvalho de Melo
@ 2020-07-03 16:18     ` Steven Rostedt
  0 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2020-07-03 16:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, linux-trace-devel, Ingo Molnar, Jiri Olsa,
	Namhyung Kim, Andrew Morton, Tzvetomir Stoyanov (VMware)

On Fri, 3 Jul 2020 08:31:07 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Thu, Jul 02, 2020 at 02:53:45PM -0400, Steven Rostedt escreveu:
> > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> > 
> > Add the functions kbuffer_subbuf_timestamp() and kbuffer_ptr_delta() to get
> > the timing data stored in the ring buffer that is used to produced the time
> > stamps of the records.
> > 
> > This is useful for tools like trace-cmd to be able to display the content of
> > the read data to understand why the records show the time stamps that they
> > do.
> > 
> > Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-2-tz.stoyanov@gmail.com
> > 
> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> > [ Ported from trace-cmd.git ]
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>  
> 
> Humm, is that intentional, i.e. two signed-off-by you?
> 

Yes.

It was originally written by me at Red Hat (thus the "From: Steven
Rostedt (Red Hat)"), and that signed-off-by is from Red Hat Steven.
Then Tzvetomir ported it over to libtraceveent in the kernel tree,
which requires his signed-off-by, and then I pull Tzvetomir's work and
the VMware Steven Rostedt signed it off. ;-)

As Signed-off-by does have some legal meaning for "right to use this
code" I feel it's more than prudent to include both the Red Hat
Steven's signed-off-by as well as the VMware Steven's signed-off-by.

Make sense?

-- Steve

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

* Re: [PATCH v2 03/15] tools lib traceevent: Add tep_load_plugins_hook() API
  2020-07-02 18:53 ` [PATCH v2 03/15] tools lib traceevent: Add tep_load_plugins_hook() API Steven Rostedt
@ 2020-07-07 14:55   ` Namhyung Kim
  2020-07-07 15:35     ` Steven Rostedt
  0 siblings, 1 reply; 28+ messages in thread
From: Namhyung Kim @ 2020-07-07 14:55 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Linux Trace Devel, Arnaldo Carvalho de Melo,
	Ingo Molnar, Jiri Olsa, Andrew Morton,
	Tzvetomir Stoyanov (VMware)

Hi Steve and Tzvetomir,

On Fri, Jul 3, 2020 at 3:57 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>
>
> Add the API function tep_load_plugins_hook() to the traceevent API to allow
> tools a common method to load in the plugins that are part of the lib
> traceevent library.
>
> Link: http://lore.kernel.org/linux-trace-devel/20190802110101.14759-4-tz.stoyanov@gmail.com
> Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-4-tz.stoyanov@gmail.com
>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  tools/lib/traceevent/event-parse.h  |  6 ++++++
>  tools/lib/traceevent/event-plugin.c | 19 +++++++++----------
>  2 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
> index b77837f75a0d..776c7c24ee79 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -396,6 +396,12 @@ struct tep_plugin_list;
>  struct tep_plugin_list *tep_load_plugins(struct tep_handle *tep);
>  void tep_unload_plugins(struct tep_plugin_list *plugin_list,
>                         struct tep_handle *tep);
> +void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
> +                          void (*load_plugin)(struct tep_handle *tep,
> +                                              const char *path,
> +                                              const char *name,
> +                                              void *data),
> +                          void *data);
>  char **tep_plugin_list_options(void);
>  void tep_plugin_free_options_list(char **list);
>  int tep_plugin_add_options(const char *name,
> diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
> index e1f7ddd5a6cf..b53d9a53bcf9 100644
> --- a/tools/lib/traceevent/event-plugin.c
> +++ b/tools/lib/traceevent/event-plugin.c
> @@ -365,20 +365,19 @@ load_plugins_dir(struct tep_handle *tep, const char *suffix,
>         closedir(dir);
>  }
>
> -static void
> -load_plugins(struct tep_handle *tep, const char *suffix,
> -            void (*load_plugin)(struct tep_handle *tep,
> -                                const char *path,
> -                                const char *name,
> -                                void *data),
> -            void *data)
> +void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
> +                          void (*load_plugin)(struct tep_handle *tep,
> +                                              const char *path,
> +                                              const char *name,
> +                                              void *data),
> +                          void *data)

Can we have a comment (or a doc) for this API?  I'm not sure how it's used..


>  {
>         char *home;
>         char *path;
>         char *envdir;
>         int ret;
>
> -       if (tep->flags & TEP_DISABLE_PLUGINS)
> +       if (tep && tep->flags & TEP_DISABLE_PLUGINS)
>                 return;

Is it ok to call with a NULL tep handle?

Thanks
Namhyung


>
>         /*
> @@ -386,7 +385,7 @@ load_plugins(struct tep_handle *tep, const char *suffix,
>          * check that first.
>          */
>  #ifdef PLUGIN_DIR
> -       if (!(tep->flags & TEP_DISABLE_SYS_PLUGINS))
> +       if (!tep || !(tep->flags & TEP_DISABLE_SYS_PLUGINS))
>                 load_plugins_dir(tep, suffix, PLUGIN_DIR,
>                                  load_plugin, data);
>  #endif
> @@ -423,7 +422,7 @@ tep_load_plugins(struct tep_handle *tep)
>  {
>         struct tep_plugin_list *list = NULL;
>
> -       load_plugins(tep, ".so", load_plugin, &list);
> +       tep_load_plugins_hook(tep, ".so", load_plugin, &list);
>         return list;
>  }
>
> --
> 2.26.2
>
>

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

* Re: [PATCH v2 04/15] tools lib traceevent: Add interface for options to plugins
  2020-07-02 18:53 ` [PATCH v2 04/15] tools lib traceevent: Add interface for options to plugins Steven Rostedt
@ 2020-07-07 15:03   ` Namhyung Kim
  0 siblings, 0 replies; 28+ messages in thread
From: Namhyung Kim @ 2020-07-07 15:03 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Linux Trace Devel, Arnaldo Carvalho de Melo,
	Ingo Molnar, Jiri Olsa, Andrew Morton,
	Tzvetomir Stoyanov (VMware)

On Fri, Jul 3, 2020 at 3:57 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>
>
> Add tep_plugin_add_option() and tep_plugin_print_options() to lib traceevent
> library that allows plugins to have their own options. For example, the
> function plugin by default does not print the parent, as it uses the parent
> to do the indenting. The "parent" option is created by the function plugin
> that will print the parent of the function like it does in the trace file.
>
> The tep_plugin_print_options() will print out the list of options that a
> plugin has defined.
>
> Link: http://lore.kernel.org/linux-trace-devel/20190802110101.14759-3-tz.stoyanov@gmail.com
> Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-5-tz.stoyanov@gmail.com
>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  tools/lib/traceevent/event-parse.h  |   2 +
>  tools/lib/traceevent/event-plugin.c | 172 ++++++++++++++++++++++++++++
>  2 files changed, 174 insertions(+)
>
> diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
> index 776c7c24ee79..02c0438527de 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -406,7 +406,9 @@ char **tep_plugin_list_options(void);
>  void tep_plugin_free_options_list(char **list);
>  int tep_plugin_add_options(const char *name,
>                            struct tep_plugin_option *options);
> +int tep_plugin_add_option(const char *name, const char *val);
>  void tep_plugin_remove_options(struct tep_plugin_option *options);
> +void tep_plugin_print_options(struct trace_seq *s);
>  void tep_print_plugins(struct trace_seq *s,
>                         const char *prefix, const char *suffix,
>                         const struct tep_plugin_list *list);
> diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
> index b53d9a53bcf9..e8f4329ba8e0 100644
> --- a/tools/lib/traceevent/event-plugin.c
> +++ b/tools/lib/traceevent/event-plugin.c
> @@ -13,6 +13,7 @@
>  #include <sys/stat.h>
>  #include <unistd.h>
>  #include <dirent.h>
> +#include <errno.h>
>  #include "event-parse.h"
>  #include "event-parse-local.h"
>  #include "event-utils.h"
> @@ -247,6 +248,166 @@ void tep_plugin_remove_options(struct tep_plugin_option *options)
>         }
>  }
>
> +static void parse_option_name(char **option, char **plugin)
> +{
> +       char *p;
> +
> +       *plugin = NULL;
> +
> +       if ((p = strstr(*option, ":"))) {
> +               *plugin = *option;
> +               *p = '\0';
> +               *option = strdup(p + 1);
> +               if (!*option)
> +                       return;

It needs to pass the error somehow..


> +       }
> +}
> +
> +static struct tep_plugin_option *
> +find_registered_option(const char *plugin, const char *option)
> +{
> +       struct registered_plugin_options *reg;
> +       struct tep_plugin_option *op;
> +       const char *op_plugin;
> +
> +       for (reg = registered_options; reg; reg = reg->next) {
> +               for (op = reg->options; op->name; op++) {
> +                       if (op->plugin_alias)
> +                               op_plugin = op->plugin_alias;
> +                       else
> +                               op_plugin = op->file;
> +
> +                       if (plugin && strcmp(plugin, op_plugin) != 0)
> +                               continue;
> +                       if (strcmp(option, op->name) != 0)
> +                               continue;
> +
> +                       return op;
> +               }
> +       }
> +
> +       return NULL;
> +}
> +
> +static int process_option(const char *plugin, const char *option, const char *val)
> +{
> +       struct tep_plugin_option *op;
> +
> +       op = find_registered_option(plugin, option);
> +       if (!op)
> +               return 0;
> +
> +       return update_option_value(op, val);
> +}
> +
> +/**
> + * tep_plugin_add_option - add an option/val pair to set plugin options
> + * @name: The name of the option (format: <plugin>:<option> or just <option>)
> + * @val: (optiona) the value for the option

Typo.

> + *
> + * Modify a plugin option. If @val is given than the value of the option
> + * is set (note, some options just take a boolean, so @val must be either
> + * "1" or "0" or "true" or "false").
> + */
> +int tep_plugin_add_option(const char *name, const char *val)
> +{
> +       struct trace_plugin_options *op;
> +       char *option_str;
> +       char *plugin;
> +
> +       option_str = strdup(name);
> +       if (!option_str)
> +               return -ENOMEM;
> +
> +       parse_option_name(&option_str, &plugin);
> +
> +       /* If the option exists, update the val */
> +       for (op = trace_plugin_options; op; op = op->next) {
> +               /* Both must be NULL or not NULL */
> +               if ((!plugin || !op->plugin) && plugin != op->plugin)
> +                       continue;
> +               if (plugin && strcmp(plugin, op->plugin) != 0)
> +                       continue;
> +               if (strcmp(op->option, option_str) != 0)
> +                       continue;
> +
> +               /* update option */
> +               free(op->value);
> +               if (val) {
> +                       op->value = strdup(val);
> +                       if (!op->value)
> +                               goto out_free;
> +               } else
> +                       op->value = NULL;
> +
> +               /* plugin and option_str don't get freed at the end */
> +               free(plugin);
> +               free(option_str);
> +
> +               plugin = op->plugin;
> +               option_str = op->option;
> +               break;
> +       }
> +
> +       /* If not found, create */
> +       if (!op) {
> +               op = malloc(sizeof(*op));
> +               if (!op)
> +                       return -ENOMEM;

goto out_free ?

> +               memset(op, 0, sizeof(*op));
> +               op->next = trace_plugin_options;
> +               trace_plugin_options = op;

I think it's better doing this after setting the value below..

> +
> +               op->plugin = plugin;
> +               op->option = option_str;
> +
> +               if (val) {
> +                       op->value = strdup(val);
> +                       if (!op->value)
> +                               goto out_free;

The option_str will be freed and op will have a dangling pointer..

> +               }
> +       }
> +
> +       return process_option(plugin, option_str, val);
> + out_free:
> +       free(option_str);

Why not free plugin too?

Thanks
Namhyung


> +       return -ENOMEM;
> +}
> +
> +static void print_op_data(struct trace_seq *s, const char *name,
> +                         const char *op)
> +{
> +       if (op)
> +               trace_seq_printf(s, "%8s:\t%s\n", name, op);
> +}
> +
> +/**
> + * tep_plugin_print_options - print out the registered plugin options
> + * @s: The trace_seq descriptor to write the plugin options into
> + *
> + * Writes a list of options into trace_seq @s.
> + */
> +void tep_plugin_print_options(struct trace_seq *s)
> +{
> +       struct registered_plugin_options *reg;
> +       struct tep_plugin_option *op;
> +
> +       for (reg = registered_options; reg; reg = reg->next) {
> +               if (reg != registered_options)
> +                       trace_seq_printf(s, "============\n");
> +               for (op = reg->options; op->name; op++) {
> +                       if (op != reg->options)
> +                               trace_seq_printf(s, "------------\n");
> +                       print_op_data(s, "file", op->file);
> +                       print_op_data(s, "plugin", op->plugin_alias);
> +                       print_op_data(s, "option", op->name);
> +                       print_op_data(s, "desc", op->description);
> +                       print_op_data(s, "value", op->value);
> +                       trace_seq_printf(s, "%8s:\t%d\n", "set", op->set);
> +               }
> +       }
> +}
> +
>  /**
>   * tep_print_plugins - print out the list of plugins loaded
>   * @s: the trace_seq descripter to write to
> @@ -273,6 +434,7 @@ load_plugin(struct tep_handle *tep, const char *path,
>             const char *file, void *data)
>  {
>         struct tep_plugin_list **plugin_list = data;
> +       struct tep_plugin_option *options;
>         tep_plugin_load_func func;
>         struct tep_plugin_list *list;
>         const char *alias;
> @@ -297,6 +459,16 @@ load_plugin(struct tep_handle *tep, const char *path,
>         if (!alias)
>                 alias = file;
>
> +       options = dlsym(handle, TEP_PLUGIN_OPTIONS_NAME);
> +       if (options) {
> +               while (options->name) {
> +                       ret = update_option(alias, options);
> +                       if (ret < 0)
> +                               goto out_free;
> +                       options++;
> +               }
> +       }
> +
>         func = dlsym(handle, TEP_PLUGIN_LOADER_NAME);
>         if (!func) {
>                 warning("could not find func '%s' in plugin '%s'\n%s\n",
> --
> 2.26.2
>
>

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

* Re: [PATCH v2 05/15] tools lib traceevent: Introduced new traceevent API, for adding new plugins directories.
  2020-07-02 18:53 ` [PATCH v2 05/15] tools lib traceevent: Introduced new traceevent API, for adding new plugins directories Steven Rostedt
@ 2020-07-07 15:06   ` Namhyung Kim
  2020-07-07 15:24     ` Steven Rostedt
  0 siblings, 1 reply; 28+ messages in thread
From: Namhyung Kim @ 2020-07-07 15:06 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Linux Trace Devel, Arnaldo Carvalho de Melo,
	Ingo Molnar, Jiri Olsa, Andrew Morton,
	Tzvetomir Stoyanov (VMware)

On Fri, Jul 3, 2020 at 3:57 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>
>
> Implement new traceevent plugin API, which can be used to add new plugins
> directories:
> enum tep_plugin_load_priority {
>         TEP_PLUGIN_FIRST,
>         TEP_PLUGIN_LAST,
> };
> int tep_add_plugin_path(struct tep_handle *tep, char *path,
>                         enum tep_plugin_load_priority prio);
>
> It adds the "path" as new plugin directory, in the context of
> the handler "tep". The tep_load_plugins() API searches for plugins
> in this new location. Depending of the priority "prio", the plugins
> from this directory are loaded before (TEP_PLUGIN_FIRST) or after
> (TEP_PLUGIN_LAST) the ordinary libtraceevent plugin locations.
>
> Link: http://lore.kernel.org/linux-trace-devel/20191007114947.17104-2-tz.stoyanov@gmail.com
> Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-6-tz.stoyanov@gmail.com
>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  tools/lib/traceevent/event-parse-local.h |  5 +-
>  tools/lib/traceevent/event-parse.c       |  1 +
>  tools/lib/traceevent/event-parse.h       |  7 +++
>  tools/lib/traceevent/event-plugin.c      | 70 ++++++++++++++++++++++++
>  4 files changed, 82 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
> index cee469803a34..96a0b0ca0675 100644
> --- a/tools/lib/traceevent/event-parse-local.h
> +++ b/tools/lib/traceevent/event-parse-local.h
> @@ -13,6 +13,7 @@ struct func_map;
>  struct func_list;
>  struct event_handler;
>  struct func_resolver;
> +struct tep_plugins_dir;
>
>  struct tep_handle {
>         int ref_count;
> @@ -47,7 +48,6 @@ struct tep_handle {
>         struct printk_list *printklist;
>         unsigned int printk_count;
>
> -
>         struct tep_event **events;
>         int nr_events;
>         struct tep_event **sort_events;
> @@ -81,10 +81,13 @@ struct tep_handle {
>
>         /* cache */
>         struct tep_event *last_event;
> +
> +       struct tep_plugins_dir *plugins_dir;
>  };
>
>  void tep_free_event(struct tep_event *event);
>  void tep_free_format_field(struct tep_format_field *field);
> +void tep_free_plugin_paths(struct tep_handle *tep);
>
>  unsigned short tep_data2host2(struct tep_handle *tep, unsigned short data);
>  unsigned int tep_data2host4(struct tep_handle *tep, unsigned int data);
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index e1bd2a93c6db..064c100d2d5a 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -7065,6 +7065,7 @@ void tep_free(struct tep_handle *tep)
>         free(tep->events);
>         free(tep->sort_events);
>         free(tep->func_resolver);
> +       tep_free_plugin_paths(tep);
>
>         free(tep);
>  }
> diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
> index 02c0438527de..91f462f5a606 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -393,6 +393,13 @@ struct tep_plugin_list;
>
>  #define INVALID_PLUGIN_LIST_OPTION     ((char **)((unsigned long)-1))
>
> +enum tep_plugin_load_priority {
> +       TEP_PLUGIN_FIRST,
> +       TEP_PLUGIN_LAST,
> +};
> +
> +int tep_add_plugin_path(struct tep_handle *tep, char *path,
> +                       enum tep_plugin_load_priority prio);
>  struct tep_plugin_list *tep_load_plugins(struct tep_handle *tep);
>  void tep_unload_plugins(struct tep_plugin_list *plugin_list,
>                         struct tep_handle *tep);
> diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
> index e8f4329ba8e0..1d4f1809cf17 100644
> --- a/tools/lib/traceevent/event-plugin.c
> +++ b/tools/lib/traceevent/event-plugin.c
> @@ -39,6 +39,12 @@ struct tep_plugin_list {
>         void                    *handle;
>  };
>
> +struct tep_plugins_dir {
> +       struct tep_plugins_dir          *next;
> +       char                            *path;
> +       enum tep_plugin_load_priority   prio;
> +};
> +
>  static void lower_case(char *str)
>  {
>         if (!str)
> @@ -544,6 +550,7 @@ void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
>                                                void *data),
>                            void *data)
>  {
> +       struct tep_plugins_dir *dir = NULL;
>         char *home;
>         char *path;
>         char *envdir;
> @@ -552,6 +559,15 @@ void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
>         if (tep && tep->flags & TEP_DISABLE_PLUGINS)
>                 return;
>
> +       if (tep)
> +               dir = tep->plugins_dir;
> +       while (dir) {
> +               if (dir->prio == TEP_PLUGIN_FIRST)
> +                       load_plugins_dir(tep, suffix, dir->path,
> +                                        load_plugin, data);
> +               dir = dir->next;
> +       }
> +
>         /*
>          * If a system plugin directory was defined,
>          * check that first.
> @@ -586,6 +602,15 @@ void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
>
>         load_plugins_dir(tep, suffix, path, load_plugin, data);
>
> +       if (tep)
> +               dir = tep->plugins_dir;
> +       while (dir) {
> +               if (dir->prio == TEP_PLUGIN_LAST)
> +                       load_plugins_dir(tep, suffix, dir->path,
> +                                        load_plugin, data);
> +               dir = dir->next;
> +       }
> +
>         free(path);
>  }
>
> @@ -598,6 +623,51 @@ tep_load_plugins(struct tep_handle *tep)
>         return list;
>  }
>
> +/**
> + * tep_add_plugin_path - Add a new plugin directory.
> + * @tep: Trace event handler.
> + * @path: Path to a directory. All files with extension .so in that

Is the extension (".so") fixed?  I think a new API has the suffix argument
which may change it... ?


> + *       directory will be loaded as plugins.
> + *@prio: Load priority of the plugins in that directory.
> + *
> + * Returns -1 in case of an error, 0 otherwise.
> + */
> +int tep_add_plugin_path(struct tep_handle *tep, char *path,
> +                       enum tep_plugin_load_priority prio)
> +{
> +       struct tep_plugins_dir *dir;
> +
> +       if (!tep || !path)
> +               return -1;
> +
> +       dir = calloc(1, sizeof(*dir));
> +       if (!dir)
> +               return -1;
> +
> +       dir->path = strdup(path);

It needs to check the return value..

Thanks
Namhyung


> +       dir->prio = prio;
> +       dir->next = tep->plugins_dir;
> +       tep->plugins_dir = dir;
> +
> +       return 0;
> +}
> +
> +void tep_free_plugin_paths(struct tep_handle *tep)
> +{
> +       struct tep_plugins_dir *dir;
> +
> +       if (!tep)
> +               return;
> +
> +       dir = tep->plugins_dir;
> +       while (dir) {
> +               tep->plugins_dir = tep->plugins_dir->next;
> +               free(dir->path);
> +               free(dir);
> +               dir = tep->plugins_dir;
> +       }
> +}
> +
>  void
>  tep_unload_plugins(struct tep_plugin_list *plugin_list, struct tep_handle *tep)
>  {
> --
> 2.26.2
>
>

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

* Re: [PATCH v2 06/15] tools lib traceevent: Add support for more printk format specifiers
  2020-07-02 18:53 ` [PATCH v2 06/15] tools lib traceevent: Add support for more printk format specifiers Steven Rostedt
@ 2020-07-07 15:09   ` Namhyung Kim
  0 siblings, 0 replies; 28+ messages in thread
From: Namhyung Kim @ 2020-07-07 15:09 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Linux Trace Devel, Arnaldo Carvalho de Melo,
	Ingo Molnar, Jiri Olsa, Andrew Morton, Johannes Berg,
	Tzvetomir Stoyanov (VMware)

On Fri, Jul 3, 2020 at 3:57 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>
>
> The printk format specifiers used in event's print format files extend
> the standard printf formats. There are a lot of new options related to
> printing pointers and kernel specific structures. Currently trace-cmd
> does not support many of them.
>
> Support for these new printk specifiers is added to the pretty_print()
> function:
>  - UUID/GUID address: %pU[bBlL]
>  - Raw buffer as a hex string: %*ph[CDN]
>
> These are improved:
>  - MAC address: %pMF, %pM and %pmR
>  - IPv4 adderss: %p[Ii]4[hnbl]
>
> Function pretty_print() is refactored. The logic for printing pointers
> %p[...] is moved to its own function.
>
> Link: https://lore.kernel.org/linux-trace-devel/20200515053754.3695335-1-tz.stoyanov@gmail.com
> Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-7-tz.stoyanov@gmail.com
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=207605
> Reported-by: Johannes Berg <johannes@sipsolutions.net>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
[SNIP]
> +static int parse_ip4_print_args(struct tep_handle *tep,
> +                               const char *ptr, bool *reverse)
> +{
> +       int ret = 0;
> +
> +       *reverse = false;
> +
> +       /* hnbl */
> +       switch (*ptr) {
> +       case 'h':
> +               if (tep->file_bigendian)
> +                       *reverse = false;
> +               else
> +                       *reverse = true;
> +               ret++;
> +       break;

Indentation is broken..

> +       case 'l':
> +               *reverse = true;
> +               ret++;
> +       break;

Ditto.

Thanks
Namhyung


> +       case 'n':
> +       case 'b':
> +               ret++;
> +               /* fall through */
> +       default:
> +               *reverse = false;
> +               break;
> +       }
> +
> +       return ret;
>  }

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

* Re: [PATCH v2 07/15] tools lib traceevent: Optimize pretty_print() function
  2020-07-02 18:53 ` [PATCH v2 07/15] tools lib traceevent: Optimize pretty_print() function Steven Rostedt
@ 2020-07-07 15:11   ` Namhyung Kim
  2020-07-07 15:31     ` Steven Rostedt
  0 siblings, 1 reply; 28+ messages in thread
From: Namhyung Kim @ 2020-07-07 15:11 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Linux Trace Devel, Arnaldo Carvalho de Melo,
	Ingo Molnar, Jiri Olsa, Andrew Morton,
	Tzvetomir Stoyanov (VMware)

On Fri, Jul 3, 2020 at 3:57 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>
>
> Each time the pretty_print() function is called to print an event,
> the event's format string is parsed. As this format string does not
> change, this parsing can be done only once - when the event struct
> is initialized.
>
> Link: https://lore.kernel.org/linux-trace-devel/20200529134929.537110-1-tz.stoyanov@gmail.com
> Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-8-tz.stoyanov@gmail.com
>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  tools/lib/traceevent/event-parse-local.h |  17 +
>  tools/lib/traceevent/event-parse.c       | 672 ++++++++++++++++-------
>  tools/lib/traceevent/event-parse.h       |   3 +
>  3 files changed, 495 insertions(+), 197 deletions(-)
>
> diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
> index 96a0b0ca0675..e71296a62236 100644
> --- a/tools/lib/traceevent/event-parse-local.h
> +++ b/tools/lib/traceevent/event-parse-local.h
> @@ -85,6 +85,23 @@ struct tep_handle {
>         struct tep_plugins_dir *plugins_dir;
>  };
>
> +enum tep_print_parse_type {
> +       PRINT_FMT_STING,

STRING ?

> +       PRINT_FMT_ARG_DIGIT,
> +       PRINT_FMT_ARG_POINTER,
> +       PRINT_FMT_ARG_STRING,
> +};

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

* Re: [PATCH v2 05/15] tools lib traceevent: Introduced new traceevent API, for adding new plugins directories.
  2020-07-07 15:06   ` Namhyung Kim
@ 2020-07-07 15:24     ` Steven Rostedt
  2020-07-08  0:02       ` Namhyung Kim
  0 siblings, 1 reply; 28+ messages in thread
From: Steven Rostedt @ 2020-07-07 15:24 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: linux-kernel, Linux Trace Devel, Arnaldo Carvalho de Melo,
	Ingo Molnar, Jiri Olsa, Andrew Morton,
	Tzvetomir Stoyanov (VMware)

On Wed, 8 Jul 2020 00:06:38 +0900
Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > +/**
> > + * tep_add_plugin_path - Add a new plugin directory.
> > + * @tep: Trace event handler.
> > + * @path: Path to a directory. All files with extension .so in that  
> 
> Is the extension (".so") fixed?  I think a new API has the suffix argument
> which may change it... ?

So this should add a "suffix" argument? NULL for ".so"?

> 
> 
> > + *       directory will be loaded as plugins.
> > + *@prio: Load priority of the plugins in that directory.
> > + *
> > + * Returns -1 in case of an error, 0 otherwise.
> > + */
> > +int tep_add_plugin_path(struct tep_handle *tep, char *path,
> > +                       enum tep_plugin_load_priority prio)
> > +{
> > +       struct tep_plugins_dir *dir;
> > +
> > +       if (!tep || !path)
> > +               return -1;
> > +
> > +       dir = calloc(1, sizeof(*dir));
> > +       if (!dir)
> > +               return -1;
> > +
> > +       dir->path = strdup(path);  
> 
> It needs to check the return value..

Yes it does indeed.

BTW, since these patches are already in trace-cmd.git, would be OK if
we just write patches on top of this series to address your concerns.
This way, we would be also adding them to trace-cmd.git as well.

I eventually want a separate libraries repo on kernel.org that this
lives in and remove it from the tools/lib directory of the kernel.

-- Steve


> 
> > +       dir->prio = prio;
> > +       dir->next = tep->plugins_dir;
> > +       tep->plugins_dir = dir;
> > +
> > +       return 0;
> > +}
> > +
> > +void tep_free_plugin_paths(struct tep_handle *tep)
> > +{
> > +       struct tep_plugins_dir *dir;
> > +
> > +       if (!tep)
> > +               return;
> > +
> > +       dir = tep->plugins_dir;
> > +       while (dir) {
> > +               tep->plugins_dir = tep->plugins_dir->next;
> > +               free(dir->path);
> > +               free(dir);
> > +               dir = tep->plugins_dir;
> > +       }
> > +}
> > +
> >  void
> >  tep_unload_plugins(struct tep_plugin_list *plugin_list, struct tep_handle *tep)
> >  {
> > --
> > 2.26.2
> >
> >  


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

* Re: [PATCH v2 07/15] tools lib traceevent: Optimize pretty_print() function
  2020-07-07 15:11   ` Namhyung Kim
@ 2020-07-07 15:31     ` Steven Rostedt
  0 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2020-07-07 15:31 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: linux-kernel, Linux Trace Devel, Arnaldo Carvalho de Melo,
	Ingo Molnar, Jiri Olsa, Andrew Morton,
	Tzvetomir Stoyanov (VMware)

On Wed, 8 Jul 2020 00:11:10 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> > --- a/tools/lib/traceevent/event-parse-local.h
> > +++ b/tools/lib/traceevent/event-parse-local.h
> > @@ -85,6 +85,23 @@ struct tep_handle {
> >         struct tep_plugins_dir *plugins_dir;
> >  };
> >
> > +enum tep_print_parse_type {
> > +       PRINT_FMT_STING,  
> 
> STRING ?

Nice catch! ;-)

-- Steve

> 
> > +       PRINT_FMT_ARG_DIGIT,
> > +       PRINT_FMT_ARG_POINTER,
> > +       PRINT_FMT_ARG_STRING,

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

* Re: [PATCH v2 03/15] tools lib traceevent: Add tep_load_plugins_hook() API
  2020-07-07 14:55   ` Namhyung Kim
@ 2020-07-07 15:35     ` Steven Rostedt
  0 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2020-07-07 15:35 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: linux-kernel, Linux Trace Devel, Arnaldo Carvalho de Melo,
	Ingo Molnar, Jiri Olsa, Andrew Morton,
	Tzvetomir Stoyanov (VMware)

On Tue, 7 Jul 2020 23:55:34 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> > -static void
> > -load_plugins(struct tep_handle *tep, const char *suffix,
> > -            void (*load_plugin)(struct tep_handle *tep,
> > -                                const char *path,
> > -                                const char *name,
> > -                                void *data),
> > -            void *data)
> > +void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
> > +                          void (*load_plugin)(struct tep_handle *tep,
> > +                                              const char *path,
> > +                                              const char *name,
> > +                                              void *data),
> > +                          void *data)  
> 
> Can we have a comment (or a doc) for this API?  I'm not sure how it's used..

Actually, this requires a man page.

> 
> 
> >  {
> >         char *home;
> >         char *path;
> >         char *envdir;
> >         int ret;
> >
> > -       if (tep->flags & TEP_DISABLE_PLUGINS)
> > +       if (tep && tep->flags & TEP_DISABLE_PLUGINS)
> >                 return;  
> 
> Is it ok to call with a NULL tep handle?

Hmm, it looks like we could possibly pass this in without a tep handle,
if the plugins don't need a it. I'm not sure we have a use case for
that. I'll need to look deeper at this.

Thanks for the review!

-- Steve

> >
> >         /*
> > @@ -386,7 +385,7 @@ load_plugins(struct tep_handle *tep, const char
> > *suffix,
> >          * check that first.
> >          */
> >  #ifdef PLUGIN_DIR
> > -       if (!(tep->flags & TEP_DISABLE_SYS_PLUGINS))
> > +       if (!tep || !(tep->flags & TEP_DISABLE_SYS_PLUGINS))
> >                 load_plugins_dir(tep, suffix, PLUGIN_DIR,
> >                                  load_plugin, data);
> >  #endif
> > @@ -423,7 +422,7 @@ tep_load_plugins(struct tep_handle *tep)
> >  {
> >         struct tep_plugin_list *list = NULL;
> >
> > -       load_plugins(tep, ".so", load_plugin, &list);
> > +       tep_load_plugins_hook(tep, ".so", load_plugin, &list);
> >         return list;
> >  }

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

* Re: [PATCH v2 05/15] tools lib traceevent: Introduced new traceevent API, for adding new plugins directories.
  2020-07-07 15:24     ` Steven Rostedt
@ 2020-07-08  0:02       ` Namhyung Kim
  0 siblings, 0 replies; 28+ messages in thread
From: Namhyung Kim @ 2020-07-08  0:02 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Linux Trace Devel, Arnaldo Carvalho de Melo,
	Ingo Molnar, Jiri Olsa, Andrew Morton,
	Tzvetomir Stoyanov (VMware)

On Wed, Jul 8, 2020 at 12:24 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 8 Jul 2020 00:06:38 +0900
> Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > +/**
> > > + * tep_add_plugin_path - Add a new plugin directory.
> > > + * @tep: Trace event handler.
> > > + * @path: Path to a directory. All files with extension .so in that
> >
> > Is the extension (".so") fixed?  I think a new API has the suffix argument
> > which may change it... ?
>
> So this should add a "suffix" argument? NULL for ".so"?

I think it's just fine to change the comment.  The file extension handling
belongs to the plugin load API and we are adding the directory path
here IMHO.

>
> >
> >
> > > + *       directory will be loaded as plugins.
> > > + *@prio: Load priority of the plugins in that directory.
> > > + *
> > > + * Returns -1 in case of an error, 0 otherwise.
> > > + */
> > > +int tep_add_plugin_path(struct tep_handle *tep, char *path,
> > > +                       enum tep_plugin_load_priority prio)
> > > +{
> > > +       struct tep_plugins_dir *dir;
> > > +
> > > +       if (!tep || !path)
> > > +               return -1;
> > > +
> > > +       dir = calloc(1, sizeof(*dir));
> > > +       if (!dir)
> > > +               return -1;
> > > +
> > > +       dir->path = strdup(path);
> >
> > It needs to check the return value..
>
> Yes it does indeed.
>
> BTW, since these patches are already in trace-cmd.git, would be OK if
> we just write patches on top of this series to address your concerns.
> This way, we would be also adding them to trace-cmd.git as well.

I'm ok with it.  I'll review more patches soon..

Thanks
Namhyung

>
> I eventually want a separate libraries repo on kernel.org that this
> lives in and remove it from the tools/lib directory of the kernel.
>
> -- Steve
>
>
> >
> > > +       dir->prio = prio;
> > > +       dir->next = tep->plugins_dir;
> > > +       tep->plugins_dir = dir;
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +void tep_free_plugin_paths(struct tep_handle *tep)
> > > +{
> > > +       struct tep_plugins_dir *dir;
> > > +
> > > +       if (!tep)
> > > +               return;
> > > +
> > > +       dir = tep->plugins_dir;
> > > +       while (dir) {
> > > +               tep->plugins_dir = tep->plugins_dir->next;
> > > +               free(dir->path);
> > > +               free(dir);
> > > +               dir = tep->plugins_dir;
> > > +       }
> > > +}
> > > +
> > >  void
> > >  tep_unload_plugins(struct tep_plugin_list *plugin_list, struct tep_handle *tep)
> > >  {
> > > --
> > > 2.26.2
> > >
> > >
>

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

* Re: [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo
  2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
                   ` (14 preceding siblings ...)
  2020-07-02 18:53 ` [PATCH v2 15/15] tools lib traceevent: Fix reporting of unknown SVM exit reasons Steven Rostedt
@ 2020-07-08  2:02 ` Namhyung Kim
  15 siblings, 0 replies; 28+ messages in thread
From: Namhyung Kim @ 2020-07-08  2:02 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Linux Trace Devel, Arnaldo Carvalho de Melo,
	Ingo Molnar, Jiri Olsa, Andrew Morton

On Fri, Jul 3, 2020 at 3:57 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Hi Arnaldo,
>
> I was missing a few patches, here's take two. I also changed some of the
> patches to state that they are coming from trace-cmd.git.
>
> -- Steve
>
> We noticed that the libtraceevent in trace-cmd.git is a bit out of sync with
> what is in the kernel. These patches can help bring it by in sync again.

With future changes to address my earlier comments..

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks
Namhyung

>
>
> Jan Kiszka (2):
>       tools lib traceevent: Add more SVM exit reasons
>       tools lib traceevent: Fix reporting of unknown SVM exit reasons
>
> Julia Cartwright (1):
>       tools lib traceevent: Add plugin for decoding syscalls/sys_enter_futex
>
> Steven Rostedt (Red Hat) (2):
>       tools lib traceevent: Add API to read time information from kbuffer
>       tools lib traceevent: Add plugin for tlb_flush
>
> Steven Rostedt (VMware) (3):
>       tools lib traceevent: Add offset option for function plugin
>       tools lib traceevent: Add builtin handler for trace_marker_raw
>       tools lib traceevent: Change to SPDX License format
>
> Tom Zanussi (1):
>       tools lib traceevent: Add proper KBUFFER_TYPE_TIME_STAMP handling
>
> Tzvetomir Stoyanov (VMware) (6):
>       tools lib traceevent: Add tep_load_plugins_hook() API
>       tools lib traceevent: Add interface for options to plugins
>       tools lib traceevent: Introduced new traceevent API, for adding new plugins directories.
>       tools lib traceevent: Add support for more printk format specifiers
>       tools lib traceevent: Optimize pretty_print() function
>       tools lib traceevent: Move kernel_stack event handler to "function" plugin.
>
> ----
>  tools/lib/traceevent/event-parse-local.h           |   22 +-
>  tools/lib/traceevent/event-parse.c                 | 1004 +++++++++++++++-----
>  tools/lib/traceevent/event-parse.h                 |   34 +-
>  tools/lib/traceevent/event-plugin.c                |  261 ++++-
>  tools/lib/traceevent/kbuffer-parse.c               |   43 +-
>  tools/lib/traceevent/kbuffer.h                     |   19 +-
>  tools/lib/traceevent/plugins/Build                 |    2 +
>  tools/lib/traceevent/plugins/Makefile              |    2 +
>  tools/lib/traceevent/plugins/plugin_function.c     |  123 ++-
>  tools/lib/traceevent/plugins/plugin_futex.c        |  123 +++
>  tools/lib/traceevent/plugins/plugin_hrtimer.c      |   17 +-
>  tools/lib/traceevent/plugins/plugin_jbd2.c         |   17 +-
>  tools/lib/traceevent/plugins/plugin_kmem.c         |   17 +-
>  tools/lib/traceevent/plugins/plugin_kvm.c          |   42 +-
>  tools/lib/traceevent/plugins/plugin_mac80211.c     |   17 +-
>  tools/lib/traceevent/plugins/plugin_sched_switch.c |   17 +-
>  tools/lib/traceevent/plugins/plugin_tlb.c          |   66 ++
>  17 files changed, 1406 insertions(+), 420 deletions(-)
>  create mode 100644 tools/lib/traceevent/plugins/plugin_futex.c
>  create mode 100644 tools/lib/traceevent/plugins/plugin_tlb.c

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

end of thread, other threads:[~2020-07-08  2:02 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-02 18:53 [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Steven Rostedt
2020-07-02 18:53 ` [PATCH v2 01/15] tools lib traceevent: Add API to read time information from kbuffer Steven Rostedt
2020-07-03 11:31   ` Arnaldo Carvalho de Melo
2020-07-03 16:18     ` Steven Rostedt
2020-07-02 18:53 ` [PATCH v2 02/15] tools lib traceevent: Add proper KBUFFER_TYPE_TIME_STAMP handling Steven Rostedt
2020-07-02 18:53 ` [PATCH v2 03/15] tools lib traceevent: Add tep_load_plugins_hook() API Steven Rostedt
2020-07-07 14:55   ` Namhyung Kim
2020-07-07 15:35     ` Steven Rostedt
2020-07-02 18:53 ` [PATCH v2 04/15] tools lib traceevent: Add interface for options to plugins Steven Rostedt
2020-07-07 15:03   ` Namhyung Kim
2020-07-02 18:53 ` [PATCH v2 05/15] tools lib traceevent: Introduced new traceevent API, for adding new plugins directories Steven Rostedt
2020-07-07 15:06   ` Namhyung Kim
2020-07-07 15:24     ` Steven Rostedt
2020-07-08  0:02       ` Namhyung Kim
2020-07-02 18:53 ` [PATCH v2 06/15] tools lib traceevent: Add support for more printk format specifiers Steven Rostedt
2020-07-07 15:09   ` Namhyung Kim
2020-07-02 18:53 ` [PATCH v2 07/15] tools lib traceevent: Optimize pretty_print() function Steven Rostedt
2020-07-07 15:11   ` Namhyung Kim
2020-07-07 15:31     ` Steven Rostedt
2020-07-02 18:53 ` [PATCH v2 08/15] tools lib traceevent: Add plugin for tlb_flush Steven Rostedt
2020-07-02 18:53 ` [PATCH v2 09/15] tools lib traceevent: Add more SVM exit reasons Steven Rostedt
2020-07-02 18:53 ` [PATCH v2 10/15] tools lib traceevent: Add offset option for function plugin Steven Rostedt
2020-07-02 18:53 ` [PATCH v2 11/15] tools lib traceevent: Add plugin for decoding syscalls/sys_enter_futex Steven Rostedt
2020-07-02 18:53 ` [PATCH v2 12/15] tools lib traceevent: Move kernel_stack event handler to "function" plugin Steven Rostedt
2020-07-02 18:53 ` [PATCH v2 13/15] tools lib traceevent: Add builtin handler for trace_marker_raw Steven Rostedt
2020-07-02 18:53 ` [PATCH v2 14/15] tools lib traceevent: Change to SPDX License format Steven Rostedt
2020-07-02 18:53 ` [PATCH v2 15/15] tools lib traceevent: Fix reporting of unknown SVM exit reasons Steven Rostedt
2020-07-08  2:02 ` [PATCH v2 00/15] tools lib traceevent: Patches from the trace-cmd repo Namhyung Kim

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