[PATCHSET 00/17] tools lib traceevent: Plugin unload enhancement

[PATCHSET 00/17] tools lib traceevent: Plugin unload enhancement

[Namhyung Kim]

Hello,

When plugins are loaded, they register various event and function
handlers.  But they don't unregister even after the plugins unloaded
so that events could have refererences to non-existing handlers.

This patchset added relevant unregister functions to handle that.

Note that this is not a problem as of now since all of users unload
plugins after finishing their access to events.  But being a generic
library, it should be handled properly IMHO.

The patch 1-4 are resend of the previous die removal series so
independent to this series.

I put the series on the 'libtraceevent/plugin-v1' branch in my tree

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Any comments are welcome, thanks
Namhyung


Namhyung Kim (17):
  tools lib traceevent: Add state member to struct trace_seq
  tools lib traceevent: Check return value of realloc()
  tools lib traceevent: Get rid of malloc_or_die() in trace_seq_init()
  tools lib traceevent: Get rid of die() finally!!
  tools lib traceevent: Make plugin unload function receive pevent
  tools lib traceevent: Add pevent_unregister_event_handler()
  tools lib traceevent: Add pevent_unregister_print_function()
  tools lib traceevent: Unregister handler when function plugin
    unloaded
  tools lib traceevent: Unregister handler when hrtimer plugin unloaded
  tools lib traceevent: Unregister handler when kmem plugin unloaded
  tools lib traceevent: Unregister handler when kvm plugin unloaded
  tools lib traceevent: Unregister handler when sched_switch plugin
    unloaded
  tools lib traceevent: Unregister handler when mac80211 plugin
    unloaded
  tools lib traceevent: Unregister handler when cfg80211 plugin
    unloaded
  tools lib traceevent: Unregister handler when jbd2 plugin unloaded
  tools lib traceevent: Unregister handler when scsi plugin unloaded
  tools lib traceevent: Unregister handler when xen plugin unloaded

 tools/lib/traceevent/Makefile              |   2 +-
 tools/lib/traceevent/event-parse.c         | 136 ++++++++++++++++++++++++++---
 tools/lib/traceevent/event-parse.h         |  19 +++-
 tools/lib/traceevent/event-plugin.c        |   4 +-
 tools/lib/traceevent/event-utils.h         |   4 -
 tools/lib/traceevent/parse-utils.c         |  44 ----------
 tools/lib/traceevent/plugin_cfg80211.c     |   7 ++
 tools/lib/traceevent/plugin_function.c     |   5 +-
 tools/lib/traceevent/plugin_hrtimer.c      |  10 +++
 tools/lib/traceevent/plugin_jbd2.c         |  11 +++
 tools/lib/traceevent/plugin_kmem.c         |  22 +++++
 tools/lib/traceevent/plugin_kvm.c          |  30 +++++++
 tools/lib/traceevent/plugin_mac80211.c     |   7 ++
 tools/lib/traceevent/plugin_sched_switch.c |  12 +++
 tools/lib/traceevent/plugin_scsi.c         |   7 ++
 tools/lib/traceevent/plugin_xen.c          |   7 ++
 tools/lib/traceevent/trace-seq.c           |  58 ++++++++++--
 tools/perf/util/trace-event.c              |   2 +-
 18 files changed, 310 insertions(+), 77 deletions(-)

-- 
1.7.11.7

[PATCH 01/17] tools lib traceevent: Add state member to struct trace_seq

[Namhyung Kim]

The trace_seq->state is for tracking errors during the use of
trace_seq APIs and getting rid of die() in it.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/Makefile      |  2 +-
 tools/lib/traceevent/event-parse.h |  7 ++++++
 tools/lib/traceevent/trace-seq.c   | 46 +++++++++++++++++++++++++++++++++-----
 3 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index f778d48ac609..56d52a33a3df 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -136,7 +136,7 @@ export Q VERBOSE
 
 EVENT_PARSE_VERSION = $(EP_VERSION).$(EP_PATCHLEVEL).$(EP_EXTRAVERSION)
 
-INCLUDES = -I. $(CONFIG_INCLUDES)
+INCLUDES = -I. -I $(srctree)/../../include $(CONFIG_INCLUDES)
 
 # Set compile option CFLAGS if not set elsewhere
 CFLAGS ?= -g -Wall
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index cf5db9013f2c..3c890cb28db7 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -58,6 +58,12 @@ struct pevent_record {
 #endif
 };
 
+enum trace_seq_fail {
+	TRACE_SEQ__GOOD,
+	TRACE_SEQ__BUFFER_POISONED,
+	TRACE_SEQ__MEM_ALLOC_FAILED,
+};
+
 /*
  * Trace sequences are used to allow a function to call several other functions
  * to create a string of data to use (up to a max of PAGE_SIZE).
@@ -68,6 +74,7 @@ struct trace_seq {
 	unsigned int		buffer_size;
 	unsigned int		len;
 	unsigned int		readpos;
+	enum trace_seq_fail	state;
 };
 
 void trace_seq_init(struct trace_seq *s);
diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
index d7f2e68bc5b9..7f24e8989401 100644
--- a/tools/lib/traceevent/trace-seq.c
+++ b/tools/lib/traceevent/trace-seq.c
@@ -22,6 +22,7 @@
 #include <string.h>
 #include <stdarg.h>
 
+#include <asm/bug.h>
 #include "event-parse.h"
 #include "event-utils.h"
 
@@ -32,8 +33,9 @@
 #define TRACE_SEQ_POISON	((void *)0xdeadbeef)
 #define TRACE_SEQ_CHECK(s)						\
 do {									\
-	if ((s)->buffer == TRACE_SEQ_POISON)			\
-		die("Usage of trace_seq after it was destroyed");	\
+	if (WARN_ONCE((s)->buffer == TRACE_SEQ_POISON,			\
+		      "Usage of trace_seq after it was destroyed"))	\
+		(s)->state = TRACE_SEQ__BUFFER_POISONED;		\
 } while (0)
 
 /**
@@ -46,6 +48,7 @@ void trace_seq_init(struct trace_seq *s)
 	s->readpos = 0;
 	s->buffer_size = TRACE_SEQ_BUF_SIZE;
 	s->buffer = malloc_or_die(s->buffer_size);
+	s->state = TRACE_SEQ__GOOD;
 }
 
 /**
@@ -80,8 +83,9 @@ static void expand_buffer(struct trace_seq *s)
 {
 	s->buffer_size += TRACE_SEQ_BUF_SIZE;
 	s->buffer = realloc(s->buffer, s->buffer_size);
-	if (!s->buffer)
-		die("Can't allocate trace_seq buffer memory");
+	if (WARN_ONCE(!s->buffer,
+		      "Can't allocate trace_seq buffer memory"))
+		s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
 }
 
 /**
@@ -108,6 +112,9 @@ trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
 	TRACE_SEQ_CHECK(s);
 
  try_again:
+	if (s->state != TRACE_SEQ__GOOD)
+		return 0;
+
 	len = (s->buffer_size - 1) - s->len;
 
 	va_start(ap, fmt);
@@ -144,6 +151,9 @@ trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args)
 	TRACE_SEQ_CHECK(s);
 
  try_again:
+	if (s->state != TRACE_SEQ__GOOD)
+		return 0;
+
 	len = (s->buffer_size - 1) - s->len;
 
 	ret = vsnprintf(s->buffer + s->len, len, fmt, args);
@@ -174,11 +184,17 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
 
 	TRACE_SEQ_CHECK(s);
 
+	if (s->state != TRACE_SEQ__GOOD)
+		return 0;
+
 	len = strlen(str);
 
 	while (len > ((s->buffer_size - 1) - s->len))
 		expand_buffer(s);
 
+	if (s->state != TRACE_SEQ__GOOD)
+		return 0;
+
 	memcpy(s->buffer + s->len, str, len);
 	s->len += len;
 
@@ -189,9 +205,15 @@ int trace_seq_putc(struct trace_seq *s, unsigned char c)
 {
 	TRACE_SEQ_CHECK(s);
 
+	if (s->state != TRACE_SEQ__GOOD)
+		return 0;
+
 	while (s->len >= (s->buffer_size - 1))
 		expand_buffer(s);
 
+	if (s->state != TRACE_SEQ__GOOD)
+		return 0;
+
 	s->buffer[s->len++] = c;
 
 	return 1;
@@ -201,6 +223,9 @@ void trace_seq_terminate(struct trace_seq *s)
 {
 	TRACE_SEQ_CHECK(s);
 
+	if (s->state != TRACE_SEQ__GOOD)
+		return;
+
 	/* There's always one character left on the buffer */
 	s->buffer[s->len] = 0;
 }
@@ -208,5 +233,16 @@ void trace_seq_terminate(struct trace_seq *s)
 int trace_seq_do_printf(struct trace_seq *s)
 {
 	TRACE_SEQ_CHECK(s);
-	return printf("%.*s", s->len, s->buffer);
+
+	switch (s->state) {
+	case TRACE_SEQ__GOOD:
+		return printf("%.*s", s->len, s->buffer);
+	case TRACE_SEQ__BUFFER_POISONED:
+		puts("Usage of trace_seq after it was destroyed");
+		break;
+	case TRACE_SEQ__MEM_ALLOC_FAILED:
+		puts("Can't allocate trace_seq buffer memory");
+		break;
+	}
+	return -1;
 }
-- 
1.7.11.7

[PATCH 02/17] tools lib traceevent: Check return value of realloc()

[Namhyung Kim]

If realloc() fails, it'll leak the buffer.  Also increate buffer size
only if the allocation succeeded.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/trace-seq.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
index 7f24e8989401..941d35d2cf87 100644
--- a/tools/lib/traceevent/trace-seq.c
+++ b/tools/lib/traceevent/trace-seq.c
@@ -81,11 +81,16 @@ void trace_seq_destroy(struct trace_seq *s)
 
 static void expand_buffer(struct trace_seq *s)
 {
-	s->buffer_size += TRACE_SEQ_BUF_SIZE;
-	s->buffer = realloc(s->buffer, s->buffer_size);
-	if (WARN_ONCE(!s->buffer,
-		      "Can't allocate trace_seq buffer memory"))
+	char *buf;
+
+	buf = realloc(s->buffer, s->buffer_size + TRACE_SEQ_BUF_SIZE);
+	if (WARN_ONCE(!buf, "Can't allocate trace_seq buffer memory")) {
 		s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
+		return;
+	}
+
+	s->buffer = buf;
+	s->buffer_size += TRACE_SEQ_BUF_SIZE;
 }
 
 /**
-- 
1.7.11.7

[PATCH 03/17] tools lib traceevent: Get rid of malloc_or_die() in trace_seq_init()

[Namhyung Kim]

Use plain malloc() and check its return value.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/trace-seq.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
index 941d35d2cf87..339a0ffa21d5 100644
--- a/tools/lib/traceevent/trace-seq.c
+++ b/tools/lib/traceevent/trace-seq.c
@@ -47,8 +47,11 @@ void trace_seq_init(struct trace_seq *s)
 	s->len = 0;
 	s->readpos = 0;
 	s->buffer_size = TRACE_SEQ_BUF_SIZE;
-	s->buffer = malloc_or_die(s->buffer_size);
-	s->state = TRACE_SEQ__GOOD;
+	s->buffer = malloc(s->buffer_size);
+	if (s->buffer != NULL)
+		s->state = TRACE_SEQ__GOOD;
+	else
+		s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
 }
 
 /**
-- 
1.7.11.7

[PATCH 04/17] tools lib traceevent: Get rid of die() finally!!

[Namhyung Kim]

Now all of its users were gone. :)

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/event-utils.h |  4 ----
 tools/lib/traceevent/parse-utils.c | 44 --------------------------------------
 2 files changed, 48 deletions(-)

diff --git a/tools/lib/traceevent/event-utils.h b/tools/lib/traceevent/event-utils.h
index e76c9acb92cd..d1dc2170e402 100644
--- a/tools/lib/traceevent/event-utils.h
+++ b/tools/lib/traceevent/event-utils.h
@@ -23,18 +23,14 @@
 #include <ctype.h>
 
 /* Can be overridden */
-void die(const char *fmt, ...);
-void *malloc_or_die(unsigned int size);
 void warning(const char *fmt, ...);
 void pr_stat(const char *fmt, ...);
 void vpr_stat(const char *fmt, va_list ap);
 
 /* Always available */
-void __die(const char *fmt, ...);
 void __warning(const char *fmt, ...);
 void __pr_stat(const char *fmt, ...);
 
-void __vdie(const char *fmt, ...);
 void __vwarning(const char *fmt, ...);
 void __vpr_stat(const char *fmt, ...);
 
diff --git a/tools/lib/traceevent/parse-utils.c b/tools/lib/traceevent/parse-utils.c
index bba701cf10e6..eda07fa31dca 100644
--- a/tools/lib/traceevent/parse-utils.c
+++ b/tools/lib/traceevent/parse-utils.c
@@ -25,40 +25,6 @@
 
 #define __weak __attribute__((weak))
 
-void __vdie(const char *fmt, va_list ap)
-{
-	int ret = errno;
-
-	if (errno)
-		perror("trace-cmd");
-	else
-		ret = -1;
-
-	fprintf(stderr, "  ");
-	vfprintf(stderr, fmt, ap);
-
-	fprintf(stderr, "\n");
-	exit(ret);
-}
-
-void __die(const char *fmt, ...)
-{
-	va_list ap;
-
-	va_start(ap, fmt);
-	__vdie(fmt, ap);
-	va_end(ap);
-}
-
-void __weak die(const char *fmt, ...)
-{
-	va_list ap;
-
-	va_start(ap, fmt);
-	__vdie(fmt, ap);
-	va_end(ap);
-}
-
 void __vwarning(const char *fmt, va_list ap)
 {
 	if (errno)
@@ -117,13 +83,3 @@ void __weak pr_stat(const char *fmt, ...)
 	__vpr_stat(fmt, ap);
 	va_end(ap);
 }
-
-void __weak *malloc_or_die(unsigned int size)
-{
-	void *data;
-
-	data = malloc(size);
-	if (!data)
-		die("malloc");
-	return data;
-}
-- 
1.7.11.7

[PATCH 05/17] tools lib traceevent: Make plugin unload function receive pevent

[Namhyung Kim]

The PEVENT_PLUGIN_UNLOADER function might need some cleanup using
pevent like unregister some handlers.  So pass pevent as argument.

Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/event-parse.h     | 7 ++++---
 tools/lib/traceevent/event-plugin.c    | 4 ++--
 tools/lib/traceevent/plugin_function.c | 2 +-
 tools/perf/util/trace-event.c          | 2 +-
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 3c890cb28db7..a3beca56cb35 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -105,7 +105,7 @@ typedef int (*pevent_event_handler_func)(struct trace_seq *s,
 					 void *context);
 
 typedef int (*pevent_plugin_load_func)(struct pevent *pevent);
-typedef int (*pevent_plugin_unload_func)(void);
+typedef int (*pevent_plugin_unload_func)(struct pevent *pevent);
 
 struct plugin_option {
 	struct plugin_option		*next;
@@ -130,7 +130,7 @@ struct plugin_option {
  * PEVENT_PLUGIN_UNLOADER:  (optional)
  *   The function called just before unloading
  *
- *   int PEVENT_PLUGIN_UNLOADER(void)
+ *   int PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
  *
  * PEVENT_PLUGIN_OPTIONS:  (optional)
  *   Plugin options that can be set before loading
@@ -411,7 +411,8 @@ enum pevent_errno {
 struct plugin_list;
 
 struct plugin_list *traceevent_load_plugins(struct pevent *pevent);
-void traceevent_unload_plugins(struct plugin_list *plugin_list);
+void traceevent_unload_plugins(struct plugin_list *plugin_list,
+			       struct pevent *pevent);
 
 struct cmdline;
 struct cmdline_list;
diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
index 125f5676bcb5..0c8bf6780e4d 100644
--- a/tools/lib/traceevent/event-plugin.c
+++ b/tools/lib/traceevent/event-plugin.c
@@ -197,7 +197,7 @@ traceevent_load_plugins(struct pevent *pevent)
 }
 
 void
-traceevent_unload_plugins(struct plugin_list *plugin_list)
+traceevent_unload_plugins(struct plugin_list *plugin_list, struct pevent *pevent)
 {
 	pevent_plugin_unload_func func;
 	struct plugin_list *list;
@@ -207,7 +207,7 @@ traceevent_unload_plugins(struct plugin_list *plugin_list)
 		plugin_list = list->next;
 		func = dlsym(list->handle, PEVENT_PLUGIN_UNLOADER_NAME);
 		if (func)
-			func();
+			func(pevent);
 		dlclose(list->handle);
 		free(list->name);
 		free(list);
diff --git a/tools/lib/traceevent/plugin_function.c b/tools/lib/traceevent/plugin_function.c
index aad92ad5e96f..39461485f9a7 100644
--- a/tools/lib/traceevent/plugin_function.c
+++ b/tools/lib/traceevent/plugin_function.c
@@ -144,7 +144,7 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
 	return 0;
 }
 
-void PEVENT_PLUGIN_UNLOADER(void)
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
 {
 	int i, x;
 
diff --git a/tools/perf/util/trace-event.c b/tools/perf/util/trace-event.c
index d9f5f6137ab3..6322d37164c5 100644
--- a/tools/perf/util/trace-event.c
+++ b/tools/perf/util/trace-event.c
@@ -34,8 +34,8 @@ int trace_event__init(struct trace_event *t)
 
 void trace_event__cleanup(struct trace_event *t)
 {
+	traceevent_unload_plugins(t->plugin_list, t->pevent);
 	pevent_free(t->pevent);
-	traceevent_unload_plugins(t->plugin_list);
 }
 
 static struct event_format*
-- 
1.7.11.7

[PATCH 06/17] tools lib traceevent: Add pevent_unregister_event_handler()

[Namhyung Kim]

When a plugin unloaded it needs to unregister its handler from pevent.
So add unregister function to do it.

Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/event-parse.c | 113 ++++++++++++++++++++++++++++++++-----
 tools/lib/traceevent/event-parse.h |   3 +
 2 files changed, 102 insertions(+), 14 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 2ce565a73dd5..05ae4b4ed0c8 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -5560,6 +5560,29 @@ int pevent_register_print_function(struct pevent *pevent,
 	return ret;
 }
 
+static struct event_format *pevent_search_event(struct pevent *pevent, int id,
+						const char *sys_name,
+						const char *event_name)
+{
+	struct event_format *event;
+
+	if (id >= 0) {
+		/* search by id */
+		event = pevent_find_event(pevent, id);
+		if (!event)
+			return NULL;
+		if (event_name && (strcmp(event_name, event->name) != 0))
+			return NULL;
+		if (sys_name && (strcmp(sys_name, event->system) != 0))
+			return NULL;
+	} else {
+		event = pevent_find_event_by_name(pevent, sys_name, event_name);
+		if (!event)
+			return NULL;
+	}
+	return event;
+}
+
 /**
  * pevent_register_event_handler - register a way to parse an event
  * @pevent: the handle to the pevent
@@ -5584,20 +5607,9 @@ int pevent_register_event_handler(struct pevent *pevent, int id,
 	struct event_format *event;
 	struct event_handler *handle;
 
-	if (id >= 0) {
-		/* search by id */
-		event = pevent_find_event(pevent, id);
-		if (!event)
-			goto not_found;
-		if (event_name && (strcmp(event_name, event->name) != 0))
-			goto not_found;
-		if (sys_name && (strcmp(sys_name, event->system) != 0))
-			goto not_found;
-	} else {
-		event = pevent_find_event_by_name(pevent, sys_name, event_name);
-		if (!event)
-			goto not_found;
-	}
+	event = pevent_search_event(pevent, id, sys_name, event_name);
+	if (event == NULL)
+		goto not_found;
 
 	pr_stat("overriding event (%d) %s:%s with new print handler",
 		event->id, event->system, event->name);
@@ -5637,6 +5649,79 @@ int pevent_register_event_handler(struct pevent *pevent, int id,
 	return -1;
 }
 
+static int handle_matches(struct event_handler *handler, int id,
+			  const char *sys_name, const char *event_name,
+			  pevent_event_handler_func func, void *context)
+{
+	if (id >= 0 && id != handler->id)
+		return 0;
+
+	if (event_name && (strcmp(event_name, handler->event_name) != 0))
+		return 0;
+
+	if (sys_name && (strcmp(sys_name, handler->sys_name) != 0))
+		return 0;
+
+	if (func != handler->func || context != handler->context)
+		return 0;
+
+	return 1;
+}
+
+/**
+ * pevent_unregister_event_handler - unregister an existing event handler
+ * @pevent: the handle to the pevent
+ * @id: the id of the event to unregister
+ * @sys_name: the system name the handler belongs to
+ * @event_name: the name of the event handler
+ * @func: the function to call to parse the event information
+ * @context: the data to be passed to @func
+ *
+ * This function removes existing event handler (parser).
+ *
+ * If @id is >= 0, then it is used to find the event.
+ * else @sys_name and @event_name are used.
+ *
+ * Returns 0 if handler was removed successfully, -1 if event was not found.
+ */
+int pevent_unregister_event_handler(struct pevent *pevent, int id,
+				    const char *sys_name, const char *event_name,
+				    pevent_event_handler_func func, void *context)
+{
+	struct event_format *event;
+	struct event_handler *handle;
+	struct event_handler **next;
+
+	event = pevent_search_event(pevent, id, sys_name, event_name);
+	if (event == NULL)
+		goto not_found;
+
+	if (event->handler == func && event->context == context) {
+		pr_stat("overriding event (%d) %s:%s with default print handler",
+			event->id, event->system, event->name);
+
+		event->handler = NULL;
+		event->context = NULL;
+		return 0;
+	}
+
+not_found:
+	for (next = &pevent->handlers; *next; next = &(*next)->next) {
+		handle = *next;
+		if (handle_matches(handle, id, sys_name, event_name,
+				   func, context))
+			break;
+	}
+
+	if (!(*next))
+		return -1;
+
+	*next = handle->next;
+	free_handler(handle);
+
+	return 0;
+}
+
 /**
  * pevent_alloc - create a pevent handle
  */
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index a3beca56cb35..c48acfbc6230 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -624,6 +624,9 @@ int pevent_print_func_field(struct trace_seq *s, const char *fmt,
 int pevent_register_event_handler(struct pevent *pevent, int id,
 				  const char *sys_name, const char *event_name,
 				  pevent_event_handler_func func, void *context);
+int pevent_unregister_event_handler(struct pevent *pevent, int id,
+				    const char *sys_name, const char *event_name,
+				    pevent_event_handler_func func, void *context);
 int pevent_register_print_function(struct pevent *pevent,
 				   pevent_func_handler func,
 				   enum pevent_func_arg_type ret_type,
-- 
1.7.11.7

[PATCH 07/17] tools lib traceevent: Add pevent_unregister_print_function()

[Namhyung Kim]

When a plugin unloaded it needs to unregister its print handler from
pevent.  So add unregister function to do it.

Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/event-parse.c | 23 +++++++++++++++++++++++
 tools/lib/traceevent/event-parse.h |  2 ++
 2 files changed, 25 insertions(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 05ae4b4ed0c8..fe962fbb156c 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -5560,6 +5560,29 @@ int pevent_register_print_function(struct pevent *pevent,
 	return ret;
 }
 
+/**
+ * pevent_unregister_print_function - unregister a helper function
+ * @pevent: the handle to the pevent
+ * @func: the function to process the helper function
+ * @name: the name of the helper function
+ *
+ * This function removes existing print handler for function @name.
+ *
+ * Returns 0 if the handler was removed successully, -1 otherwise.
+ */
+int pevent_unregister_print_function(struct pevent *pevent,
+				     pevent_func_handler func, char *name)
+{
+	struct pevent_function_handler *func_handle;
+
+	func_handle = find_func_handler(pevent, name);
+	if (func_handle && func_handle->func == func) {
+		remove_func_handler(pevent, name);
+		return 0;
+	}
+	return -1;
+}
+
 static struct event_format *pevent_search_event(struct pevent *pevent, int id,
 						const char *sys_name,
 						const char *event_name)
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index c48acfbc6230..791c539374c7 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -631,6 +631,8 @@ int pevent_register_print_function(struct pevent *pevent,
 				   pevent_func_handler func,
 				   enum pevent_func_arg_type ret_type,
 				   char *name, ...);
+int pevent_unregister_print_function(struct pevent *pevent,
+				     pevent_func_handler func, char *name);
 
 struct format_field *pevent_find_common_field(struct event_format *event, const char *name);
 struct format_field *pevent_find_field(struct event_format *event, const char *name);
-- 
1.7.11.7

[PATCH 08/17] tools lib traceevent: Unregister handler when function plugin unloaded

[Namhyung Kim]

The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.

Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/plugin_function.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/lib/traceevent/plugin_function.c b/tools/lib/traceevent/plugin_function.c
index 39461485f9a7..80ba4ff1fe84 100644
--- a/tools/lib/traceevent/plugin_function.c
+++ b/tools/lib/traceevent/plugin_function.c
@@ -148,6 +148,9 @@ void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
 {
 	int i, x;
 
+	pevent_unregister_event_handler(pevent, -1, "ftrace", "function",
+					function_handler, NULL);
+
 	for (i = 0; i <= cpus; i++) {
 		for (x = 0; x < fstack[i].size && fstack[i].stack[x]; x++)
 			free(fstack[i].stack[x]);
-- 
1.7.11.7

[PATCH 09/17] tools lib traceevent: Unregister handler when hrtimer plugin unloaded

[Namhyung Kim]

The timer handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.

Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/plugin_hrtimer.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/lib/traceevent/plugin_hrtimer.c b/tools/lib/traceevent/plugin_hrtimer.c
index 0b0ebf30aa44..12bf14cc1152 100644
--- a/tools/lib/traceevent/plugin_hrtimer.c
+++ b/tools/lib/traceevent/plugin_hrtimer.c
@@ -76,3 +76,13 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
 				      timer_start_handler, NULL);
 	return 0;
 }
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+	pevent_unregister_event_handler(pevent, -1,
+					"timer", "hrtimer_expire_entry",
+					timer_expire_handler, NULL);
+
+	pevent_unregister_event_handler(pevent, -1, "timer", "hrtimer_start",
+					timer_start_handler, NULL);
+}
-- 
1.7.11.7

[PATCH 10/17] tools lib traceevent: Unregister handler when kmem plugin unloaded

[Namhyung Kim]

The kmem handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.

Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/plugin_kmem.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tools/lib/traceevent/plugin_kmem.c b/tools/lib/traceevent/plugin_kmem.c
index 7115c8037ea8..70650ff48d78 100644
--- a/tools/lib/traceevent/plugin_kmem.c
+++ b/tools/lib/traceevent/plugin_kmem.c
@@ -70,3 +70,25 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
 				      call_site_handler, NULL);
 	return 0;
 }
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+	pevent_unregister_event_handler(pevent, -1, "kmem", "kfree",
+					call_site_handler, NULL);
+
+	pevent_unregister_event_handler(pevent, -1, "kmem", "kmalloc",
+					call_site_handler, NULL);
+
+	pevent_unregister_event_handler(pevent, -1, "kmem", "kmalloc_node",
+					call_site_handler, NULL);
+
+	pevent_unregister_event_handler(pevent, -1, "kmem", "kmem_cache_alloc",
+					call_site_handler, NULL);
+
+	pevent_unregister_event_handler(pevent, -1, "kmem",
+					"kmem_cache_alloc_node",
+					call_site_handler, NULL);
+
+	pevent_unregister_event_handler(pevent, -1, "kmem", "kmem_cache_free",
+					call_site_handler, NULL);
+}
-- 
1.7.11.7

[PATCH 11/17] tools lib traceevent: Unregister handler when kvm plugin unloaded

[Namhyung Kim]

The kvm handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.

Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/plugin_kvm.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tools/lib/traceevent/plugin_kvm.c b/tools/lib/traceevent/plugin_kvm.c
index a0e282c6b967..6a1eeadbf7a1 100644
--- a/tools/lib/traceevent/plugin_kvm.c
+++ b/tools/lib/traceevent/plugin_kvm.c
@@ -434,3 +434,33 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
 				       PEVENT_FUNC_ARG_VOID);
 	return 0;
 }
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+	pevent_unregister_event_handler(pevent, -1, "kvm", "kvm_exit",
+					kvm_exit_handler, NULL);
+
+	pevent_unregister_event_handler(pevent, -1, "kvm", "kvm_emulate_insn",
+					kvm_emulate_insn_handler, NULL);
+
+	pevent_unregister_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_get_page",
+					kvm_mmu_get_page_handler, NULL);
+
+	pevent_unregister_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_sync_page",
+					kvm_mmu_print_role, NULL);
+
+	pevent_unregister_event_handler(pevent, -1,
+					"kvmmmu", "kvm_mmu_unsync_page",
+					kvm_mmu_print_role, NULL);
+
+	pevent_unregister_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_zap_page",
+					kvm_mmu_print_role, NULL);
+
+	pevent_unregister_event_handler(pevent, -1, "kvmmmu",
+			"kvm_mmu_prepare_zap_page", kvm_mmu_print_role,
+			NULL);
+
+	pevent_unregister_print_function(pevent,
+					 process_is_writable_pte,
+					 "is_writable_pte");
+}
-- 
1.7.11.7

[PATCH 12/17] tools lib traceevent: Unregister handler when sched_switch plugin unloaded

[Namhyung Kim]

The event handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.

Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/plugin_sched_switch.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/lib/traceevent/plugin_sched_switch.c b/tools/lib/traceevent/plugin_sched_switch.c
index fea3724aa24f..f1ce60065258 100644
--- a/tools/lib/traceevent/plugin_sched_switch.c
+++ b/tools/lib/traceevent/plugin_sched_switch.c
@@ -146,3 +146,15 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
 				      sched_wakeup_handler, NULL);
 	return 0;
 }
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+	pevent_unregister_event_handler(pevent, -1, "sched", "sched_switch",
+					sched_switch_handler, NULL);
+
+	pevent_unregister_event_handler(pevent, -1, "sched", "sched_wakeup",
+					sched_wakeup_handler, NULL);
+
+	pevent_unregister_event_handler(pevent, -1, "sched", "sched_wakeup_new",
+					sched_wakeup_handler, NULL);
+}
-- 
1.7.11.7

[PATCH 13/17] tools lib traceevent: Unregister handler when mac80211 plugin unloaded

[Namhyung Kim]

The event handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.

Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/plugin_mac80211.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/lib/traceevent/plugin_mac80211.c b/tools/lib/traceevent/plugin_mac80211.c
index 558a3b91c046..7e15a0f1c2fd 100644
--- a/tools/lib/traceevent/plugin_mac80211.c
+++ b/tools/lib/traceevent/plugin_mac80211.c
@@ -93,3 +93,10 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
 				      drv_bss_info_changed, NULL);
 	return 0;
 }
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+	pevent_unregister_event_handler(pevent, -1, "mac80211",
+					"drv_bss_info_changed",
+					drv_bss_info_changed, NULL);
+}
-- 
1.7.11.7

[PATCH 14/17] tools lib traceevent: Unregister handler when cfg80211 plugin unloaded

[Namhyung Kim]

The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.

Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/plugin_cfg80211.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/lib/traceevent/plugin_cfg80211.c b/tools/lib/traceevent/plugin_cfg80211.c
index dcab8e873c21..7b07d149557b 100644
--- a/tools/lib/traceevent/plugin_cfg80211.c
+++ b/tools/lib/traceevent/plugin_cfg80211.c
@@ -22,3 +22,10 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
 				       PEVENT_FUNC_ARG_VOID);
 	return 0;
 }
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+	pevent_unregister_print_function(pevent,
+					 process___le16_to_cpup,
+					 "__le16_to_cpup");
+}
-- 
1.7.11.7

[PATCH 15/17] tools lib traceevent: Unregister handler when jbd2 plugin unloaded

[Namhyung Kim]

The function handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.

Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/plugin_jbd2.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/lib/traceevent/plugin_jbd2.c b/tools/lib/traceevent/plugin_jbd2.c
index 2f93f81f0bac..88fe6fe872d5 100644
--- a/tools/lib/traceevent/plugin_jbd2.c
+++ b/tools/lib/traceevent/plugin_jbd2.c
@@ -66,3 +66,14 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
 				       PEVENT_FUNC_ARG_VOID);
 	return 0;
 }
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+	pevent_unregister_print_function(pevent,
+					 process_jbd2_dev_to_name,
+					 "jbd2_dev_to_name");
+
+	pevent_unregister_print_function(pevent,
+					 process_jiffies_to_msecs,
+					 "jiffies_to_msecs");
+}
-- 
1.7.11.7

[PATCH 16/17] tools lib traceevent: Unregister handler when scsi plugin unloaded

[Namhyung Kim]

The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.

Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/plugin_scsi.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/lib/traceevent/plugin_scsi.c b/tools/lib/traceevent/plugin_scsi.c
index 6fb8e3e3fcad..64d688534518 100644
--- a/tools/lib/traceevent/plugin_scsi.c
+++ b/tools/lib/traceevent/plugin_scsi.c
@@ -421,3 +421,10 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
 				       PEVENT_FUNC_ARG_VOID);
 	return 0;
 }
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+	pevent_unregister_print_function(pevent,
+					 process_scsi_trace_parse_cdb,
+					 "scsi_trace_parse_cdb");
+}
-- 
1.7.11.7

[PATCH 17/17] tools lib traceevent: Unregister handler when xen plugin unloaded

[Namhyung Kim]

The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.

Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/plugin_xen.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/lib/traceevent/plugin_xen.c b/tools/lib/traceevent/plugin_xen.c
index e7794298f3a9..a65353f44302 100644
--- a/tools/lib/traceevent/plugin_xen.c
+++ b/tools/lib/traceevent/plugin_xen.c
@@ -128,3 +128,10 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
 				       PEVENT_FUNC_ARG_VOID);
 	return 0;
 }
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+	pevent_unregister_print_function(pevent,
+					 process_xen_hypercall_name,
+					 "xen_hypercall_name");
+}
-- 
1.7.11.7

Re: [PATCH 01/17] tools lib traceevent: Add state member to struct trace_seq

[Steven Rostedt]

On Wed, 15 Jan 2014 10:45:24 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

>  
> @@ -32,8 +33,9 @@
>  #define TRACE_SEQ_POISON	((void *)0xdeadbeef)
>  #define TRACE_SEQ_CHECK(s)						\
>  do {									\
> -	if ((s)->buffer == TRACE_SEQ_POISON)			\
> -		die("Usage of trace_seq after it was destroyed");	\
> +	if (WARN_ONCE((s)->buffer == TRACE_SEQ_POISON,			\
> +		      "Usage of trace_seq after it was destroyed"))	\
> +		(s)->state = TRACE_SEQ__BUFFER_POISONED;		\
>  } while (0)
>  
>  /**
> @@ -46,6 +48,7 @@ void trace_seq_init(struct trace_seq *s)
>  	s->readpos = 0;
>  	s->buffer_size = TRACE_SEQ_BUF_SIZE;
>  	s->buffer = malloc_or_die(s->buffer_size);
> +	s->state = TRACE_SEQ__GOOD;
>  }
>  
>  /**
> @@ -80,8 +83,9 @@ static void expand_buffer(struct trace_seq *s)
>  {
>  	s->buffer_size += TRACE_SEQ_BUF_SIZE;
>  	s->buffer = realloc(s->buffer, s->buffer_size);
> -	if (!s->buffer)
> -		die("Can't allocate trace_seq buffer memory");
> +	if (WARN_ONCE(!s->buffer,
> +		      "Can't allocate trace_seq buffer memory"))
> +		s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
>  }
>  
>  /**
> @@ -108,6 +112,9 @@ trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
>  	TRACE_SEQ_CHECK(s);
>  
>   try_again:
> +	if (s->state != TRACE_SEQ__GOOD)
> +		return 0;
> +
>  	len = (s->buffer_size - 1) - s->len;
>  
>  	va_start(ap, fmt);
> @@ -144,6 +151,9 @@ trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args)
>  	TRACE_SEQ_CHECK(s);
>  
>   try_again:
> +	if (s->state != TRACE_SEQ__GOOD)
> +		return 0;
> +
>  	len = (s->buffer_size - 1) - s->len;
>  
>  	ret = vsnprintf(s->buffer + s->len, len, fmt, args);
> @@ -174,11 +184,17 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
>  
>  	TRACE_SEQ_CHECK(s);
>  
> +	if (s->state != TRACE_SEQ__GOOD)
> +		return 0;
> +
>  	len = strlen(str);
>  
>  	while (len > ((s->buffer_size - 1) - s->len))
>  		expand_buffer(s);
>  
> +	if (s->state != TRACE_SEQ__GOOD)
> +		return 0;
> +
>  	memcpy(s->buffer + s->len, str, len);
>  	s->len += len;
>  
> @@ -189,9 +205,15 @@ int trace_seq_putc(struct trace_seq *s, unsigned char c)
>  {
>  	TRACE_SEQ_CHECK(s);
>  
> +	if (s->state != TRACE_SEQ__GOOD)
> +		return 0;
> +

Instead of adding all of these, we can extend the macro
TRACE_SEQ_CHECK() which does a
	if (s->state != TRACE_SEQ__GOOD)
		return;

and a TRACE_SEQ_CHECK_RET() that does a return 0;

-- Steve

>  	while (s->len >= (s->buffer_size - 1))
>  		expand_buffer(s);
>  
> +	if (s->state != TRACE_SEQ__GOOD)
> +		return 0;
> +
>  	s->buffer[s->len++] = c;
>  
>  	return 1;
> @@ -201,6 +223,9 @@ void trace_seq_terminate(struct trace_seq *s)
>  {
>  	TRACE_SEQ_CHECK(s);
>  
> +	if (s->state != TRACE_SEQ__GOOD)
> +		return;
> +
>  	/* There's always one character left on the buffer */
>  	s->buffer[s->len] = 0;
>  }
> @@ -208,5 +233,16 @@ void trace_seq_terminate(struct trace_seq *s)
>  int trace_seq_do_printf(struct trace_seq *s)
>  {
>  	TRACE_SEQ_CHECK(s);
> -	return printf("%.*s", s->len, s->buffer);
> +
> +	switch (s->state) {
> +	case TRACE_SEQ__GOOD:
> +		return printf("%.*s", s->len, s->buffer);
> +	case TRACE_SEQ__BUFFER_POISONED:
> +		puts("Usage of trace_seq after it was destroyed");
> +		break;
> +	case TRACE_SEQ__MEM_ALLOC_FAILED:
> +		puts("Can't allocate trace_seq buffer memory");
> +		break;
> +	}
> +	return -1;
>  }

Re: [PATCH 02/17] tools lib traceevent: Check return value of realloc()

[Steven Rostedt]

On Wed, 15 Jan 2014 10:45:25 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> If realloc() fails, it'll leak the buffer.  Also increate buffer size
> only if the allocation succeeded.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

Re: [PATCH 03/17] tools lib traceevent: Get rid of malloc_or_die() in trace_seq_init()

[Steven Rostedt]

On Wed, 15 Jan 2014 10:45:26 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> Use plain malloc() and check its return value.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

> ---
>  tools/lib/traceevent/trace-seq.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
> index 941d35d2cf87..339a0ffa21d5 100644
> --- a/tools/lib/traceevent/trace-seq.c
> +++ b/tools/lib/traceevent/trace-seq.c
> @@ -47,8 +47,11 @@ void trace_seq_init(struct trace_seq *s)
>  	s->len = 0;
>  	s->readpos = 0;
>  	s->buffer_size = TRACE_SEQ_BUF_SIZE;
> -	s->buffer = malloc_or_die(s->buffer_size);
> -	s->state = TRACE_SEQ__GOOD;
> +	s->buffer = malloc(s->buffer_size);
> +	if (s->buffer != NULL)
> +		s->state = TRACE_SEQ__GOOD;
> +	else
> +		s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
>  }
>  
>  /**

Re: [PATCH 05/17] tools lib traceevent: Make plugin unload function receive pevent

[Steven Rostedt]

On Wed, 15 Jan 2014 10:45:28 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> The PEVENT_PLUGIN_UNLOADER function might need some cleanup using
> pevent like unregister some handlers.  So pass pevent as argument.
> 
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Steven Rostedt <rostedt@goodmis.org>

> ---
>  tools/lib/traceevent/event-parse.h     | 7 ++++---
>  tools/lib/traceevent/event-plugin.c    | 4 ++--
>  tools/lib/traceevent/plugin_function.c | 2 +-
>  tools/perf/util/trace-event.c          | 2 +-
>  4 files changed, 8 insertions(+), 7 deletions(-)
> 

Re: [PATCH 01/17] tools lib traceevent: Add state member to struct trace_seq

[Namhyung Kim]

Hi Steve,

On Tue, 14 Jan 2014 21:00:58 -0500, Steven Rostedt wrote:
> On Wed, 15 Jan 2014 10:45:24 +0900
> Namhyung Kim <namhyung@kernel.org> wrote:
>
>>  
>> @@ -32,8 +33,9 @@
>>  #define TRACE_SEQ_POISON	((void *)0xdeadbeef)
>>  #define TRACE_SEQ_CHECK(s)						\
>>  do {									\
>> -	if ((s)->buffer == TRACE_SEQ_POISON)			\
>> -		die("Usage of trace_seq after it was destroyed");	\
>> +	if (WARN_ONCE((s)->buffer == TRACE_SEQ_POISON,			\
>> +		      "Usage of trace_seq after it was destroyed"))	\
>> +		(s)->state = TRACE_SEQ__BUFFER_POISONED;		\
>>  } while (0)
>>  
>> @@ -189,9 +205,15 @@ int trace_seq_putc(struct trace_seq *s, unsigned char c)
>>  {
>>  	TRACE_SEQ_CHECK(s);
>>  
>> +	if (s->state != TRACE_SEQ__GOOD)
>> +		return 0;
>> +
>
> Instead of adding all of these, we can extend the macro
> TRACE_SEQ_CHECK() which does a
> 	if (s->state != TRACE_SEQ__GOOD)
> 		return;
>
> and a TRACE_SEQ_CHECK_RET() that does a return 0;

Oh, it looks better.  But I'd like to TRACE_SEQ_CHECK() as is for some
cases.  How about this?


>From 0b43e4f8a9689ab47bdf55d5eddf122f300577e7 Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung@kernel.org>
Date: Thu, 19 Dec 2013 18:17:44 +0900
Subject: [PATCH] tools lib traceevent: Add state member to struct trace_seq

The trace_seq->state is for tracking errors during the use of
trace_seq APIs and getting rid of die() in it.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/Makefile      |  2 +-
 tools/lib/traceevent/event-parse.h |  7 +++++
 tools/lib/traceevent/trace-seq.c   | 53 +++++++++++++++++++++++++++++---------
 3 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index f778d48ac609..56d52a33a3df 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -136,7 +136,7 @@ export Q VERBOSE
 
 EVENT_PARSE_VERSION = $(EP_VERSION).$(EP_PATCHLEVEL).$(EP_EXTRAVERSION)
 
-INCLUDES = -I. $(CONFIG_INCLUDES)
+INCLUDES = -I. -I $(srctree)/../../include $(CONFIG_INCLUDES)
 
 # Set compile option CFLAGS if not set elsewhere
 CFLAGS ?= -g -Wall
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index cf5db9013f2c..3c890cb28db7 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -58,6 +58,12 @@ struct pevent_record {
 #endif
 };
 
+enum trace_seq_fail {
+	TRACE_SEQ__GOOD,
+	TRACE_SEQ__BUFFER_POISONED,
+	TRACE_SEQ__MEM_ALLOC_FAILED,
+};
+
 /*
  * Trace sequences are used to allow a function to call several other functions
  * to create a string of data to use (up to a max of PAGE_SIZE).
@@ -68,6 +74,7 @@ struct trace_seq {
 	unsigned int		buffer_size;
 	unsigned int		len;
 	unsigned int		readpos;
+	enum trace_seq_fail	state;
 };
 
 void trace_seq_init(struct trace_seq *s);
diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
index d7f2e68bc5b9..c39048b0de08 100644
--- a/tools/lib/traceevent/trace-seq.c
+++ b/tools/lib/traceevent/trace-seq.c
@@ -22,6 +22,7 @@
 #include <string.h>
 #include <stdarg.h>
 
+#include <asm/bug.h>
 #include "event-parse.h"
 #include "event-utils.h"
 
@@ -32,10 +33,21 @@
 #define TRACE_SEQ_POISON	((void *)0xdeadbeef)
 #define TRACE_SEQ_CHECK(s)						\
 do {									\
-	if ((s)->buffer == TRACE_SEQ_POISON)			\
-		die("Usage of trace_seq after it was destroyed");	\
+	if (WARN_ONCE((s)->buffer == TRACE_SEQ_POISON,			\
+		      "Usage of trace_seq after it was destroyed"))	\
+		(s)->state = TRACE_SEQ__BUFFER_POISONED;		\
 } while (0)
 
+#define TRACE_SEQ_CHECK_RET_N(s, n)		\
+do {						\
+	TRACE_SEQ_CHECK(s);			\
+	if ((s)->state != TRACE_SEQ__GOOD)	\
+		return n; 			\
+} while (0)
+
+#define TRACE_SEQ_CHECK_RET(s)   TRACE_SEQ_CHECK_RET_N(s, )
+#define TRACE_SEQ_CHECK_RET0(s)  TRACE_SEQ_CHECK_RET_N(s, 0)
+
 /**
  * trace_seq_init - initialize the trace_seq structure
  * @s: a pointer to the trace_seq structure to initialize
@@ -46,6 +58,7 @@ void trace_seq_init(struct trace_seq *s)
 	s->readpos = 0;
 	s->buffer_size = TRACE_SEQ_BUF_SIZE;
 	s->buffer = malloc_or_die(s->buffer_size);
+	s->state = TRACE_SEQ__GOOD;
 }
 
 /**
@@ -80,8 +93,9 @@ static void expand_buffer(struct trace_seq *s)
 {
 	s->buffer_size += TRACE_SEQ_BUF_SIZE;
 	s->buffer = realloc(s->buffer, s->buffer_size);
-	if (!s->buffer)
-		die("Can't allocate trace_seq buffer memory");
+	if (WARN_ONCE(!s->buffer,
+		      "Can't allocate trace_seq buffer memory"))
+		s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
 }
 
 /**
@@ -105,9 +119,9 @@ trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
 	int len;
 	int ret;
 
-	TRACE_SEQ_CHECK(s);
-
  try_again:
+	TRACE_SEQ_CHECK_RET0(s);
+
 	len = (s->buffer_size - 1) - s->len;
 
 	va_start(ap, fmt);
@@ -141,9 +155,9 @@ trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args)
 	int len;
 	int ret;
 
-	TRACE_SEQ_CHECK(s);
-
  try_again:
+	TRACE_SEQ_CHECK_RET0(s);
+
 	len = (s->buffer_size - 1) - s->len;
 
 	ret = vsnprintf(s->buffer + s->len, len, fmt, args);
@@ -172,13 +186,15 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
 {
 	int len;
 
-	TRACE_SEQ_CHECK(s);
+	TRACE_SEQ_CHECK_RET0(s);
 
 	len = strlen(str);
 
 	while (len > ((s->buffer_size - 1) - s->len))
 		expand_buffer(s);
 
+	TRACE_SEQ_CHECK_RET0(s);
+
 	memcpy(s->buffer + s->len, str, len);
 	s->len += len;
 
@@ -187,11 +203,13 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
 
 int trace_seq_putc(struct trace_seq *s, unsigned char c)
 {
-	TRACE_SEQ_CHECK(s);
+	TRACE_SEQ_CHECK_RET0(s);
 
 	while (s->len >= (s->buffer_size - 1))
 		expand_buffer(s);
 
+	TRACE_SEQ_CHECK_RET0(s);
+
 	s->buffer[s->len++] = c;
 
 	return 1;
@@ -199,7 +217,7 @@ int trace_seq_putc(struct trace_seq *s, unsigned char c)
 
 void trace_seq_terminate(struct trace_seq *s)
 {
-	TRACE_SEQ_CHECK(s);
+	TRACE_SEQ_CHECK_RET(s);
 
 	/* There's always one character left on the buffer */
 	s->buffer[s->len] = 0;
@@ -208,5 +226,16 @@ void trace_seq_terminate(struct trace_seq *s)
 int trace_seq_do_printf(struct trace_seq *s)
 {
 	TRACE_SEQ_CHECK(s);
-	return printf("%.*s", s->len, s->buffer);
+
+	switch (s->state) {
+	case TRACE_SEQ__GOOD:
+		return printf("%.*s", s->len, s->buffer);
+	case TRACE_SEQ__BUFFER_POISONED:
+		puts("Usage of trace_seq after it was destroyed");
+		break;
+	case TRACE_SEQ__MEM_ALLOC_FAILED:
+		puts("Can't allocate trace_seq buffer memory");
+		break;
+	}
+	return -1;
 }
-- 
1.7.11.7

Re: [PATCH 01/17] tools lib traceevent: Add state member to struct trace_seq

[Steven Rostedt]

On Wed, 15 Jan 2014 11:49:28 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> Hi Steve,
> 
> On Tue, 14 Jan 2014 21:00:58 -0500, Steven Rostedt wrote:
> > On Wed, 15 Jan 2014 10:45:24 +0900
> > Namhyung Kim <namhyung@kernel.org> wrote:
> >
> >>  
> >> @@ -32,8 +33,9 @@
> >>  #define TRACE_SEQ_POISON	((void *)0xdeadbeef)
> >>  #define TRACE_SEQ_CHECK(s)						\
> >>  do {									\
> >> -	if ((s)->buffer == TRACE_SEQ_POISON)			\
> >> -		die("Usage of trace_seq after it was destroyed");	\
> >> +	if (WARN_ONCE((s)->buffer == TRACE_SEQ_POISON,			\
> >> +		      "Usage of trace_seq after it was destroyed"))	\
> >> +		(s)->state = TRACE_SEQ__BUFFER_POISONED;		\
> >>  } while (0)
> >>  
> >> @@ -189,9 +205,15 @@ int trace_seq_putc(struct trace_seq *s, unsigned char c)
> >>  {
> >>  	TRACE_SEQ_CHECK(s);
> >>  
> >> +	if (s->state != TRACE_SEQ__GOOD)
> >> +		return 0;
> >> +
> >
> > Instead of adding all of these, we can extend the macro
> > TRACE_SEQ_CHECK() which does a
> > 	if (s->state != TRACE_SEQ__GOOD)
> > 		return;
> >
> > and a TRACE_SEQ_CHECK_RET() that does a return 0;
> 
> Oh, it looks better.  But I'd like to TRACE_SEQ_CHECK() as is for some
> cases.  How about this?
> 
> 

Looks good to me.

Acked-by: Steven Rostedt <rostedt@goodmis.org>

I'll try to look at the rest of the patches tomorrow.

-- Steve

Re: [PATCH 01/17] tools lib traceevent: Add state member to struct trace_seq

[Namhyung Kim]

On Tue, 14 Jan 2014 21:56:55 -0500, Steven Rostedt wrote:
> On Wed, 15 Jan 2014 11:49:28 +0900
> Namhyung Kim <namhyung@kernel.org> wrote:
>> Oh, it looks better.  But I'd like to TRACE_SEQ_CHECK() as is for some
>> cases.  How about this?
>
> Looks good to me.
>
> Acked-by: Steven Rostedt <rostedt@goodmis.org>
>
> I'll try to look at the rest of the patches tomorrow.

Thanks, and I found that trace_seq_destroy() should use
TRACE_SEQ_TRACE_RET instead.  Here's an updated patch.

Thanks,
Namhyung


>From 539a5dd1cb3ba4cb03c283115a9a84f8e345514e Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung@kernel.org>
Date: Thu, 19 Dec 2013 18:17:44 +0900
Subject: [PATCH] tools lib traceevent: Add state member to struct trace_seq

The trace_seq->state is for tracking errors during the use of
trace_seq APIs and getting rid of die() in it.

Acked-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/Makefile      |  2 +-
 tools/lib/traceevent/event-parse.h |  7 +++++
 tools/lib/traceevent/trace-seq.c   | 55 +++++++++++++++++++++++++++++---------
 3 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index f778d48ac609..56d52a33a3df 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -136,7 +136,7 @@ export Q VERBOSE
 
 EVENT_PARSE_VERSION = $(EP_VERSION).$(EP_PATCHLEVEL).$(EP_EXTRAVERSION)
 
-INCLUDES = -I. $(CONFIG_INCLUDES)
+INCLUDES = -I. -I $(srctree)/../../include $(CONFIG_INCLUDES)
 
 # Set compile option CFLAGS if not set elsewhere
 CFLAGS ?= -g -Wall
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index cf5db9013f2c..3c890cb28db7 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -58,6 +58,12 @@ struct pevent_record {
 #endif
 };
 
+enum trace_seq_fail {
+	TRACE_SEQ__GOOD,
+	TRACE_SEQ__BUFFER_POISONED,
+	TRACE_SEQ__MEM_ALLOC_FAILED,
+};
+
 /*
  * Trace sequences are used to allow a function to call several other functions
  * to create a string of data to use (up to a max of PAGE_SIZE).
@@ -68,6 +74,7 @@ struct trace_seq {
 	unsigned int		buffer_size;
 	unsigned int		len;
 	unsigned int		readpos;
+	enum trace_seq_fail	state;
 };
 
 void trace_seq_init(struct trace_seq *s);
diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
index d7f2e68bc5b9..f7112138e6af 100644
--- a/tools/lib/traceevent/trace-seq.c
+++ b/tools/lib/traceevent/trace-seq.c
@@ -22,6 +22,7 @@
 #include <string.h>
 #include <stdarg.h>
 
+#include <asm/bug.h>
 #include "event-parse.h"
 #include "event-utils.h"
 
@@ -32,10 +33,21 @@
 #define TRACE_SEQ_POISON	((void *)0xdeadbeef)
 #define TRACE_SEQ_CHECK(s)						\
 do {									\
-	if ((s)->buffer == TRACE_SEQ_POISON)			\
-		die("Usage of trace_seq after it was destroyed");	\
+	if (WARN_ONCE((s)->buffer == TRACE_SEQ_POISON,			\
+		      "Usage of trace_seq after it was destroyed"))	\
+		(s)->state = TRACE_SEQ__BUFFER_POISONED;		\
 } while (0)
 
+#define TRACE_SEQ_CHECK_RET_N(s, n)		\
+do {						\
+	TRACE_SEQ_CHECK(s);			\
+	if ((s)->state != TRACE_SEQ__GOOD)	\
+		return n; 			\
+} while (0)
+
+#define TRACE_SEQ_CHECK_RET(s)   TRACE_SEQ_CHECK_RET_N(s, )
+#define TRACE_SEQ_CHECK_RET0(s)  TRACE_SEQ_CHECK_RET_N(s, 0)
+
 /**
  * trace_seq_init - initialize the trace_seq structure
  * @s: a pointer to the trace_seq structure to initialize
@@ -46,6 +58,7 @@ void trace_seq_init(struct trace_seq *s)
 	s->readpos = 0;
 	s->buffer_size = TRACE_SEQ_BUF_SIZE;
 	s->buffer = malloc_or_die(s->buffer_size);
+	s->state = TRACE_SEQ__GOOD;
 }
 
 /**
@@ -71,7 +84,7 @@ void trace_seq_destroy(struct trace_seq *s)
 {
 	if (!s)
 		return;
-	TRACE_SEQ_CHECK(s);
+	TRACE_SEQ_CHECK_RET(s);
 	free(s->buffer);
 	s->buffer = TRACE_SEQ_POISON;
 }
@@ -80,8 +93,9 @@ static void expand_buffer(struct trace_seq *s)
 {
 	s->buffer_size += TRACE_SEQ_BUF_SIZE;
 	s->buffer = realloc(s->buffer, s->buffer_size);
-	if (!s->buffer)
-		die("Can't allocate trace_seq buffer memory");
+	if (WARN_ONCE(!s->buffer,
+		      "Can't allocate trace_seq buffer memory"))
+		s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
 }
 
 /**
@@ -105,9 +119,9 @@ trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
 	int len;
 	int ret;
 
-	TRACE_SEQ_CHECK(s);
-
  try_again:
+	TRACE_SEQ_CHECK_RET0(s);
+
 	len = (s->buffer_size - 1) - s->len;
 
 	va_start(ap, fmt);
@@ -141,9 +155,9 @@ trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args)
 	int len;
 	int ret;
 
-	TRACE_SEQ_CHECK(s);
-
  try_again:
+	TRACE_SEQ_CHECK_RET0(s);
+
 	len = (s->buffer_size - 1) - s->len;
 
 	ret = vsnprintf(s->buffer + s->len, len, fmt, args);
@@ -172,13 +186,15 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
 {
 	int len;
 
-	TRACE_SEQ_CHECK(s);
+	TRACE_SEQ_CHECK_RET0(s);
 
 	len = strlen(str);
 
 	while (len > ((s->buffer_size - 1) - s->len))
 		expand_buffer(s);
 
+	TRACE_SEQ_CHECK_RET0(s);
+
 	memcpy(s->buffer + s->len, str, len);
 	s->len += len;
 
@@ -187,11 +203,13 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
 
 int trace_seq_putc(struct trace_seq *s, unsigned char c)
 {
-	TRACE_SEQ_CHECK(s);
+	TRACE_SEQ_CHECK_RET0(s);
 
 	while (s->len >= (s->buffer_size - 1))
 		expand_buffer(s);
 
+	TRACE_SEQ_CHECK_RET0(s);
+
 	s->buffer[s->len++] = c;
 
 	return 1;
@@ -199,7 +217,7 @@ int trace_seq_putc(struct trace_seq *s, unsigned char c)
 
 void trace_seq_terminate(struct trace_seq *s)
 {
-	TRACE_SEQ_CHECK(s);
+	TRACE_SEQ_CHECK_RET(s);
 
 	/* There's always one character left on the buffer */
 	s->buffer[s->len] = 0;
@@ -208,5 +226,16 @@ void trace_seq_terminate(struct trace_seq *s)
 int trace_seq_do_printf(struct trace_seq *s)
 {
 	TRACE_SEQ_CHECK(s);
-	return printf("%.*s", s->len, s->buffer);
+
+	switch (s->state) {
+	case TRACE_SEQ__GOOD:
+		return printf("%.*s", s->len, s->buffer);
+	case TRACE_SEQ__BUFFER_POISONED:
+		puts("Usage of trace_seq after it was destroyed");
+		break;
+	case TRACE_SEQ__MEM_ALLOC_FAILED:
+		puts("Can't allocate trace_seq buffer memory");
+		break;
+	}
+	return -1;
 }
-- 
1.7.11.7

Re: [PATCHSET 00/17] tools lib traceevent: Plugin unload enhancement

[Jiri Olsa]

On Wed, Jan 15, 2014 at 10:45:23AM +0900, Namhyung Kim wrote:
> Hello,
> 
> When plugins are loaded, they register various event and function
> handlers.  But they don't unregister even after the plugins unloaded
> so that events could have refererences to non-existing handlers.
> 
> This patchset added relevant unregister functions to handle that.
> 
> Note that this is not a problem as of now since all of users unload
> plugins after finishing their access to events.  But being a generic
> library, it should be handled properly IMHO.
> 
> The patch 1-4 are resend of the previous die removal series so
> independent to this series.
> 
> I put the series on the 'libtraceevent/plugin-v1' branch in my tree
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> Any comments are welcome, thanks
> Namhyung

looks ok,

Reviewed-by: Jiri Olsa <jolsa@redhat.com>

jirka

Re: [PATCH 01/17] tools lib traceevent: Add state member to struct trace_seq

[Arnaldo Carvalho de Melo]

Em Wed, Jan 15, 2014 at 02:03:20PM +0900, Namhyung Kim escreveu:
> On Tue, 14 Jan 2014 21:56:55 -0500, Steven Rostedt wrote:
> > On Wed, 15 Jan 2014 11:49:28 +0900
> > Namhyung Kim <namhyung@kernel.org> wrote:
> >> Oh, it looks better.  But I'd like to TRACE_SEQ_CHECK() as is for some
> >> cases.  How about this?
> >
> > Looks good to me.
> >
> > Acked-by: Steven Rostedt <rostedt@goodmis.org>
> >
> > I'll try to look at the rest of the patches tomorrow.

Hi Steven, can I keep your ackd-by?
 
> Thanks, and I found that trace_seq_destroy() should use
> TRACE_SEQ_TRACE_RET instead.  Here's an updated patch.
> 
> Thanks,
> Namhyung
> 
> 
> From 539a5dd1cb3ba4cb03c283115a9a84f8e345514e Mon Sep 17 00:00:00 2001
> From: Namhyung Kim <namhyung@kernel.org>
> Date: Thu, 19 Dec 2013 18:17:44 +0900
> Subject: [PATCH] tools lib traceevent: Add state member to struct trace_seq
> 
> The trace_seq->state is for tracking errors during the use of
> trace_seq APIs and getting rid of die() in it.
> 
> Acked-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/lib/traceevent/Makefile      |  2 +-
>  tools/lib/traceevent/event-parse.h |  7 +++++
>  tools/lib/traceevent/trace-seq.c   | 55 +++++++++++++++++++++++++++++---------
>  3 files changed, 50 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
> index f778d48ac609..56d52a33a3df 100644
> --- a/tools/lib/traceevent/Makefile
> +++ b/tools/lib/traceevent/Makefile
> @@ -136,7 +136,7 @@ export Q VERBOSE
>  
>  EVENT_PARSE_VERSION = $(EP_VERSION).$(EP_PATCHLEVEL).$(EP_EXTRAVERSION)
>  
> -INCLUDES = -I. $(CONFIG_INCLUDES)
> +INCLUDES = -I. -I $(srctree)/../../include $(CONFIG_INCLUDES)
>  
>  # Set compile option CFLAGS if not set elsewhere
>  CFLAGS ?= -g -Wall
> diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
> index cf5db9013f2c..3c890cb28db7 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -58,6 +58,12 @@ struct pevent_record {
>  #endif
>  };
>  
> +enum trace_seq_fail {
> +	TRACE_SEQ__GOOD,
> +	TRACE_SEQ__BUFFER_POISONED,
> +	TRACE_SEQ__MEM_ALLOC_FAILED,
> +};
> +
>  /*
>   * Trace sequences are used to allow a function to call several other functions
>   * to create a string of data to use (up to a max of PAGE_SIZE).
> @@ -68,6 +74,7 @@ struct trace_seq {
>  	unsigned int		buffer_size;
>  	unsigned int		len;
>  	unsigned int		readpos;
> +	enum trace_seq_fail	state;
>  };
>  
>  void trace_seq_init(struct trace_seq *s);
> diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
> index d7f2e68bc5b9..f7112138e6af 100644
> --- a/tools/lib/traceevent/trace-seq.c
> +++ b/tools/lib/traceevent/trace-seq.c
> @@ -22,6 +22,7 @@
>  #include <string.h>
>  #include <stdarg.h>
>  
> +#include <asm/bug.h>
>  #include "event-parse.h"
>  #include "event-utils.h"
>  
> @@ -32,10 +33,21 @@
>  #define TRACE_SEQ_POISON	((void *)0xdeadbeef)
>  #define TRACE_SEQ_CHECK(s)						\
>  do {									\
> -	if ((s)->buffer == TRACE_SEQ_POISON)			\
> -		die("Usage of trace_seq after it was destroyed");	\
> +	if (WARN_ONCE((s)->buffer == TRACE_SEQ_POISON,			\
> +		      "Usage of trace_seq after it was destroyed"))	\
> +		(s)->state = TRACE_SEQ__BUFFER_POISONED;		\
>  } while (0)
>  
> +#define TRACE_SEQ_CHECK_RET_N(s, n)		\
> +do {						\
> +	TRACE_SEQ_CHECK(s);			\
> +	if ((s)->state != TRACE_SEQ__GOOD)	\
> +		return n; 			\
> +} while (0)
> +
> +#define TRACE_SEQ_CHECK_RET(s)   TRACE_SEQ_CHECK_RET_N(s, )
> +#define TRACE_SEQ_CHECK_RET0(s)  TRACE_SEQ_CHECK_RET_N(s, 0)
> +
>  /**
>   * trace_seq_init - initialize the trace_seq structure
>   * @s: a pointer to the trace_seq structure to initialize
> @@ -46,6 +58,7 @@ void trace_seq_init(struct trace_seq *s)
>  	s->readpos = 0;
>  	s->buffer_size = TRACE_SEQ_BUF_SIZE;
>  	s->buffer = malloc_or_die(s->buffer_size);
> +	s->state = TRACE_SEQ__GOOD;
>  }
>  
>  /**
> @@ -71,7 +84,7 @@ void trace_seq_destroy(struct trace_seq *s)
>  {
>  	if (!s)
>  		return;
> -	TRACE_SEQ_CHECK(s);
> +	TRACE_SEQ_CHECK_RET(s);
>  	free(s->buffer);
>  	s->buffer = TRACE_SEQ_POISON;
>  }
> @@ -80,8 +93,9 @@ static void expand_buffer(struct trace_seq *s)
>  {
>  	s->buffer_size += TRACE_SEQ_BUF_SIZE;
>  	s->buffer = realloc(s->buffer, s->buffer_size);
> -	if (!s->buffer)
> -		die("Can't allocate trace_seq buffer memory");
> +	if (WARN_ONCE(!s->buffer,
> +		      "Can't allocate trace_seq buffer memory"))
> +		s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
>  }
>  
>  /**
> @@ -105,9 +119,9 @@ trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
>  	int len;
>  	int ret;
>  
> -	TRACE_SEQ_CHECK(s);
> -
>   try_again:
> +	TRACE_SEQ_CHECK_RET0(s);
> +
>  	len = (s->buffer_size - 1) - s->len;
>  
>  	va_start(ap, fmt);
> @@ -141,9 +155,9 @@ trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args)
>  	int len;
>  	int ret;
>  
> -	TRACE_SEQ_CHECK(s);
> -
>   try_again:
> +	TRACE_SEQ_CHECK_RET0(s);
> +
>  	len = (s->buffer_size - 1) - s->len;
>  
>  	ret = vsnprintf(s->buffer + s->len, len, fmt, args);
> @@ -172,13 +186,15 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
>  {
>  	int len;
>  
> -	TRACE_SEQ_CHECK(s);
> +	TRACE_SEQ_CHECK_RET0(s);
>  
>  	len = strlen(str);
>  
>  	while (len > ((s->buffer_size - 1) - s->len))
>  		expand_buffer(s);
>  
> +	TRACE_SEQ_CHECK_RET0(s);
> +
>  	memcpy(s->buffer + s->len, str, len);
>  	s->len += len;
>  
> @@ -187,11 +203,13 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
>  
>  int trace_seq_putc(struct trace_seq *s, unsigned char c)
>  {
> -	TRACE_SEQ_CHECK(s);
> +	TRACE_SEQ_CHECK_RET0(s);
>  
>  	while (s->len >= (s->buffer_size - 1))
>  		expand_buffer(s);
>  
> +	TRACE_SEQ_CHECK_RET0(s);
> +
>  	s->buffer[s->len++] = c;
>  
>  	return 1;
> @@ -199,7 +217,7 @@ int trace_seq_putc(struct trace_seq *s, unsigned char c)
>  
>  void trace_seq_terminate(struct trace_seq *s)
>  {
> -	TRACE_SEQ_CHECK(s);
> +	TRACE_SEQ_CHECK_RET(s);
>  
>  	/* There's always one character left on the buffer */
>  	s->buffer[s->len] = 0;
> @@ -208,5 +226,16 @@ void trace_seq_terminate(struct trace_seq *s)
>  int trace_seq_do_printf(struct trace_seq *s)
>  {
>  	TRACE_SEQ_CHECK(s);
> -	return printf("%.*s", s->len, s->buffer);
> +
> +	switch (s->state) {
> +	case TRACE_SEQ__GOOD:
> +		return printf("%.*s", s->len, s->buffer);
> +	case TRACE_SEQ__BUFFER_POISONED:
> +		puts("Usage of trace_seq after it was destroyed");
> +		break;
> +	case TRACE_SEQ__MEM_ALLOC_FAILED:
> +		puts("Can't allocate trace_seq buffer memory");
> +		break;
> +	}
> +	return -1;
>  }
> -- 
> 1.7.11.7

Re: [PATCH 01/17] tools lib traceevent: Add state member to struct trace_seq

[Steven Rostedt]

On Wed, 15 Jan 2014 11:15:19 -0300
Arnaldo Carvalho de Melo <acme@ghostprotocols.net> wrote:

> Em Wed, Jan 15, 2014 at 02:03:20PM +0900, Namhyung Kim escreveu:
> > On Tue, 14 Jan 2014 21:56:55 -0500, Steven Rostedt wrote:
> > > On Wed, 15 Jan 2014 11:49:28 +0900
> > > Namhyung Kim <namhyung@kernel.org> wrote:
> > >> Oh, it looks better.  But I'd like to TRACE_SEQ_CHECK() as is for some
> > >> cases.  How about this?
> > >
> > > Looks good to me.
> > >
> > > Acked-by: Steven Rostedt <rostedt@goodmis.org>
> > >
> > > I'll try to look at the rest of the patches tomorrow.
> 
> Hi Steven, can I keep your ackd-by?
>  

Yep.

-- Steve

Re: [PATCH 06/17] tools lib traceevent: Add pevent_unregister_event_handler()

[Steven Rostedt]

On Wed, 15 Jan 2014 10:45:29 +0900
Namhyung Kim <namhyung@kernel.org> wrote:
  
>  	pr_stat("overriding event (%d) %s:%s with new print handler",
>  		event->id, event->system, event->name);
> @@ -5637,6 +5649,79 @@ int pevent_register_event_handler(struct pevent *pevent, int id,
>  	return -1;
>  }
>  
> +static int handle_matches(struct event_handler *handler, int id,
> +			  const char *sys_name, const char *event_name,
> +			  pevent_event_handler_func func, void *context)
> +{
> +	if (id >= 0 && id != handler->id)
> +		return 0;
> +
> +	if (event_name && (strcmp(event_name, handler->event_name) != 0))
> +		return 0;
> +
> +	if (sys_name && (strcmp(sys_name, handler->sys_name) != 0))
> +		return 0;
> +
> +	if (func != handler->func || context != handler->context)
> +		return 0;
> +
> +	return 1;
> +}
> +
> +/**
> + * pevent_unregister_event_handler - unregister an existing event handler
> + * @pevent: the handle to the pevent
> + * @id: the id of the event to unregister
> + * @sys_name: the system name the handler belongs to
> + * @event_name: the name of the event handler
> + * @func: the function to call to parse the event information
> + * @context: the data to be passed to @func
> + *
> + * This function removes existing event handler (parser).
> + *
> + * If @id is >= 0, then it is used to find the event.
> + * else @sys_name and @event_name are used.
> + *
> + * Returns 0 if handler was removed successfully, -1 if event was not found.
> + */
> +int pevent_unregister_event_handler(struct pevent *pevent, int id,
> +				    const char *sys_name, const char *event_name,
> +				    pevent_event_handler_func func, void *context)
> +{
> +	struct event_format *event;
> +	struct event_handler *handle;
> +	struct event_handler **next;
> +
> +	event = pevent_search_event(pevent, id, sys_name, event_name);
> +	if (event == NULL)
> +		goto not_found;
> +
> +	if (event->handler == func && event->context == context) {
> +		pr_stat("overriding event (%d) %s:%s with default print handler",

Should we use the word "overriding" again? Perhaps saying:

"removing override print handler (%d) %s:%s. Going back to default handler."

Or something?

-- Steve

> +			event->id, event->system, event->name);
> +
> +		event->handler = NULL;
> +		event->context = NULL;
> +		return 0;
> +	}
> +
> +not_found:
> +	for (next = &pevent->handlers; *next; next = &(*next)->next) {
> +		handle = *next;
> +		if (handle_matches(handle, id, sys_name, event_name,
> +				   func, context))
> +			break;
> +	}
> +
> +	if (!(*next))
> +		return -1;
> +
> +	*next = handle->next;
> +	free_handler(handle);
> +
> +	return 0;
> +}
> +

Re: [PATCH 07/17] tools lib traceevent: Add pevent_unregister_print_function()

[Steven Rostedt]

On Wed, 15 Jan 2014 10:45:30 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> When a plugin unloaded it needs to unregister its print handler from
> pevent.  So add unregister function to do it.
> 
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

Re: [PATCH 08/17] tools lib traceevent: Unregister handler when function plugin unloaded

[Steven Rostedt]

On Wed, 15 Jan 2014 10:45:31 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> The function handler should be unregistered when the plugin unloaded
> otherwise it'll try to access invalid memory.
> 
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve


> ---
>  tools/lib/traceevent/plugin_function.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/lib/traceevent/plugin_function.c b/tools/lib/traceevent/plugin_function.c
> index 39461485f9a7..80ba4ff1fe84 100644
> --- a/tools/lib/traceevent/plugin_function.c
> +++ b/tools/lib/traceevent/plugin_function.c
> @@ -148,6 +148,9 @@ void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
>  {
>  	int i, x;
>  
> +	pevent_unregister_event_handler(pevent, -1, "ftrace", "function",
> +					function_handler, NULL);
> +
>  	for (i = 0; i <= cpus; i++) {
>  		for (x = 0; x < fstack[i].size && fstack[i].stack[x]; x++)
>  			free(fstack[i].stack[x]);

Re: [PATCH 09/17] tools lib traceevent: Unregister handler when hrtimer plugin unloaded

[Steven Rostedt]

On Wed, 15 Jan 2014 10:45:32 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> The timer handlers should be unregistered when the plugin unloaded
> otherwise they'll try to access invalid memory.
> 
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

Re: [PATCH 06/17] tools lib traceevent: Add pevent_unregister_event_handler()

[Arnaldo Carvalho de Melo]

Em Wed, Jan 15, 2014 at 11:18:39AM -0500, Steven Rostedt escreveu:
> On Wed, 15 Jan 2014 10:45:29 +0900
> Namhyung Kim <namhyung@kernel.org> wrote:
>   
> >  	pr_stat("overriding event (%d) %s:%s with new print handler",
> >  		event->id, event->system, event->name);
> > @@ -5637,6 +5649,79 @@ int pevent_register_event_handler(struct pevent *pevent, int id,
> >  	return -1;
> >  }
> >  
> > +static int handle_matches(struct event_handler *handler, int id,
> > +			  const char *sys_name, const char *event_name,
> > +			  pevent_event_handler_func func, void *context)
> > +{
> > +	if (id >= 0 && id != handler->id)
> > +		return 0;
> > +
> > +	if (event_name && (strcmp(event_name, handler->event_name) != 0))
> > +		return 0;
> > +
> > +	if (sys_name && (strcmp(sys_name, handler->sys_name) != 0))
> > +		return 0;
> > +
> > +	if (func != handler->func || context != handler->context)
> > +		return 0;
> > +
> > +	return 1;
> > +}
> > +
> > +/**
> > + * pevent_unregister_event_handler - unregister an existing event handler
> > + * @pevent: the handle to the pevent
> > + * @id: the id of the event to unregister
> > + * @sys_name: the system name the handler belongs to
> > + * @event_name: the name of the event handler
> > + * @func: the function to call to parse the event information
> > + * @context: the data to be passed to @func
> > + *
> > + * This function removes existing event handler (parser).
> > + *
> > + * If @id is >= 0, then it is used to find the event.
> > + * else @sys_name and @event_name are used.
> > + *
> > + * Returns 0 if handler was removed successfully, -1 if event was not found.
> > + */
> > +int pevent_unregister_event_handler(struct pevent *pevent, int id,
> > +				    const char *sys_name, const char *event_name,
> > +				    pevent_event_handler_func func, void *context)
> > +{
> > +	struct event_format *event;
> > +	struct event_handler *handle;
> > +	struct event_handler **next;
> > +
> > +	event = pevent_search_event(pevent, id, sys_name, event_name);
> > +	if (event == NULL)
> > +		goto not_found;
> > +
> > +	if (event->handler == func && event->context == context) {
> > +		pr_stat("overriding event (%d) %s:%s with default print handler",
> 
> Should we use the word "overriding" again? Perhaps saying:
> 
> "removing override print handler (%d) %s:%s. Going back to default handler."
> 
> Or something?

Applied up to the patch just before this one, waiting for a resolution.
 
> -- Steve
> 
> > +			event->id, event->system, event->name);
> > +
> > +		event->handler = NULL;
> > +		event->context = NULL;
> > +		return 0;
> > +	}
> > +
> > +not_found:
> > +	for (next = &pevent->handlers; *next; next = &(*next)->next) {
> > +		handle = *next;
> > +		if (handle_matches(handle, id, sys_name, event_name,
> > +				   func, context))
> > +			break;
> > +	}
> > +
> > +	if (!(*next))
> > +		return -1;
> > +
> > +	*next = handle->next;
> > +	free_handler(handle);
> > +
> > +	return 0;
> > +}
> > +

Re: [PATCH 13/17] tools lib traceevent: Unregister handler when mac80211 plugin unloaded

[Steven Rostedt]

On Wed, 15 Jan 2014 10:45:36 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> The event handler should be unregistered when the plugin unloaded
> otherwise it'll try to access invalid memory.
> 
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

For PATCH 10-13,

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

Re: [PATCH 14/17] tools lib traceevent: Unregister handler when cfg80211 plugin unloaded

[Steven Rostedt]

On Wed, 15 Jan 2014 10:45:37 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> The function handler should be unregistered when the plugin unloaded
> otherwise it'll try to access invalid memory.
> 
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/lib/traceevent/plugin_cfg80211.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tools/lib/traceevent/plugin_cfg80211.c b/tools/lib/traceevent/plugin_cfg80211.c
> index dcab8e873c21..7b07d149557b 100644
> --- a/tools/lib/traceevent/plugin_cfg80211.c
> +++ b/tools/lib/traceevent/plugin_cfg80211.c
> @@ -22,3 +22,10 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
>  				       PEVENT_FUNC_ARG_VOID);
>  	return 0;
>  }
> +
> +void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
> +{
> +	pevent_unregister_print_function(pevent,
> +					 process___le16_to_cpup,
> +					 "__le16_to_cpup");
> +}

Can we format this a little nicer:

	pevent_unregister_print_function(pevent, process___le16_to_cpup,
					 "__le16_to_cpup");

I know the register had that separated, but that's because it had args
too, and it made it look nicer. But this don't work here. It looks odd.

-- Steve

Re: [PATCH 15/17] tools lib traceevent: Unregister handler when jbd2 plugin unloaded

[Steven Rostedt]

On Wed, 15 Jan 2014 10:45:38 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> The function handlers should be unregistered when the plugin unloaded
> otherwise they'll try to access invalid memory.
> 
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/lib/traceevent/plugin_jbd2.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/tools/lib/traceevent/plugin_jbd2.c b/tools/lib/traceevent/plugin_jbd2.c
> index 2f93f81f0bac..88fe6fe872d5 100644
> --- a/tools/lib/traceevent/plugin_jbd2.c
> +++ b/tools/lib/traceevent/plugin_jbd2.c
> @@ -66,3 +66,14 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
>  				       PEVENT_FUNC_ARG_VOID);
>  	return 0;
>  }
> +
> +void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
> +{
> +	pevent_unregister_print_function(pevent,
> +					 process_jbd2_dev_to_name,
> +					 "jbd2_dev_to_name");
> +
> +	pevent_unregister_print_function(pevent,
> +					 process_jiffies_to_msecs,
> +					 "jiffies_to_msecs");
> +}

Same here. I don't know about others, but I think it looks a bit nicer
if we move the function up to the first line.

-- Steve

Re: [PATCH 16/17] tools lib traceevent: Unregister handler when scsi plugin unloaded

[Steven Rostedt]

On Wed, 15 Jan 2014 10:45:39 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> The function handler should be unregistered when the plugin unloaded
> otherwise it'll try to access invalid memory.
> 
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/lib/traceevent/plugin_scsi.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tools/lib/traceevent/plugin_scsi.c b/tools/lib/traceevent/plugin_scsi.c
> index 6fb8e3e3fcad..64d688534518 100644
> --- a/tools/lib/traceevent/plugin_scsi.c
> +++ b/tools/lib/traceevent/plugin_scsi.c
> @@ -421,3 +421,10 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
>  				       PEVENT_FUNC_ARG_VOID);
>  	return 0;
>  }
> +
> +void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
> +{
> +	pevent_unregister_print_function(pevent,
> +					 process_scsi_trace_parse_cdb,
> +					 "scsi_trace_parse_cdb");
> +}

Seems we have a trend ;-)

Same for patch 17.

-- Steve

Re: [PATCH 06/17] tools lib traceevent: Add pevent_unregister_event_handler()

[Namhyung Kim]

Hi Steve,

2014-01-16 AM 1:18, Steven Rostedt wrote:
> On Wed, 15 Jan 2014 10:45:29 +0900
> Namhyung Kim <namhyung@kernel.org> wrote:
>> +	if (event->handler == func && event->context == context) {
>> +		pr_stat("overriding event (%d) %s:%s with default print handler",
>
> Should we use the word "overriding" again? Perhaps saying:
>
> "removing override print handler (%d) %s:%s. Going back to default handler."
>
> Or something?

What about this:

"removing override handler for event (%d) %s:%s. Going back to default 
handler."

Thanks,
Namhyung

Re: [PATCH 06/17] tools lib traceevent: Add pevent_unregister_event_handler()

[Steven Rostedt]

On Thu, 16 Jan 2014 09:00:28 +0900
Namhyung Kim <namhyung.kim@lge.com> wrote:

> Hi Steve,
> 
> 2014-01-16 AM 1:18, Steven Rostedt wrote:
> > On Wed, 15 Jan 2014 10:45:29 +0900
> > Namhyung Kim <namhyung@kernel.org> wrote:
> >> +	if (event->handler == func && event->context == context) {
> >> +		pr_stat("overriding event (%d) %s:%s with default print handler",
> >
> > Should we use the word "overriding" again? Perhaps saying:
> >
> > "removing override print handler (%d) %s:%s. Going back to default handler."
> >
> > Or something?
> 
> What about this:
> 
> "removing override handler for event (%d) %s:%s. Going back to default 
> handler."
> 

Sounds good.

-- Steve

Re: [PATCH 06/17] tools lib traceevent: Add pevent_unregister_event_handler()

[Namhyung Kim]

On Wed, 15 Jan 2014 20:52:59 -0500, Steven Rostedt wrote:
> On Thu, 16 Jan 2014 09:00:28 +0900
> Namhyung Kim <namhyung.kim@lge.com> wrote:
>
>> Hi Steve,
>> 
>> 2014-01-16 AM 1:18, Steven Rostedt wrote:
>> > On Wed, 15 Jan 2014 10:45:29 +0900
>> > Namhyung Kim <namhyung@kernel.org> wrote:
>> >> +	if (event->handler == func && event->context == context) {
>> >> +		pr_stat("overriding event (%d) %s:%s with default print handler",
>> >
>> > Should we use the word "overriding" again? Perhaps saying:
>> >
>> > "removing override print handler (%d) %s:%s. Going back to default handler."
>> >
>> > Or something?
>> 
>> What about this:
>> 
>> "removing override handler for event (%d) %s:%s. Going back to default 
>> handler."
>> 
>
> Sounds good.

Thanks, can I add your ack with this change?

Namhyung

Re: [PATCH 06/17] tools lib traceevent: Add pevent_unregister_event_handler()

[Steven Rostedt]

On Thu, 16 Jan 2014 10:58:05 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> Thanks, can I add your ack with this change?

Yeah, go ahead.

-- Steve

[tip:perf/core] tools lib traceevent: Add state member to struct trace_seq

[tip-bot for Namhyung Kim]

Commit-ID:  3c6d8d84423932f1d9949179c6acdf2405515ee4
Gitweb:     http://git.kernel.org/tip/3c6d8d84423932f1d9949179c6acdf2405515ee4
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 19 Dec 2013 18:17:44 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 15 Jan 2014 15:10:19 -0300

tools lib traceevent: Add state member to struct trace_seq

The trace_seq->state is for tracking errors during the use of trace_seq
APIs and getting rid of die() in it.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/87fvopalbb.fsf@sejong.aot.lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/Makefile      |  2 +-
 tools/lib/traceevent/event-parse.h |  7 +++++
 tools/lib/traceevent/trace-seq.c   | 55 +++++++++++++++++++++++++++++---------
 3 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index f778d48..56d52a3 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -136,7 +136,7 @@ export Q VERBOSE
 
 EVENT_PARSE_VERSION = $(EP_VERSION).$(EP_PATCHLEVEL).$(EP_EXTRAVERSION)
 
-INCLUDES = -I. $(CONFIG_INCLUDES)
+INCLUDES = -I. -I $(srctree)/../../include $(CONFIG_INCLUDES)
 
 # Set compile option CFLAGS if not set elsewhere
 CFLAGS ?= -g -Wall
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index cf5db90..3c890cb 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -58,6 +58,12 @@ struct pevent_record {
 #endif
 };
 
+enum trace_seq_fail {
+	TRACE_SEQ__GOOD,
+	TRACE_SEQ__BUFFER_POISONED,
+	TRACE_SEQ__MEM_ALLOC_FAILED,
+};
+
 /*
  * Trace sequences are used to allow a function to call several other functions
  * to create a string of data to use (up to a max of PAGE_SIZE).
@@ -68,6 +74,7 @@ struct trace_seq {
 	unsigned int		buffer_size;
 	unsigned int		len;
 	unsigned int		readpos;
+	enum trace_seq_fail	state;
 };
 
 void trace_seq_init(struct trace_seq *s);
diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
index d7f2e68..f711213 100644
--- a/tools/lib/traceevent/trace-seq.c
+++ b/tools/lib/traceevent/trace-seq.c
@@ -22,6 +22,7 @@
 #include <string.h>
 #include <stdarg.h>
 
+#include <asm/bug.h>
 #include "event-parse.h"
 #include "event-utils.h"
 
@@ -32,10 +33,21 @@
 #define TRACE_SEQ_POISON	((void *)0xdeadbeef)
 #define TRACE_SEQ_CHECK(s)						\
 do {									\
-	if ((s)->buffer == TRACE_SEQ_POISON)			\
-		die("Usage of trace_seq after it was destroyed");	\
+	if (WARN_ONCE((s)->buffer == TRACE_SEQ_POISON,			\
+		      "Usage of trace_seq after it was destroyed"))	\
+		(s)->state = TRACE_SEQ__BUFFER_POISONED;		\
 } while (0)
 
+#define TRACE_SEQ_CHECK_RET_N(s, n)		\
+do {						\
+	TRACE_SEQ_CHECK(s);			\
+	if ((s)->state != TRACE_SEQ__GOOD)	\
+		return n; 			\
+} while (0)
+
+#define TRACE_SEQ_CHECK_RET(s)   TRACE_SEQ_CHECK_RET_N(s, )
+#define TRACE_SEQ_CHECK_RET0(s)  TRACE_SEQ_CHECK_RET_N(s, 0)
+
 /**
  * trace_seq_init - initialize the trace_seq structure
  * @s: a pointer to the trace_seq structure to initialize
@@ -46,6 +58,7 @@ void trace_seq_init(struct trace_seq *s)
 	s->readpos = 0;
 	s->buffer_size = TRACE_SEQ_BUF_SIZE;
 	s->buffer = malloc_or_die(s->buffer_size);
+	s->state = TRACE_SEQ__GOOD;
 }
 
 /**
@@ -71,7 +84,7 @@ void trace_seq_destroy(struct trace_seq *s)
 {
 	if (!s)
 		return;
-	TRACE_SEQ_CHECK(s);
+	TRACE_SEQ_CHECK_RET(s);
 	free(s->buffer);
 	s->buffer = TRACE_SEQ_POISON;
 }
@@ -80,8 +93,9 @@ static void expand_buffer(struct trace_seq *s)
 {
 	s->buffer_size += TRACE_SEQ_BUF_SIZE;
 	s->buffer = realloc(s->buffer, s->buffer_size);
-	if (!s->buffer)
-		die("Can't allocate trace_seq buffer memory");
+	if (WARN_ONCE(!s->buffer,
+		      "Can't allocate trace_seq buffer memory"))
+		s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
 }
 
 /**
@@ -105,9 +119,9 @@ trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
 	int len;
 	int ret;
 
-	TRACE_SEQ_CHECK(s);
-
  try_again:
+	TRACE_SEQ_CHECK_RET0(s);
+
 	len = (s->buffer_size - 1) - s->len;
 
 	va_start(ap, fmt);
@@ -141,9 +155,9 @@ trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args)
 	int len;
 	int ret;
 
-	TRACE_SEQ_CHECK(s);
-
  try_again:
+	TRACE_SEQ_CHECK_RET0(s);
+
 	len = (s->buffer_size - 1) - s->len;
 
 	ret = vsnprintf(s->buffer + s->len, len, fmt, args);
@@ -172,13 +186,15 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
 {
 	int len;
 
-	TRACE_SEQ_CHECK(s);
+	TRACE_SEQ_CHECK_RET0(s);
 
 	len = strlen(str);
 
 	while (len > ((s->buffer_size - 1) - s->len))
 		expand_buffer(s);
 
+	TRACE_SEQ_CHECK_RET0(s);
+
 	memcpy(s->buffer + s->len, str, len);
 	s->len += len;
 
@@ -187,11 +203,13 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
 
 int trace_seq_putc(struct trace_seq *s, unsigned char c)
 {
-	TRACE_SEQ_CHECK(s);
+	TRACE_SEQ_CHECK_RET0(s);
 
 	while (s->len >= (s->buffer_size - 1))
 		expand_buffer(s);
 
+	TRACE_SEQ_CHECK_RET0(s);
+
 	s->buffer[s->len++] = c;
 
 	return 1;
@@ -199,7 +217,7 @@ int trace_seq_putc(struct trace_seq *s, unsigned char c)
 
 void trace_seq_terminate(struct trace_seq *s)
 {
-	TRACE_SEQ_CHECK(s);
+	TRACE_SEQ_CHECK_RET(s);
 
 	/* There's always one character left on the buffer */
 	s->buffer[s->len] = 0;
@@ -208,5 +226,16 @@ void trace_seq_terminate(struct trace_seq *s)
 int trace_seq_do_printf(struct trace_seq *s)
 {
 	TRACE_SEQ_CHECK(s);
-	return printf("%.*s", s->len, s->buffer);
+
+	switch (s->state) {
+	case TRACE_SEQ__GOOD:
+		return printf("%.*s", s->len, s->buffer);
+	case TRACE_SEQ__BUFFER_POISONED:
+		puts("Usage of trace_seq after it was destroyed");
+		break;
+	case TRACE_SEQ__MEM_ALLOC_FAILED:
+		puts("Can't allocate trace_seq buffer memory");
+		break;
+	}
+	return -1;
 }

[tip:perf/core] tools lib traceevent: Check return value of realloc()

[tip-bot for Namhyung Kim]

Commit-ID:  3026bba3c37711234771349ca020d9a85e572f60
Gitweb:     http://git.kernel.org/tip/3026bba3c37711234771349ca020d9a85e572f60
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Wed, 15 Jan 2014 10:45:25 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 15 Jan 2014 15:10:28 -0300

tools lib traceevent: Check return value of realloc()

If realloc() fails, it'll leak the buffer.  Also increate buffer size
only if the allocation succeeded.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/1389750340-15965-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/trace-seq.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
index f711213..e454a2c 100644
--- a/tools/lib/traceevent/trace-seq.c
+++ b/tools/lib/traceevent/trace-seq.c
@@ -91,11 +91,16 @@ void trace_seq_destroy(struct trace_seq *s)
 
 static void expand_buffer(struct trace_seq *s)
 {
-	s->buffer_size += TRACE_SEQ_BUF_SIZE;
-	s->buffer = realloc(s->buffer, s->buffer_size);
-	if (WARN_ONCE(!s->buffer,
-		      "Can't allocate trace_seq buffer memory"))
+	char *buf;
+
+	buf = realloc(s->buffer, s->buffer_size + TRACE_SEQ_BUF_SIZE);
+	if (WARN_ONCE(!buf, "Can't allocate trace_seq buffer memory")) {
 		s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
+		return;
+	}
+
+	s->buffer = buf;
+	s->buffer_size += TRACE_SEQ_BUF_SIZE;
 }
 
 /**

[tip:perf/core] tools lib traceevent: Get rid of malloc_or_die() in trace_seq_init()

[tip-bot for Namhyung Kim]

Commit-ID:  504586e0954bcf9550dfdea37d3234174ed1d68f
Gitweb:     http://git.kernel.org/tip/504586e0954bcf9550dfdea37d3234174ed1d68f
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Wed, 15 Jan 2014 10:45:26 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 15 Jan 2014 15:10:32 -0300

tools lib traceevent: Get rid of malloc_or_die() in trace_seq_init()

Use plain malloc() and check its return value.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/1389750340-15965-4-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/trace-seq.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
index e454a2c..ec3bd16 100644
--- a/tools/lib/traceevent/trace-seq.c
+++ b/tools/lib/traceevent/trace-seq.c
@@ -57,8 +57,11 @@ void trace_seq_init(struct trace_seq *s)
 	s->len = 0;
 	s->readpos = 0;
 	s->buffer_size = TRACE_SEQ_BUF_SIZE;
-	s->buffer = malloc_or_die(s->buffer_size);
-	s->state = TRACE_SEQ__GOOD;
+	s->buffer = malloc(s->buffer_size);
+	if (s->buffer != NULL)
+		s->state = TRACE_SEQ__GOOD;
+	else
+		s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
 }
 
 /**

[tip:perf/core] tools lib traceevent: Get rid of die() finally!!

[tip-bot for Namhyung Kim]

Commit-ID:  e825e756f84eab0e68d7d6644c018c3412748406
Gitweb:     http://git.kernel.org/tip/e825e756f84eab0e68d7d6644c018c3412748406
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Wed, 15 Jan 2014 10:45:27 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 15 Jan 2014 15:10:36 -0300

tools lib traceevent: Get rid of die() finally!!

Now all of its users were gone. :)

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: Jiri Olsa <jolsa@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/1389750340-15965-5-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/event-utils.h |  4 ----
 tools/lib/traceevent/parse-utils.c | 44 --------------------------------------
 2 files changed, 48 deletions(-)

diff --git a/tools/lib/traceevent/event-utils.h b/tools/lib/traceevent/event-utils.h
index e76c9ac..d1dc217 100644
--- a/tools/lib/traceevent/event-utils.h
+++ b/tools/lib/traceevent/event-utils.h
@@ -23,18 +23,14 @@
 #include <ctype.h>
 
 /* Can be overridden */
-void die(const char *fmt, ...);
-void *malloc_or_die(unsigned int size);
 void warning(const char *fmt, ...);
 void pr_stat(const char *fmt, ...);
 void vpr_stat(const char *fmt, va_list ap);
 
 /* Always available */
-void __die(const char *fmt, ...);
 void __warning(const char *fmt, ...);
 void __pr_stat(const char *fmt, ...);
 
-void __vdie(const char *fmt, ...);
 void __vwarning(const char *fmt, ...);
 void __vpr_stat(const char *fmt, ...);
 
diff --git a/tools/lib/traceevent/parse-utils.c b/tools/lib/traceevent/parse-utils.c
index bba701c..eda07fa 100644
--- a/tools/lib/traceevent/parse-utils.c
+++ b/tools/lib/traceevent/parse-utils.c
@@ -25,40 +25,6 @@
 
 #define __weak __attribute__((weak))
 
-void __vdie(const char *fmt, va_list ap)
-{
-	int ret = errno;
-
-	if (errno)
-		perror("trace-cmd");
-	else
-		ret = -1;
-
-	fprintf(stderr, "  ");
-	vfprintf(stderr, fmt, ap);
-
-	fprintf(stderr, "\n");
-	exit(ret);
-}
-
-void __die(const char *fmt, ...)
-{
-	va_list ap;
-
-	va_start(ap, fmt);
-	__vdie(fmt, ap);
-	va_end(ap);
-}
-
-void __weak die(const char *fmt, ...)
-{
-	va_list ap;
-
-	va_start(ap, fmt);
-	__vdie(fmt, ap);
-	va_end(ap);
-}
-
 void __vwarning(const char *fmt, va_list ap)
 {
 	if (errno)
@@ -117,13 +83,3 @@ void __weak pr_stat(const char *fmt, ...)
 	__vpr_stat(fmt, ap);
 	va_end(ap);
 }
-
-void __weak *malloc_or_die(unsigned int size)
-{
-	void *data;
-
-	data = malloc(size);
-	if (!data)
-		die("malloc");
-	return data;
-}

[tip:perf/core] tools lib traceevent: Make plugin unload function receive pevent

[tip-bot for Namhyung Kim]

Commit-ID:  8d0c2224ca6e04ba51c403805e7e1e2ca536520b
Gitweb:     http://git.kernel.org/tip/8d0c2224ca6e04ba51c403805e7e1e2ca536520b
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Wed, 15 Jan 2014 10:45:28 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 15 Jan 2014 15:10:40 -0300

tools lib traceevent: Make plugin unload function receive pevent

The PEVENT_PLUGIN_UNLOADER function might need some cleanup using pevent
like unregister some handlers.  So pass pevent as argument.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/1389750340-15965-6-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/event-parse.h     | 7 ++++---
 tools/lib/traceevent/event-plugin.c    | 4 ++--
 tools/lib/traceevent/plugin_function.c | 2 +-
 tools/perf/util/trace-event.c          | 2 +-
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 3c890cb..a3beca5 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -105,7 +105,7 @@ typedef int (*pevent_event_handler_func)(struct trace_seq *s,
 					 void *context);
 
 typedef int (*pevent_plugin_load_func)(struct pevent *pevent);
-typedef int (*pevent_plugin_unload_func)(void);
+typedef int (*pevent_plugin_unload_func)(struct pevent *pevent);
 
 struct plugin_option {
 	struct plugin_option		*next;
@@ -130,7 +130,7 @@ struct plugin_option {
  * PEVENT_PLUGIN_UNLOADER:  (optional)
  *   The function called just before unloading
  *
- *   int PEVENT_PLUGIN_UNLOADER(void)
+ *   int PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
  *
  * PEVENT_PLUGIN_OPTIONS:  (optional)
  *   Plugin options that can be set before loading
@@ -411,7 +411,8 @@ enum pevent_errno {
 struct plugin_list;
 
 struct plugin_list *traceevent_load_plugins(struct pevent *pevent);
-void traceevent_unload_plugins(struct plugin_list *plugin_list);
+void traceevent_unload_plugins(struct plugin_list *plugin_list,
+			       struct pevent *pevent);
 
 struct cmdline;
 struct cmdline_list;
diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
index 125f567..0c8bf67 100644
--- a/tools/lib/traceevent/event-plugin.c
+++ b/tools/lib/traceevent/event-plugin.c
@@ -197,7 +197,7 @@ traceevent_load_plugins(struct pevent *pevent)
 }
 
 void
-traceevent_unload_plugins(struct plugin_list *plugin_list)
+traceevent_unload_plugins(struct plugin_list *plugin_list, struct pevent *pevent)
 {
 	pevent_plugin_unload_func func;
 	struct plugin_list *list;
@@ -207,7 +207,7 @@ traceevent_unload_plugins(struct plugin_list *plugin_list)
 		plugin_list = list->next;
 		func = dlsym(list->handle, PEVENT_PLUGIN_UNLOADER_NAME);
 		if (func)
-			func();
+			func(pevent);
 		dlclose(list->handle);
 		free(list->name);
 		free(list);
diff --git a/tools/lib/traceevent/plugin_function.c b/tools/lib/traceevent/plugin_function.c
index aad92ad..3946148 100644
--- a/tools/lib/traceevent/plugin_function.c
+++ b/tools/lib/traceevent/plugin_function.c
@@ -144,7 +144,7 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
 	return 0;
 }
 
-void PEVENT_PLUGIN_UNLOADER(void)
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
 {
 	int i, x;
 
diff --git a/tools/perf/util/trace-event.c b/tools/perf/util/trace-event.c
index d9f5f61..6322d37 100644
--- a/tools/perf/util/trace-event.c
+++ b/tools/perf/util/trace-event.c
@@ -34,8 +34,8 @@ int trace_event__init(struct trace_event *t)
 
 void trace_event__cleanup(struct trace_event *t)
 {
+	traceevent_unload_plugins(t->plugin_list, t->pevent);
 	pevent_free(t->pevent);
-	traceevent_unload_plugins(t->plugin_list);
 }
 
 static struct event_format*