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.
Changes in v2:
- Update the comment of patch [2/5] for explaining more
detail of the backgroud.
---
Masami Hiramatsu (5):
[BUGFIX] tracing: Returns -EBUSY when event_enable_func fails to get module
perf: Swap the 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
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");
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;
}
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);
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)
Swap the 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.
The new interface is:
bool strglobmatch(const char *glob, const char *str);
Actually this patch is not technically needed, but from
the viewpoint of coding, it would better be changed.
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);
And 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.
Changes in v2:
- Update the comment of patch [2/5] for explaining more
detail of the backgroud.
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);
A blast from the past! (I'm cleaning out my inbox).
On Wed, 22 May 2013 11:19:03 +0900
Masami Hiramatsu <[email protected]> wrote:
> 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.
>
> Changes in v2:
> - Update the comment of patch [2/5] for explaining more
> detail of the backgroud.
>
> ---
>
> Masami Hiramatsu (5):
> [BUGFIX] tracing: Returns -EBUSY when event_enable_func fails to get module
This is applied.
> perf: Swap the 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
These don't look to be applied. Looks like this patch series was lost.
Masami, are these still relevant?
-- Steve
>
>
> 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(-)
>
(2014/06/19 10:27), Steven Rostedt wrote:
>
> A blast from the past! (I'm cleaning out my inbox).
>
> On Wed, 22 May 2013 11:19:03 +0900
> Masami Hiramatsu <[email protected]> wrote:
>
>> 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.
>>
>> Changes in v2:
>> - Update the comment of patch [2/5] for explaining more
>> detail of the backgroud.
>>
>> ---
>>
>> Masami Hiramatsu (5):
>> [BUGFIX] tracing: Returns -EBUSY when event_enable_func fails to get module
>
> This is applied.
>
>
>> perf: Swap the 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
>
> These don't look to be applied. Looks like this patch series was lost.
>
> Masami, are these still relevant?
These are separated enhancement. Feel free to apply the 1st bugfix patch.
I'll update the series :)
Thank you,
>
> -- Steve
>
>
>>
>>
>> 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(-)
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
On Thu, 19 Jun 2014 15:36:53 +0900
Masami Hiramatsu <[email protected]> wrote:
> These are separated enhancement. Feel free to apply the 1st bugfix patch.
That first bugfix patch is already in mainline.
> I'll update the series :)
Thanks!
-- Steve