Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

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

4 views
Skip to first unread message

Namhyung Kim

unread,
Jan 14, 2014, 8:50:01 PM1/14/14
to
Now all of its users were gone. :)

Signed-off-by: Namhyung Kim <namh...@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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Namhyung Kim

unread,
Jan 14, 2014, 8:50:01 PM1/14/14
to
The kmem handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.

Cc: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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);
+}

Namhyung Kim

unread,
Jan 14, 2014, 8:50:01 PM1/14/14
to
The PEVENT_PLUGIN_UNLOADER function might need some cleanup using
pevent like unregister some handlers. So pass pevent as argument.

Cc: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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*

Namhyung Kim

unread,
Jan 14, 2014, 8:50:02 PM1/14/14
to
Use plain malloc() and check its return value.

Signed-off-by: Namhyung Kim <namh...@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;
}

/**

Namhyung Kim

unread,
Jan 14, 2014, 8:50:02 PM1/14/14
to
When a plugin unloaded it needs to unregister its print handler from
pevent. So add unregister function to do it.

Cc: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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);

Namhyung Kim

unread,
Jan 14, 2014, 8:50:02 PM1/14/14
to
When a plugin unloaded it needs to unregister its handler from pevent.
So add unregister function to do it.

Cc: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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,

Namhyung Kim

unread,
Jan 14, 2014, 8:50:02 PM1/14/14
to
The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.

Cc: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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");
+}

Namhyung Kim

unread,
Jan 14, 2014, 8:50:02 PM1/14/14
to
The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.

Cc: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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]);

Namhyung Kim

unread,
Jan 14, 2014, 8:50:02 PM1/14/14
to
The function handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.

Cc: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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");
+}

Namhyung Kim

unread,
Jan 14, 2014, 8:50:02 PM1/14/14
to
The event handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.

Cc: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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);

Namhyung Kim

unread,
Jan 14, 2014, 8:50:02 PM1/14/14
to
The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.

Cc: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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");

Namhyung Kim

unread,
Jan 14, 2014, 8:50:02 PM1/14/14
to
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 <namh...@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;

Namhyung Kim

unread,
Jan 14, 2014, 8:50:02 PM1/14/14
to
The event handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.

Cc: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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);
+}

Namhyung Kim

unread,
Jan 14, 2014, 8:50:02 PM1/14/14
to
The timer handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.

Cc: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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);

Namhyung Kim

unread,
Jan 14, 2014, 8:50:02 PM1/14/14
to
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(-)

Namhyung Kim

unread,
Jan 14, 2014, 8:50:01 PM1/14/14
to
The kvm handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.

Cc: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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");
+}

Namhyung Kim

unread,
Jan 14, 2014, 8:50:02 PM1/14/14
to
The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.

Cc: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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");
+}

Namhyung Kim

unread,
Jan 14, 2014, 9:00:02 PM1/14/14
to
If realloc() fails, it'll leak the buffer. Also increate buffer size
only if the allocation succeeded.

Signed-off-by: Namhyung Kim <namh...@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;
}

/**

Steven Rostedt

unread,
Jan 14, 2014, 9:10:01 PM1/14/14
to
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

Steven Rostedt

unread,
Jan 14, 2014, 9:10:01 PM1/14/14
to
On Wed, 15 Jan 2014 10:45:25 +0900
Namhyung Kim <namh...@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 <namh...@kernel.org>
> ---

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

-- Steve

Steven Rostedt

unread,
Jan 14, 2014, 9:10:01 PM1/14/14
to
On Wed, 15 Jan 2014 10:45:26 +0900
Namhyung Kim <namh...@kernel.org> wrote:

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

Acked-by: Steven Rostedt <ros...@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;
> }
>
> /**

--

Steven Rostedt

unread,
Jan 14, 2014, 9:30:01 PM1/14/14
to
On Wed, 15 Jan 2014 10:45:28 +0900
Namhyung Kim <namh...@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 <jo...@redhat.com>
> Signed-off-by: Namhyung Kim <namh...@kernel.org>

Acked-by: Steven Rostedt <ros...@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(-)
>


Namhyung Kim

unread,
Jan 14, 2014, 9:50:01 PM1/14/14
to
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 <namh...@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 <namh...@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 <namh...@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(-)
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

Steven Rostedt

unread,
Jan 14, 2014, 10:00:02 PM1/14/14
to
On Wed, 15 Jan 2014 11:49:28 +0900
Looks good to me.

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

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

-- Steve

Namhyung Kim

unread,
Jan 15, 2014, 12:10:01 AM1/15/14
to
On Tue, 14 Jan 2014 21:56:55 -0500, Steven Rostedt wrote:
> On Wed, 15 Jan 2014 11:49:28 +0900
> Namhyung Kim <namh...@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 <ros...@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 <namh...@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 <ros...@goodmis.org>
Signed-off-by: Namhyung Kim <namh...@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(-)
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;
int trace_seq_putc(struct trace_seq *s, unsigned char c)
{

Jiri Olsa

unread,
Jan 15, 2014, 8:40:02 AM1/15/14
to
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 <jo...@redhat.com>

jirka

Arnaldo Carvalho de Melo

unread,
Jan 15, 2014, 9:30:02 AM1/15/14
to
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 <namh...@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 <ros...@goodmis.org>
> >
> > I'll try to look at the rest of the patches tomorrow.

Hi Steven, can I keep your ackd-by?

Steven Rostedt

unread,
Jan 15, 2014, 9:30:02 AM1/15/14
to
On Wed, 15 Jan 2014 11:15:19 -0300
Arnaldo Carvalho de Melo <ac...@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 <namh...@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 <ros...@goodmis.org>
> > >
> > > I'll try to look at the rest of the patches tomorrow.
>
> Hi Steven, can I keep your ackd-by?
>

Yep.

-- Steve

Steven Rostedt

unread,
Jan 15, 2014, 11:20:02 AM1/15/14
to
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;
> +}
> +

Steven Rostedt

unread,
Jan 15, 2014, 11:30:02 AM1/15/14
to
On Wed, 15 Jan 2014 10:45:30 +0900
Namhyung Kim <namh...@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 <jo...@redhat.com>
> Signed-off-by: Namhyung Kim <namh...@kernel.org>
> ---

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

-- Steve

Steven Rostedt

unread,
Jan 15, 2014, 11:30:02 AM1/15/14
to
On Wed, 15 Jan 2014 10:45:31 +0900
Namhyung Kim <namh...@kernel.org> wrote:

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

Acked-by: Steven Rostedt <ros...@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]);

--

Steven Rostedt

unread,
Jan 15, 2014, 11:30:02 AM1/15/14
to
On Wed, 15 Jan 2014 10:45:32 +0900
Namhyung Kim <namh...@kernel.org> wrote:

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

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

-- Steve

Arnaldo Carvalho de Melo

unread,
Jan 15, 2014, 1:20:03 PM1/15/14
to
Applied up to the patch just before this one, waiting for a resolution.

Steven Rostedt

unread,
Jan 15, 2014, 3:40:02 PM1/15/14
to
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

Steven Rostedt

unread,
Jan 15, 2014, 3:40:02 PM1/15/14
to
On Wed, 15 Jan 2014 10:45:39 +0900
Namhyung Kim <namh...@kernel.org> wrote:

> The function handler should be unregistered when the plugin unloaded
> otherwise it'll try to access invalid memory.
>
> Cc: Jiri Olsa <jo...@redhat.com>
> Signed-off-by: Namhyung Kim <namh...@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

Arnaldo Carvalho de Melo

unread,
Jan 15, 2014, 3:40:02 PM1/15/14
to
From: Namhyung Kim <namh...@kernel.org>

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 <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389750340-15965-6-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@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 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.8.1.4

Arnaldo Carvalho de Melo

unread,
Jan 15, 2014, 3:40:02 PM1/15/14
to
From: Namhyung Kim <namh...@kernel.org>

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 <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/87fvopa...@sejong.aot.lge.com
Signed-off-by: Arnaldo Carvalho de Melo <ac...@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 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

Arnaldo Carvalho de Melo

unread,
Jan 15, 2014, 3:40:02 PM1/15/14
to
From: Namhyung Kim <namh...@kernel.org>

Now all of its users were gone. :)

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389750340-15965-5-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@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 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;
-}

Arnaldo Carvalho de Melo

unread,
Jan 15, 2014, 3:40:02 PM1/15/14
to
From: Namhyung Kim <namh...@kernel.org>

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

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389750340-15965-3-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@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 f7112138e6af..e454a2c66cac 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;
}

/**

Arnaldo Carvalho de Melo

unread,
Jan 15, 2014, 3:40:02 PM1/15/14
to
From: Namhyung Kim <namh...@kernel.org>

The hist_entry__add_cpumode_period() and hist_entry__decay() functions
are dealing with hist_entry's stat fields only.

Make them he_stat methods then.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Acked-by: Jiri Olsa <jo...@redhat.com>
Cc: Andi Kleen <an...@firstfloor.org>
Cc: Arun Sharma <ash...@fb.com>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Paul Mackerras <pau...@samba.org>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Rodrigo Campos <rod...@sdfg.com.ar>
Link: http://lkml.kernel.org/r/1389677157-30513-5-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/perf/util/hist.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 4ed3e883240d..e4e6249b87d4 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -181,21 +181,21 @@ void hists__output_recalc_col_len(struct hists *hists, int max_rows)
}
}

-static void hist_entry__add_cpumode_period(struct hist_entry *he,
- unsigned int cpumode, u64 period)
+static void he_stat__add_cpumode_period(struct he_stat *he_stat,
+ unsigned int cpumode, u64 period)
{
switch (cpumode) {
case PERF_RECORD_MISC_KERNEL:
- he->stat.period_sys += period;
+ he_stat->period_sys += period;
break;
case PERF_RECORD_MISC_USER:
- he->stat.period_us += period;
+ he_stat->period_us += period;
break;
case PERF_RECORD_MISC_GUEST_KERNEL:
- he->stat.period_guest_sys += period;
+ he_stat->period_guest_sys += period;
break;
case PERF_RECORD_MISC_GUEST_USER:
- he->stat.period_guest_us += period;
+ he_stat->period_guest_us += period;
break;
default:
break;
@@ -222,10 +222,10 @@ static void he_stat__add_stat(struct he_stat *dest, struct he_stat *src)
dest->weight += src->weight;
}

-static void hist_entry__decay(struct hist_entry *he)
+static void he_stat__decay(struct he_stat *he_stat)
{
- he->stat.period = (he->stat.period * 7) / 8;
- he->stat.nr_events = (he->stat.nr_events * 7) / 8;
+ he_stat->period = (he_stat->period * 7) / 8;
+ he_stat->nr_events = (he_stat->nr_events * 7) / 8;
/* XXX need decay for weight too? */
}

@@ -236,7 +236,7 @@ static bool hists__decay_entry(struct hists *hists, struct hist_entry *he)
if (prev_period == 0)
return true;

- hist_entry__decay(he);
+ he_stat__decay(&he->stat);

if (!he->filtered)
hists->stats.total_period -= prev_period - he->stat.period;
@@ -402,7 +402,7 @@ static struct hist_entry *add_hist_entry(struct hists *hists,
rb_link_node(&he->rb_node_in, parent, p);
rb_insert_color(&he->rb_node_in, hists->entries_in);
out:
- hist_entry__add_cpumode_period(he, al->cpumode, period);
+ he_stat__add_cpumode_period(&he->stat, al->cpumode, period);
return he;

Steven Rostedt

unread,
Jan 15, 2014, 3:40:02 PM1/15/14
to
On Wed, 15 Jan 2014 10:45:37 +0900
Namhyung Kim <namh...@kernel.org> wrote:

> The function handler should be unregistered when the plugin unloaded
> otherwise it'll try to access invalid memory.
>
> Cc: Jiri Olsa <jo...@redhat.com>
> Signed-off-by: Namhyung Kim <namh...@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

Steven Rostedt

unread,
Jan 15, 2014, 3:40:02 PM1/15/14
to
On Wed, 15 Jan 2014 10:45:36 +0900
Namhyung Kim <namh...@kernel.org> wrote:

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

For PATCH 10-13,

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

Arnaldo Carvalho de Melo

unread,
Jan 15, 2014, 3:40:02 PM1/15/14
to
From: Namhyung Kim <namh...@kernel.org>

It failed to build perf on my ubuntu 10.04 box (gcc 4.4.3):

CC util/strlist.o
cc1: warnings being treated as errors
util/strlist.c: In function ‘str_node__delete’:
util/strlist.c:42: error: dereferencing type-punned pointer will break strict-aliasing rules
util/strlist.c:42: error: dereferencing type-punned pointer will break strict-aliasing rules
CC util/strfilter.o
make: *** [util/strlist.o] Error 1

CC util/srcline.o
cc1: warnings being treated as errors
util/srcline.c: In function ‘addr2line_init’:
util/srcline.c:132: error: dereferencing type-punned pointer will break strict-aliasing rules
util/srcline.c:132: error: dereferencing type-punned pointer will break strict-aliasing rules
util/srcline.c: In function ‘addr2line_cleanup’:
util/srcline.c:143: error: dereferencing type-punned pointer will break strict-aliasing rules
util/srcline.c:143: error: dereferencing type-punned pointer will break strict-aliasing rules
make: *** [util/srcline.o] Error 1

It seems it only allows to remove 'const' qualifier.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Mark Rutland <mark.r...@arm.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <pet...@infradead.org>
Link: http://lkml.kernel.org/r/1389276479-9047-1-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/perf/util/srcline.c | 4 ++--
tools/perf/util/strlist.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
index 7e67879ebd25..f3e4bc5fe5d2 100644
--- a/tools/perf/util/srcline.c
+++ b/tools/perf/util/srcline.c
@@ -129,7 +129,7 @@ static struct a2l_data *addr2line_init(const char *path)

out:
if (a2l) {
- zfree((void **)&a2l->input);
+ zfree((char **)&a2l->input);
free(a2l);
}
bfd_close(abfd);
@@ -140,7 +140,7 @@ static void addr2line_cleanup(struct a2l_data *a2l)
{
if (a2l->abfd)
bfd_close(a2l->abfd);
- zfree((void **)&a2l->input);
+ zfree((char **)&a2l->input);
zfree(&a2l->syms);
free(a2l);
}
diff --git a/tools/perf/util/strlist.c b/tools/perf/util/strlist.c
index 61a90bf24b4d..71f9d102b96f 100644
--- a/tools/perf/util/strlist.c
+++ b/tools/perf/util/strlist.c
@@ -39,7 +39,7 @@ out_delete:
static void str_node__delete(struct str_node *snode, bool dupstr)
{
if (dupstr)
- zfree((void **)&snode->s);
+ zfree((char **)&snode->s);
free(snode);
}

--
1.8.1.4

Arnaldo Carvalho de Melo

unread,
Jan 15, 2014, 3:40:03 PM1/15/14
to
From: Namhyung Kim <namh...@kernel.org>

The machine__resolve_callchain() is called only if symbol_conf.
use_callchain is set so no need to check it again.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Cc: Andi Kleen <an...@firstfloor.org>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Paul Mackerras <pau...@samba.org>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Rodrigo Campos <rod...@sdfg.com.ar>
Link: http://lkml.kernel.org/r/1389677157-30513-2-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/perf/util/machine.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 0130279aac51..ded74590b92f 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1314,8 +1314,6 @@ static int machine__resolve_callchain_sample(struct machine *machine,
*root_al = al;
callchain_cursor_reset(&callchain_cursor);
}
- if (!symbol_conf.use_callchain)
- break;
}

err = callchain_cursor_append(&callchain_cursor,

Arnaldo Carvalho de Melo

unread,
Jan 15, 2014, 3:40:03 PM1/15/14
to
From: Namhyung Kim <namh...@kernel.org>

Use plain malloc() and check its return value.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389750340-15965-4-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@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 e454a2c66cac..ec3bd16a5488 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;
}

/**

Arnaldo Carvalho de Melo

unread,
Jan 15, 2014, 3:40:03 PM1/15/14
to
From: Mark Rutland <mark.r...@arm.com>

Currently the feature-checks Makefile does not inherit $(CC), and calls
cc rather than $(CROSS_COMPILE)gcc. Thus the feature checks invoke the
native toolchain rather than the cross toolchain, and can identify
features as available when they are not. This can break the build.

Additionally the native pkg-config is always called as opposed to
$(CROSS_COMPILE)pkg-config, so the wrong flags and paths may be passed
to the cross compiler.

This patch passes CROSS_COMPILE down to the feature-checks Makefile, and
forces its use. Additionally pkg-config is replaced with
$(CROSS_COMPILE)pkg-config via a new $(PKG_CONFIG) variable. This patch
has been build tested on x86_64 and arm.

Signed-off-by: Mark Rutland <mark.r...@arm.com>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Will Deacon <will....@arm.com>
Link: http://lkml.kernel.org/r/1389782648-4417-4-git-...@arm.com
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/perf/Makefile.perf | 1 +
tools/perf/config/Makefile | 6 +++---
tools/perf/config/feature-checks/Makefile | 9 +++++----
3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 87d7726cee2d..7257e7e9e38a 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -76,6 +76,7 @@ $(OUTPUT)PERF-VERSION-FILE: ../../.git/HEAD

CC = $(CROSS_COMPILE)gcc
AR = $(CROSS_COMPILE)ar
+PKG_CONFIG = $(CROSS_COMPILE)pkg-config

RM = rm -f
LN = ln -f
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 01dd43df0d04..d604e50fc167 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -372,7 +372,7 @@ ifndef NO_SLANG
endif

ifndef NO_GTK2
- FLAGS_GTK2=$(CFLAGS) $(LDFLAGS) $(EXTLIBS) $(shell pkg-config --libs --cflags gtk+-2.0 2>/dev/null)
+ FLAGS_GTK2=$(CFLAGS) $(LDFLAGS) $(EXTLIBS) $(shell $(PKG_CONFIG) --libs --cflags gtk+-2.0 2>/dev/null)
ifneq ($(feature-gtk2), 1)
msg := $(warning GTK2 not found, disables GTK2 support. Please install gtk2-devel or libgtk2.0-dev);
NO_GTK2 := 1
@@ -381,8 +381,8 @@ ifndef NO_GTK2
GTK_CFLAGS := -DHAVE_GTK_INFO_BAR_SUPPORT
endif
CFLAGS += -DHAVE_GTK2_SUPPORT
- GTK_CFLAGS += $(shell pkg-config --cflags gtk+-2.0 2>/dev/null)
- GTK_LIBS := $(shell pkg-config --libs gtk+-2.0 2>/dev/null)
+ GTK_CFLAGS += $(shell $(PKG_CONFIG) --cflags gtk+-2.0 2>/dev/null)
+ GTK_LIBS := $(shell $(PKG_CONFIG) --libs gtk+-2.0 2>/dev/null)
EXTLIBS += -ldl
endif
endif
diff --git a/tools/perf/config/feature-checks/Makefile b/tools/perf/config/feature-checks/Makefile
index 7cf6fcdacebe..12e551346fa6 100644
--- a/tools/perf/config/feature-checks/Makefile
+++ b/tools/perf/config/feature-checks/Makefile
@@ -28,7 +28,8 @@ FILES= \
test-stackprotector-all.bin \
test-timerfd.bin

-CC := $(CC) -MD
+CC := $(CROSS_COMPILE)gcc -MD
+PKG_CONFIG := $(CROSS_COMPILE)pkg-config

all: $(FILES)

@@ -37,7 +38,7 @@ BUILD = $(CC) $(CFLAGS) -o $(OUTPUT)$@ $(patsubst %.bin,%.c,$@) $(LDFLAGS)
###############################

test-all.bin:
- $(BUILD) -Werror -fstack-protector-all -O2 -Werror -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -laudit -I/usr/include/slang -lslang $(shell pkg-config --libs --cflags gtk+-2.0 2>/dev/null) $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl
+ $(BUILD) -Werror -fstack-protector-all -O2 -Werror -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -laudit -I/usr/include/slang -lslang $(shell $(PKG_CONFIG) --libs --cflags gtk+-2.0 2>/dev/null) $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl

test-hello.bin:
$(BUILD)
@@ -82,10 +83,10 @@ test-libslang.bin:
$(BUILD) -I/usr/include/slang -lslang

test-gtk2.bin:
- $(BUILD) $(shell pkg-config --libs --cflags gtk+-2.0 2>/dev/null)
+ $(BUILD) $(shell $(PKG_CONFIG) --libs --cflags gtk+-2.0 2>/dev/null)

test-gtk2-infobar.bin:
- $(BUILD) $(shell pkg-config --libs --cflags gtk+-2.0 2>/dev/null)
+ $(BUILD) $(shell $(PKG_CONFIG) --libs --cflags gtk+-2.0 2>/dev/null)

grep-libs = $(filter -l%,$(1))
strip-libs = $(filter-out -l%,$(1))

Arnaldo Carvalho de Melo

unread,
Jan 15, 2014, 3:40:03 PM1/15/14
to
From: Namhyung Kim <namh...@kernel.org>

The report__resolve_callchain() can be shared with perf top code as it
doesn't really depend on the perf report code. Factor it out as
sample__resolve_callchain(). The same goes to the hist_entry__append_
callchain() too.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Acked-by: Jiri Olsa <jo...@redhat.com>
Cc: Andi Kleen <an...@firstfloor.org>
Cc: Arun Sharma <ash...@fb.com>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Paul Mackerras <pau...@samba.org>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Rodrigo Campos <rod...@sdfg.com.ar>
Link: http://lkml.kernel.org/r/1389677157-30513-3-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/perf/builtin-report.c | 24 +++---------------------
tools/perf/builtin-top.c | 22 +++++++---------------
tools/perf/util/callchain.c | 23 +++++++++++++++++++++++
tools/perf/util/callchain.h | 6 ++++++
4 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 46864dd7eb83..3c53ec268fbc 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -75,24 +75,6 @@ static int report__config(const char *var, const char *value, void *cb)
return perf_default_config(var, value, cb);
}

-static int report__resolve_callchain(struct report *rep, struct symbol **parent,
- struct perf_evsel *evsel, struct addr_location *al,
- struct perf_sample *sample)
-{
- if ((sort__has_parent || symbol_conf.use_callchain) && sample->callchain) {
- return machine__resolve_callchain(al->machine, evsel, al->thread, sample,
- parent, al, rep->max_stack);
- }
- return 0;
-}
-
-static int hist_entry__append_callchain(struct hist_entry *he, struct perf_sample *sample)
-{
- if (!symbol_conf.use_callchain)
- return 0;
- return callchain_append(he->callchain, &callchain_cursor, sample->period);
-}
-
static int report__add_mem_hist_entry(struct perf_tool *tool, struct addr_location *al,
struct perf_sample *sample, struct perf_evsel *evsel,
union perf_event *event)
@@ -103,7 +85,7 @@ static int report__add_mem_hist_entry(struct perf_tool *tool, struct addr_locati
struct hist_entry *he;
struct mem_info *mi, *mx;
uint64_t cost;
- int err = report__resolve_callchain(rep, &parent, evsel, al, sample);
+ int err = sample__resolve_callchain(sample, &parent, evsel, al, rep->max_stack);

if (err)
return err;
@@ -155,7 +137,7 @@ static int report__add_branch_hist_entry(struct perf_tool *tool, struct addr_loc
unsigned i;
struct hist_entry *he;
struct branch_info *bi, *bx;
- int err = report__resolve_callchain(rep, &parent, evsel, al, sample);
+ int err = sample__resolve_callchain(sample, &parent, evsel, al, rep->max_stack);

if (err)
return err;
@@ -208,7 +190,7 @@ static int report__add_hist_entry(struct perf_tool *tool, struct perf_evsel *evs
struct report *rep = container_of(tool, struct report, tool);
struct symbol *parent = NULL;
struct hist_entry *he;
- int err = report__resolve_callchain(rep, &parent, evsel, al, sample);
+ int err = sample__resolve_callchain(sample, &parent, evsel, al, rep->max_stack);

if (err)
return err;
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 569dd87690ef..76cd510d34d0 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -743,15 +743,10 @@ static void perf_event__process_sample(struct perf_tool *tool,
if (al.sym == NULL || !al.sym->ignore) {
struct hist_entry *he;

- if ((sort__has_parent || symbol_conf.use_callchain) &&
- sample->callchain) {
- err = machine__resolve_callchain(machine, evsel,
- al.thread, sample,
- &parent, &al,
- top->max_stack);
- if (err)
- return;
- }
+ err = sample__resolve_callchain(sample, &parent, evsel, &al,
+ top->max_stack);
+ if (err)
+ return;

he = perf_evsel__add_hist_entry(evsel, &al, sample);
if (he == NULL) {
@@ -759,12 +754,9 @@ static void perf_event__process_sample(struct perf_tool *tool,
return;
}

- if (symbol_conf.use_callchain) {
- err = callchain_append(he->callchain, &callchain_cursor,
- sample->period);
- if (err)
- return;
- }
+ err = hist_entry__append_callchain(he, sample);
+ if (err)
+ return;

if (sort__has_sym)
perf_top__record_precise_ip(top, he, evsel->idx, ip);
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index e3970e3eaacf..9eb4f57f8663 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -17,6 +17,8 @@

#include "hist.h"
#include "util.h"
+#include "sort.h"
+#include "machine.h"
#include "callchain.h"

__thread struct callchain_cursor callchain_cursor;
@@ -531,3 +533,24 @@ int callchain_cursor_append(struct callchain_cursor *cursor,

return 0;
}
+
+int sample__resolve_callchain(struct perf_sample *sample, struct symbol **parent,
+ struct perf_evsel *evsel, struct addr_location *al,
+ int max_stack)
+{
+ if (sample->callchain == NULL)
+ return 0;
+
+ if (symbol_conf.use_callchain || sort__has_parent) {
+ return machine__resolve_callchain(al->machine, evsel, al->thread,
+ sample, parent, al, max_stack);
+ }
+ return 0;
+}
+
+int hist_entry__append_callchain(struct hist_entry *he, struct perf_sample *sample)
+{
+ if (!symbol_conf.use_callchain)
+ return 0;
+ return callchain_append(he->callchain, &callchain_cursor, sample->period);
+}
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index 08b25af9eea1..8ad97e9b119f 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -145,10 +145,16 @@ static inline void callchain_cursor_advance(struct callchain_cursor *cursor)
}

struct option;
+struct hist_entry;

int record_parse_callchain(const char *arg, struct record_opts *opts);
int record_parse_callchain_opt(const struct option *opt, const char *arg, int unset);
int record_callchain_opt(const struct option *opt, const char *arg, int unset);

+int sample__resolve_callchain(struct perf_sample *sample, struct symbol **parent,
+ struct perf_evsel *evsel, struct addr_location *al,
+ int max_stack);
+int hist_entry__append_callchain(struct hist_entry *he, struct perf_sample *sample);
+
extern const char record_callchain_help[];
#endif /* __PERF_CALLCHAIN_H */

Arnaldo Carvalho de Melo

unread,
Jan 15, 2014, 3:40:03 PM1/15/14
to
From: Arnaldo Carvalho de Melo <ac...@redhat.com>

To be consistent with the equivalent option in 'stat', also, for the
same reason, use -D as the one letter alias.

Suggested-by: Ingo Molnar <mi...@kernel.org>
Cc: Adrian Hunter <adrian...@intel.com>
Cc: David Ahern <dsa...@gmail.com>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Mike Galbraith <efa...@gmx.de>
Cc: Namhyung Kim <namh...@kernel.org>
Cc: Paul Mackerras <pau...@samba.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Stephane Eranian <era...@google.com>
Link: http://lkml.kernel.org/n/tip-p5yjnopajb...@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/perf/Documentation/perf-record.txt | 3 ++-
tools/perf/builtin-record.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 3a35a8523e6d..c71b0f36d9e8 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -208,7 +208,8 @@ overrides that and uses per-thread mmaps. A side-effect of that is that
inheritance is automatically disabled. --per-thread is ignored with a warning
if combined with -a or -C options.

---initial-delay msecs::
+-D::
+--delay=::
After starting the program, wait msecs before measuring. This is useful to
filter out the startup phase of the program, which is often very different.

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 78e790f4c54f..3c394bf16fa8 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -882,7 +882,7 @@ const struct option record_options[] = {
OPT_CALLBACK('G', "cgroup", &record.evlist, "name",
"monitor event in cgroup name only",
parse_cgroups),
- OPT_UINTEGER(0, "initial-delay", &record.opts.initial_delay,
+ OPT_UINTEGER('D', "delay", &record.opts.initial_delay,
"ms to wait before starting measurement after program start"),
OPT_STRING('u', "uid", &record.opts.target.uid_str, "user",
"user to profile"),

Arnaldo Carvalho de Melo

unread,
Jan 15, 2014, 3:40:03 PM1/15/14
to
From: Mark Rutland <mark.r...@arm.com>

The scsi and cfg80211 plugins cast between unsigned long long and
pointers, which is problematic for architectures where unsigned long
long is wider than the native pointer size:

linux/tools/lib/traceevent/plugin_scsi.c: In function ‘process_scsi_trace_parse_cdb’:
linux/tools/lib/traceevent/plugin_scsi.c:408:26: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
scsi_trace_parse_cdb(s, (unsigned char *) args[1], args[2]);

linux/tools/lib/traceevent/plugin_cfg80211.c: In function ‘process___le16_to_cpup’:
linux/tools/lib/traceevent/plugin_cfg80211.c:11:18: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
uint16_t *val = (uint16_t *) args[0];

This patch adds an intermediate cast to unsigned long, silencing the
warning.

Signed-off-by: Mark Rutland <mark.r...@arm.com>
Acked-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Will Deacon <will....@arm.com>
Link: http://lkml.kernel.org/r/1389782648-4417-3-git-...@arm.com
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/lib/traceevent/plugin_cfg80211.c | 2 +-
tools/lib/traceevent/plugin_scsi.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lib/traceevent/plugin_cfg80211.c b/tools/lib/traceevent/plugin_cfg80211.c
index dcab8e873c21..57e98221db20 100644
--- a/tools/lib/traceevent/plugin_cfg80211.c
+++ b/tools/lib/traceevent/plugin_cfg80211.c
@@ -8,7 +8,7 @@ static unsigned long long
process___le16_to_cpup(struct trace_seq *s,
unsigned long long *args)
{
- uint16_t *val = (uint16_t *) args[0];
+ uint16_t *val = (uint16_t *) (unsigned long) args[0];
return val ? (long long) le16toh(*val) : 0;
}

diff --git a/tools/lib/traceevent/plugin_scsi.c b/tools/lib/traceevent/plugin_scsi.c
index 6fb8e3e3fcad..7ef16cc96562 100644
--- a/tools/lib/traceevent/plugin_scsi.c
+++ b/tools/lib/traceevent/plugin_scsi.c
@@ -405,7 +405,7 @@ scsi_trace_parse_cdb(struct trace_seq *p, unsigned char *cdb, int len)
unsigned long long process_scsi_trace_parse_cdb(struct trace_seq *s,
unsigned long long *args)
{
- scsi_trace_parse_cdb(s, (unsigned char *) args[1], args[2]);
+ scsi_trace_parse_cdb(s, (unsigned char *) (unsigned long) args[1], args[2]);
return 0;

Steven Rostedt

unread,
Jan 15, 2014, 4:20:01 PM1/15/14
to
On Wed, 15 Jan 2014 17:33:05 -0300
Arnaldo Carvalho de Melo <ac...@infradead.org> wrote:

> From: Namhyung Kim <namh...@kernel.org>
>
> Now all of its users were gone. :)
>
> Signed-off-by: Namhyung Kim <namh...@kernel.org>
> Reviewed-by: Jiri Olsa <jo...@redhat.com>
> Cc: Frederic Weisbecker <fwei...@gmail.com>
> Cc: Ingo Molnar <mi...@kernel.org>
> Cc: Jiri Olsa <jo...@redhat.com>
> Cc: Namhyung Kim <namhyu...@lge.com>
> Cc: Peter Zijlstra <a.p.zi...@chello.nl>
> Cc: Steven Rostedt <ros...@goodmis.org>

I just couldn't bring myself to Ack the death of die! }-,:


-- Steve

> Link: http://lkml.kernel.org/r/1389750340-15965-5-gi...@kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>

Namhyung Kim

unread,
Jan 15, 2014, 7:10:02 PM1/15/14
to
Hi Steve,

2014-01-16 AM 1:18, Steven Rostedt wrote:
> On Wed, 15 Jan 2014 10:45:29 +0900
> Namhyung Kim <namh...@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

Steven Rostedt

unread,
Jan 15, 2014, 9:00:02 PM1/15/14
to
On Thu, 16 Jan 2014 09:00:28 +0900
Namhyung Kim <namhyu...@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 <namh...@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

Namhyung Kim

unread,
Jan 15, 2014, 9:00:02 PM1/15/14
to
On Wed, 15 Jan 2014 20:52:59 -0500, Steven Rostedt wrote:
> On Thu, 16 Jan 2014 09:00:28 +0900
> Namhyung Kim <namhyu...@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 <namh...@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

Steven Rostedt

unread,
Jan 15, 2014, 9:10:02 PM1/15/14
to
On Thu, 16 Jan 2014 10:58:05 +0900
Namhyung Kim <namh...@kernel.org> wrote:

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

Yeah, go ahead.

-- Steve

Namhyung Kim

unread,
Jan 15, 2014, 9:40:01 PM1/15/14
to
The event handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.

Acked-by: Steven Rostedt <ros...@goodmis.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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

Namhyung Kim

unread,
Jan 15, 2014, 9:40:01 PM1/15/14
to
The kvm handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.

Acked-by: Steven Rostedt <ros...@goodmis.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
tools/lib/traceevent/plugin_kvm.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)

diff --git a/tools/lib/traceevent/plugin_kvm.c b/tools/lib/traceevent/plugin_kvm.c
index a0e282c6b967..9e0e8c61b43b 100644
--- a/tools/lib/traceevent/plugin_kvm.c
+++ b/tools/lib/traceevent/plugin_kvm.c
@@ -434,3 +434,32 @@ 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");

Namhyung Kim

unread,
Jan 15, 2014, 9:40:01 PM1/15/14
to
The function handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.

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

diff --git a/tools/lib/traceevent/plugin_jbd2.c b/tools/lib/traceevent/plugin_jbd2.c
index 2f93f81f0bac..0db714c721be 100644
--- a/tools/lib/traceevent/plugin_jbd2.c
+++ b/tools/lib/traceevent/plugin_jbd2.c
@@ -66,3 +66,12 @@ 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");

Namhyung Kim

unread,
Jan 15, 2014, 9:40:02 PM1/15/14
to
The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.

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

diff --git a/tools/lib/traceevent/plugin_xen.c b/tools/lib/traceevent/plugin_xen.c
index e7794298f3a9..3a413eaada68 100644
--- a/tools/lib/traceevent/plugin_xen.c
+++ b/tools/lib/traceevent/plugin_xen.c
@@ -128,3 +128,9 @@ 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");

Namhyung Kim

unread,
Jan 15, 2014, 9:40:02 PM1/15/14
to
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.

* changes in v2)
- add ack and review tags from Steve and Jiri
- improve message on handler unregister (Steve)
- cleanup coding style issue (Steve)


I put the series on the 'libtraceevent/plugin-v2' 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 (12):
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/event-parse.c | 136 ++++++++++++++++++++++++++---
tools/lib/traceevent/event-parse.h | 5 ++
tools/lib/traceevent/plugin_cfg80211.c | 6 ++
tools/lib/traceevent/plugin_function.c | 3 +
tools/lib/traceevent/plugin_hrtimer.c | 10 +++
tools/lib/traceevent/plugin_jbd2.c | 9 ++
tools/lib/traceevent/plugin_kmem.c | 22 +++++
tools/lib/traceevent/plugin_kvm.c | 29 ++++++
tools/lib/traceevent/plugin_mac80211.c | 7 ++
tools/lib/traceevent/plugin_sched_switch.c | 12 +++
tools/lib/traceevent/plugin_scsi.c | 6 ++
tools/lib/traceevent/plugin_xen.c | 6 ++
12 files changed, 237 insertions(+), 14 deletions(-)

Namhyung Kim

unread,
Jan 15, 2014, 9:40:02 PM1/15/14
to
The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.

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

diff --git a/tools/lib/traceevent/plugin_scsi.c b/tools/lib/traceevent/plugin_scsi.c
index 7ef16cc96562..eda326fc8620 100644
--- a/tools/lib/traceevent/plugin_scsi.c
+++ b/tools/lib/traceevent/plugin_scsi.c
@@ -421,3 +421,9 @@ 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");
+}

Namhyung Kim

unread,
Jan 15, 2014, 9:40:02 PM1/15/14
to
The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.

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

diff --git a/tools/lib/traceevent/plugin_cfg80211.c b/tools/lib/traceevent/plugin_cfg80211.c
index 57e98221db20..c066b25905f8 100644
--- a/tools/lib/traceevent/plugin_cfg80211.c
+++ b/tools/lib/traceevent/plugin_cfg80211.c
@@ -22,3 +22,9 @@ 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");
+}

Namhyung Kim

unread,
Jan 15, 2014, 9:40:02 PM1/15/14
to
The event handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.

Acked-by: Steven Rostedt <ros...@goodmis.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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);
+}

Namhyung Kim

unread,
Jan 15, 2014, 9:40:02 PM1/15/14
to
The timer handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.

Acked-by: Steven Rostedt <ros...@goodmis.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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);
+}

Namhyung Kim

unread,
Jan 15, 2014, 9:40:02 PM1/15/14
to
When a plugin unloaded it needs to unregister its handler from pevent.
So add unregister function to do it.

Acked-by: Steven Rostedt <ros...@goodmis.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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..d1973cb8388b 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("removing override handler for event (%d) %s:%s. Going back to default 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,

Namhyung Kim

unread,
Jan 15, 2014, 9:40:02 PM1/15/14
to
The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.

Acked-by: Steven Rostedt <ros...@goodmis.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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]);

Namhyung Kim

unread,
Jan 15, 2014, 9:40:03 PM1/15/14
to
The kmem handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.

Acked-by: Steven Rostedt <ros...@goodmis.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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);
+}

Namhyung Kim

unread,
Jan 15, 2014, 9:40:03 PM1/15/14
to
When a plugin unloaded it needs to unregister its print handler from
pevent. So add unregister function to do it.

Acked-by: Steven Rostedt <ros...@goodmis.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Signed-off-by: Namhyung Kim <namh...@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 d1973cb8388b..1587ea392ad6 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);

Steven Rostedt

unread,
Jan 15, 2014, 10:30:02 PM1/15/14
to
On Thu, 16 Jan 2014 11:31:15 +0900
Namhyung Kim <namh...@kernel.org> wrote:

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

You can add my Acked-by to patches 9-12 too.

-- Steve

> ---
> tools/lib/traceevent/plugin_cfg80211.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/tools/lib/traceevent/plugin_cfg80211.c b/tools/lib/traceevent/plugin_cfg80211.c
> index 57e98221db20..c066b25905f8 100644
> --- a/tools/lib/traceevent/plugin_cfg80211.c
> +++ b/tools/lib/traceevent/plugin_cfg80211.c
> @@ -22,3 +22,9 @@ 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");
> +}

--

Ingo Molnar

unread,
Jan 16, 2014, 3:40:02 AM1/16/14
to

* Steven Rostedt <ros...@goodmis.org> wrote:

> On Wed, 15 Jan 2014 17:33:05 -0300
> Arnaldo Carvalho de Melo <ac...@infradead.org> wrote:
>
> > From: Namhyung Kim <namh...@kernel.org>
> >
> > Now all of its users were gone. :)
> >

Now that all users of death are dead, death can die as well?

> > Signed-off-by: Namhyung Kim <namh...@kernel.org>
> > Reviewed-by: Jiri Olsa <jo...@redhat.com>
> > Cc: Frederic Weisbecker <fwei...@gmail.com>
> > Cc: Ingo Molnar <mi...@kernel.org>
> > Cc: Jiri Olsa <jo...@redhat.com>
> > Cc: Namhyung Kim <namhyu...@lge.com>
> > Cc: Peter Zijlstra <a.p.zi...@chello.nl>
> > Cc: Steven Rostedt <ros...@goodmis.org>
>
> I just couldn't bring myself to Ack the death of die! }-,:

Indeed not something to ack to death!

Thanks,

Ingo

tip-bot for Namhyung Kim

unread,
Jan 16, 2014, 8:40:02 AM1/16/14
to
Commit-ID: 3026bba3c37711234771349ca020d9a85e572f60
Gitweb: http://git.kernel.org/tip/3026bba3c37711234771349ca020d9a85e572f60
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Wed, 15 Jan 2014 10:45:25 +0900
Committer: Arnaldo Carvalho de Melo <ac...@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 <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389750340-15965-3-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@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-bot for Namhyung Kim

unread,
Jan 16, 2014, 8:40:02 AM1/16/14
to
Commit-ID: e825e756f84eab0e68d7d6644c018c3412748406
Gitweb: http://git.kernel.org/tip/e825e756f84eab0e68d7d6644c018c3412748406
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Wed, 15 Jan 2014 10:45:27 +0900
Committer: Arnaldo Carvalho de Melo <ac...@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 <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389750340-15965-5-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@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-bot for Namhyung Kim

unread,
Jan 16, 2014, 8:40:03 AM1/16/14
to
Commit-ID: 8d0c2224ca6e04ba51c403805e7e1e2ca536520b
Gitweb: http://git.kernel.org/tip/8d0c2224ca6e04ba51c403805e7e1e2ca536520b
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Wed, 15 Jan 2014 10:45:28 +0900
Committer: Arnaldo Carvalho de Melo <ac...@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 <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389750340-15965-6-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@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*

tip-bot for Namhyung Kim

unread,
Jan 16, 2014, 8:50:02 AM1/16/14
to
Commit-ID: 504586e0954bcf9550dfdea37d3234174ed1d68f
Gitweb: http://git.kernel.org/tip/504586e0954bcf9550dfdea37d3234174ed1d68f
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Wed, 15 Jan 2014 10:45:26 +0900
Committer: Arnaldo Carvalho de Melo <ac...@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 <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389750340-15965-4-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@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-bot for Arnaldo Carvalho de Melo

unread,
Jan 16, 2014, 8:50:02 AM1/16/14
to
Commit-ID: a6205a35ba2d56dcfb79b08cc11b8c7d549826aa
Gitweb: http://git.kernel.org/tip/a6205a35ba2d56dcfb79b08cc11b8c7d549826aa
Author: Arnaldo Carvalho de Melo <ac...@redhat.com>
AuthorDate: Tue, 14 Jan 2014 17:58:12 -0300
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Tue, 14 Jan 2014 17:58:12 -0300

perf record: Rename --initial-delay to --delay

To be consistent with the equivalent option in 'stat', also, for the
same reason, use -D as the one letter alias.

Suggested-by: Ingo Molnar <mi...@kernel.org>
Cc: Adrian Hunter <adrian...@intel.com>
Cc: David Ahern <dsa...@gmail.com>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Mike Galbraith <efa...@gmx.de>
Cc: Namhyung Kim <namh...@kernel.org>
Cc: Paul Mackerras <pau...@samba.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Stephane Eranian <era...@google.com>
Link: http://lkml.kernel.org/n/tip-p5yjnopajb...@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/perf/Documentation/perf-record.txt | 3 ++-
tools/perf/builtin-record.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 3a35a85..c71b0f3 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -208,7 +208,8 @@ overrides that and uses per-thread mmaps. A side-effect of that is that
inheritance is automatically disabled. --per-thread is ignored with a warning
if combined with -a or -C options.

---initial-delay msecs::
+-D::
+--delay=::
After starting the program, wait msecs before measuring. This is useful to
filter out the startup phase of the program, which is often very different.

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 78e790f..3c394bf 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -882,7 +882,7 @@ const struct option record_options[] = {
OPT_CALLBACK('G', "cgroup", &record.evlist, "name",
"monitor event in cgroup name only",
parse_cgroups),
- OPT_UINTEGER(0, "initial-delay", &record.opts.initial_delay,
+ OPT_UINTEGER('D', "delay", &record.opts.initial_delay,
"ms to wait before starting measurement after program start"),
OPT_STRING('u', "uid", &record.opts.target.uid_str, "user",
"user to profile"),

tip-bot for Namhyung Kim

unread,
Jan 16, 2014, 8:50:03 AM1/16/14
to
Commit-ID: 3c6d8d84423932f1d9949179c6acdf2405515ee4
Gitweb: http://git.kernel.org/tip/3c6d8d84423932f1d9949179c6acdf2405515ee4
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Thu, 19 Dec 2013 18:17:44 +0900
Committer: Arnaldo Carvalho de Melo <ac...@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 <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/87fvopa...@sejong.aot.lge.com
Signed-off-by: Arnaldo Carvalho de Melo <ac...@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-bot for Namhyung Kim

unread,
Jan 19, 2014, 7:30:02 AM1/19/14
to
Commit-ID: ac668c7b7d87c1895faf196a337b5ddfd32a0e21
Gitweb: http://git.kernel.org/tip/ac668c7b7d87c1895faf196a337b5ddfd32a0e21
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Thu, 16 Jan 2014 11:31:09 +0900
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Thu, 16 Jan 2014 16:26:24 -0300

tools lib traceevent: Unregister handler when function plugin is unloaded

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

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389839478-5887-4-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
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 3946148..80ba4ff 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]);
--

tip-bot for Namhyung Kim

unread,
Jan 19, 2014, 7:30:02 AM1/19/14
to
Commit-ID: 0247a967d4c1986821b3c7cbad324468fa6f2ae3
Gitweb: http://git.kernel.org/tip/0247a967d4c1986821b3c7cbad324468fa6f2ae3
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Thu, 16 Jan 2014 11:31:16 +0900
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Thu, 16 Jan 2014 16:26:25 -0300

tools lib traceevent: Unregister handler when jbd2 plugin is is unloaded

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

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389839478-5887-11-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/lib/traceevent/plugin_jbd2.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/tools/lib/traceevent/plugin_jbd2.c b/tools/lib/traceevent/plugin_jbd2.c
index 2f93f81..0db714c 100644
--- a/tools/lib/traceevent/plugin_jbd2.c
+++ b/tools/lib/traceevent/plugin_jbd2.c
@@ -66,3 +66,12 @@ 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");
+}
--

tip-bot for Namhyung Kim

unread,
Jan 19, 2014, 7:30:03 AM1/19/14
to
Commit-ID: ad13701d4905e820f32ce3c2590e19ca65765d63
Gitweb: http://git.kernel.org/tip/ad13701d4905e820f32ce3c2590e19ca65765d63
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Thu, 16 Jan 2014 11:31:07 +0900
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Thu, 16 Jan 2014 16:26:24 -0300

tools lib traceevent: Add pevent_unregister_event_handler()

When a plugin is unloaded it needs to unregister its handler from pevent.

So add an unregister function to do it.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389839478-5887-2-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
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 2ce565a..d1973cb 100644
index a3beca5..c48acfb 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,
--

tip-bot for Namhyung Kim

unread,
Jan 19, 2014, 7:30:03 AM1/19/14
to
Commit-ID: 02bafd377c1137d0705f224881cd21de123204f0
Gitweb: http://git.kernel.org/tip/02bafd377c1137d0705f224881cd21de123204f0
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Thu, 16 Jan 2014 11:31:13 +0900
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Thu, 16 Jan 2014 16:26:25 -0300

tools lib traceevent: Unregister handler when sched_switch plugin is unloaded

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

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389839478-5887-8-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
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 fea3724..f1ce600 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);
+}
--

tip-bot for Namhyung Kim

unread,
Jan 19, 2014, 7:30:04 AM1/19/14
to
Commit-ID: 6024cf3898d25088b01025d72a6929839de9c7b6
Gitweb: http://git.kernel.org/tip/6024cf3898d25088b01025d72a6929839de9c7b6
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Thu, 16 Jan 2014 11:31:15 +0900
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Thu, 16 Jan 2014 16:26:25 -0300

tools lib traceevent: Unregister handler when cfg80211 plugin is unloaded

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

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389839478-5887-10-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/lib/traceevent/plugin_cfg80211.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/tools/lib/traceevent/plugin_cfg80211.c b/tools/lib/traceevent/plugin_cfg80211.c
index 57e9822..c066b25 100644
--- a/tools/lib/traceevent/plugin_cfg80211.c
+++ b/tools/lib/traceevent/plugin_cfg80211.c
@@ -22,3 +22,9 @@ 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");
+}
--

tip-bot for Namhyung Kim

unread,
Jan 19, 2014, 7:30:04 AM1/19/14
to
Commit-ID: 11e99c55414ebade1031a0ed3b49915824c7c3ea
Gitweb: http://git.kernel.org/tip/11e99c55414ebade1031a0ed3b49915824c7c3ea
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Thu, 16 Jan 2014 11:31:11 +0900
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Thu, 16 Jan 2014 16:26:24 -0300

tools lib traceevent: Unregister handler when kmem plugin is unloaded

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

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389839478-5887-6-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
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 7115c80..70650ff 100644

tip-bot for Namhyung Kim

unread,
Jan 19, 2014, 7:30:03 AM1/19/14
to
Commit-ID: ec7c6debdd446ad2262f236d13964efae90ba0f7
Gitweb: http://git.kernel.org/tip/ec7c6debdd446ad2262f236d13964efae90ba0f7
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Thu, 16 Jan 2014 11:31:14 +0900
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Thu, 16 Jan 2014 16:26:25 -0300

tools lib traceevent: Unregister handler when mac80211 plugin is unloaded

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

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389839478-5887-9-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
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 558a3b9..7e15a0f 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);

tip-bot for Namhyung Kim

unread,
Jan 19, 2014, 7:30:03 AM1/19/14
to
Commit-ID: bf6b3a95ff439b1dcd6151b3f38810f3cec1e319
Gitweb: http://git.kernel.org/tip/bf6b3a95ff439b1dcd6151b3f38810f3cec1e319
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Thu, 16 Jan 2014 11:31:18 +0900
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Thu, 16 Jan 2014 16:26:25 -0300

tools lib traceevent: Unregister handler when xen plugin is unloaded

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

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389839478-5887-13-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/lib/traceevent/plugin_xen.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/tools/lib/traceevent/plugin_xen.c b/tools/lib/traceevent/plugin_xen.c
index e779429..3a413ea 100644
--- a/tools/lib/traceevent/plugin_xen.c
+++ b/tools/lib/traceevent/plugin_xen.c
@@ -128,3 +128,9 @@ 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");
+}
--

tip-bot for Namhyung Kim

unread,
Jan 19, 2014, 7:30:03 AM1/19/14
to
Commit-ID: 4061edaa54744dca833051119e763f073dd3c334
Gitweb: http://git.kernel.org/tip/4061edaa54744dca833051119e763f073dd3c334
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Thu, 16 Jan 2014 11:31:10 +0900
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Thu, 16 Jan 2014 16:26:24 -0300

tools lib traceevent: Unregister handler when hrtimer plugin is unloaded

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

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389839478-5887-5-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
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 0b0ebf3..12bf14c 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);
+}
--

tip-bot for Namhyung Kim

unread,
Jan 19, 2014, 7:30:03 AM1/19/14
to
Commit-ID: 354a2bd0318e0758f93b8b24553f3376fa9dfa21
Gitweb: http://git.kernel.org/tip/354a2bd0318e0758f93b8b24553f3376fa9dfa21
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Thu, 16 Jan 2014 11:31:12 +0900
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Thu, 16 Jan 2014 16:26:24 -0300

tools lib traceevent: Unregister handler when kvm plugin is unloaded

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

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389839478-5887-7-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/lib/traceevent/plugin_kvm.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)

diff --git a/tools/lib/traceevent/plugin_kvm.c b/tools/lib/traceevent/plugin_kvm.c
index a0e282c..9e0e8c6 100644
--- a/tools/lib/traceevent/plugin_kvm.c
+++ b/tools/lib/traceevent/plugin_kvm.c
@@ -434,3 +434,32 @@ 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");

tip-bot for Namhyung Kim

unread,
Jan 19, 2014, 7:30:03 AM1/19/14
to
Commit-ID: a157112cfc67b3889f6493933cbd32620aa4be18
Gitweb: http://git.kernel.org/tip/a157112cfc67b3889f6493933cbd32620aa4be18
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Thu, 16 Jan 2014 11:31:17 +0900
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Thu, 16 Jan 2014 16:26:25 -0300

tools lib traceevent: Unregister handler when scsi plugin is unloaded

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

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389839478-5887-12-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/lib/traceevent/plugin_scsi.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/tools/lib/traceevent/plugin_scsi.c b/tools/lib/traceevent/plugin_scsi.c
index 7ef16cc..eda326f 100644
--- a/tools/lib/traceevent/plugin_scsi.c
+++ b/tools/lib/traceevent/plugin_scsi.c
@@ -421,3 +421,9 @@ 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");

tip-bot for Namhyung Kim

unread,
Jan 19, 2014, 7:30:04 AM1/19/14
to
Commit-ID: 20c7e5abbd0cdfaa656f46af052a6e6a8ce94775
Gitweb: http://git.kernel.org/tip/20c7e5abbd0cdfaa656f46af052a6e6a8ce94775
Author: Namhyung Kim <namh...@kernel.org>
AuthorDate: Thu, 16 Jan 2014 11:31:08 +0900
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Thu, 16 Jan 2014 16:26:24 -0300

tools lib traceevent: Add pevent_unregister_print_function()

When a plugin unloaded it needs to unregister its print handler from
pevent.

So add an unregister function to do it.

Signed-off-by: Namhyung Kim <namh...@kernel.org>
Reviewed-by: Jiri Olsa <jo...@redhat.com>
Acked-by: Steven Rostedt <ros...@goodmis.org>
Cc: Frederic Weisbecker <fwei...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Namhyung Kim <namhyu...@lge.com>
Cc: Peter Zijlstra <a.p.zi...@chello.nl>
Cc: Steven Rostedt <ros...@goodmis.org>
Link: http://lkml.kernel.org/r/1389839478-5887-3-gi...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
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 d1973cb..1587ea39 100644
index c48acfb..791c539 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);
--
0 new messages