Hi,
Here is a series of ftrace/perf updates to support multiple
event select operation by glob-based wild cards.
I've ported strglobmatch from perf-tools (with recursive call
limitation) for this use. It is easier to use (just replacing
strcmp) but slower than current parser-based matching.
I don't care about the speed of matching because the all of
the matching which I've introduced in this series are done
on slow-path.
---
Masami Hiramatsu (5):
[BUGFIX] tracing: Returns -EBUSY when event_enable_func fails to get module
perf: Reorder parameters of strglobmatch
lib/string: Add a generic wildcard string matching function
tracing/kprobes: Allow user to delete kprobe events by wild cards
tracing: Support enable/disable multiple events trigger by wild cards
Documentation/trace/ftrace.txt | 12 ++-
Documentation/trace/kprobetrace.txt | 19 +++--
include/linux/string.h | 8 ++
kernel/trace/trace_events.c | 121 +++++++++++++++++++++++++----------
kernel/trace/trace_kprobe.c | 97 ++++++++++++++++++++--------
lib/string.c | 91 ++++++++++++++++++++++++++
tools/perf/util/parse-events.c | 14 ++--
tools/perf/util/probe-event.c | 2 -
tools/perf/util/strfilter.c | 2 -
tools/perf/util/string.c | 16 ++---
tools/perf/util/util.h | 4 +
11 files changed, 295 insertions(+), 91 deletions(-)
--
Masami Hiramatsu <[email protected]>
IT Management Research Dept. and Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
Since try_module_get returns false( = 0) when it fails to
pindown a module, event_enable_func() returns 0 which means
"succeed". This can cause a kernel panic when the entry
is removed, because the event is already released.
This fixes the bug by returning -EBUSY, because the reason
why it fails is under removing module at that time.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
---
kernel/trace/trace_events.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 7a0cf68..27963e2 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2072,8 +2072,10 @@ event_enable_func(struct ftrace_hash *hash,
out_reg:
/* Don't let event modules unload while probe registered */
ret = try_module_get(file->event_call->mod);
- if (!ret)
+ if (!ret) {
+ ret = -EBUSY;
goto out_free;
+ }
ret = __ftrace_event_enable_disable(file, 1, 1);
if (ret < 0)
Reorder parameters of strglobmatch() so that the first
parameter is the glob pattern as like as regexec(),
because the subjective parameter of strglobmatch() must
be the glob pattern, but not a sample string.
So, the new interface is:
bool strglobmatch(const char *glob, const char *str);
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Irina Tirdea <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: David Ahern <[email protected]>
---
tools/perf/util/parse-events.c | 14 +++++++-------
tools/perf/util/probe-event.c | 2 +-
tools/perf/util/strfilter.c | 2 +-
tools/perf/util/string.c | 16 ++++++++--------
tools/perf/util/util.h | 4 ++--
5 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 6c8bb0f..26fb64a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -402,7 +402,7 @@ static int add_tracepoint_multi_event(struct list_head **list, int *idx,
|| !strcmp(evt_ent->d_name, "filter"))
continue;
- if (!strglobmatch(evt_ent->d_name, evt_name))
+ if (!strglobmatch(evt_name, evt_ent->d_name))
continue;
ret = add_tracepoint(list, idx, sys_name, evt_ent->d_name);
@@ -441,7 +441,7 @@ static int add_tracepoint_multi_sys(struct list_head **list, int *idx,
|| !strcmp(events_ent->d_name, "header_page"))
continue;
- if (!strglobmatch(events_ent->d_name, sys_name))
+ if (!strglobmatch(sys_name, events_ent->d_name))
continue;
ret = add_tracepoint_event(list, idx, events_ent->d_name,
@@ -955,7 +955,7 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
for_each_subsystem(sys_dir, sys_dirent, sys_next) {
if (subsys_glob != NULL &&
- !strglobmatch(sys_dirent.d_name, subsys_glob))
+ !strglobmatch(subsys_glob, sys_dirent.d_name))
continue;
snprintf(dir_path, MAXPATHLEN, "%s/%s", tracing_events_path,
@@ -966,7 +966,7 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
for_each_event(sys_dirent, evt_dir, evt_dirent, evt_next) {
if (event_glob != NULL &&
- !strglobmatch(evt_dirent.d_name, event_glob))
+ !strglobmatch(event_glob, evt_dirent.d_name))
continue;
if (name_only) {
@@ -1065,7 +1065,7 @@ int print_hwcache_events(const char *event_glob, bool name_only)
for (i = 0; i < PERF_COUNT_HW_CACHE_RESULT_MAX; i++) {
__perf_evsel__hw_cache_type_op_res_name(type, op, i,
name, sizeof(name));
- if (event_glob != NULL && !strglobmatch(name, event_glob))
+ if (event_glob != NULL && !strglobmatch(event_glob, name))
continue;
if (name_only)
@@ -1091,8 +1091,8 @@ static void print_symbol_events(const char *event_glob, unsigned type,
for (i = 0; i < max; i++, syms++) {
if (event_glob != NULL &&
- !(strglobmatch(syms->symbol, event_glob) ||
- (syms->alias && strglobmatch(syms->alias, event_glob))))
+ !(strglobmatch(event_glob, syms->symbol) ||
+ (syms->alias && strglobmatch(event_glob, syms->alias))))
continue;
if (name_only) {
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index aa04bf9..4d3b498 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2106,7 +2106,7 @@ static int del_trace_probe_event(int fd, const char *buf,
if (strpbrk(buf, "*?")) { /* Glob-exp */
strlist__for_each_safe(ent, n, namelist)
- if (strglobmatch(ent->s, buf)) {
+ if (strglobmatch(buf, ent->s)) {
ret = __del_trace_probe_event(fd, ent);
if (ret < 0)
break;
diff --git a/tools/perf/util/strfilter.c b/tools/perf/util/strfilter.c
index 834c8eb..e50bfc5 100644
--- a/tools/perf/util/strfilter.c
+++ b/tools/perf/util/strfilter.c
@@ -186,7 +186,7 @@ static bool strfilter_node__compare(struct strfilter_node *self,
case '!': /* NOT */
return !strfilter_node__compare(self->r, str);
default:
- return strglobmatch(str, self->p);
+ return strglobmatch(self->p, str);
}
}
diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index 29c7b2c..f4204a5 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -223,7 +223,7 @@ error:
}
/* Glob/lazy pattern matching */
-static bool __match_glob(const char *str, const char *pat, bool ignore_space)
+static bool __match_glob(const char *pat, const char *str, bool ignore_space)
{
while (*str && *pat && *pat != '*') {
if (ignore_space) {
@@ -259,7 +259,7 @@ static bool __match_glob(const char *str, const char *pat, bool ignore_space)
if (!*pat) /* Tail wild card matches all */
return true;
while (*str)
- if (__match_glob(str++, pat, ignore_space))
+ if (__match_glob(pat, str++, ignore_space))
return true;
}
return !*str && !*pat;
@@ -267,8 +267,8 @@ static bool __match_glob(const char *str, const char *pat, bool ignore_space)
/**
* strglobmatch - glob expression pattern matching
- * @str: the target string to match
* @pat: the pattern string to match
+ * @str: the target string to match
*
* This returns true if the @str matches @pat. @pat can includes wildcards
* ('*','?') and character classes ([CHARS], complementation and ranges are
@@ -277,22 +277,22 @@ static bool __match_glob(const char *str, const char *pat, bool ignore_space)
*
* Note: if @pat syntax is broken, this always returns false.
*/
-bool strglobmatch(const char *str, const char *pat)
+bool strglobmatch(const char *pat, const char *str)
{
- return __match_glob(str, pat, false);
+ return __match_glob(pat, str, false);
}
/**
* strlazymatch - matching pattern strings lazily with glob pattern
- * @str: the target string to match
* @pat: the pattern string to match
+ * @str: the target string to match
*
* This is similar to strglobmatch, except this ignores spaces in
* the target string.
*/
-bool strlazymatch(const char *str, const char *pat)
+bool strlazymatch(const char *pat, const char *str)
{
- return __match_glob(str, pat, true);
+ return __match_glob(pat, str, true);
}
/**
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index a45710b..f493590 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -239,8 +239,8 @@ int copyfile(const char *from, const char *to);
s64 perf_atoll(const char *str);
char **argv_split(const char *str, int *argcp);
void argv_free(char **argv);
-bool strglobmatch(const char *str, const char *pat);
-bool strlazymatch(const char *str, const char *pat);
+bool strglobmatch(const char *pat, const char *str);
+bool strlazymatch(const char *pat, const char *str);
int strtailcmp(const char *s1, const char *s2);
char *strxfrchar(char *s, char from, char to);
unsigned long convert_unit(unsigned long value, char *unit);
Add strglobmatch() for generic wildcard string matching.
This code is originally from perf-tools. For porting in
the kernel, the limitation of the number of wildcards is
introduced, because of the limitation of the stack.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Akinobu Mita <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: David Howells <[email protected]>
---
include/linux/string.h | 8 ++++
lib/string.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 99 insertions(+)
diff --git a/include/linux/string.h b/include/linux/string.h
index ac889c5..fcb1506 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -122,6 +122,14 @@ extern void argv_free(char **argv);
extern bool sysfs_streq(const char *s1, const char *s2);
extern int strtobool(const char *s, bool *res);
+/*
+ * Maximum number of wildcards in a glob pattern
+ * This limits the number of wildcards in a glob pattern, because
+ * more wildcards consume more kernel stack.
+ */
+#define MAX_GLOB_WILDCARDS 10
+extern bool strglobmatch(const char *glob, const char *str);
+
#ifdef CONFIG_BINARY_PRINTF
int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args);
int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf);
diff --git a/lib/string.c b/lib/string.c
index e5878de..640fb10 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -824,3 +824,94 @@ void *memchr_inv(const void *start, int c, size_t bytes)
return check_bytes8(start, value, bytes % 8);
}
EXPORT_SYMBOL(memchr_inv);
+
+/* Character class matching */
+static bool __match_charclass(const char *pat, char c, const char **npat)
+{
+ bool complement = false, ret = true;
+
+ if (*pat == '!') {
+ complement = true;
+ pat++;
+ }
+ if (*pat++ == c) /* First character is special */
+ goto end;
+
+ while (*pat && *pat != ']') { /* Matching */
+ if (*pat == '-' && *(pat + 1) != ']') { /* Range */
+ if (*(pat - 1) <= c && c <= *(pat + 1))
+ goto end;
+ if (*(pat - 1) > *(pat + 1))
+ goto error;
+ pat += 2;
+ } else if (*pat++ == c)
+ goto end;
+ }
+ if (!*pat)
+ goto error;
+ ret = false;
+
+end:
+ while (*pat && *pat != ']') /* Searching closing */
+ pat++;
+ if (!*pat)
+ goto error;
+ *npat = pat + 1;
+ return complement ? !ret : ret;
+
+error:
+ return false;
+}
+
+/* Glob pattern(wildcard and char-class) matching */
+static bool __match_glob(const char *pat, const char *str, int nest)
+{
+ if (nest > MAX_GLOB_WILDCARDS)
+ return false;
+
+ while (*str && *pat && *pat != '*') {
+ if (*pat == '?') { /* Matches any single character */
+ str++;
+ pat++;
+ continue;
+ } else if (*pat == '[') /* Character classes/Ranges */
+ if (__match_charclass(pat + 1, *str, &pat)) {
+ str++;
+ continue;
+ } else
+ return false;
+ else if (*pat == '\\') /* Escaped char match as normal char */
+ pat++;
+ if (*str++ != *pat++)
+ return false;
+ }
+ /* Check wild card */
+ if (*pat == '*') {
+ while (*pat == '*')
+ pat++;
+ if (!*pat) /* Tail wild card matches all */
+ return true;
+ while (*str)
+ if (__match_glob(pat, str++, nest + 1))
+ return true;
+ }
+ return !*str && !*pat;
+}
+
+/**
+ * strglobmatch - glob expression pattern matching
+ * @pat: the pattern string to match
+ * @str: the target string to match
+ *
+ * This returns true if the @str matches @pat. @pat can includes wildcards
+ * ('*','?') and character classes ([CHARS], complementation and ranges are
+ * also supported). Also, this supports escape character ('\') to use special
+ * characters as normal character.
+ *
+ * Note: if @pat syntax is broken, this always returns false.
+ */
+bool strglobmatch(const char *pat, const char *str)
+{
+ return __match_glob(pat, str, 0);
+}
+EXPORT_SYMBOL(strglobmatch);
Support enable/disable multiple events trigger on ftrace
by using wild cards. This makes enabling multiple events
at once easy.
e.g.)
# echo vfs_symlink:enable_event:\*:\*rq\* > set_ftrace_filter
# cat set_ftrace_filter
#### all functions enabled ####
vfs_symlink:enable_event:*:*rq*:unlimited
# grep 0\\* -r events/*/*/enable
events/block/block_getrq/enable:0*
events/block/block_rq_abort/enable:0*
events/block/block_rq_complete/enable:0*
events/block/block_rq_insert/enable:0*
events/block/block_rq_issue/enable:0*
events/block/block_rq_remap/enable:0*
events/block/block_rq_requeue/enable:0*
events/block/block_sleeprq/enable:0*
events/irq/irq_handler_entry/enable:0*
events/irq/irq_handler_exit/enable:0*
events/irq/softirq_entry/enable:0*
events/irq/softirq_exit/enable:0*
events/irq/softirq_raise/enable:0*
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
---
Documentation/trace/ftrace.txt | 12 ++--
kernel/trace/trace_events.c | 123 +++++++++++++++++++++++++++++-----------
2 files changed, 95 insertions(+), 40 deletions(-)
diff --git a/Documentation/trace/ftrace.txt b/Documentation/trace/ftrace.txt
index bfe8c29..92ca5236 100644
--- a/Documentation/trace/ftrace.txt
+++ b/Documentation/trace/ftrace.txt
@@ -2407,11 +2407,11 @@ The following commands are supported:
echo '!native_flush_tlb_others:snapshot:0' > set_ftrace_filter
- enable_event/disable_event
- These commands can enable or disable a trace event. Note, because
+ These commands can enable or disable trace events. Note, because
function tracing callbacks are very sensitive, when these commands
- are registered, the trace point is activated, but disabled in
- a "soft" mode. That is, the tracepoint will be called, but
- just will not be traced. The event tracepoint stays in this mode
+ are registered, the tracepoints are activated, but disabled in
+ a "soft" mode. That is, the tracepoints will be called, but
+ just will not be traced. The event tracepoints stay in this mode
as long as there's a command that triggers it.
echo 'try_to_wake_up:enable_event:sched:sched_switch:2' > \
@@ -2422,8 +2422,10 @@ The following commands are supported:
<function>:enable_event:<system>:<event>[:count]
<function>:disable_event:<system>:<event>[:count]
- To remove the events commands:
+ Note that the system and event accept wildcards for operating
+ multiple events at once.
+ To remove the events commands:
echo '!try_to_wake_up:enable_event:sched:sched_switch:0' > \
set_ftrace_filter
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 27963e2..1376bb4 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1850,17 +1850,21 @@ __trace_add_event_dirs(struct trace_array *tr)
#define DISABLE_EVENT_STR "disable_event"
struct event_probe_data {
- struct ftrace_event_file *file;
+ struct ftrace_event_file **files;
+ char *event;
+ char *system;
unsigned long count;
int ref;
bool enable;
};
-static struct ftrace_event_file *
-find_event_file(struct trace_array *tr, const char *system, const char *event)
+static int
+find_event_files(struct trace_array *tr, const char *system, const char *event,
+ struct ftrace_event_file **files, int size)
{
struct ftrace_event_file *file;
struct ftrace_event_call *call;
+ int nr = 0;
list_for_each_entry(file, &tr->events, list) {
@@ -1872,11 +1876,14 @@ find_event_file(struct trace_array *tr, const char *system, const char *event)
if (call->flags & TRACE_EVENT_FL_IGNORE_ENABLE)
continue;
- if (strcmp(event, call->name) == 0 &&
- strcmp(system, call->class->system) == 0)
- return file;
+ if (strglobmatch(event, call->name) &&
+ strglobmatch(system, call->class->system)) {
+ if (files && nr < size)
+ files[nr] = file;
+ nr++;
+ }
}
- return NULL;
+ return nr;
}
static void
@@ -1884,14 +1891,24 @@ event_enable_probe(unsigned long ip, unsigned long parent_ip, void **_data)
{
struct event_probe_data **pdata = (struct event_probe_data **)_data;
struct event_probe_data *data = *pdata;
+ struct ftrace_event_file **file;
if (!data)
return;
- if (data->enable)
- clear_bit(FTRACE_EVENT_FL_SOFT_DISABLED_BIT, &data->file->flags);
- else
- set_bit(FTRACE_EVENT_FL_SOFT_DISABLED_BIT, &data->file->flags);
+ file = data->files;
+ if (unlikely(!file))
+ return;
+
+ while (*file) {
+ if (data->enable)
+ clear_bit(FTRACE_EVENT_FL_SOFT_DISABLED_BIT,
+ &(*file)->flags);
+ else
+ set_bit(FTRACE_EVENT_FL_SOFT_DISABLED_BIT,
+ &(*file)->flags);
+ file++;
+ }
}
static void
@@ -1906,10 +1923,6 @@ event_enable_count_probe(unsigned long ip, unsigned long parent_ip, void **_data
if (!data->count)
return;
- /* Skip if the event is in a state we want to switch to */
- if (data->enable == !(data->file->flags & FTRACE_EVENT_FL_SOFT_DISABLED))
- return;
-
if (data->count != -1)
(data->count)--;
@@ -1926,8 +1939,7 @@ event_enable_print(struct seq_file *m, unsigned long ip,
seq_printf(m, "%s:%s:%s",
data->enable ? ENABLE_EVENT_STR : DISABLE_EVENT_STR,
- data->file->event_call->class->system,
- data->file->event_call->name);
+ data->system, data->event);
if (data->count == -1)
seq_printf(m, ":unlimited\n");
@@ -1948,6 +1960,40 @@ event_enable_init(struct ftrace_probe_ops *ops, unsigned long ip,
return 0;
}
+static int event_files_soft_mode(struct ftrace_event_file **files, int enable)
+{
+ struct ftrace_event_file **file = files;
+
+ if (!file)
+ return -EINVAL;
+
+ while (*file) {
+ /* Don't let event modules unload while probe registered */
+ if (enable) {
+ if (!try_module_get((*file)->event_call->mod))
+ goto rollback;
+ }
+
+ __ftrace_event_enable_disable(*file, enable, 1);
+
+ if (!enable)
+ module_put((*file)->event_call->mod);
+
+ file++;
+ }
+
+ return 0;
+
+ rollback:
+ while (file != files) {
+ file--;
+ __ftrace_event_enable_disable(*file, 0, 1);
+ module_put((*file)->event_call->mod);
+ }
+
+ return -EBUSY;
+}
+
static void
event_enable_free(struct ftrace_probe_ops *ops, unsigned long ip,
void **_data)
@@ -1960,9 +2006,11 @@ event_enable_free(struct ftrace_probe_ops *ops, unsigned long ip,
data->ref--;
if (!data->ref) {
- /* Remove the SOFT_MODE flag */
- __ftrace_event_enable_disable(data->file, 0, 1);
- module_put(data->file->event_call->mod);
+ /* We don't need wait rcu because no one refers data here */
+ event_files_soft_mode(data->files, 0);
+ kfree(data->files);
+ kfree(data->event);
+ kfree(data->system);
kfree(data);
}
*pdata = NULL;
@@ -2001,13 +2049,13 @@ event_enable_func(struct ftrace_hash *hash,
char *glob, char *cmd, char *param, int enabled)
{
struct trace_array *tr = top_trace_array();
- struct ftrace_event_file *file;
struct ftrace_probe_ops *ops;
struct event_probe_data *data;
const char *system;
const char *event;
char *number;
bool enable;
+ int nr_files;
int ret;
/* hash funcs only work with set_ftrace_filter */
@@ -2026,8 +2074,8 @@ event_enable_func(struct ftrace_hash *hash,
mutex_lock(&event_mutex);
ret = -EINVAL;
- file = find_event_file(tr, system, event);
- if (!file)
+ nr_files = find_event_files(tr, system, event, NULL, 0);
+ if (nr_files == 0)
goto out;
enable = strcmp(cmd, ENABLE_EVENT_STR) == 0;
@@ -2050,7 +2098,17 @@ event_enable_func(struct ftrace_hash *hash,
data->enable = enable;
data->count = -1;
- data->file = file;
+ data->files = kzalloc((nr_files + 1) * sizeof(*data->files),
+ GFP_KERNEL);
+ if (!data->files)
+ goto out_free;
+
+ find_event_files(tr, system, event, data->files, nr_files);
+
+ data->event = kstrdup(event, GFP_KERNEL);
+ data->system = kstrdup(system, GFP_KERNEL);
+ if (!data->event || !data->system)
+ goto out_free;
if (!param)
goto out_reg;
@@ -2070,16 +2128,10 @@ event_enable_func(struct ftrace_hash *hash,
goto out_free;
out_reg:
- /* Don't let event modules unload while probe registered */
- ret = try_module_get(file->event_call->mod);
- if (!ret) {
- ret = -EBUSY;
+ ret = event_files_soft_mode(data->files, 1);
+ if (ret < 0)
goto out_free;
- }
- ret = __ftrace_event_enable_disable(file, 1, 1);
- if (ret < 0)
- goto out_put;
ret = register_ftrace_function_probe(glob, ops, data);
/*
* The above returns on success the # of functions enabled,
@@ -2098,10 +2150,11 @@ event_enable_func(struct ftrace_hash *hash,
return ret;
out_disable:
- __ftrace_event_enable_disable(file, 0, 1);
- out_put:
- module_put(file->event_call->mod);
+ event_files_soft_mode(data->files, 0);
out_free:
+ kfree(data->files);
+ kfree(data->event);
+ kfree(data->system);
kfree(data);
goto out;
}
Allow user to delete multiple kprobe events by using
wild cards. This makes removing events on a specific
function easy.
e.g.)
# echo p vfs_symlink >> kprobe_events
# echo p vfs_symlink+5 >> kprobe_events
# echo p vfs_read >> kprobe_events
# cat kprobe_events
p:kprobes/p_vfs_symlink_0 vfs_symlink
p:kprobes/p_vfs_symlink_5 vfs_symlink+5
p:kprobes/p_vfs_read_0 vfs_read
# echo -:kprobes/\*vfs_symlink_\* >> kprobe_events
# cat kprobe_events
p:kprobes/p_vfs_read_0 vfs_read
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
---
Documentation/trace/kprobetrace.txt | 19 ++++---
kernel/trace/trace_kprobe.c | 97 +++++++++++++++++++++++++----------
2 files changed, 82 insertions(+), 34 deletions(-)
diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
index d68ea5f..04f22be 100644
--- a/Documentation/trace/kprobetrace.txt
+++ b/Documentation/trace/kprobetrace.txt
@@ -26,9 +26,9 @@ Synopsis of kprobe_events
r[:[GRP/]EVENT] [MOD:]SYM[+0] [FETCHARGS] : Set a return probe
-:[GRP/]EVENT : Clear a probe
- GRP : Group name. If omitted, use "kprobes" for it.
+ GRP : Group name. If omitted, use "kprobes" for it.(*)
EVENT : Event name. If omitted, the event name is generated
- based on SYM+offs or MEMADDR.
+ based on SYM+offs or MEMADDR.(*)
MOD : Module name which has given SYM.
SYM[+offs] : Symbol+offset where the probe is inserted.
MEMADDR : Address where the probe is inserted.
@@ -39,15 +39,16 @@ Synopsis of kprobe_events
@SYM[+|-offs] : Fetch memory at SYM +|- offs (SYM should be a data symbol)
$stackN : Fetch Nth entry of stack (N >= 0)
$stack : Fetch stack address.
- $retval : Fetch return value.(*)
- +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
+ $retval : Fetch return value.(**)
+ +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(***)
NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
(u8/u16/u32/u64/s8/s16/s32/s64), "string" and bitfield
are supported.
- (*) only for return probe.
- (**) this is useful for fetching a field of data structures.
+ (*) both GRP and EVENT accept glob-style wild cards when clearing probes.
+ (**) only for return probe.
+ (***) this is useful for fetching a field of data structures.
Types
-----
@@ -143,7 +144,11 @@ REC->dfd, REC->filename, REC->flags, REC->mode
echo -:myprobe >> kprobe_events
- This clears probe points selectively.
+ This removes a probe points selectively. Since the event name and group
+ name accept wild cards only when removing, you can clear all event as
+ below too.
+
+ echo '-:*/*' >> kprobe_events
Right after definition, each event is disabled by default. For tracing these
events, you need to enable it.
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 9f46e98..b619853 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -171,16 +171,32 @@ static void free_trace_probe(struct trace_probe *tp)
kfree(tp);
}
-static struct trace_probe *find_trace_probe(const char *event,
- const char *group)
+/**
+ * find_trace_probes - find matched trace_probe and return the number of it
+ * @event: event name (glob pattern)
+ * @group: group(subsystem) name (glob pattern)
+ * @buf: the address of an array of trace_probe *
+ * @size: the size of @buf array
+ *
+ * Search trace_probe matched with given name (pattern) and returns
+ * the number of it. If @buf is not NULL, it records the address of
+ * the matched trace_probe on it.
+ */
+static int
+find_trace_probes(const char *event, const char *group,
+ struct trace_probe **buf, int size)
{
struct trace_probe *tp;
+ int count = 0;
list_for_each_entry(tp, &probe_list, list)
- if (strcmp(tp->call.name, event) == 0 &&
- strcmp(tp->call.class->system, group) == 0)
- return tp;
- return NULL;
+ if (strglobmatch(event, tp->call.name) &&
+ strglobmatch(group, tp->call.class->system)) {
+ if (buf && count < size)
+ buf[count] = tp;
+ count++;
+ }
+ return count;
}
static int trace_probe_nr_files(struct trace_probe *tp)
@@ -414,8 +430,9 @@ static int register_trace_probe(struct trace_probe *tp)
mutex_lock(&probe_lock);
/* Delete old (same name) event if exist */
- old_tp = find_trace_probe(tp->call.name, tp->call.class->system);
- if (old_tp) {
+ ret = find_trace_probes(tp->call.name, tp->call.class->system,
+ &old_tp, 1);
+ if (ret) {
ret = unregister_trace_probe(old_tp);
if (ret < 0)
goto end;
@@ -475,6 +492,49 @@ static struct notifier_block trace_probe_module_nb = {
.priority = 1 /* Invoked after kprobe module callback */
};
+static int delete_trace_probe(const char *event, const char *group)
+{
+ struct trace_probe **tps;
+ int nr_tps, i, ret = 0;
+
+ if (!event) {
+ pr_info("Delete command needs an event name.\n");
+ return -EINVAL;
+ }
+
+ mutex_lock(&probe_lock);
+
+ nr_tps = find_trace_probes(event, group, NULL, 0);
+ if (nr_tps == 0) {
+ pr_info("Event %s/%s doesn't matched.\n", group, event);
+ ret = -ENOENT;
+ goto out_unlock;
+ }
+
+ tps = kzalloc(nr_tps * sizeof(*tps), GFP_KERNEL);
+ if (!tps) {
+ ret = -ENOMEM;
+ goto out_unlock;
+ }
+
+ find_trace_probes(event, group, tps, nr_tps);
+
+ for (i = 0; i < nr_tps; i++) {
+ ret = unregister_trace_probe(tps[i]);
+ if (ret < 0) {
+ pr_info("Failed to delete event: %d\n", ret);
+ continue; /* Greedy cleanup */
+ }
+ free_trace_probe(tps[i]);
+ }
+ ret = 0;
+ kfree(tps);
+
+ out_unlock:
+ mutex_unlock(&probe_lock);
+ return ret;
+}
+
static int create_trace_probe(int argc, char **argv)
{
/*
@@ -536,25 +596,8 @@ static int create_trace_probe(int argc, char **argv)
if (!group)
group = KPROBE_EVENT_SYSTEM;
- if (is_delete) {
- if (!event) {
- pr_info("Delete command needs an event name.\n");
- return -EINVAL;
- }
- mutex_lock(&probe_lock);
- tp = find_trace_probe(event, group);
- if (!tp) {
- mutex_unlock(&probe_lock);
- pr_info("Event %s/%s doesn't exist.\n", group, event);
- return -ENOENT;
- }
- /* delete an event */
- ret = unregister_trace_probe(tp);
- if (ret == 0)
- free_trace_probe(tp);
- mutex_unlock(&probe_lock);
- return ret;
- }
+ if (is_delete)
+ return delete_trace_probe(event, group);
if (argc < 2) {
pr_info("Probe point is not specified.\n");
On Thu, 2013-05-16 at 20:48 +0900, Masami Hiramatsu wrote:
> Reorder parameters of strglobmatch() so that the first
> parameter is the glob pattern as like as regexec(),
> because the subjective parameter of strglobmatch() must
> be the glob pattern, but not a sample string.
> So, the new interface is:
I'm a bit confused to the rational here. Can you explain in more detail
to why this patch is actually needed?
It just looks like you swapped two parameters. I still do not understand
what this was done for.
Thanks,
-- Steve
>
> bool strglobmatch(const char *glob, const char *str);
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Irina Tirdea <[email protected]>
> Cc: Pekka Enberg <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: David Ahern <[email protected]>
> ---
> tools/perf/util/parse-events.c | 14 +++++++-------
> tools/perf/util/probe-event.c | 2 +-
> tools/perf/util/strfilter.c | 2 +-
> tools/perf/util/string.c | 16 ++++++++--------
> tools/perf/util/util.h | 4 ++--
> 5 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 6c8bb0f..26fb64a 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -402,7 +402,7 @@ static int add_tracepoint_multi_event(struct list_head **list, int *idx,
> || !strcmp(evt_ent->d_name, "filter"))
> continue;
>
> - if (!strglobmatch(evt_ent->d_name, evt_name))
> + if (!strglobmatch(evt_name, evt_ent->d_name))
> continue;
>
> ret = add_tracepoint(list, idx, sys_name, evt_ent->d_name);
> @@ -441,7 +441,7 @@ static int add_tracepoint_multi_sys(struct list_head **list, int *idx,
> || !strcmp(events_ent->d_name, "header_page"))
> continue;
>
> - if (!strglobmatch(events_ent->d_name, sys_name))
> + if (!strglobmatch(sys_name, events_ent->d_name))
> continue;
>
> ret = add_tracepoint_event(list, idx, events_ent->d_name,
> @@ -955,7 +955,7 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
>
> for_each_subsystem(sys_dir, sys_dirent, sys_next) {
> if (subsys_glob != NULL &&
> - !strglobmatch(sys_dirent.d_name, subsys_glob))
> + !strglobmatch(subsys_glob, sys_dirent.d_name))
> continue;
>
> snprintf(dir_path, MAXPATHLEN, "%s/%s", tracing_events_path,
> @@ -966,7 +966,7 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
>
> for_each_event(sys_dirent, evt_dir, evt_dirent, evt_next) {
> if (event_glob != NULL &&
> - !strglobmatch(evt_dirent.d_name, event_glob))
> + !strglobmatch(event_glob, evt_dirent.d_name))
> continue;
>
> if (name_only) {
> @@ -1065,7 +1065,7 @@ int print_hwcache_events(const char *event_glob, bool name_only)
> for (i = 0; i < PERF_COUNT_HW_CACHE_RESULT_MAX; i++) {
> __perf_evsel__hw_cache_type_op_res_name(type, op, i,
> name, sizeof(name));
> - if (event_glob != NULL && !strglobmatch(name, event_glob))
> + if (event_glob != NULL && !strglobmatch(event_glob, name))
> continue;
>
> if (name_only)
> @@ -1091,8 +1091,8 @@ static void print_symbol_events(const char *event_glob, unsigned type,
> for (i = 0; i < max; i++, syms++) {
>
> if (event_glob != NULL &&
> - !(strglobmatch(syms->symbol, event_glob) ||
> - (syms->alias && strglobmatch(syms->alias, event_glob))))
> + !(strglobmatch(event_glob, syms->symbol) ||
> + (syms->alias && strglobmatch(event_glob, syms->alias))))
> continue;
>
> if (name_only) {
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index aa04bf9..4d3b498 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2106,7 +2106,7 @@ static int del_trace_probe_event(int fd, const char *buf,
>
> if (strpbrk(buf, "*?")) { /* Glob-exp */
> strlist__for_each_safe(ent, n, namelist)
> - if (strglobmatch(ent->s, buf)) {
> + if (strglobmatch(buf, ent->s)) {
> ret = __del_trace_probe_event(fd, ent);
> if (ret < 0)
> break;
> diff --git a/tools/perf/util/strfilter.c b/tools/perf/util/strfilter.c
> index 834c8eb..e50bfc5 100644
> --- a/tools/perf/util/strfilter.c
> +++ b/tools/perf/util/strfilter.c
> @@ -186,7 +186,7 @@ static bool strfilter_node__compare(struct strfilter_node *self,
> case '!': /* NOT */
> return !strfilter_node__compare(self->r, str);
> default:
> - return strglobmatch(str, self->p);
> + return strglobmatch(self->p, str);
> }
> }
>
> diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
> index 29c7b2c..f4204a5 100644
> --- a/tools/perf/util/string.c
> +++ b/tools/perf/util/string.c
> @@ -223,7 +223,7 @@ error:
> }
>
> /* Glob/lazy pattern matching */
> -static bool __match_glob(const char *str, const char *pat, bool ignore_space)
> +static bool __match_glob(const char *pat, const char *str, bool ignore_space)
> {
> while (*str && *pat && *pat != '*') {
> if (ignore_space) {
> @@ -259,7 +259,7 @@ static bool __match_glob(const char *str, const char *pat, bool ignore_space)
> if (!*pat) /* Tail wild card matches all */
> return true;
> while (*str)
> - if (__match_glob(str++, pat, ignore_space))
> + if (__match_glob(pat, str++, ignore_space))
> return true;
> }
> return !*str && !*pat;
> @@ -267,8 +267,8 @@ static bool __match_glob(const char *str, const char *pat, bool ignore_space)
>
> /**
> * strglobmatch - glob expression pattern matching
> - * @str: the target string to match
> * @pat: the pattern string to match
> + * @str: the target string to match
> *
> * This returns true if the @str matches @pat. @pat can includes wildcards
> * ('*','?') and character classes ([CHARS], complementation and ranges are
> @@ -277,22 +277,22 @@ static bool __match_glob(const char *str, const char *pat, bool ignore_space)
> *
> * Note: if @pat syntax is broken, this always returns false.
> */
> -bool strglobmatch(const char *str, const char *pat)
> +bool strglobmatch(const char *pat, const char *str)
> {
> - return __match_glob(str, pat, false);
> + return __match_glob(pat, str, false);
> }
>
> /**
> * strlazymatch - matching pattern strings lazily with glob pattern
> - * @str: the target string to match
> * @pat: the pattern string to match
> + * @str: the target string to match
> *
> * This is similar to strglobmatch, except this ignores spaces in
> * the target string.
> */
> -bool strlazymatch(const char *str, const char *pat)
> +bool strlazymatch(const char *pat, const char *str)
> {
> - return __match_glob(str, pat, true);
> + return __match_glob(pat, str, true);
> }
>
> /**
> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
> index a45710b..f493590 100644
> --- a/tools/perf/util/util.h
> +++ b/tools/perf/util/util.h
> @@ -239,8 +239,8 @@ int copyfile(const char *from, const char *to);
> s64 perf_atoll(const char *str);
> char **argv_split(const char *str, int *argcp);
> void argv_free(char **argv);
> -bool strglobmatch(const char *str, const char *pat);
> -bool strlazymatch(const char *str, const char *pat);
> +bool strglobmatch(const char *pat, const char *str);
> +bool strlazymatch(const char *pat, const char *str);
> int strtailcmp(const char *s1, const char *s2);
> char *strxfrchar(char *s, char from, char to);
> unsigned long convert_unit(unsigned long value, char *unit);
On Thu, 2013-05-16 at 20:48 +0900, Masami Hiramatsu wrote:
> Since try_module_get returns false( = 0) when it fails to
> pindown a module, event_enable_func() returns 0 which means
> "succeed". This can cause a kernel panic when the entry
> is removed, because the event is already released.
>
> This fixes the bug by returning -EBUSY, because the reason
> why it fails is under removing module at that time.
>
Thanks, this looks to be something that needs to go in right away.
-- Steve
> Signed-off-by: Masami Hiramatsu <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> ---
> kernel/trace/trace_events.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 7a0cf68..27963e2 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -2072,8 +2072,10 @@ event_enable_func(struct ftrace_hash *hash,
> out_reg:
> /* Don't let event modules unload while probe registered */
> ret = try_module_get(file->event_call->mod);
> - if (!ret)
> + if (!ret) {
> + ret = -EBUSY;
> goto out_free;
> + }
>
> ret = __ftrace_event_enable_disable(file, 1, 1);
> if (ret < 0)
(2013/05/16 23:55), Steven Rostedt wrote:
> On Thu, 2013-05-16 at 20:48 +0900, Masami Hiramatsu wrote:
>> Reorder parameters of strglobmatch() so that the first
>> parameter is the glob pattern as like as regexec(),
>> because the subjective parameter of strglobmatch() must
>> be the glob pattern, but not a sample string.
>> So, the new interface is:
>
> I'm a bit confused to the rational here. Can you explain in more detail
> to why this patch is actually needed?
Yes, actually, this patch is not needed from the viewpoint of execution,
but less misuse for future use of the strglobmatch, I think.
For example, glob(3) has the pattern parameter as the first one,
int glob(const char *pattern, int flags,
int (*errfunc) (const char *epath, int eerrno),
glob_t *pglob);
regexec(3) also has the compiled regexp at the first parameter,
int regexec(const regex_t *preg, const char *string, size_t nmatch,
regmatch_t pmatch[], int eflags);
Thus, I think a new user of strglobmatch() may guess that the first
parameter should be the glob pattern.
So, this patch is not technically needed, but from the viewpoint of coding
naturally, it should be changed, IMHO.
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
Em Fri, May 17, 2013 at 11:21:11AM +0900, Masami Hiramatsu escreveu:
> (2013/05/16 23:55), Steven Rostedt wrote:
> > I'm a bit confused to the rational here. Can you explain in more detail
> > to why this patch is actually needed?
> Yes, actually, this patch is not needed from the viewpoint of execution,
> but less misuse for future use of the strglobmatch, I think.
> For example, glob(3) has the pattern parameter as the first one,
>
> int glob(const char *pattern, int flags,
> int (*errfunc) (const char *epath, int eerrno),
> glob_t *pglob);
> regexec(3) also has the compiled regexp at the first parameter,
> int regexec(const regex_t *preg, const char *string, size_t nmatch,
> regmatch_t pmatch[], int eflags);
> Thus, I think a new user of strglobmatch() may guess that the first
> parameter should be the glob pattern.
> So, this patch is not technically needed, but from the viewpoint of coding
> naturally, it should be changed, IMHO.
So I suggest you state this in the changeset comment :-)
- Arnaldo
(2013/05/21 18:19), Arnaldo Carvalho de Melo wrote:
> Em Fri, May 17, 2013 at 11:21:11AM +0900, Masami Hiramatsu escreveu:
>> (2013/05/16 23:55), Steven Rostedt wrote:
>>> I'm a bit confused to the rational here. Can you explain in more detail
>>> to why this patch is actually needed?
>
>> Yes, actually, this patch is not needed from the viewpoint of execution,
>> but less misuse for future use of the strglobmatch, I think.
>
>> For example, glob(3) has the pattern parameter as the first one,
>>
>> int glob(const char *pattern, int flags,
>> int (*errfunc) (const char *epath, int eerrno),
>> glob_t *pglob);
>
>> regexec(3) also has the compiled regexp at the first parameter,
>
>> int regexec(const regex_t *preg, const char *string, size_t nmatch,
>> regmatch_t pmatch[], int eflags);
>
>> Thus, I think a new user of strglobmatch() may guess that the first
>> parameter should be the glob pattern.
>
>> So, this patch is not technically needed, but from the viewpoint of coding
>> naturally, it should be changed, IMHO.
>
> So I suggest you state this in the changeset comment :-)
Agreed, I must update it ...
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]