Hello,
When plugins are loaded, they register various event and function
handlers. But they don't unregister even after the plugins unloaded
so that events could have refererences to non-existing handlers.
This patchset added relevant unregister functions to handle that.
Note that this is not a problem as of now since all of users unload
plugins after finishing their access to events. But being a generic
library, it should be handled properly IMHO.
The patch 1-4 are resend of the previous die removal series so
independent to this series.
I put the series on the 'libtraceevent/plugin-v1' branch in my tree
git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
Any comments are welcome, thanks
Namhyung
Namhyung Kim (17):
tools lib traceevent: Add state member to struct trace_seq
tools lib traceevent: Check return value of realloc()
tools lib traceevent: Get rid of malloc_or_die() in trace_seq_init()
tools lib traceevent: Get rid of die() finally!!
tools lib traceevent: Make plugin unload function receive pevent
tools lib traceevent: Add pevent_unregister_event_handler()
tools lib traceevent: Add pevent_unregister_print_function()
tools lib traceevent: Unregister handler when function plugin
unloaded
tools lib traceevent: Unregister handler when hrtimer plugin unloaded
tools lib traceevent: Unregister handler when kmem plugin unloaded
tools lib traceevent: Unregister handler when kvm plugin unloaded
tools lib traceevent: Unregister handler when sched_switch plugin
unloaded
tools lib traceevent: Unregister handler when mac80211 plugin
unloaded
tools lib traceevent: Unregister handler when cfg80211 plugin
unloaded
tools lib traceevent: Unregister handler when jbd2 plugin unloaded
tools lib traceevent: Unregister handler when scsi plugin unloaded
tools lib traceevent: Unregister handler when xen plugin unloaded
tools/lib/traceevent/Makefile | 2 +-
tools/lib/traceevent/event-parse.c | 136 ++++++++++++++++++++++++++---
tools/lib/traceevent/event-parse.h | 19 +++-
tools/lib/traceevent/event-plugin.c | 4 +-
tools/lib/traceevent/event-utils.h | 4 -
tools/lib/traceevent/parse-utils.c | 44 ----------
tools/lib/traceevent/plugin_cfg80211.c | 7 ++
tools/lib/traceevent/plugin_function.c | 5 +-
tools/lib/traceevent/plugin_hrtimer.c | 10 +++
tools/lib/traceevent/plugin_jbd2.c | 11 +++
tools/lib/traceevent/plugin_kmem.c | 22 +++++
tools/lib/traceevent/plugin_kvm.c | 30 +++++++
tools/lib/traceevent/plugin_mac80211.c | 7 ++
tools/lib/traceevent/plugin_sched_switch.c | 12 +++
tools/lib/traceevent/plugin_scsi.c | 7 ++
tools/lib/traceevent/plugin_xen.c | 7 ++
tools/lib/traceevent/trace-seq.c | 58 ++++++++++--
tools/perf/util/trace-event.c | 2 +-
18 files changed, 310 insertions(+), 77 deletions(-)
--
1.7.11.7
The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/plugin_xen.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tools/lib/traceevent/plugin_xen.c b/tools/lib/traceevent/plugin_xen.c
index e7794298f3a9..a65353f44302 100644
--- a/tools/lib/traceevent/plugin_xen.c
+++ b/tools/lib/traceevent/plugin_xen.c
@@ -128,3 +128,10 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
PEVENT_FUNC_ARG_VOID);
return 0;
}
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+ pevent_unregister_print_function(pevent,
+ process_xen_hypercall_name,
+ "xen_hypercall_name");
+}
--
1.7.11.7
The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/plugin_scsi.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tools/lib/traceevent/plugin_scsi.c b/tools/lib/traceevent/plugin_scsi.c
index 6fb8e3e3fcad..64d688534518 100644
--- a/tools/lib/traceevent/plugin_scsi.c
+++ b/tools/lib/traceevent/plugin_scsi.c
@@ -421,3 +421,10 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
PEVENT_FUNC_ARG_VOID);
return 0;
}
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+ pevent_unregister_print_function(pevent,
+ process_scsi_trace_parse_cdb,
+ "scsi_trace_parse_cdb");
+}
--
1.7.11.7
The function handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/plugin_jbd2.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/tools/lib/traceevent/plugin_jbd2.c b/tools/lib/traceevent/plugin_jbd2.c
index 2f93f81f0bac..88fe6fe872d5 100644
--- a/tools/lib/traceevent/plugin_jbd2.c
+++ b/tools/lib/traceevent/plugin_jbd2.c
@@ -66,3 +66,14 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
PEVENT_FUNC_ARG_VOID);
return 0;
}
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+ pevent_unregister_print_function(pevent,
+ process_jbd2_dev_to_name,
+ "jbd2_dev_to_name");
+
+ pevent_unregister_print_function(pevent,
+ process_jiffies_to_msecs,
+ "jiffies_to_msecs");
+}
--
1.7.11.7
The timer handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/plugin_hrtimer.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tools/lib/traceevent/plugin_hrtimer.c b/tools/lib/traceevent/plugin_hrtimer.c
index 0b0ebf30aa44..12bf14cc1152 100644
--- a/tools/lib/traceevent/plugin_hrtimer.c
+++ b/tools/lib/traceevent/plugin_hrtimer.c
@@ -76,3 +76,13 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
timer_start_handler, NULL);
return 0;
}
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+ pevent_unregister_event_handler(pevent, -1,
+ "timer", "hrtimer_expire_entry",
+ timer_expire_handler, NULL);
+
+ pevent_unregister_event_handler(pevent, -1, "timer", "hrtimer_start",
+ timer_start_handler, NULL);
+}
--
1.7.11.7
Use plain malloc() and check its return value.
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/trace-seq.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
index 941d35d2cf87..339a0ffa21d5 100644
--- a/tools/lib/traceevent/trace-seq.c
+++ b/tools/lib/traceevent/trace-seq.c
@@ -47,8 +47,11 @@ void trace_seq_init(struct trace_seq *s)
s->len = 0;
s->readpos = 0;
s->buffer_size = TRACE_SEQ_BUF_SIZE;
- s->buffer = malloc_or_die(s->buffer_size);
- s->state = TRACE_SEQ__GOOD;
+ s->buffer = malloc(s->buffer_size);
+ if (s->buffer != NULL)
+ s->state = TRACE_SEQ__GOOD;
+ else
+ s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
}
/**
--
1.7.11.7
The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/plugin_cfg80211.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tools/lib/traceevent/plugin_cfg80211.c b/tools/lib/traceevent/plugin_cfg80211.c
index dcab8e873c21..7b07d149557b 100644
--- a/tools/lib/traceevent/plugin_cfg80211.c
+++ b/tools/lib/traceevent/plugin_cfg80211.c
@@ -22,3 +22,10 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
PEVENT_FUNC_ARG_VOID);
return 0;
}
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+ pevent_unregister_print_function(pevent,
+ process___le16_to_cpup,
+ "__le16_to_cpup");
+}
--
1.7.11.7
The kvm handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/plugin_kvm.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/tools/lib/traceevent/plugin_kvm.c b/tools/lib/traceevent/plugin_kvm.c
index a0e282c6b967..6a1eeadbf7a1 100644
--- a/tools/lib/traceevent/plugin_kvm.c
+++ b/tools/lib/traceevent/plugin_kvm.c
@@ -434,3 +434,33 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
PEVENT_FUNC_ARG_VOID);
return 0;
}
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+ pevent_unregister_event_handler(pevent, -1, "kvm", "kvm_exit",
+ kvm_exit_handler, NULL);
+
+ pevent_unregister_event_handler(pevent, -1, "kvm", "kvm_emulate_insn",
+ kvm_emulate_insn_handler, NULL);
+
+ pevent_unregister_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_get_page",
+ kvm_mmu_get_page_handler, NULL);
+
+ pevent_unregister_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_sync_page",
+ kvm_mmu_print_role, NULL);
+
+ pevent_unregister_event_handler(pevent, -1,
+ "kvmmmu", "kvm_mmu_unsync_page",
+ kvm_mmu_print_role, NULL);
+
+ pevent_unregister_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_zap_page",
+ kvm_mmu_print_role, NULL);
+
+ pevent_unregister_event_handler(pevent, -1, "kvmmmu",
+ "kvm_mmu_prepare_zap_page", kvm_mmu_print_role,
+ NULL);
+
+ pevent_unregister_print_function(pevent,
+ process_is_writable_pte,
+ "is_writable_pte");
+}
--
1.7.11.7
The event handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/plugin_sched_switch.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tools/lib/traceevent/plugin_sched_switch.c b/tools/lib/traceevent/plugin_sched_switch.c
index fea3724aa24f..f1ce60065258 100644
--- a/tools/lib/traceevent/plugin_sched_switch.c
+++ b/tools/lib/traceevent/plugin_sched_switch.c
@@ -146,3 +146,15 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
sched_wakeup_handler, NULL);
return 0;
}
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+ pevent_unregister_event_handler(pevent, -1, "sched", "sched_switch",
+ sched_switch_handler, NULL);
+
+ pevent_unregister_event_handler(pevent, -1, "sched", "sched_wakeup",
+ sched_wakeup_handler, NULL);
+
+ pevent_unregister_event_handler(pevent, -1, "sched", "sched_wakeup_new",
+ sched_wakeup_handler, NULL);
+}
--
1.7.11.7
The event handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
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
The kmem handlers should be unregistered when the plugin unloaded
otherwise they'll try to access invalid memory.
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/plugin_kmem.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/tools/lib/traceevent/plugin_kmem.c b/tools/lib/traceevent/plugin_kmem.c
index 7115c8037ea8..70650ff48d78 100644
--- a/tools/lib/traceevent/plugin_kmem.c
+++ b/tools/lib/traceevent/plugin_kmem.c
@@ -70,3 +70,25 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
call_site_handler, NULL);
return 0;
}
+
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
+{
+ pevent_unregister_event_handler(pevent, -1, "kmem", "kfree",
+ call_site_handler, NULL);
+
+ pevent_unregister_event_handler(pevent, -1, "kmem", "kmalloc",
+ call_site_handler, NULL);
+
+ pevent_unregister_event_handler(pevent, -1, "kmem", "kmalloc_node",
+ call_site_handler, NULL);
+
+ pevent_unregister_event_handler(pevent, -1, "kmem", "kmem_cache_alloc",
+ call_site_handler, NULL);
+
+ pevent_unregister_event_handler(pevent, -1, "kmem",
+ "kmem_cache_alloc_node",
+ call_site_handler, NULL);
+
+ pevent_unregister_event_handler(pevent, -1, "kmem", "kmem_cache_free",
+ call_site_handler, NULL);
+}
--
1.7.11.7
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 <[email protected]>
---
tools/lib/traceevent/Makefile | 2 +-
tools/lib/traceevent/event-parse.h | 7 ++++++
tools/lib/traceevent/trace-seq.c | 46 +++++++++++++++++++++++++++++++++-----
3 files changed, 49 insertions(+), 6 deletions(-)
diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index f778d48ac609..56d52a33a3df 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -136,7 +136,7 @@ export Q VERBOSE
EVENT_PARSE_VERSION = $(EP_VERSION).$(EP_PATCHLEVEL).$(EP_EXTRAVERSION)
-INCLUDES = -I. $(CONFIG_INCLUDES)
+INCLUDES = -I. -I $(srctree)/../../include $(CONFIG_INCLUDES)
# Set compile option CFLAGS if not set elsewhere
CFLAGS ?= -g -Wall
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index cf5db9013f2c..3c890cb28db7 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -58,6 +58,12 @@ struct pevent_record {
#endif
};
+enum trace_seq_fail {
+ TRACE_SEQ__GOOD,
+ TRACE_SEQ__BUFFER_POISONED,
+ TRACE_SEQ__MEM_ALLOC_FAILED,
+};
+
/*
* Trace sequences are used to allow a function to call several other functions
* to create a string of data to use (up to a max of PAGE_SIZE).
@@ -68,6 +74,7 @@ struct trace_seq {
unsigned int buffer_size;
unsigned int len;
unsigned int readpos;
+ enum trace_seq_fail state;
};
void trace_seq_init(struct trace_seq *s);
diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
index d7f2e68bc5b9..7f24e8989401 100644
--- a/tools/lib/traceevent/trace-seq.c
+++ b/tools/lib/traceevent/trace-seq.c
@@ -22,6 +22,7 @@
#include <string.h>
#include <stdarg.h>
+#include <asm/bug.h>
#include "event-parse.h"
#include "event-utils.h"
@@ -32,8 +33,9 @@
#define TRACE_SEQ_POISON ((void *)0xdeadbeef)
#define TRACE_SEQ_CHECK(s) \
do { \
- if ((s)->buffer == TRACE_SEQ_POISON) \
- die("Usage of trace_seq after it was destroyed"); \
+ if (WARN_ONCE((s)->buffer == TRACE_SEQ_POISON, \
+ "Usage of trace_seq after it was destroyed")) \
+ (s)->state = TRACE_SEQ__BUFFER_POISONED; \
} while (0)
/**
@@ -46,6 +48,7 @@ void trace_seq_init(struct trace_seq *s)
s->readpos = 0;
s->buffer_size = TRACE_SEQ_BUF_SIZE;
s->buffer = malloc_or_die(s->buffer_size);
+ s->state = TRACE_SEQ__GOOD;
}
/**
@@ -80,8 +83,9 @@ static void expand_buffer(struct trace_seq *s)
{
s->buffer_size += TRACE_SEQ_BUF_SIZE;
s->buffer = realloc(s->buffer, s->buffer_size);
- if (!s->buffer)
- die("Can't allocate trace_seq buffer memory");
+ if (WARN_ONCE(!s->buffer,
+ "Can't allocate trace_seq buffer memory"))
+ s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
}
/**
@@ -108,6 +112,9 @@ trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
TRACE_SEQ_CHECK(s);
try_again:
+ if (s->state != TRACE_SEQ__GOOD)
+ return 0;
+
len = (s->buffer_size - 1) - s->len;
va_start(ap, fmt);
@@ -144,6 +151,9 @@ trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args)
TRACE_SEQ_CHECK(s);
try_again:
+ if (s->state != TRACE_SEQ__GOOD)
+ return 0;
+
len = (s->buffer_size - 1) - s->len;
ret = vsnprintf(s->buffer + s->len, len, fmt, args);
@@ -174,11 +184,17 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
TRACE_SEQ_CHECK(s);
+ if (s->state != TRACE_SEQ__GOOD)
+ return 0;
+
len = strlen(str);
while (len > ((s->buffer_size - 1) - s->len))
expand_buffer(s);
+ if (s->state != TRACE_SEQ__GOOD)
+ return 0;
+
memcpy(s->buffer + s->len, str, len);
s->len += len;
@@ -189,9 +205,15 @@ int trace_seq_putc(struct trace_seq *s, unsigned char c)
{
TRACE_SEQ_CHECK(s);
+ if (s->state != TRACE_SEQ__GOOD)
+ return 0;
+
while (s->len >= (s->buffer_size - 1))
expand_buffer(s);
+ if (s->state != TRACE_SEQ__GOOD)
+ return 0;
+
s->buffer[s->len++] = c;
return 1;
@@ -201,6 +223,9 @@ void trace_seq_terminate(struct trace_seq *s)
{
TRACE_SEQ_CHECK(s);
+ if (s->state != TRACE_SEQ__GOOD)
+ return;
+
/* There's always one character left on the buffer */
s->buffer[s->len] = 0;
}
@@ -208,5 +233,16 @@ void trace_seq_terminate(struct trace_seq *s)
int trace_seq_do_printf(struct trace_seq *s)
{
TRACE_SEQ_CHECK(s);
- return printf("%.*s", s->len, s->buffer);
+
+ switch (s->state) {
+ case TRACE_SEQ__GOOD:
+ return printf("%.*s", s->len, s->buffer);
+ case TRACE_SEQ__BUFFER_POISONED:
+ puts("Usage of trace_seq after it was destroyed");
+ break;
+ case TRACE_SEQ__MEM_ALLOC_FAILED:
+ puts("Can't allocate trace_seq buffer memory");
+ break;
+ }
+ return -1;
}
--
1.7.11.7
The function handler should be unregistered when the plugin unloaded
otherwise it'll try to access invalid memory.
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/plugin_function.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/lib/traceevent/plugin_function.c b/tools/lib/traceevent/plugin_function.c
index 39461485f9a7..80ba4ff1fe84 100644
--- a/tools/lib/traceevent/plugin_function.c
+++ b/tools/lib/traceevent/plugin_function.c
@@ -148,6 +148,9 @@ void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
{
int i, x;
+ pevent_unregister_event_handler(pevent, -1, "ftrace", "function",
+ function_handler, NULL);
+
for (i = 0; i <= cpus; i++) {
for (x = 0; x < fstack[i].size && fstack[i].stack[x]; x++)
free(fstack[i].stack[x]);
--
1.7.11.7
When a plugin unloaded it needs to unregister its print handler from
pevent. So add unregister function to do it.
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/event-parse.c | 23 +++++++++++++++++++++++
tools/lib/traceevent/event-parse.h | 2 ++
2 files changed, 25 insertions(+)
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 05ae4b4ed0c8..fe962fbb156c 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -5560,6 +5560,29 @@ int pevent_register_print_function(struct pevent *pevent,
return ret;
}
+/**
+ * pevent_unregister_print_function - unregister a helper function
+ * @pevent: the handle to the pevent
+ * @func: the function to process the helper function
+ * @name: the name of the helper function
+ *
+ * This function removes existing print handler for function @name.
+ *
+ * Returns 0 if the handler was removed successully, -1 otherwise.
+ */
+int pevent_unregister_print_function(struct pevent *pevent,
+ pevent_func_handler func, char *name)
+{
+ struct pevent_function_handler *func_handle;
+
+ func_handle = find_func_handler(pevent, name);
+ if (func_handle && func_handle->func == func) {
+ remove_func_handler(pevent, name);
+ return 0;
+ }
+ return -1;
+}
+
static struct event_format *pevent_search_event(struct pevent *pevent, int id,
const char *sys_name,
const char *event_name)
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index c48acfbc6230..791c539374c7 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -631,6 +631,8 @@ int pevent_register_print_function(struct pevent *pevent,
pevent_func_handler func,
enum pevent_func_arg_type ret_type,
char *name, ...);
+int pevent_unregister_print_function(struct pevent *pevent,
+ pevent_func_handler func, char *name);
struct format_field *pevent_find_common_field(struct event_format *event, const char *name);
struct format_field *pevent_find_field(struct event_format *event, const char *name);
--
1.7.11.7
When a plugin unloaded it needs to unregister its handler from pevent.
So add unregister function to do it.
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/event-parse.c | 113 ++++++++++++++++++++++++++++++++-----
tools/lib/traceevent/event-parse.h | 3 +
2 files changed, 102 insertions(+), 14 deletions(-)
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 2ce565a73dd5..05ae4b4ed0c8 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -5560,6 +5560,29 @@ int pevent_register_print_function(struct pevent *pevent,
return ret;
}
+static struct event_format *pevent_search_event(struct pevent *pevent, int id,
+ const char *sys_name,
+ const char *event_name)
+{
+ struct event_format *event;
+
+ if (id >= 0) {
+ /* search by id */
+ event = pevent_find_event(pevent, id);
+ if (!event)
+ return NULL;
+ if (event_name && (strcmp(event_name, event->name) != 0))
+ return NULL;
+ if (sys_name && (strcmp(sys_name, event->system) != 0))
+ return NULL;
+ } else {
+ event = pevent_find_event_by_name(pevent, sys_name, event_name);
+ if (!event)
+ return NULL;
+ }
+ return event;
+}
+
/**
* pevent_register_event_handler - register a way to parse an event
* @pevent: the handle to the pevent
@@ -5584,20 +5607,9 @@ int pevent_register_event_handler(struct pevent *pevent, int id,
struct event_format *event;
struct event_handler *handle;
- if (id >= 0) {
- /* search by id */
- event = pevent_find_event(pevent, id);
- if (!event)
- goto not_found;
- if (event_name && (strcmp(event_name, event->name) != 0))
- goto not_found;
- if (sys_name && (strcmp(sys_name, event->system) != 0))
- goto not_found;
- } else {
- event = pevent_find_event_by_name(pevent, sys_name, event_name);
- if (!event)
- goto not_found;
- }
+ event = pevent_search_event(pevent, id, sys_name, event_name);
+ if (event == NULL)
+ goto not_found;
pr_stat("overriding event (%d) %s:%s with new print handler",
event->id, event->system, event->name);
@@ -5637,6 +5649,79 @@ int pevent_register_event_handler(struct pevent *pevent, int id,
return -1;
}
+static int handle_matches(struct event_handler *handler, int id,
+ const char *sys_name, const char *event_name,
+ pevent_event_handler_func func, void *context)
+{
+ if (id >= 0 && id != handler->id)
+ return 0;
+
+ if (event_name && (strcmp(event_name, handler->event_name) != 0))
+ return 0;
+
+ if (sys_name && (strcmp(sys_name, handler->sys_name) != 0))
+ return 0;
+
+ if (func != handler->func || context != handler->context)
+ return 0;
+
+ return 1;
+}
+
+/**
+ * pevent_unregister_event_handler - unregister an existing event handler
+ * @pevent: the handle to the pevent
+ * @id: the id of the event to unregister
+ * @sys_name: the system name the handler belongs to
+ * @event_name: the name of the event handler
+ * @func: the function to call to parse the event information
+ * @context: the data to be passed to @func
+ *
+ * This function removes existing event handler (parser).
+ *
+ * If @id is >= 0, then it is used to find the event.
+ * else @sys_name and @event_name are used.
+ *
+ * Returns 0 if handler was removed successfully, -1 if event was not found.
+ */
+int pevent_unregister_event_handler(struct pevent *pevent, int id,
+ const char *sys_name, const char *event_name,
+ pevent_event_handler_func func, void *context)
+{
+ struct event_format *event;
+ struct event_handler *handle;
+ struct event_handler **next;
+
+ event = pevent_search_event(pevent, id, sys_name, event_name);
+ if (event == NULL)
+ goto not_found;
+
+ if (event->handler == func && event->context == context) {
+ pr_stat("overriding event (%d) %s:%s with default print handler",
+ event->id, event->system, event->name);
+
+ event->handler = NULL;
+ event->context = NULL;
+ return 0;
+ }
+
+not_found:
+ for (next = &pevent->handlers; *next; next = &(*next)->next) {
+ handle = *next;
+ if (handle_matches(handle, id, sys_name, event_name,
+ func, context))
+ break;
+ }
+
+ if (!(*next))
+ return -1;
+
+ *next = handle->next;
+ free_handler(handle);
+
+ return 0;
+}
+
/**
* pevent_alloc - create a pevent handle
*/
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index a3beca56cb35..c48acfbc6230 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -624,6 +624,9 @@ int pevent_print_func_field(struct trace_seq *s, const char *fmt,
int pevent_register_event_handler(struct pevent *pevent, int id,
const char *sys_name, const char *event_name,
pevent_event_handler_func func, void *context);
+int pevent_unregister_event_handler(struct pevent *pevent, int id,
+ const char *sys_name, const char *event_name,
+ pevent_event_handler_func func, void *context);
int pevent_register_print_function(struct pevent *pevent,
pevent_func_handler func,
enum pevent_func_arg_type ret_type,
--
1.7.11.7
The PEVENT_PLUGIN_UNLOADER function might need some cleanup using
pevent like unregister some handlers. So pass pevent as argument.
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/event-parse.h | 7 ++++---
tools/lib/traceevent/event-plugin.c | 4 ++--
tools/lib/traceevent/plugin_function.c | 2 +-
tools/perf/util/trace-event.c | 2 +-
4 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 3c890cb28db7..a3beca56cb35 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -105,7 +105,7 @@ typedef int (*pevent_event_handler_func)(struct trace_seq *s,
void *context);
typedef int (*pevent_plugin_load_func)(struct pevent *pevent);
-typedef int (*pevent_plugin_unload_func)(void);
+typedef int (*pevent_plugin_unload_func)(struct pevent *pevent);
struct plugin_option {
struct plugin_option *next;
@@ -130,7 +130,7 @@ struct plugin_option {
* PEVENT_PLUGIN_UNLOADER: (optional)
* The function called just before unloading
*
- * int PEVENT_PLUGIN_UNLOADER(void)
+ * int PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
*
* PEVENT_PLUGIN_OPTIONS: (optional)
* Plugin options that can be set before loading
@@ -411,7 +411,8 @@ enum pevent_errno {
struct plugin_list;
struct plugin_list *traceevent_load_plugins(struct pevent *pevent);
-void traceevent_unload_plugins(struct plugin_list *plugin_list);
+void traceevent_unload_plugins(struct plugin_list *plugin_list,
+ struct pevent *pevent);
struct cmdline;
struct cmdline_list;
diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
index 125f5676bcb5..0c8bf6780e4d 100644
--- a/tools/lib/traceevent/event-plugin.c
+++ b/tools/lib/traceevent/event-plugin.c
@@ -197,7 +197,7 @@ traceevent_load_plugins(struct pevent *pevent)
}
void
-traceevent_unload_plugins(struct plugin_list *plugin_list)
+traceevent_unload_plugins(struct plugin_list *plugin_list, struct pevent *pevent)
{
pevent_plugin_unload_func func;
struct plugin_list *list;
@@ -207,7 +207,7 @@ traceevent_unload_plugins(struct plugin_list *plugin_list)
plugin_list = list->next;
func = dlsym(list->handle, PEVENT_PLUGIN_UNLOADER_NAME);
if (func)
- func();
+ func(pevent);
dlclose(list->handle);
free(list->name);
free(list);
diff --git a/tools/lib/traceevent/plugin_function.c b/tools/lib/traceevent/plugin_function.c
index aad92ad5e96f..39461485f9a7 100644
--- a/tools/lib/traceevent/plugin_function.c
+++ b/tools/lib/traceevent/plugin_function.c
@@ -144,7 +144,7 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
return 0;
}
-void PEVENT_PLUGIN_UNLOADER(void)
+void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
{
int i, x;
diff --git a/tools/perf/util/trace-event.c b/tools/perf/util/trace-event.c
index d9f5f6137ab3..6322d37164c5 100644
--- a/tools/perf/util/trace-event.c
+++ b/tools/perf/util/trace-event.c
@@ -34,8 +34,8 @@ int trace_event__init(struct trace_event *t)
void trace_event__cleanup(struct trace_event *t)
{
+ traceevent_unload_plugins(t->plugin_list, t->pevent);
pevent_free(t->pevent);
- traceevent_unload_plugins(t->plugin_list);
}
static struct event_format*
--
1.7.11.7
Now all of its users were gone. :)
Signed-off-by: Namhyung Kim <[email protected]>
---
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
If realloc() fails, it'll leak the buffer. Also increate buffer size
only if the allocation succeeded.
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/trace-seq.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
index 7f24e8989401..941d35d2cf87 100644
--- a/tools/lib/traceevent/trace-seq.c
+++ b/tools/lib/traceevent/trace-seq.c
@@ -81,11 +81,16 @@ void trace_seq_destroy(struct trace_seq *s)
static void expand_buffer(struct trace_seq *s)
{
- s->buffer_size += TRACE_SEQ_BUF_SIZE;
- s->buffer = realloc(s->buffer, s->buffer_size);
- if (WARN_ONCE(!s->buffer,
- "Can't allocate trace_seq buffer memory"))
+ char *buf;
+
+ buf = realloc(s->buffer, s->buffer_size + TRACE_SEQ_BUF_SIZE);
+ if (WARN_ONCE(!buf, "Can't allocate trace_seq buffer memory")) {
s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
+ return;
+ }
+
+ s->buffer = buf;
+ s->buffer_size += TRACE_SEQ_BUF_SIZE;
}
/**
--
1.7.11.7
On Wed, 15 Jan 2014 10:45:24 +0900
Namhyung Kim <[email protected]> wrote:
>
> @@ -32,8 +33,9 @@
> #define TRACE_SEQ_POISON ((void *)0xdeadbeef)
> #define TRACE_SEQ_CHECK(s) \
> do { \
> - if ((s)->buffer == TRACE_SEQ_POISON) \
> - die("Usage of trace_seq after it was destroyed"); \
> + if (WARN_ONCE((s)->buffer == TRACE_SEQ_POISON, \
> + "Usage of trace_seq after it was destroyed")) \
> + (s)->state = TRACE_SEQ__BUFFER_POISONED; \
> } while (0)
>
> /**
> @@ -46,6 +48,7 @@ void trace_seq_init(struct trace_seq *s)
> s->readpos = 0;
> s->buffer_size = TRACE_SEQ_BUF_SIZE;
> s->buffer = malloc_or_die(s->buffer_size);
> + s->state = TRACE_SEQ__GOOD;
> }
>
> /**
> @@ -80,8 +83,9 @@ static void expand_buffer(struct trace_seq *s)
> {
> s->buffer_size += TRACE_SEQ_BUF_SIZE;
> s->buffer = realloc(s->buffer, s->buffer_size);
> - if (!s->buffer)
> - die("Can't allocate trace_seq buffer memory");
> + if (WARN_ONCE(!s->buffer,
> + "Can't allocate trace_seq buffer memory"))
> + s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
> }
>
> /**
> @@ -108,6 +112,9 @@ trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
> TRACE_SEQ_CHECK(s);
>
> try_again:
> + if (s->state != TRACE_SEQ__GOOD)
> + return 0;
> +
> len = (s->buffer_size - 1) - s->len;
>
> va_start(ap, fmt);
> @@ -144,6 +151,9 @@ trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args)
> TRACE_SEQ_CHECK(s);
>
> try_again:
> + if (s->state != TRACE_SEQ__GOOD)
> + return 0;
> +
> len = (s->buffer_size - 1) - s->len;
>
> ret = vsnprintf(s->buffer + s->len, len, fmt, args);
> @@ -174,11 +184,17 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
>
> TRACE_SEQ_CHECK(s);
>
> + if (s->state != TRACE_SEQ__GOOD)
> + return 0;
> +
> len = strlen(str);
>
> while (len > ((s->buffer_size - 1) - s->len))
> expand_buffer(s);
>
> + if (s->state != TRACE_SEQ__GOOD)
> + return 0;
> +
> memcpy(s->buffer + s->len, str, len);
> s->len += len;
>
> @@ -189,9 +205,15 @@ int trace_seq_putc(struct trace_seq *s, unsigned char c)
> {
> TRACE_SEQ_CHECK(s);
>
> + if (s->state != TRACE_SEQ__GOOD)
> + return 0;
> +
Instead of adding all of these, we can extend the macro
TRACE_SEQ_CHECK() which does a
if (s->state != TRACE_SEQ__GOOD)
return;
and a TRACE_SEQ_CHECK_RET() that does a return 0;
-- Steve
> while (s->len >= (s->buffer_size - 1))
> expand_buffer(s);
>
> + if (s->state != TRACE_SEQ__GOOD)
> + return 0;
> +
> s->buffer[s->len++] = c;
>
> return 1;
> @@ -201,6 +223,9 @@ void trace_seq_terminate(struct trace_seq *s)
> {
> TRACE_SEQ_CHECK(s);
>
> + if (s->state != TRACE_SEQ__GOOD)
> + return;
> +
> /* There's always one character left on the buffer */
> s->buffer[s->len] = 0;
> }
> @@ -208,5 +233,16 @@ void trace_seq_terminate(struct trace_seq *s)
> int trace_seq_do_printf(struct trace_seq *s)
> {
> TRACE_SEQ_CHECK(s);
> - return printf("%.*s", s->len, s->buffer);
> +
> + switch (s->state) {
> + case TRACE_SEQ__GOOD:
> + return printf("%.*s", s->len, s->buffer);
> + case TRACE_SEQ__BUFFER_POISONED:
> + puts("Usage of trace_seq after it was destroyed");
> + break;
> + case TRACE_SEQ__MEM_ALLOC_FAILED:
> + puts("Can't allocate trace_seq buffer memory");
> + break;
> + }
> + return -1;
> }
On Wed, 15 Jan 2014 10:45:25 +0900
Namhyung Kim <[email protected]> wrote:
> If realloc() fails, it'll leak the buffer. Also increate buffer size
> only if the allocation succeeded.
>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
Acked-by: Steven Rostedt <[email protected]>
-- Steve
On Wed, 15 Jan 2014 10:45:26 +0900
Namhyung Kim <[email protected]> wrote:
> Use plain malloc() and check its return value.
>
> Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
-- 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;
> }
>
> /**
On Wed, 15 Jan 2014 10:45:28 +0900
Namhyung Kim <[email protected]> wrote:
> The PEVENT_PLUGIN_UNLOADER function might need some cleanup using
> pevent like unregister some handlers. So pass pevent as argument.
>
> Cc: Jiri Olsa <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
> ---
> 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(-)
>
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 <[email protected]> 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 <[email protected]>
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 <[email protected]>
---
tools/lib/traceevent/Makefile | 2 +-
tools/lib/traceevent/event-parse.h | 7 +++++
tools/lib/traceevent/trace-seq.c | 53 +++++++++++++++++++++++++++++---------
3 files changed, 49 insertions(+), 13 deletions(-)
diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index f778d48ac609..56d52a33a3df 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -136,7 +136,7 @@ export Q VERBOSE
EVENT_PARSE_VERSION = $(EP_VERSION).$(EP_PATCHLEVEL).$(EP_EXTRAVERSION)
-INCLUDES = -I. $(CONFIG_INCLUDES)
+INCLUDES = -I. -I $(srctree)/../../include $(CONFIG_INCLUDES)
# Set compile option CFLAGS if not set elsewhere
CFLAGS ?= -g -Wall
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index cf5db9013f2c..3c890cb28db7 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -58,6 +58,12 @@ struct pevent_record {
#endif
};
+enum trace_seq_fail {
+ TRACE_SEQ__GOOD,
+ TRACE_SEQ__BUFFER_POISONED,
+ TRACE_SEQ__MEM_ALLOC_FAILED,
+};
+
/*
* Trace sequences are used to allow a function to call several other functions
* to create a string of data to use (up to a max of PAGE_SIZE).
@@ -68,6 +74,7 @@ struct trace_seq {
unsigned int buffer_size;
unsigned int len;
unsigned int readpos;
+ enum trace_seq_fail state;
};
void trace_seq_init(struct trace_seq *s);
diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
index d7f2e68bc5b9..c39048b0de08 100644
--- a/tools/lib/traceevent/trace-seq.c
+++ b/tools/lib/traceevent/trace-seq.c
@@ -22,6 +22,7 @@
#include <string.h>
#include <stdarg.h>
+#include <asm/bug.h>
#include "event-parse.h"
#include "event-utils.h"
@@ -32,10 +33,21 @@
#define TRACE_SEQ_POISON ((void *)0xdeadbeef)
#define TRACE_SEQ_CHECK(s) \
do { \
- if ((s)->buffer == TRACE_SEQ_POISON) \
- die("Usage of trace_seq after it was destroyed"); \
+ if (WARN_ONCE((s)->buffer == TRACE_SEQ_POISON, \
+ "Usage of trace_seq after it was destroyed")) \
+ (s)->state = TRACE_SEQ__BUFFER_POISONED; \
} while (0)
+#define TRACE_SEQ_CHECK_RET_N(s, n) \
+do { \
+ TRACE_SEQ_CHECK(s); \
+ if ((s)->state != TRACE_SEQ__GOOD) \
+ return n; \
+} while (0)
+
+#define TRACE_SEQ_CHECK_RET(s) TRACE_SEQ_CHECK_RET_N(s, )
+#define TRACE_SEQ_CHECK_RET0(s) TRACE_SEQ_CHECK_RET_N(s, 0)
+
/**
* trace_seq_init - initialize the trace_seq structure
* @s: a pointer to the trace_seq structure to initialize
@@ -46,6 +58,7 @@ void trace_seq_init(struct trace_seq *s)
s->readpos = 0;
s->buffer_size = TRACE_SEQ_BUF_SIZE;
s->buffer = malloc_or_die(s->buffer_size);
+ s->state = TRACE_SEQ__GOOD;
}
/**
@@ -80,8 +93,9 @@ static void expand_buffer(struct trace_seq *s)
{
s->buffer_size += TRACE_SEQ_BUF_SIZE;
s->buffer = realloc(s->buffer, s->buffer_size);
- if (!s->buffer)
- die("Can't allocate trace_seq buffer memory");
+ if (WARN_ONCE(!s->buffer,
+ "Can't allocate trace_seq buffer memory"))
+ s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
}
/**
@@ -105,9 +119,9 @@ trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
int len;
int ret;
- TRACE_SEQ_CHECK(s);
-
try_again:
+ TRACE_SEQ_CHECK_RET0(s);
+
len = (s->buffer_size - 1) - s->len;
va_start(ap, fmt);
@@ -141,9 +155,9 @@ trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args)
int len;
int ret;
- TRACE_SEQ_CHECK(s);
-
try_again:
+ TRACE_SEQ_CHECK_RET0(s);
+
len = (s->buffer_size - 1) - s->len;
ret = vsnprintf(s->buffer + s->len, len, fmt, args);
@@ -172,13 +186,15 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
{
int len;
- TRACE_SEQ_CHECK(s);
+ TRACE_SEQ_CHECK_RET0(s);
len = strlen(str);
while (len > ((s->buffer_size - 1) - s->len))
expand_buffer(s);
+ TRACE_SEQ_CHECK_RET0(s);
+
memcpy(s->buffer + s->len, str, len);
s->len += len;
@@ -187,11 +203,13 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
int trace_seq_putc(struct trace_seq *s, unsigned char c)
{
- TRACE_SEQ_CHECK(s);
+ TRACE_SEQ_CHECK_RET0(s);
while (s->len >= (s->buffer_size - 1))
expand_buffer(s);
+ TRACE_SEQ_CHECK_RET0(s);
+
s->buffer[s->len++] = c;
return 1;
@@ -199,7 +217,7 @@ int trace_seq_putc(struct trace_seq *s, unsigned char c)
void trace_seq_terminate(struct trace_seq *s)
{
- TRACE_SEQ_CHECK(s);
+ TRACE_SEQ_CHECK_RET(s);
/* There's always one character left on the buffer */
s->buffer[s->len] = 0;
@@ -208,5 +226,16 @@ void trace_seq_terminate(struct trace_seq *s)
int trace_seq_do_printf(struct trace_seq *s)
{
TRACE_SEQ_CHECK(s);
- return printf("%.*s", s->len, s->buffer);
+
+ switch (s->state) {
+ case TRACE_SEQ__GOOD:
+ return printf("%.*s", s->len, s->buffer);
+ case TRACE_SEQ__BUFFER_POISONED:
+ puts("Usage of trace_seq after it was destroyed");
+ break;
+ case TRACE_SEQ__MEM_ALLOC_FAILED:
+ puts("Can't allocate trace_seq buffer memory");
+ break;
+ }
+ return -1;
}
--
1.7.11.7
On Wed, 15 Jan 2014 11:49:28 +0900
Namhyung Kim <[email protected]> wrote:
> Hi Steve,
>
> On Tue, 14 Jan 2014 21:00:58 -0500, Steven Rostedt wrote:
> > On Wed, 15 Jan 2014 10:45:24 +0900
> > Namhyung Kim <[email protected]> wrote:
> >
> >>
> >> @@ -32,8 +33,9 @@
> >> #define TRACE_SEQ_POISON ((void *)0xdeadbeef)
> >> #define TRACE_SEQ_CHECK(s) \
> >> do { \
> >> - if ((s)->buffer == TRACE_SEQ_POISON) \
> >> - die("Usage of trace_seq after it was destroyed"); \
> >> + if (WARN_ONCE((s)->buffer == TRACE_SEQ_POISON, \
> >> + "Usage of trace_seq after it was destroyed")) \
> >> + (s)->state = TRACE_SEQ__BUFFER_POISONED; \
> >> } while (0)
> >>
> >> @@ -189,9 +205,15 @@ int trace_seq_putc(struct trace_seq *s, unsigned char c)
> >> {
> >> TRACE_SEQ_CHECK(s);
> >>
> >> + if (s->state != TRACE_SEQ__GOOD)
> >> + return 0;
> >> +
> >
> > Instead of adding all of these, we can extend the macro
> > TRACE_SEQ_CHECK() which does a
> > if (s->state != TRACE_SEQ__GOOD)
> > return;
> >
> > and a TRACE_SEQ_CHECK_RET() that does a return 0;
>
> Oh, it looks better. But I'd like to TRACE_SEQ_CHECK() as is for some
> cases. How about this?
>
>
Looks good to me.
Acked-by: Steven Rostedt <[email protected]>
I'll try to look at the rest of the patches tomorrow.
-- Steve
On Tue, 14 Jan 2014 21:56:55 -0500, Steven Rostedt wrote:
> On Wed, 15 Jan 2014 11:49:28 +0900
> Namhyung Kim <[email protected]> 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 <[email protected]>
>
> 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 <[email protected]>
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 <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/Makefile | 2 +-
tools/lib/traceevent/event-parse.h | 7 +++++
tools/lib/traceevent/trace-seq.c | 55 +++++++++++++++++++++++++++++---------
3 files changed, 50 insertions(+), 14 deletions(-)
diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index f778d48ac609..56d52a33a3df 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -136,7 +136,7 @@ export Q VERBOSE
EVENT_PARSE_VERSION = $(EP_VERSION).$(EP_PATCHLEVEL).$(EP_EXTRAVERSION)
-INCLUDES = -I. $(CONFIG_INCLUDES)
+INCLUDES = -I. -I $(srctree)/../../include $(CONFIG_INCLUDES)
# Set compile option CFLAGS if not set elsewhere
CFLAGS ?= -g -Wall
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index cf5db9013f2c..3c890cb28db7 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -58,6 +58,12 @@ struct pevent_record {
#endif
};
+enum trace_seq_fail {
+ TRACE_SEQ__GOOD,
+ TRACE_SEQ__BUFFER_POISONED,
+ TRACE_SEQ__MEM_ALLOC_FAILED,
+};
+
/*
* Trace sequences are used to allow a function to call several other functions
* to create a string of data to use (up to a max of PAGE_SIZE).
@@ -68,6 +74,7 @@ struct trace_seq {
unsigned int buffer_size;
unsigned int len;
unsigned int readpos;
+ enum trace_seq_fail state;
};
void trace_seq_init(struct trace_seq *s);
diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
index d7f2e68bc5b9..f7112138e6af 100644
--- a/tools/lib/traceevent/trace-seq.c
+++ b/tools/lib/traceevent/trace-seq.c
@@ -22,6 +22,7 @@
#include <string.h>
#include <stdarg.h>
+#include <asm/bug.h>
#include "event-parse.h"
#include "event-utils.h"
@@ -32,10 +33,21 @@
#define TRACE_SEQ_POISON ((void *)0xdeadbeef)
#define TRACE_SEQ_CHECK(s) \
do { \
- if ((s)->buffer == TRACE_SEQ_POISON) \
- die("Usage of trace_seq after it was destroyed"); \
+ if (WARN_ONCE((s)->buffer == TRACE_SEQ_POISON, \
+ "Usage of trace_seq after it was destroyed")) \
+ (s)->state = TRACE_SEQ__BUFFER_POISONED; \
} while (0)
+#define TRACE_SEQ_CHECK_RET_N(s, n) \
+do { \
+ TRACE_SEQ_CHECK(s); \
+ if ((s)->state != TRACE_SEQ__GOOD) \
+ return n; \
+} while (0)
+
+#define TRACE_SEQ_CHECK_RET(s) TRACE_SEQ_CHECK_RET_N(s, )
+#define TRACE_SEQ_CHECK_RET0(s) TRACE_SEQ_CHECK_RET_N(s, 0)
+
/**
* trace_seq_init - initialize the trace_seq structure
* @s: a pointer to the trace_seq structure to initialize
@@ -46,6 +58,7 @@ void trace_seq_init(struct trace_seq *s)
s->readpos = 0;
s->buffer_size = TRACE_SEQ_BUF_SIZE;
s->buffer = malloc_or_die(s->buffer_size);
+ s->state = TRACE_SEQ__GOOD;
}
/**
@@ -71,7 +84,7 @@ void trace_seq_destroy(struct trace_seq *s)
{
if (!s)
return;
- TRACE_SEQ_CHECK(s);
+ TRACE_SEQ_CHECK_RET(s);
free(s->buffer);
s->buffer = TRACE_SEQ_POISON;
}
@@ -80,8 +93,9 @@ static void expand_buffer(struct trace_seq *s)
{
s->buffer_size += TRACE_SEQ_BUF_SIZE;
s->buffer = realloc(s->buffer, s->buffer_size);
- if (!s->buffer)
- die("Can't allocate trace_seq buffer memory");
+ if (WARN_ONCE(!s->buffer,
+ "Can't allocate trace_seq buffer memory"))
+ s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
}
/**
@@ -105,9 +119,9 @@ trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
int len;
int ret;
- TRACE_SEQ_CHECK(s);
-
try_again:
+ TRACE_SEQ_CHECK_RET0(s);
+
len = (s->buffer_size - 1) - s->len;
va_start(ap, fmt);
@@ -141,9 +155,9 @@ trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args)
int len;
int ret;
- TRACE_SEQ_CHECK(s);
-
try_again:
+ TRACE_SEQ_CHECK_RET0(s);
+
len = (s->buffer_size - 1) - s->len;
ret = vsnprintf(s->buffer + s->len, len, fmt, args);
@@ -172,13 +186,15 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
{
int len;
- TRACE_SEQ_CHECK(s);
+ TRACE_SEQ_CHECK_RET0(s);
len = strlen(str);
while (len > ((s->buffer_size - 1) - s->len))
expand_buffer(s);
+ TRACE_SEQ_CHECK_RET0(s);
+
memcpy(s->buffer + s->len, str, len);
s->len += len;
@@ -187,11 +203,13 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
int trace_seq_putc(struct trace_seq *s, unsigned char c)
{
- TRACE_SEQ_CHECK(s);
+ TRACE_SEQ_CHECK_RET0(s);
while (s->len >= (s->buffer_size - 1))
expand_buffer(s);
+ TRACE_SEQ_CHECK_RET0(s);
+
s->buffer[s->len++] = c;
return 1;
@@ -199,7 +217,7 @@ int trace_seq_putc(struct trace_seq *s, unsigned char c)
void trace_seq_terminate(struct trace_seq *s)
{
- TRACE_SEQ_CHECK(s);
+ TRACE_SEQ_CHECK_RET(s);
/* There's always one character left on the buffer */
s->buffer[s->len] = 0;
@@ -208,5 +226,16 @@ void trace_seq_terminate(struct trace_seq *s)
int trace_seq_do_printf(struct trace_seq *s)
{
TRACE_SEQ_CHECK(s);
- return printf("%.*s", s->len, s->buffer);
+
+ switch (s->state) {
+ case TRACE_SEQ__GOOD:
+ return printf("%.*s", s->len, s->buffer);
+ case TRACE_SEQ__BUFFER_POISONED:
+ puts("Usage of trace_seq after it was destroyed");
+ break;
+ case TRACE_SEQ__MEM_ALLOC_FAILED:
+ puts("Can't allocate trace_seq buffer memory");
+ break;
+ }
+ return -1;
}
--
1.7.11.7
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 <[email protected]>
jirka
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 <[email protected]> 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 <[email protected]>
> >
> > I'll try to look at the rest of the patches tomorrow.
Hi Steven, can I keep your ackd-by?
> Thanks, and I found that trace_seq_destroy() should use
> TRACE_SEQ_TRACE_RET instead. Here's an updated patch.
>
> Thanks,
> Namhyung
>
>
> From 539a5dd1cb3ba4cb03c283115a9a84f8e345514e Mon Sep 17 00:00:00 2001
> From: Namhyung Kim <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> tools/lib/traceevent/Makefile | 2 +-
> tools/lib/traceevent/event-parse.h | 7 +++++
> tools/lib/traceevent/trace-seq.c | 55 +++++++++++++++++++++++++++++---------
> 3 files changed, 50 insertions(+), 14 deletions(-)
>
> diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
> index f778d48ac609..56d52a33a3df 100644
> --- a/tools/lib/traceevent/Makefile
> +++ b/tools/lib/traceevent/Makefile
> @@ -136,7 +136,7 @@ export Q VERBOSE
>
> EVENT_PARSE_VERSION = $(EP_VERSION).$(EP_PATCHLEVEL).$(EP_EXTRAVERSION)
>
> -INCLUDES = -I. $(CONFIG_INCLUDES)
> +INCLUDES = -I. -I $(srctree)/../../include $(CONFIG_INCLUDES)
>
> # Set compile option CFLAGS if not set elsewhere
> CFLAGS ?= -g -Wall
> diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
> index cf5db9013f2c..3c890cb28db7 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -58,6 +58,12 @@ struct pevent_record {
> #endif
> };
>
> +enum trace_seq_fail {
> + TRACE_SEQ__GOOD,
> + TRACE_SEQ__BUFFER_POISONED,
> + TRACE_SEQ__MEM_ALLOC_FAILED,
> +};
> +
> /*
> * Trace sequences are used to allow a function to call several other functions
> * to create a string of data to use (up to a max of PAGE_SIZE).
> @@ -68,6 +74,7 @@ struct trace_seq {
> unsigned int buffer_size;
> unsigned int len;
> unsigned int readpos;
> + enum trace_seq_fail state;
> };
>
> void trace_seq_init(struct trace_seq *s);
> diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
> index d7f2e68bc5b9..f7112138e6af 100644
> --- a/tools/lib/traceevent/trace-seq.c
> +++ b/tools/lib/traceevent/trace-seq.c
> @@ -22,6 +22,7 @@
> #include <string.h>
> #include <stdarg.h>
>
> +#include <asm/bug.h>
> #include "event-parse.h"
> #include "event-utils.h"
>
> @@ -32,10 +33,21 @@
> #define TRACE_SEQ_POISON ((void *)0xdeadbeef)
> #define TRACE_SEQ_CHECK(s) \
> do { \
> - if ((s)->buffer == TRACE_SEQ_POISON) \
> - die("Usage of trace_seq after it was destroyed"); \
> + if (WARN_ONCE((s)->buffer == TRACE_SEQ_POISON, \
> + "Usage of trace_seq after it was destroyed")) \
> + (s)->state = TRACE_SEQ__BUFFER_POISONED; \
> } while (0)
>
> +#define TRACE_SEQ_CHECK_RET_N(s, n) \
> +do { \
> + TRACE_SEQ_CHECK(s); \
> + if ((s)->state != TRACE_SEQ__GOOD) \
> + return n; \
> +} while (0)
> +
> +#define TRACE_SEQ_CHECK_RET(s) TRACE_SEQ_CHECK_RET_N(s, )
> +#define TRACE_SEQ_CHECK_RET0(s) TRACE_SEQ_CHECK_RET_N(s, 0)
> +
> /**
> * trace_seq_init - initialize the trace_seq structure
> * @s: a pointer to the trace_seq structure to initialize
> @@ -46,6 +58,7 @@ void trace_seq_init(struct trace_seq *s)
> s->readpos = 0;
> s->buffer_size = TRACE_SEQ_BUF_SIZE;
> s->buffer = malloc_or_die(s->buffer_size);
> + s->state = TRACE_SEQ__GOOD;
> }
>
> /**
> @@ -71,7 +84,7 @@ void trace_seq_destroy(struct trace_seq *s)
> {
> if (!s)
> return;
> - TRACE_SEQ_CHECK(s);
> + TRACE_SEQ_CHECK_RET(s);
> free(s->buffer);
> s->buffer = TRACE_SEQ_POISON;
> }
> @@ -80,8 +93,9 @@ static void expand_buffer(struct trace_seq *s)
> {
> s->buffer_size += TRACE_SEQ_BUF_SIZE;
> s->buffer = realloc(s->buffer, s->buffer_size);
> - if (!s->buffer)
> - die("Can't allocate trace_seq buffer memory");
> + if (WARN_ONCE(!s->buffer,
> + "Can't allocate trace_seq buffer memory"))
> + s->state = TRACE_SEQ__MEM_ALLOC_FAILED;
> }
>
> /**
> @@ -105,9 +119,9 @@ trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
> int len;
> int ret;
>
> - TRACE_SEQ_CHECK(s);
> -
> try_again:
> + TRACE_SEQ_CHECK_RET0(s);
> +
> len = (s->buffer_size - 1) - s->len;
>
> va_start(ap, fmt);
> @@ -141,9 +155,9 @@ trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args)
> int len;
> int ret;
>
> - TRACE_SEQ_CHECK(s);
> -
> try_again:
> + TRACE_SEQ_CHECK_RET0(s);
> +
> len = (s->buffer_size - 1) - s->len;
>
> ret = vsnprintf(s->buffer + s->len, len, fmt, args);
> @@ -172,13 +186,15 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
> {
> int len;
>
> - TRACE_SEQ_CHECK(s);
> + TRACE_SEQ_CHECK_RET0(s);
>
> len = strlen(str);
>
> while (len > ((s->buffer_size - 1) - s->len))
> expand_buffer(s);
>
> + TRACE_SEQ_CHECK_RET0(s);
> +
> memcpy(s->buffer + s->len, str, len);
> s->len += len;
>
> @@ -187,11 +203,13 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
>
> int trace_seq_putc(struct trace_seq *s, unsigned char c)
> {
> - TRACE_SEQ_CHECK(s);
> + TRACE_SEQ_CHECK_RET0(s);
>
> while (s->len >= (s->buffer_size - 1))
> expand_buffer(s);
>
> + TRACE_SEQ_CHECK_RET0(s);
> +
> s->buffer[s->len++] = c;
>
> return 1;
> @@ -199,7 +217,7 @@ int trace_seq_putc(struct trace_seq *s, unsigned char c)
>
> void trace_seq_terminate(struct trace_seq *s)
> {
> - TRACE_SEQ_CHECK(s);
> + TRACE_SEQ_CHECK_RET(s);
>
> /* There's always one character left on the buffer */
> s->buffer[s->len] = 0;
> @@ -208,5 +226,16 @@ void trace_seq_terminate(struct trace_seq *s)
> int trace_seq_do_printf(struct trace_seq *s)
> {
> TRACE_SEQ_CHECK(s);
> - return printf("%.*s", s->len, s->buffer);
> +
> + switch (s->state) {
> + case TRACE_SEQ__GOOD:
> + return printf("%.*s", s->len, s->buffer);
> + case TRACE_SEQ__BUFFER_POISONED:
> + puts("Usage of trace_seq after it was destroyed");
> + break;
> + case TRACE_SEQ__MEM_ALLOC_FAILED:
> + puts("Can't allocate trace_seq buffer memory");
> + break;
> + }
> + return -1;
> }
> --
> 1.7.11.7
On Wed, 15 Jan 2014 11:15:19 -0300
Arnaldo Carvalho de Melo <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > >
> > > I'll try to look at the rest of the patches tomorrow.
>
> Hi Steven, can I keep your ackd-by?
>
Yep.
-- Steve
On Wed, 15 Jan 2014 10:45:29 +0900
Namhyung Kim <[email protected]> wrote:
> pr_stat("overriding event (%d) %s:%s with new print handler",
> event->id, event->system, event->name);
> @@ -5637,6 +5649,79 @@ int pevent_register_event_handler(struct pevent *pevent, int id,
> return -1;
> }
>
> +static int handle_matches(struct event_handler *handler, int id,
> + const char *sys_name, const char *event_name,
> + pevent_event_handler_func func, void *context)
> +{
> + if (id >= 0 && id != handler->id)
> + return 0;
> +
> + if (event_name && (strcmp(event_name, handler->event_name) != 0))
> + return 0;
> +
> + if (sys_name && (strcmp(sys_name, handler->sys_name) != 0))
> + return 0;
> +
> + if (func != handler->func || context != handler->context)
> + return 0;
> +
> + return 1;
> +}
> +
> +/**
> + * pevent_unregister_event_handler - unregister an existing event handler
> + * @pevent: the handle to the pevent
> + * @id: the id of the event to unregister
> + * @sys_name: the system name the handler belongs to
> + * @event_name: the name of the event handler
> + * @func: the function to call to parse the event information
> + * @context: the data to be passed to @func
> + *
> + * This function removes existing event handler (parser).
> + *
> + * If @id is >= 0, then it is used to find the event.
> + * else @sys_name and @event_name are used.
> + *
> + * Returns 0 if handler was removed successfully, -1 if event was not found.
> + */
> +int pevent_unregister_event_handler(struct pevent *pevent, int id,
> + const char *sys_name, const char *event_name,
> + pevent_event_handler_func func, void *context)
> +{
> + struct event_format *event;
> + struct event_handler *handle;
> + struct event_handler **next;
> +
> + event = pevent_search_event(pevent, id, sys_name, event_name);
> + if (event == NULL)
> + goto not_found;
> +
> + if (event->handler == func && event->context == context) {
> + pr_stat("overriding event (%d) %s:%s with default print handler",
Should we use the word "overriding" again? Perhaps saying:
"removing override print handler (%d) %s:%s. Going back to default handler."
Or something?
-- Steve
> + event->id, event->system, event->name);
> +
> + event->handler = NULL;
> + event->context = NULL;
> + return 0;
> + }
> +
> +not_found:
> + for (next = &pevent->handlers; *next; next = &(*next)->next) {
> + handle = *next;
> + if (handle_matches(handle, id, sys_name, event_name,
> + func, context))
> + break;
> + }
> +
> + if (!(*next))
> + return -1;
> +
> + *next = handle->next;
> + free_handler(handle);
> +
> + return 0;
> +}
> +
On Wed, 15 Jan 2014 10:45:30 +0900
Namhyung Kim <[email protected]> wrote:
> When a plugin unloaded it needs to unregister its print handler from
> pevent. So add unregister function to do it.
>
> Cc: Jiri Olsa <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
Acked-by: Steven Rostedt <[email protected]>
-- Steve
On Wed, 15 Jan 2014 10:45:31 +0900
Namhyung Kim <[email protected]> wrote:
> The function handler should be unregistered when the plugin unloaded
> otherwise it'll try to access invalid memory.
>
> Cc: Jiri Olsa <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
-- 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]);
On Wed, 15 Jan 2014 10:45:32 +0900
Namhyung Kim <[email protected]> wrote:
> The timer handlers should be unregistered when the plugin unloaded
> otherwise they'll try to access invalid memory.
>
> Cc: Jiri Olsa <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
-- Steve
Em Wed, Jan 15, 2014 at 11:18:39AM -0500, Steven Rostedt escreveu:
> On Wed, 15 Jan 2014 10:45:29 +0900
> Namhyung Kim <[email protected]> wrote:
>
> > pr_stat("overriding event (%d) %s:%s with new print handler",
> > event->id, event->system, event->name);
> > @@ -5637,6 +5649,79 @@ int pevent_register_event_handler(struct pevent *pevent, int id,
> > return -1;
> > }
> >
> > +static int handle_matches(struct event_handler *handler, int id,
> > + const char *sys_name, const char *event_name,
> > + pevent_event_handler_func func, void *context)
> > +{
> > + if (id >= 0 && id != handler->id)
> > + return 0;
> > +
> > + if (event_name && (strcmp(event_name, handler->event_name) != 0))
> > + return 0;
> > +
> > + if (sys_name && (strcmp(sys_name, handler->sys_name) != 0))
> > + return 0;
> > +
> > + if (func != handler->func || context != handler->context)
> > + return 0;
> > +
> > + return 1;
> > +}
> > +
> > +/**
> > + * pevent_unregister_event_handler - unregister an existing event handler
> > + * @pevent: the handle to the pevent
> > + * @id: the id of the event to unregister
> > + * @sys_name: the system name the handler belongs to
> > + * @event_name: the name of the event handler
> > + * @func: the function to call to parse the event information
> > + * @context: the data to be passed to @func
> > + *
> > + * This function removes existing event handler (parser).
> > + *
> > + * If @id is >= 0, then it is used to find the event.
> > + * else @sys_name and @event_name are used.
> > + *
> > + * Returns 0 if handler was removed successfully, -1 if event was not found.
> > + */
> > +int pevent_unregister_event_handler(struct pevent *pevent, int id,
> > + const char *sys_name, const char *event_name,
> > + pevent_event_handler_func func, void *context)
> > +{
> > + struct event_format *event;
> > + struct event_handler *handle;
> > + struct event_handler **next;
> > +
> > + event = pevent_search_event(pevent, id, sys_name, event_name);
> > + if (event == NULL)
> > + goto not_found;
> > +
> > + if (event->handler == func && event->context == context) {
> > + pr_stat("overriding event (%d) %s:%s with default print handler",
>
> Should we use the word "overriding" again? Perhaps saying:
>
> "removing override print handler (%d) %s:%s. Going back to default handler."
>
> Or something?
Applied up to the patch just before this one, waiting for a resolution.
> -- Steve
>
> > + event->id, event->system, event->name);
> > +
> > + event->handler = NULL;
> > + event->context = NULL;
> > + return 0;
> > + }
> > +
> > +not_found:
> > + for (next = &pevent->handlers; *next; next = &(*next)->next) {
> > + handle = *next;
> > + if (handle_matches(handle, id, sys_name, event_name,
> > + func, context))
> > + break;
> > + }
> > +
> > + if (!(*next))
> > + return -1;
> > +
> > + *next = handle->next;
> > + free_handler(handle);
> > +
> > + return 0;
> > +}
> > +
On Wed, 15 Jan 2014 10:45:36 +0900
Namhyung Kim <[email protected]> wrote:
> The event handler should be unregistered when the plugin unloaded
> otherwise it'll try to access invalid memory.
>
> Cc: Jiri Olsa <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
For PATCH 10-13,
Acked-by: Steven Rostedt <[email protected]>
-- Steve
On Wed, 15 Jan 2014 10:45:37 +0900
Namhyung Kim <[email protected]> wrote:
> The function handler should be unregistered when the plugin unloaded
> otherwise it'll try to access invalid memory.
>
> Cc: Jiri Olsa <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> 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
On Wed, 15 Jan 2014 10:45:38 +0900
Namhyung Kim <[email protected]> wrote:
> The function handlers should be unregistered when the plugin unloaded
> otherwise they'll try to access invalid memory.
>
> Cc: Jiri Olsa <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> tools/lib/traceevent/plugin_jbd2.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/tools/lib/traceevent/plugin_jbd2.c b/tools/lib/traceevent/plugin_jbd2.c
> index 2f93f81f0bac..88fe6fe872d5 100644
> --- a/tools/lib/traceevent/plugin_jbd2.c
> +++ b/tools/lib/traceevent/plugin_jbd2.c
> @@ -66,3 +66,14 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
> PEVENT_FUNC_ARG_VOID);
> return 0;
> }
> +
> +void PEVENT_PLUGIN_UNLOADER(struct pevent *pevent)
> +{
> + pevent_unregister_print_function(pevent,
> + process_jbd2_dev_to_name,
> + "jbd2_dev_to_name");
> +
> + pevent_unregister_print_function(pevent,
> + process_jiffies_to_msecs,
> + "jiffies_to_msecs");
> +}
Same here. I don't know about others, but I think it looks a bit nicer
if we move the function up to the first line.
-- Steve
On Wed, 15 Jan 2014 10:45:39 +0900
Namhyung Kim <[email protected]> wrote:
> The function handler should be unregistered when the plugin unloaded
> otherwise it'll try to access invalid memory.
>
> Cc: Jiri Olsa <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> 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
Hi Steve,
2014-01-16 AM 1:18, Steven Rostedt wrote:
> On Wed, 15 Jan 2014 10:45:29 +0900
> Namhyung Kim <[email protected]> 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
On Thu, 16 Jan 2014 09:00:28 +0900
Namhyung Kim <[email protected]> wrote:
> Hi Steve,
>
> 2014-01-16 AM 1:18, Steven Rostedt wrote:
> > On Wed, 15 Jan 2014 10:45:29 +0900
> > Namhyung Kim <[email protected]> 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
On Wed, 15 Jan 2014 20:52:59 -0500, Steven Rostedt wrote:
> On Thu, 16 Jan 2014 09:00:28 +0900
> Namhyung Kim <[email protected]> wrote:
>
>> Hi Steve,
>>
>> 2014-01-16 AM 1:18, Steven Rostedt wrote:
>> > On Wed, 15 Jan 2014 10:45:29 +0900
>> > Namhyung Kim <[email protected]> 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
On Thu, 16 Jan 2014 10:58:05 +0900
Namhyung Kim <[email protected]> wrote:
> Thanks, can I add your ack with this change?
Yeah, go ahead.
-- Steve
Commit-ID: 3026bba3c37711234771349ca020d9a85e572f60
Gitweb: http://git.kernel.org/tip/3026bba3c37711234771349ca020d9a85e572f60
Author: Namhyung Kim <[email protected]>
AuthorDate: Wed, 15 Jan 2014 10:45:25 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Reviewed-by: Jiri Olsa <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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;
}
/**
Commit-ID: e825e756f84eab0e68d7d6644c018c3412748406
Gitweb: http://git.kernel.org/tip/e825e756f84eab0e68d7d6644c018c3412748406
Author: Namhyung Kim <[email protected]>
AuthorDate: Wed, 15 Jan 2014 10:45:27 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Reviewed-by: Jiri Olsa <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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;
-}
Commit-ID: 8d0c2224ca6e04ba51c403805e7e1e2ca536520b
Gitweb: http://git.kernel.org/tip/8d0c2224ca6e04ba51c403805e7e1e2ca536520b
Author: Namhyung Kim <[email protected]>
AuthorDate: Wed, 15 Jan 2014 10:45:28 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Reviewed-by: Jiri Olsa <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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*
Commit-ID: 504586e0954bcf9550dfdea37d3234174ed1d68f
Gitweb: http://git.kernel.org/tip/504586e0954bcf9550dfdea37d3234174ed1d68f
Author: Namhyung Kim <[email protected]>
AuthorDate: Wed, 15 Jan 2014 10:45:26 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Reviewed-by: Jiri Olsa <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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;
}
/**
Commit-ID: 3c6d8d84423932f1d9949179c6acdf2405515ee4
Gitweb: http://git.kernel.org/tip/3c6d8d84423932f1d9949179c6acdf2405515ee4
Author: Namhyung Kim <[email protected]>
AuthorDate: Thu, 19 Dec 2013 18:17:44 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Reviewed-by: Jiri Olsa <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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;
}