Subject: [PATCH perf/core v3 0/8] perf-probe: Add filtering features

Hi,

Here is a series of patches which improves perf-probe to add
filtering features for --list,--del and --funcs. This also
includes some bugfixes and cleanups.

I've fixed some patches according to Namhyung's review(Thanks!),
and splitted the strfilter related patches.

Note that first 2 bugfixes are removed from this series, those
are already picked by Arnaldo.

E.g. --list/--funcs accepts filter rules.
----
# ./perf probe -l vfs\*
probe:vfs_read (on vfs_read@ksrc/linux-3/fs/read_write.c)
# ./perf probe -l \*libc:\*
probe_libc:malloc (on __libc_malloc@malloc/malloc.c in /usr/lib64/libc-2.17.so)
----
----
# ./perf probe -F *kmalloc
__kmalloc
devm_kmalloc
mempool_kmalloc
sg_kmalloc
sock_kmalloc
----

Also, --del accepts filter rules too which is more flexible than
simple wildcard.
----
# ./perf probe -d 'vfs*|malloc'
Removed event: probe:vfs_read
Removed event: probe_libc:malloc
----

Changes in v3:
- [1/8] Split strfilter__or() and add strfilter__and() from v2 [3/8].
Also rename strfilter__add() to strfilter__append().
- [2/8] Split strfilter__string from v2 [5/8].

Changes in v2:
- [2/8] Fix to check errors except -ENOENT on deleting events.
- [5/8] Ditto.
- [5/8] Fix to check memory allocation error in strfilter__string.
- [5/8] Fix to check errors of strfilter__string for verifying
given filter.

Thank you,


---

Masami Hiramatsu (8):
perf: Improve strfilter to append additional rules
perf: Add strfilter__string to recover rules string
perf probe: Accept multiple filter options
perf probe: Accept filter argument for --list
perf probe: Allow to use filter on --del command
perf probe: Accept filter argument for --funcs
perf probe: Remove redundant cleanup of params.filter
perf probe: Cleanup and consolidate command parsers


tools/perf/Documentation/perf-probe.txt | 9 +-
tools/perf/builtin-probe.c | 160 +++++++++++++++----------------
tools/perf/util/probe-event.c | 126 +++++++++++-------------
tools/perf/util/probe-event.h | 4 -
tools/perf/util/strfilter.c | 107 +++++++++++++++++++++
tools/perf/util/strfilter.h | 35 +++++++
6 files changed, 281 insertions(+), 160 deletions(-)


--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]


Subject: [PATCH perf/core v3 1/8] perf: Improve strfilter to append additional rules

Add strfilter__or/and to append additional rules to
existing strfilter.

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/util/strfilter.c | 40 ++++++++++++++++++++++++++++++++++++++++
tools/perf/util/strfilter.h | 26 ++++++++++++++++++++++++++
2 files changed, 66 insertions(+)

diff --git a/tools/perf/util/strfilter.c b/tools/perf/util/strfilter.c
index 79a757a..f3429cd 100644
--- a/tools/perf/util/strfilter.c
+++ b/tools/perf/util/strfilter.c
@@ -170,6 +170,46 @@ struct strfilter *strfilter__new(const char *rules, const char **err)
return filter;
}

+static int strfilter__append(struct strfilter *filter, bool _or,
+ const char *rules, const char **err)
+{
+ struct strfilter_node *right, *root;
+ const char *ep = NULL;
+
+ if (!filter || !rules)
+ return -EINVAL;
+
+ right = strfilter_node__new(rules, &ep);
+ if (!right || *ep != '\0') {
+ if (err)
+ *err = ep;
+ goto error;
+ }
+ root = strfilter_node__alloc(_or ? OP_or : OP_and, filter->root, right);
+ if (!root) {
+ ep = NULL;
+ goto error;
+ }
+
+ filter->root = root;
+ return 0;
+
+error:
+ strfilter_node__delete(right);
+ return ep ? -EINVAL : -ENOMEM;
+}
+
+int strfilter__or(struct strfilter *filter, const char *rules, const char **err)
+{
+ return strfilter__append(filter, true, rules, err);
+}
+
+int strfilter__and(struct strfilter *filter, const char *rules,
+ const char **err)
+{
+ return strfilter__append(filter, false, rules, err);
+}
+
static bool strfilter_node__compare(struct strfilter_node *node,
const char *str)
{
diff --git a/tools/perf/util/strfilter.h b/tools/perf/util/strfilter.h
index fe611f3..d007cdc 100644
--- a/tools/perf/util/strfilter.h
+++ b/tools/perf/util/strfilter.h
@@ -29,6 +29,32 @@ struct strfilter {
struct strfilter *strfilter__new(const char *rules, const char **err);

/**
+ * strfilter__or - Append an additional rule by logical-or
+ * @filter: Original string filter
+ * @rules: Filter rule to be appended at left of the root of
+ * @filter by using logical-or.
+ * @err: Pointer which points an error detected on @rules
+ *
+ * Parse @rules and join it to the @filter by using logical-or.
+ * Return 0 if success, or return the error code.
+ */
+int strfilter__or(struct strfilter *filter,
+ const char *rules, const char **err);
+
+/**
+ * strfilter__add - Append an additional rule by logical-and
+ * @filter: Original string filter
+ * @rules: Filter rule to be appended at left of the root of
+ * @filter by using logical-and.
+ * @err: Pointer which points an error detected on @rules
+ *
+ * Parse @rules and join it to the @filter by using logical-and.
+ * Return 0 if success, or return the error code.
+ */
+int strfilter__and(struct strfilter *filter,
+ const char *rules, const char **err);
+
+/**
* strfilter__compare - compare given string and a string filter
* @filter: String filter
* @str: target string

Subject: [PATCH perf/core v3 2/8] perf: Add strfilter__string to recover rules string

Add strfilter__string to recover rules string from strfilter.
This will be good for debugging.

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/util/strfilter.c | 67 +++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/strfilter.h | 9 ++++++
2 files changed, 76 insertions(+)

diff --git a/tools/perf/util/strfilter.c b/tools/perf/util/strfilter.c
index f3429cd..bcae659 100644
--- a/tools/perf/util/strfilter.c
+++ b/tools/perf/util/strfilter.c
@@ -237,3 +237,70 @@ bool strfilter__compare(struct strfilter *filter, const char *str)
return false;
return strfilter_node__compare(filter->root, str);
}
+
+static int strfilter_node__sprint(struct strfilter_node *node, char *buf);
+
+/* sprint node in parenthesis if needed */
+static int strfilter_node__sprint_pt(struct strfilter_node *node, char *buf)
+{
+ int len;
+ int pt = node->r ? 2 : 0; /* don't need to check node->l */
+
+ if (buf && pt)
+ *buf++ = '(';
+ len = strfilter_node__sprint(node, buf);
+ if (len < 0)
+ return len;
+ if (buf && pt)
+ *(buf + len) = ')';
+ return len + pt;
+}
+
+static int strfilter_node__sprint(struct strfilter_node *node, char *buf)
+{
+ int len = 0, rlen;
+
+ if (!node || !node->p)
+ return -EINVAL;
+
+ switch (*node->p) {
+ case '|':
+ case '&':
+ len = strfilter_node__sprint_pt(node->l, buf);
+ if (len < 0)
+ return len;
+ case '!':
+ if (buf) {
+ *(buf + len++) = *node->p;
+ buf += len;
+ } else
+ len++;
+ rlen = strfilter_node__sprint_pt(node->r, buf);
+ if (rlen < 0)
+ return rlen;
+ len += rlen;
+ break;
+ default:
+ len = strlen(node->p);
+ if (buf)
+ strcpy(buf, node->p);
+ }
+
+ return len;
+}
+
+char *strfilter__string(struct strfilter *filter)
+{
+ int len;
+ char *ret = NULL;
+
+ len = strfilter_node__sprint(filter->root, NULL);
+ if (len < 0)
+ return NULL;
+
+ ret = malloc(len + 1);
+ if (ret)
+ strfilter_node__sprint(filter->root, ret);
+
+ return ret;
+}
diff --git a/tools/perf/util/strfilter.h b/tools/perf/util/strfilter.h
index d007cdc..cff5eda 100644
--- a/tools/perf/util/strfilter.h
+++ b/tools/perf/util/strfilter.h
@@ -71,4 +71,13 @@ bool strfilter__compare(struct strfilter *filter, const char *str);
*/
void strfilter__delete(struct strfilter *filter);

+/**
+ * strfilter__string - Reconstruct a rule string from filter
+ * @filter: String filter to reconstruct
+ *
+ * Reconstruct a rule string from @filter. This will be good for
+ * debug messages. Note that returning string must be freed afterward.
+ */
+char *strfilter__string(struct strfilter *filter);
+
#endif

Subject: [PATCH perf/core v3 3/8] perf probe: Accept multiple filter options

Accept multiple filter options. Each filters are combined
by logical-or. E.g. --filter abc* --filter *def is same
as --filter abc*|*def

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/builtin-probe.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 92dcce0..be17075 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -262,21 +262,25 @@ static int opt_set_filter(const struct option *opt __maybe_unused,
const char *str, int unset __maybe_unused)
{
const char *err;
+ int ret = 0;

if (str) {
pr_debug2("Set filter: %s\n", str);
- if (params.filter)
- strfilter__delete(params.filter);
- params.filter = strfilter__new(str, &err);
if (!params.filter) {
+ params.filter = strfilter__new(str, &err);
+ if (!params.filter)
+ ret = err ? -EINVAL : -ENOMEM;
+ } else
+ ret = strfilter__or(params.filter, str, &err);
+
+ if (ret == -EINVAL) {
pr_err("Filter parse error at %td.\n", err - str + 1);
pr_err("Source: \"%s\"\n", str);
pr_err(" %*c\n", (int)(err - str + 1), '^');
- return -EINVAL;
}
}

- return 0;
+ return ret;
}

static int init_params(void)

Subject: [PATCH perf/core v3 4/8] perf probe: Accept filter argument for --list

Currently, perf-probe --list option ignores given event filter.
----
# ./perf probe -l vfs\*
probe:vfs_read (on vfs_read@ksrc/linux-3/fs/read_write.c)
probe_libc:malloc (on __libc_malloc@malloc/malloc.c in /usr/lib64/libc-2.17.so)
----

This changes --list option to accept the event filter argument as below.
----
# ./perf probe -l vfs\*
probe:vfs_read (on vfs_read@ksrc/linux-3/fs/read_write.c)
# ./perf probe -l \*libc:\*
probe_libc:malloc (on __libc_malloc@malloc/malloc.c in /usr/lib64/libc-2.17.so)
----

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/Documentation/perf-probe.txt | 6 +--
tools/perf/builtin-probe.c | 70 ++++++++++++++++++++-----------
tools/perf/util/probe-event.c | 27 ++++++++++--
tools/perf/util/probe-event.h | 2 -
4 files changed, 73 insertions(+), 32 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index a4a3cc7..d0feb8e 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -14,7 +14,7 @@ or
or
'perf probe' [options] --del='[GROUP:]EVENT' [...]
or
-'perf probe' --list
+'perf probe' --list[=[GROUP:]EVENT]
or
'perf probe' [options] --line='LINE'
or
@@ -66,8 +66,8 @@ OPTIONS
classes(e.g. [a-z], [!A-Z]).

-l::
---list::
- List up current probe events.
+--list[=[GROUP:]EVENT]::
+ List up current probe events. This can also accept filtering patterns of event names.

-L::
--line=::
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index be17075..feca316 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -44,6 +44,7 @@

#define DEFAULT_VAR_FILTER "!__k???tab_* & !__crc_*"
#define DEFAULT_FUNC_FILTER "!_*"
+#define DEFAULT_LIST_FILTER "*:*"

/* Session management structure */
static struct {
@@ -93,6 +94,28 @@ static int parse_probe_event(const char *str)
return ret;
}

+static int params_add_filter(const char *str)
+{
+ const char *err = NULL;
+ int ret = 0;
+
+ pr_debug2("Add filter: %s\n", str);
+ if (!params.filter) {
+ params.filter = strfilter__new(str, &err);
+ if (!params.filter)
+ ret = err ? -EINVAL : -ENOMEM;
+ } else
+ ret = strfilter__or(params.filter, str, &err);
+
+ if (ret == -EINVAL) {
+ pr_err("Filter parse error at %td.\n", err - str + 1);
+ pr_err("Source: \"%s\"\n", str);
+ pr_err(" %*c\n", (int)(err - str + 1), '^');
+ }
+
+ return ret;
+}
+
static int set_target(const char *ptr)
{
int found = 0;
@@ -180,6 +203,18 @@ static int opt_del_probe_event(const struct option *opt __maybe_unused,
return 0;
}

+static int opt_list_probe_event(const struct option *opt __maybe_unused,
+ const char *str, int unset)
+{
+ if (!unset)
+ params.list_events = true;
+
+ if (str)
+ return params_add_filter(str);
+
+ return 0;
+}
+
static int opt_set_target(const struct option *opt, const char *str,
int unset __maybe_unused)
{
@@ -261,26 +296,10 @@ static int opt_show_vars(const struct option *opt __maybe_unused,
static int opt_set_filter(const struct option *opt __maybe_unused,
const char *str, int unset __maybe_unused)
{
- const char *err;
- int ret = 0;
+ if (str)
+ return params_add_filter(str);

- if (str) {
- pr_debug2("Set filter: %s\n", str);
- if (!params.filter) {
- params.filter = strfilter__new(str, &err);
- if (!params.filter)
- ret = err ? -EINVAL : -ENOMEM;
- } else
- ret = strfilter__or(params.filter, str, &err);
-
- if (ret == -EINVAL) {
- pr_err("Filter parse error at %td.\n", err - str + 1);
- pr_err("Source: \"%s\"\n", str);
- pr_err(" %*c\n", (int)(err - str + 1), '^');
- }
- }
-
- return ret;
+ return 0;
}

static int init_params(void)
@@ -320,21 +339,22 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
"perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]",
"perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]",
"perf probe [<options>] --del '[GROUP:]EVENT' ...",
- "perf probe --list",
+ "perf probe --list [GROUP:]EVENT ...",
#ifdef HAVE_DWARF_SUPPORT
"perf probe [<options>] --line 'LINEDESC'",
"perf probe [<options>] --vars 'PROBEPOINT'",
#endif
"perf probe [<options>] --funcs",
NULL
-};
+ };
struct option options[] = {
OPT_INCR('v', "verbose", &verbose,
"be more verbose (show parsed arguments, etc)"),
OPT_BOOLEAN('q', "quiet", &params.quiet,
"be quiet (do not show any mesages)"),
- OPT_BOOLEAN('l', "list", &params.list_events,
- "list up current probe events"),
+ OPT_CALLBACK_DEFAULT('l', "list", NULL, "[GROUP:]EVENT",
+ "list up probe events", opt_list_probe_event,
+ DEFAULT_LIST_FILTER),
OPT_CALLBACK('d', "del", NULL, "[GROUP:]EVENT", "delete a probe event.",
opt_del_probe_event),
OPT_CALLBACK('a', "add", NULL,
@@ -448,7 +468,9 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
pr_warning(" Error: Don't use --list with --exec.\n");
usage_with_options(probe_usage, options);
}
- ret = show_perf_probe_events();
+ ret = show_perf_probe_events(params.filter);
+ strfilter__delete(params.filter);
+ params.filter = NULL;
if (ret < 0)
pr_err_with_code(" Error: Failed to show event list.", ret);
return ret;
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 291bf23..acd9ec8 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2129,7 +2129,23 @@ static int show_perf_probe_event(struct perf_probe_event *pev,
return ret;
}

-static int __show_perf_probe_events(int fd, bool is_kprobe)
+static bool filter_probe_trace_event(struct probe_trace_event *tev,
+ struct strfilter *filter)
+{
+ char tmp[128];
+
+ /* At first, check the event name itself */
+ if (strfilter__compare(filter, tev->event))
+ return true;
+
+ /* Next, check the combination of name and group */
+ if (e_snprintf(tmp, 128, "%s:%s", tev->group, tev->event) < 0)
+ return false;
+ return strfilter__compare(filter, tmp);
+}
+
+static int __show_perf_probe_events(int fd, bool is_kprobe,
+ struct strfilter *filter)
{
int ret = 0;
struct probe_trace_event tev;
@@ -2147,12 +2163,15 @@ static int __show_perf_probe_events(int fd, bool is_kprobe)
strlist__for_each(ent, rawlist) {
ret = parse_probe_trace_command(ent->s, &tev);
if (ret >= 0) {
+ if (!filter_probe_trace_event(&tev, filter))
+ goto next;
ret = convert_to_perf_probe_event(&tev, &pev,
is_kprobe);
if (ret >= 0)
ret = show_perf_probe_event(&pev,
tev.point.module);
}
+next:
clear_perf_probe_event(&pev);
clear_probe_trace_event(&tev);
if (ret < 0)
@@ -2164,7 +2183,7 @@ static int __show_perf_probe_events(int fd, bool is_kprobe)
}

/* List up current perf-probe events */
-int show_perf_probe_events(void)
+int show_perf_probe_events(struct strfilter *filter)
{
int kp_fd, up_fd, ret;

@@ -2176,7 +2195,7 @@ int show_perf_probe_events(void)

kp_fd = open_kprobe_events(false);
if (kp_fd >= 0) {
- ret = __show_perf_probe_events(kp_fd, true);
+ ret = __show_perf_probe_events(kp_fd, true, filter);
close(kp_fd);
if (ret < 0)
goto out;
@@ -2190,7 +2209,7 @@ int show_perf_probe_events(void)
}

if (up_fd >= 0) {
- ret = __show_perf_probe_events(up_fd, false);
+ ret = __show_perf_probe_events(up_fd, false, filter);
close(up_fd);
}
out:
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index d6b7834..819e825 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -127,7 +127,7 @@ extern const char *kernel_get_module_path(const char *module);
extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
int max_probe_points, bool force_add);
extern int del_perf_probe_events(struct strlist *dellist);
-extern int show_perf_probe_events(void);
+extern int show_perf_probe_events(struct strfilter *filter);
extern int show_line_range(struct line_range *lr, const char *module,
bool user);
extern int show_available_vars(struct perf_probe_event *pevs, int npevs,

Subject: [PATCH perf/core v3 5/8] perf probe: Allow to use filter on --del command

This makes perf-probe --del option to accept filter rules
not only simple glob pattern. This simplifies the code and
improve the flexibility.

E.g. if we remove 2 different pattern events, we need 2
-d options.
----
# ./perf probe -d vfs\* -d malloc
Removed event: probe_libc:malloc
Removed event: probe:vfs_read
----

This allows you to joint the 2 patterns with '|'.

----
# ./perf probe -d 'vfs*|malloc'
Removed event: probe:vfs_read
Removed event: probe_libc:malloc
----

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/builtin-probe.c | 16 ++-----
tools/perf/util/probe-event.c | 99 +++++++++++++----------------------------
tools/perf/util/probe-event.h | 2 -
3 files changed, 39 insertions(+), 78 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index feca316..1f41b4e 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -55,12 +55,12 @@ static struct {
bool show_ext_vars;
bool show_funcs;
bool mod_events;
+ bool del_events;
bool uprobes;
bool quiet;
bool target_used;
int nevents;
struct perf_probe_event events[MAX_PROBES];
- struct strlist *dellist;
struct line_range line_range;
char *target;
int max_probe_points;
@@ -195,10 +195,8 @@ static int opt_del_probe_event(const struct option *opt __maybe_unused,
const char *str, int unset __maybe_unused)
{
if (str) {
- params.mod_events = true;
- if (!params.dellist)
- params.dellist = strlist__new(true, NULL);
- strlist__add(params.dellist, str);
+ params.del_events = true;
+ return params_add_filter(str);
}
return 0;
}
@@ -313,8 +311,6 @@ static void cleanup_params(void)

for (i = 0; i < params.nevents; i++)
clear_perf_probe_event(params.events + i);
- if (params.dellist)
- strlist__delete(params.dellist);
line_range__clear(&params.line_range);
free(params.target);
if (params.filter)
@@ -454,7 +450,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (params.max_probe_points == 0)
params.max_probe_points = MAX_PROBES;

- if ((!params.nevents && !params.dellist && !params.list_events &&
+ if ((!params.nevents && !params.del_events && !params.list_events &&
!params.show_lines && !params.show_funcs))
usage_with_options(probe_usage, options);

@@ -514,8 +510,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
}
#endif

- if (params.dellist) {
- ret = del_perf_probe_events(params.dellist);
+ if (params.del_events) {
+ ret = del_perf_probe_events(params.filter);
if (ret < 0) {
pr_err_with_code(" Error: Failed to delete events.", ret);
return ret;
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index acd9ec8..4fdb38c 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2701,40 +2701,39 @@ error:
return ret;
}

-static int del_trace_probe_event(int fd, const char *buf,
- struct strlist *namelist)
+static int del_trace_probe_events(int fd, struct strfilter *filter,
+ struct strlist *namelist)
{
- struct str_node *ent, *n;
+ struct str_node *ent;
+ const char *p;
int ret = -ENOENT;

- if (strpbrk(buf, "*?")) { /* Glob-exp */
- strlist__for_each_safe(ent, n, namelist)
- if (strglobmatch(ent->s, buf)) {
- ret = __del_trace_probe_event(fd, ent);
- if (ret < 0)
- break;
- strlist__remove(namelist, ent);
- }
- } else {
- ent = strlist__find(namelist, buf);
- if (ent) {
+ if (!namelist)
+ return -ENOENT;
+
+ strlist__for_each(ent, namelist) {
+ p = strchr(ent->s, ':');
+ if ((p && strfilter__compare(filter, p + 1)) ||
+ strfilter__compare(filter, ent->s)) {
ret = __del_trace_probe_event(fd, ent);
- if (ret >= 0)
- strlist__remove(namelist, ent);
+ if (ret < 0)
+ break;
}
}

return ret;
}

-int del_perf_probe_events(struct strlist *dellist)
+int del_perf_probe_events(struct strfilter *filter)
{
- int ret = -1, ret2, ufd = -1, kfd = -1;
- char buf[128];
- const char *group, *event;
- char *p, *str;
- struct str_node *ent;
+ int ret, ret2, ufd = -1, kfd = -1;
struct strlist *namelist = NULL, *unamelist = NULL;
+ char *str = strfilter__string(filter);
+
+ if (!str)
+ return -EINVAL;
+
+ pr_debug("Delete filter: \'%s\'\n", str);

/* Get current event names */
kfd = open_kprobe_events(true);
@@ -2747,56 +2746,21 @@ int del_perf_probe_events(struct strlist *dellist)

if (kfd < 0 && ufd < 0) {
print_both_open_warning(kfd, ufd);
+ ret = kfd;
goto error;
}

- if (namelist == NULL && unamelist == NULL) {
- ret = -ENOENT;
+ ret = del_trace_probe_events(kfd, filter, namelist);
+ if (ret < 0 && ret != -ENOENT)
goto error;
- }

- strlist__for_each(ent, dellist) {
- str = strdup(ent->s);
- if (str == NULL) {
- ret = -ENOMEM;
- goto error;
- }
- pr_debug("Parsing: %s\n", str);
- p = strchr(str, ':');
- if (p) {
- group = str;
- *p = '\0';
- event = p + 1;
- } else {
- group = "*";
- event = str;
- }
-
- ret = e_snprintf(buf, 128, "%s:%s", group, event);
- if (ret < 0) {
- pr_err("Failed to copy event.");
- free(str);
- goto error;
- }
-
- pr_debug("Group: %s, Event: %s\n", group, event);
- free(str);
-
- ret = ret2 = -ENOENT;
- if (namelist)
- ret = del_trace_probe_event(kfd, buf, namelist);
-
- if ((ret >= 0 || ret == -ENOENT) && unamelist)
- ret2 = del_trace_probe_event(ufd, buf, unamelist);
-
- /* Since we can remove probes which already removed, don't check it */
- if (ret == -ENOENT && ret2 == -ENOENT)
- pr_debug("Event \"%s\" does not exist.\n", buf);
- else if (ret < 0 || ret2 < 0) {
- if (ret >= 0)
- ret = ret2;
- break;
- }
+ ret2 = del_trace_probe_events(ufd, filter, unamelist);
+ if (ret2 < 0 && ret2 != -ENOENT)
+ ret = ret2;
+ else if (ret == -ENOENT && ret2 == -ENOENT) {
+ pr_debug("\"%s\" does not hit any event.\n", str);
+ /* Note that this is silently ignored */
+ ret = 0;
}

error:
@@ -2809,6 +2773,7 @@ error:
strlist__delete(unamelist);
close(ufd);
}
+ free(str);

return ret;
}
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 819e825..027356f 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -126,7 +126,7 @@ extern const char *kernel_get_module_path(const char *module);

extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
int max_probe_points, bool force_add);
-extern int del_perf_probe_events(struct strlist *dellist);
+extern int del_perf_probe_events(struct strfilter *filter);
extern int show_perf_probe_events(struct strfilter *filter);
extern int show_line_range(struct line_range *lr, const char *module,
bool user);

Subject: [PATCH perf/core v3 6/8] perf probe: Accept filter argument for --funcs

This allows user to pass the filter pattern directly to
--funcs option as below.
----
# ./perf probe -F *kmalloc
__kmalloc
devm_kmalloc
mempool_kmalloc
sg_kmalloc
sock_kmalloc
----

We previously need --filter option for that.

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/Documentation/perf-probe.txt | 3 ++-
tools/perf/builtin-probe.c | 19 ++++++++++++++-----
2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index d0feb8e..a272f2e 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -84,9 +84,10 @@ OPTIONS
variables.

-F::
---funcs::
+--funcs[=FILTER]::
Show available functions in given module or kernel. With -x/--exec,
can also list functions in a user space executable / shared library.
+ This also can accept a FILTER rule argument.

--filter=FILTER::
(Only for --vars and --funcs) Set filter. FILTER is a combination of glob
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 1f41b4e..5a0e8f1 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -290,6 +290,17 @@ static int opt_show_vars(const struct option *opt __maybe_unused,
return ret;
}
#endif
+static int opt_show_funcs(const struct option *opt __maybe_unused,
+ const char *str, int unset)
+{
+ if (!unset)
+ params.show_funcs = true;
+
+ if (str)
+ return params_add_filter(str);
+
+ return 0;
+}

static int opt_set_filter(const struct option *opt __maybe_unused,
const char *str, int unset __maybe_unused)
@@ -399,8 +410,9 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
OPT__DRY_RUN(&probe_event_dry_run),
OPT_INTEGER('\0', "max-probes", &params.max_probe_points,
"Set how many probe points can be found for a probe."),
- OPT_BOOLEAN('F', "funcs", &params.show_funcs,
- "Show potential probe-able functions."),
+ OPT_CALLBACK_DEFAULT('F', "funcs", NULL, "[FILTER]",
+ "Show potential probe-able functions.",
+ opt_show_funcs, DEFAULT_FUNC_FILTER),
OPT_CALLBACK('\0', "filter", NULL,
"[!]FILTER", "Set a filter (with --vars/funcs only)\n"
"\t\t\t(default: \"" DEFAULT_VAR_FILTER "\" for --vars,\n"
@@ -472,9 +484,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
return ret;
}
if (params.show_funcs) {
- if (!params.filter)
- params.filter = strfilter__new(DEFAULT_FUNC_FILTER,
- NULL);
ret = show_available_funcs(params.target, params.filter,
params.uprobes);
strfilter__delete(params.filter);

Subject: [PATCH perf/core v3 7/8] perf probe: Remove redundant cleanup of params.filter

Since params.filter will be released in cleanup_params,
we don't need to clear it in each command.

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/builtin-probe.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 5a0e8f1..08c9481 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -477,8 +477,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
usage_with_options(probe_usage, options);
}
ret = show_perf_probe_events(params.filter);
- strfilter__delete(params.filter);
- params.filter = NULL;
if (ret < 0)
pr_err_with_code(" Error: Failed to show event list.", ret);
return ret;
@@ -486,8 +484,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (params.show_funcs) {
ret = show_available_funcs(params.target, params.filter,
params.uprobes);
- strfilter__delete(params.filter);
- params.filter = NULL;
if (ret < 0)
pr_err_with_code(" Error: Failed to show functions.", ret);
return ret;
@@ -511,8 +507,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
params.target,
params.filter,
params.show_ext_vars);
- strfilter__delete(params.filter);
- params.filter = NULL;
if (ret < 0)
pr_err_with_code(" Error: Failed to show vars.", ret);
return ret;

Subject: [PATCH perf/core v3 8/8] perf probe: Cleanup and consolidate command parsers

To simplify the perf-probe command code, consolidate some
similar functions and use command short-name for command
classification, instead of separated booleans.

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/builtin-probe.c | 105 ++++++++++++++++----------------------------
1 file changed, 37 insertions(+), 68 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 08c9481..17870b8 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -48,14 +48,10 @@

/* Session management structure */
static struct {
+ int command; /* Command short_name */
bool list_events;
bool force_add;
- bool show_lines;
- bool show_vars;
bool show_ext_vars;
- bool show_funcs;
- bool mod_events;
- bool del_events;
bool uprobes;
bool quiet;
bool target_used;
@@ -175,44 +171,11 @@ static int parse_probe_event_argv(int argc, const char **argv)

len += sprintf(&buf[len], "%s ", argv[i]);
}
- params.mod_events = true;
ret = parse_probe_event(buf);
free(buf);
return ret;
}

-static int opt_add_probe_event(const struct option *opt __maybe_unused,
- const char *str, int unset __maybe_unused)
-{
- if (str) {
- params.mod_events = true;
- return parse_probe_event(str);
- } else
- return 0;
-}
-
-static int opt_del_probe_event(const struct option *opt __maybe_unused,
- const char *str, int unset __maybe_unused)
-{
- if (str) {
- params.del_events = true;
- return params_add_filter(str);
- }
- return 0;
-}
-
-static int opt_list_probe_event(const struct option *opt __maybe_unused,
- const char *str, int unset)
-{
- if (!unset)
- params.list_events = true;
-
- if (str)
- return params_add_filter(str);
-
- return 0;
-}
-
static int opt_set_target(const struct option *opt, const char *str,
int unset __maybe_unused)
{
@@ -250,8 +213,10 @@ static int opt_set_target(const struct option *opt, const char *str,
return ret;
}

+/* Command option callbacks */
+
#ifdef HAVE_DWARF_SUPPORT
-static int opt_show_lines(const struct option *opt __maybe_unused,
+static int opt_show_lines(const struct option *opt,
const char *str, int unset __maybe_unused)
{
int ret = 0;
@@ -259,19 +224,19 @@ static int opt_show_lines(const struct option *opt __maybe_unused,
if (!str)
return 0;

- if (params.show_lines) {
+ if (params.command == 'L') {
pr_warning("Warning: more than one --line options are"
" detected. Only the first one is valid.\n");
return 0;
}

- params.show_lines = true;
+ params.command = opt->short_name;
ret = parse_line_range_desc(str, &params.line_range);

return ret;
}

-static int opt_show_vars(const struct option *opt __maybe_unused,
+static int opt_show_vars(const struct option *opt,
const char *str, int unset __maybe_unused)
{
struct perf_probe_event *pev = &params.events[params.nevents];
@@ -285,16 +250,27 @@ static int opt_show_vars(const struct option *opt __maybe_unused,
pr_err(" Error: '--vars' doesn't accept arguments.\n");
return -EINVAL;
}
- params.show_vars = true;
+ params.command = opt->short_name;

return ret;
}
#endif
-static int opt_show_funcs(const struct option *opt __maybe_unused,
- const char *str, int unset)
+static int opt_add_probe_event(const struct option *opt,
+ const char *str, int unset __maybe_unused)
+{
+ if (str) {
+ params.command = opt->short_name;
+ return parse_probe_event(str);
+ }
+
+ return 0;
+}
+
+static int opt_set_filter_with_command(const struct option *opt,
+ const char *str, int unset)
{
if (!unset)
- params.show_funcs = true;
+ params.command = opt->short_name;

if (str)
return params_add_filter(str);
@@ -360,10 +336,10 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
OPT_BOOLEAN('q', "quiet", &params.quiet,
"be quiet (do not show any mesages)"),
OPT_CALLBACK_DEFAULT('l', "list", NULL, "[GROUP:]EVENT",
- "list up probe events", opt_list_probe_event,
- DEFAULT_LIST_FILTER),
+ "list up probe events",
+ opt_set_filter_with_command, DEFAULT_LIST_FILTER),
OPT_CALLBACK('d', "del", NULL, "[GROUP:]EVENT", "delete a probe event.",
- opt_del_probe_event),
+ opt_set_filter_with_command),
OPT_CALLBACK('a', "add", NULL,
#ifdef HAVE_DWARF_SUPPORT
"[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT"
@@ -412,7 +388,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
"Set how many probe points can be found for a probe."),
OPT_CALLBACK_DEFAULT('F', "funcs", NULL, "[FILTER]",
"Show potential probe-able functions.",
- opt_show_funcs, DEFAULT_FUNC_FILTER),
+ opt_set_filter_with_command, DEFAULT_FUNC_FILTER),
OPT_CALLBACK('\0', "filter", NULL,
"[!]FILTER", "Set a filter (with --vars/funcs only)\n"
"\t\t\t(default: \"" DEFAULT_VAR_FILTER "\" for --vars,\n"
@@ -462,16 +438,13 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (params.max_probe_points == 0)
params.max_probe_points = MAX_PROBES;

- if ((!params.nevents && !params.del_events && !params.list_events &&
- !params.show_lines && !params.show_funcs))
- usage_with_options(probe_usage, options);
-
/*
* Only consider the user's kernel image path if given.
*/
symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);

- if (params.list_events) {
+ switch (params.command) {
+ case 'l':
if (params.uprobes) {
pr_warning(" Error: Don't use --list with --exec.\n");
usage_with_options(probe_usage, options);
@@ -480,24 +453,20 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (ret < 0)
pr_err_with_code(" Error: Failed to show event list.", ret);
return ret;
- }
- if (params.show_funcs) {
+ case 'F':
ret = show_available_funcs(params.target, params.filter,
params.uprobes);
if (ret < 0)
pr_err_with_code(" Error: Failed to show functions.", ret);
return ret;
- }
-
#ifdef HAVE_DWARF_SUPPORT
- if (params.show_lines) {
+ case 'L':
ret = show_line_range(&params.line_range, params.target,
params.uprobes);
if (ret < 0)
pr_err_with_code(" Error: Failed to show lines.", ret);
return ret;
- }
- if (params.show_vars) {
+ case 'V':
if (!params.filter)
params.filter = strfilter__new(DEFAULT_VAR_FILTER,
NULL);
@@ -510,18 +479,15 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (ret < 0)
pr_err_with_code(" Error: Failed to show vars.", ret);
return ret;
- }
#endif
-
- if (params.del_events) {
+ case 'd':
ret = del_perf_probe_events(params.filter);
if (ret < 0) {
pr_err_with_code(" Error: Failed to delete events.", ret);
return ret;
}
- }
-
- if (params.nevents) {
+ break;
+ case 'a':
/* Ensure the last given target is used */
if (params.target && !params.target_used) {
pr_warning(" Error: -x/-m must follow the probe definitions.\n");
@@ -535,6 +501,9 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
pr_err_with_code(" Error: Failed to add events.", ret);
return ret;
}
+ break;
+ default:
+ usage_with_options(probe_usage, options);
}
return 0;
}

Subject: Re: (ltc-kernel 10702) [PATCH perf/core v3 0/8] perf-probe: Add filtering features

Ping?

On 2015/04/24 18:47, Masami Hiramatsu wrote:
> Hi,
>
> Here is a series of patches which improves perf-probe to add
> filtering features for --list,--del and --funcs. This also
> includes some bugfixes and cleanups.
>
> I've fixed some patches according to Namhyung's review(Thanks!),
> and splitted the strfilter related patches.
>
> Note that first 2 bugfixes are removed from this series, those
> are already picked by Arnaldo.
>
> E.g. --list/--funcs accepts filter rules.
> ----
> # ./perf probe -l vfs\*
> probe:vfs_read (on vfs_read@ksrc/linux-3/fs/read_write.c)
> # ./perf probe -l \*libc:\*
> probe_libc:malloc (on __libc_malloc@malloc/malloc.c in /usr/lib64/libc-2.17.so)
> ----
> ----
> # ./perf probe -F *kmalloc
> __kmalloc
> devm_kmalloc
> mempool_kmalloc
> sg_kmalloc
> sock_kmalloc
> ----
>
> Also, --del accepts filter rules too which is more flexible than
> simple wildcard.
> ----
> # ./perf probe -d 'vfs*|malloc'
> Removed event: probe:vfs_read
> Removed event: probe_libc:malloc
> ----
>
> Changes in v3:
> - [1/8] Split strfilter__or() and add strfilter__and() from v2 [3/8].
> Also rename strfilter__add() to strfilter__append().
> - [2/8] Split strfilter__string from v2 [5/8].
>
> Changes in v2:
> - [2/8] Fix to check errors except -ENOENT on deleting events.
> - [5/8] Ditto.
> - [5/8] Fix to check memory allocation error in strfilter__string.
> - [5/8] Fix to check errors of strfilter__string for verifying
> given filter.
>
> Thank you,
>
>
> ---
>
> Masami Hiramatsu (8):
> perf: Improve strfilter to append additional rules
> perf: Add strfilter__string to recover rules string
> perf probe: Accept multiple filter options
> perf probe: Accept filter argument for --list
> perf probe: Allow to use filter on --del command
> perf probe: Accept filter argument for --funcs
> perf probe: Remove redundant cleanup of params.filter
> perf probe: Cleanup and consolidate command parsers
>
>
> tools/perf/Documentation/perf-probe.txt | 9 +-
> tools/perf/builtin-probe.c | 160 +++++++++++++++----------------
> tools/perf/util/probe-event.c | 126 +++++++++++-------------
> tools/perf/util/probe-event.h | 4 -
> tools/perf/util/strfilter.c | 107 +++++++++++++++++++++
> tools/perf/util/strfilter.h | 35 +++++++
> 6 files changed, 281 insertions(+), 160 deletions(-)
>
>


--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]

2015-05-04 20:57:06

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH perf/core v3 5/8] perf probe: Allow to use filter on --del command

Em Fri, Apr 24, 2015 at 06:47:53PM +0900, Masami Hiramatsu escreveu:
> This makes perf-probe --del option to accept filter rules
> not only simple glob pattern. This simplifies the code and
> improve the flexibility.
>
> E.g. if we remove 2 different pattern events, we need 2
> -d options.
> ----
> # ./perf probe -d vfs\* -d malloc
> Removed event: probe_libc:malloc
> Removed event: probe:vfs_read
> ----

Masami,

I applied the first 4 patches in this series, but this one is
clashing, I think, with:

commit 7bedda35519840e3709c306991f9ffe028a5650d
Author: Naveen N. Rao <[email protected]>
Date: Tue Apr 28 17:35:34 2015 +0530

perf probe: Improve detection of file/function name in the probe
pattern

-------------

That was recently merged, with your ack.

Could you please take a look at refreshing the series on top of my
perf/core branch?

- Arnaldo

Subject: Re: Re: [PATCH perf/core v3 5/8] perf probe: Allow to use filter on --del command

On 2015/05/05 0:17, Arnaldo Carvalho de Melo wrote:
> Em Fri, Apr 24, 2015 at 06:47:53PM +0900, Masami Hiramatsu escreveu:
>> This makes perf-probe --del option to accept filter rules
>> not only simple glob pattern. This simplifies the code and
>> improve the flexibility.
>>
>> E.g. if we remove 2 different pattern events, we need 2
>> -d options.
>> ----
>> # ./perf probe -d vfs\* -d malloc
>> Removed event: probe_libc:malloc
>> Removed event: probe:vfs_read
>> ----
>
> Masami,
>
> I applied the first 4 patches in this series, but this one is
> clashing, I think, with:
>
> commit 7bedda35519840e3709c306991f9ffe028a5650d
> Author: Naveen N. Rao <[email protected]>
> Date: Tue Apr 28 17:35:34 2015 +0530
>
> perf probe: Improve detection of file/function name in the probe
> pattern
>
> -------------
>
> That was recently merged, with your ack.
>
> Could you please take a look at refreshing the series on top of my
> perf/core branch?

OK, I'll update and resend it :)

Thanks!

--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]

Subject: [PATCH perf/core v4 0/4] perf-probe: Add filtering features

Hi,

Here is a series of patches which improves perf-probe to add
filtering features for --del and --funcs, and cleanup code.

This version is just updated for the latest perf/core.

--del and --funcs accepts filter rules too which is more flexible
than simple wildcard.
----
# ./perf probe -d 'vfs*|malloc'
Removed event: probe:vfs_read
Removed event: probe_libc:malloc
----

Changes in v4:
- Removes patches which already merged.
- [1/4] Update for the latest perf/core.

Changes in v3:
- [1/8] Split strfilter__or() and add strfilter__and() from v2 [3/8].
Also rename strfilter__add() to strfilter__append().
- [2/8] Split strfilter__string from v2 [5/8].

Changes in v2:
- [2/8] Fix to check errors except -ENOENT on deleting events.
- [5/8] Ditto.
- [5/8] Fix to check memory allocation error in strfilter__string.
- [5/8] Fix to check errors of strfilter__string for verifying
given filter.

Thank you,


---

Masami Hiramatsu (4):
perf probe: Allow to use filter on --del command
perf probe: Accept filter argument for --funcs
perf probe: Remove redundant cleanup of params.filter
perf probe: Cleanup and consolidate command parsers


tools/perf/Documentation/perf-probe.txt | 3 -
tools/perf/builtin-probe.c | 128 ++++++++++++-------------------
tools/perf/util/probe-event.c | 102 ++++++++-----------------
tools/perf/util/probe-event.h | 2
4 files changed, 83 insertions(+), 152 deletions(-)


--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]

Subject: [PATCH perf/core v4 1/4] perf probe: Allow to use filter on --del command

This makes perf-probe --del option to accept filter rules
not only simple glob pattern. This simplifies the code and
improve the flexibility.

E.g. if we remove 2 different pattern events, we need 2
-d options.
----
# ./perf probe -d vfs\* -d malloc
Removed event: probe_libc:malloc
Removed event: probe:vfs_read
----

This allows you to joint the 2 patterns with '|'.

----
# ./perf probe -d 'vfs*|malloc'
Removed event: probe:vfs_read
Removed event: probe_libc:malloc
----

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/builtin-probe.c | 16 ++----
tools/perf/util/probe-event.c | 102 +++++++++++++----------------------------
tools/perf/util/probe-event.h | 2 -
3 files changed, 39 insertions(+), 81 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index feca316..1f41b4e 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -55,12 +55,12 @@ static struct {
bool show_ext_vars;
bool show_funcs;
bool mod_events;
+ bool del_events;
bool uprobes;
bool quiet;
bool target_used;
int nevents;
struct perf_probe_event events[MAX_PROBES];
- struct strlist *dellist;
struct line_range line_range;
char *target;
int max_probe_points;
@@ -195,10 +195,8 @@ static int opt_del_probe_event(const struct option *opt __maybe_unused,
const char *str, int unset __maybe_unused)
{
if (str) {
- params.mod_events = true;
- if (!params.dellist)
- params.dellist = strlist__new(true, NULL);
- strlist__add(params.dellist, str);
+ params.del_events = true;
+ return params_add_filter(str);
}
return 0;
}
@@ -313,8 +311,6 @@ static void cleanup_params(void)

for (i = 0; i < params.nevents; i++)
clear_perf_probe_event(params.events + i);
- if (params.dellist)
- strlist__delete(params.dellist);
line_range__clear(&params.line_range);
free(params.target);
if (params.filter)
@@ -454,7 +450,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (params.max_probe_points == 0)
params.max_probe_points = MAX_PROBES;

- if ((!params.nevents && !params.dellist && !params.list_events &&
+ if ((!params.nevents && !params.del_events && !params.list_events &&
!params.show_lines && !params.show_funcs))
usage_with_options(probe_usage, options);

@@ -514,8 +510,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
}
#endif

- if (params.dellist) {
- ret = del_perf_probe_events(params.dellist);
+ if (params.del_events) {
+ ret = del_perf_probe_events(params.filter);
if (ret < 0) {
pr_err_with_code(" Error: Failed to delete events.", ret);
return ret;
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 5995d81..abf5845 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2734,40 +2734,39 @@ error:
return ret;
}

-static int del_trace_probe_event(int fd, const char *buf,
- struct strlist *namelist)
+static int del_trace_probe_events(int fd, struct strfilter *filter,
+ struct strlist *namelist)
{
- struct str_node *ent, *n;
+ struct str_node *ent;
+ const char *p;
int ret = -ENOENT;

- if (strpbrk(buf, "*?")) { /* Glob-exp */
- strlist__for_each_safe(ent, n, namelist)
- if (strglobmatch(ent->s, buf)) {
- ret = __del_trace_probe_event(fd, ent);
- if (ret < 0)
- break;
- strlist__remove(namelist, ent);
- }
- } else {
- ent = strlist__find(namelist, buf);
- if (ent) {
+ if (!namelist)
+ return -ENOENT;
+
+ strlist__for_each(ent, namelist) {
+ p = strchr(ent->s, ':');
+ if ((p && strfilter__compare(filter, p + 1)) ||
+ strfilter__compare(filter, ent->s)) {
ret = __del_trace_probe_event(fd, ent);
- if (ret >= 0)
- strlist__remove(namelist, ent);
+ if (ret < 0)
+ break;
}
}

return ret;
}

-int del_perf_probe_events(struct strlist *dellist)
+int del_perf_probe_events(struct strfilter *filter)
{
- int ret = -1, ret2, ufd = -1, kfd = -1;
- char buf[128];
- const char *group, *event;
- char *p, *str;
- struct str_node *ent;
+ int ret, ret2, ufd = -1, kfd = -1;
struct strlist *namelist = NULL, *unamelist = NULL;
+ char *str = strfilter__string(filter);
+
+ if (!str)
+ return -EINVAL;
+
+ pr_debug("Delete filter: \'%s\'\n", str);

/* Get current event names */
kfd = open_kprobe_events(true);
@@ -2780,59 +2779,21 @@ int del_perf_probe_events(struct strlist *dellist)

if (kfd < 0 && ufd < 0) {
print_both_open_warning(kfd, ufd);
+ ret = kfd;
goto error;
}

- if (namelist == NULL && unamelist == NULL) {
- ret = -ENOENT;
+ ret = del_trace_probe_events(kfd, filter, namelist);
+ if (ret < 0 && ret != -ENOENT)
goto error;
- }
-
- strlist__for_each(ent, dellist) {
- str = strdup(ent->s);
- if (str == NULL) {
- ret = -ENOMEM;
- goto error;
- }
- pr_debug("Parsing: %s\n", str);
- p = strchr(str, ':');
- if (p) {
- group = str;
- *p = '\0';
- event = p + 1;
- } else {
- group = "*";
- event = str;
- }

- if (event && *event == '.')
- event++;
-
- ret = e_snprintf(buf, 128, "%s:%s", group, event);
- if (ret < 0) {
- pr_err("Failed to copy event.");
- free(str);
- goto error;
- }
-
- pr_debug("Group: %s, Event: %s\n", group, event);
- free(str);
-
- ret = ret2 = -ENOENT;
- if (namelist)
- ret = del_trace_probe_event(kfd, buf, namelist);
-
- if ((ret >= 0 || ret == -ENOENT) && unamelist)
- ret2 = del_trace_probe_event(ufd, buf, unamelist);
-
- /* Since we can remove probes which already removed, don't check it */
- if (ret == -ENOENT && ret2 == -ENOENT)
- pr_debug("Event \"%s\" does not exist.\n", buf);
- else if (ret < 0 || ret2 < 0) {
- if (ret >= 0)
- ret = ret2;
- break;
- }
+ ret2 = del_trace_probe_events(ufd, filter, unamelist);
+ if (ret2 < 0 && ret2 != -ENOENT)
+ ret = ret2;
+ else if (ret == -ENOENT && ret2 == -ENOENT) {
+ pr_debug("\"%s\" does not hit any event.\n", str);
+ /* Note that this is silently ignored */
+ ret = 0;
}

error:
@@ -2845,6 +2806,7 @@ error:
strlist__delete(unamelist);
close(ufd);
}
+ free(str);

return ret;
}
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index ec13362..e10aedc 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -126,7 +126,7 @@ extern const char *kernel_get_module_path(const char *module);

extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
int max_probe_points, bool force_add);
-extern int del_perf_probe_events(struct strlist *dellist);
+extern int del_perf_probe_events(struct strfilter *filter);
extern int show_perf_probe_events(struct strfilter *filter);
extern int show_line_range(struct line_range *lr, const char *module,
bool user);

Subject: [PATCH perf/core v4 2/4] perf probe: Accept filter argument for --funcs

This allows user to pass the filter pattern directly to
--funcs option as below.
----
# ./perf probe -F *kmalloc
__kmalloc
devm_kmalloc
mempool_kmalloc
sg_kmalloc
sock_kmalloc
----

We previously need --filter option for that.

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/Documentation/perf-probe.txt | 3 ++-
tools/perf/builtin-probe.c | 19 ++++++++++++++-----
2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index d0feb8e..a272f2e 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -84,9 +84,10 @@ OPTIONS
variables.

-F::
---funcs::
+--funcs[=FILTER]::
Show available functions in given module or kernel. With -x/--exec,
can also list functions in a user space executable / shared library.
+ This also can accept a FILTER rule argument.

--filter=FILTER::
(Only for --vars and --funcs) Set filter. FILTER is a combination of glob
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 1f41b4e..5a0e8f1 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -290,6 +290,17 @@ static int opt_show_vars(const struct option *opt __maybe_unused,
return ret;
}
#endif
+static int opt_show_funcs(const struct option *opt __maybe_unused,
+ const char *str, int unset)
+{
+ if (!unset)
+ params.show_funcs = true;
+
+ if (str)
+ return params_add_filter(str);
+
+ return 0;
+}

static int opt_set_filter(const struct option *opt __maybe_unused,
const char *str, int unset __maybe_unused)
@@ -399,8 +410,9 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
OPT__DRY_RUN(&probe_event_dry_run),
OPT_INTEGER('\0', "max-probes", &params.max_probe_points,
"Set how many probe points can be found for a probe."),
- OPT_BOOLEAN('F', "funcs", &params.show_funcs,
- "Show potential probe-able functions."),
+ OPT_CALLBACK_DEFAULT('F', "funcs", NULL, "[FILTER]",
+ "Show potential probe-able functions.",
+ opt_show_funcs, DEFAULT_FUNC_FILTER),
OPT_CALLBACK('\0', "filter", NULL,
"[!]FILTER", "Set a filter (with --vars/funcs only)\n"
"\t\t\t(default: \"" DEFAULT_VAR_FILTER "\" for --vars,\n"
@@ -472,9 +484,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
return ret;
}
if (params.show_funcs) {
- if (!params.filter)
- params.filter = strfilter__new(DEFAULT_FUNC_FILTER,
- NULL);
ret = show_available_funcs(params.target, params.filter,
params.uprobes);
strfilter__delete(params.filter);

Subject: [PATCH perf/core v4 3/4] perf probe: Remove redundant cleanup of params.filter

Since params.filter will be released in cleanup_params,
we don't need to clear it in each command.

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/builtin-probe.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 5a0e8f1..08c9481 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -477,8 +477,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
usage_with_options(probe_usage, options);
}
ret = show_perf_probe_events(params.filter);
- strfilter__delete(params.filter);
- params.filter = NULL;
if (ret < 0)
pr_err_with_code(" Error: Failed to show event list.", ret);
return ret;
@@ -486,8 +484,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (params.show_funcs) {
ret = show_available_funcs(params.target, params.filter,
params.uprobes);
- strfilter__delete(params.filter);
- params.filter = NULL;
if (ret < 0)
pr_err_with_code(" Error: Failed to show functions.", ret);
return ret;
@@ -511,8 +507,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
params.target,
params.filter,
params.show_ext_vars);
- strfilter__delete(params.filter);
- params.filter = NULL;
if (ret < 0)
pr_err_with_code(" Error: Failed to show vars.", ret);
return ret;

Subject: [PATCH perf/core v4 4/4] perf probe: Cleanup and consolidate command parsers

To simplify the perf-probe command code, consolidate some
similar functions and use command short-name for command
classification, instead of separated booleans.

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/builtin-probe.c | 105 ++++++++++++++++----------------------------
1 file changed, 37 insertions(+), 68 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 08c9481..17870b8 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -48,14 +48,10 @@

/* Session management structure */
static struct {
+ int command; /* Command short_name */
bool list_events;
bool force_add;
- bool show_lines;
- bool show_vars;
bool show_ext_vars;
- bool show_funcs;
- bool mod_events;
- bool del_events;
bool uprobes;
bool quiet;
bool target_used;
@@ -175,44 +171,11 @@ static int parse_probe_event_argv(int argc, const char **argv)

len += sprintf(&buf[len], "%s ", argv[i]);
}
- params.mod_events = true;
ret = parse_probe_event(buf);
free(buf);
return ret;
}

-static int opt_add_probe_event(const struct option *opt __maybe_unused,
- const char *str, int unset __maybe_unused)
-{
- if (str) {
- params.mod_events = true;
- return parse_probe_event(str);
- } else
- return 0;
-}
-
-static int opt_del_probe_event(const struct option *opt __maybe_unused,
- const char *str, int unset __maybe_unused)
-{
- if (str) {
- params.del_events = true;
- return params_add_filter(str);
- }
- return 0;
-}
-
-static int opt_list_probe_event(const struct option *opt __maybe_unused,
- const char *str, int unset)
-{
- if (!unset)
- params.list_events = true;
-
- if (str)
- return params_add_filter(str);
-
- return 0;
-}
-
static int opt_set_target(const struct option *opt, const char *str,
int unset __maybe_unused)
{
@@ -250,8 +213,10 @@ static int opt_set_target(const struct option *opt, const char *str,
return ret;
}

+/* Command option callbacks */
+
#ifdef HAVE_DWARF_SUPPORT
-static int opt_show_lines(const struct option *opt __maybe_unused,
+static int opt_show_lines(const struct option *opt,
const char *str, int unset __maybe_unused)
{
int ret = 0;
@@ -259,19 +224,19 @@ static int opt_show_lines(const struct option *opt __maybe_unused,
if (!str)
return 0;

- if (params.show_lines) {
+ if (params.command == 'L') {
pr_warning("Warning: more than one --line options are"
" detected. Only the first one is valid.\n");
return 0;
}

- params.show_lines = true;
+ params.command = opt->short_name;
ret = parse_line_range_desc(str, &params.line_range);

return ret;
}

-static int opt_show_vars(const struct option *opt __maybe_unused,
+static int opt_show_vars(const struct option *opt,
const char *str, int unset __maybe_unused)
{
struct perf_probe_event *pev = &params.events[params.nevents];
@@ -285,16 +250,27 @@ static int opt_show_vars(const struct option *opt __maybe_unused,
pr_err(" Error: '--vars' doesn't accept arguments.\n");
return -EINVAL;
}
- params.show_vars = true;
+ params.command = opt->short_name;

return ret;
}
#endif
-static int opt_show_funcs(const struct option *opt __maybe_unused,
- const char *str, int unset)
+static int opt_add_probe_event(const struct option *opt,
+ const char *str, int unset __maybe_unused)
+{
+ if (str) {
+ params.command = opt->short_name;
+ return parse_probe_event(str);
+ }
+
+ return 0;
+}
+
+static int opt_set_filter_with_command(const struct option *opt,
+ const char *str, int unset)
{
if (!unset)
- params.show_funcs = true;
+ params.command = opt->short_name;

if (str)
return params_add_filter(str);
@@ -360,10 +336,10 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
OPT_BOOLEAN('q', "quiet", &params.quiet,
"be quiet (do not show any mesages)"),
OPT_CALLBACK_DEFAULT('l', "list", NULL, "[GROUP:]EVENT",
- "list up probe events", opt_list_probe_event,
- DEFAULT_LIST_FILTER),
+ "list up probe events",
+ opt_set_filter_with_command, DEFAULT_LIST_FILTER),
OPT_CALLBACK('d', "del", NULL, "[GROUP:]EVENT", "delete a probe event.",
- opt_del_probe_event),
+ opt_set_filter_with_command),
OPT_CALLBACK('a', "add", NULL,
#ifdef HAVE_DWARF_SUPPORT
"[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT"
@@ -412,7 +388,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
"Set how many probe points can be found for a probe."),
OPT_CALLBACK_DEFAULT('F', "funcs", NULL, "[FILTER]",
"Show potential probe-able functions.",
- opt_show_funcs, DEFAULT_FUNC_FILTER),
+ opt_set_filter_with_command, DEFAULT_FUNC_FILTER),
OPT_CALLBACK('\0', "filter", NULL,
"[!]FILTER", "Set a filter (with --vars/funcs only)\n"
"\t\t\t(default: \"" DEFAULT_VAR_FILTER "\" for --vars,\n"
@@ -462,16 +438,13 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (params.max_probe_points == 0)
params.max_probe_points = MAX_PROBES;

- if ((!params.nevents && !params.del_events && !params.list_events &&
- !params.show_lines && !params.show_funcs))
- usage_with_options(probe_usage, options);
-
/*
* Only consider the user's kernel image path if given.
*/
symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);

- if (params.list_events) {
+ switch (params.command) {
+ case 'l':
if (params.uprobes) {
pr_warning(" Error: Don't use --list with --exec.\n");
usage_with_options(probe_usage, options);
@@ -480,24 +453,20 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (ret < 0)
pr_err_with_code(" Error: Failed to show event list.", ret);
return ret;
- }
- if (params.show_funcs) {
+ case 'F':
ret = show_available_funcs(params.target, params.filter,
params.uprobes);
if (ret < 0)
pr_err_with_code(" Error: Failed to show functions.", ret);
return ret;
- }
-
#ifdef HAVE_DWARF_SUPPORT
- if (params.show_lines) {
+ case 'L':
ret = show_line_range(&params.line_range, params.target,
params.uprobes);
if (ret < 0)
pr_err_with_code(" Error: Failed to show lines.", ret);
return ret;
- }
- if (params.show_vars) {
+ case 'V':
if (!params.filter)
params.filter = strfilter__new(DEFAULT_VAR_FILTER,
NULL);
@@ -510,18 +479,15 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (ret < 0)
pr_err_with_code(" Error: Failed to show vars.", ret);
return ret;
- }
#endif
-
- if (params.del_events) {
+ case 'd':
ret = del_perf_probe_events(params.filter);
if (ret < 0) {
pr_err_with_code(" Error: Failed to delete events.", ret);
return ret;
}
- }
-
- if (params.nevents) {
+ break;
+ case 'a':
/* Ensure the last given target is used */
if (params.target && !params.target_used) {
pr_warning(" Error: -x/-m must follow the probe definitions.\n");
@@ -535,6 +501,9 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
pr_err_with_code(" Error: Failed to add events.", ret);
return ret;
}
+ break;
+ default:
+ usage_with_options(probe_usage, options);
}
return 0;
}

2015-05-05 16:06:48

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: Re: [PATCH perf/core v3 5/8] perf probe: Allow to use filter on --del command

Em Tue, May 05, 2015 at 10:59:47AM +0900, Masami Hiramatsu escreveu:
> On 2015/05/05 0:17, Arnaldo Carvalho de Melo wrote:
> > I applied the first 4 patches in this series, but this one is
> > clashing, I think, with:

> > commit 7bedda35519840e3709c306991f9ffe028a5650d
> > Author: Naveen N. Rao <[email protected]>
> > Date: Tue Apr 28 17:35:34 2015 +0530
> > perf probe: Improve detection of file/function name in the probe pattern
> > -------------
> > That was recently merged, with your ack.
> >
> > Could you please take a look at refreshing the series on top of my
> > perf/core branch?
>
> OK, I'll update and resend it :)

Thanks, testing/applying them now,

- Arnaldo

2015-05-05 16:07:06

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH perf/core v4 2/4] perf probe: Accept filter argument for --funcs

Em Tue, May 05, 2015 at 11:29:50AM +0900, Masami Hiramatsu escreveu:
> This allows user to pass the filter pattern directly to
> --funcs option as below.
> ----
> # ./perf probe -F *kmalloc
> __kmalloc
> devm_kmalloc
> mempool_kmalloc
> sg_kmalloc
> sock_kmalloc
> ----

Tested, works well, so I went to try and use it in another option:

[root@ssdandy acme]# perf probe kmalloc*
Probe point 'kmalloc*' not found.
Error: Failed to add events.
[root@ssdandy acme]#

8-)

- Arnaldo

2015-05-05 16:07:12

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH perf/core v4 0/4] perf-probe: Add filtering features

Em Tue, May 05, 2015 at 11:29:46AM +0900, Masami Hiramatsu escreveu:
> Hi,
>
> Here is a series of patches which improves perf-probe to add
> filtering features for --del and --funcs, and cleanup code.
>
> This version is just updated for the latest perf/core.
>
> --del and --funcs accepts filter rules too which is more flexible
> than simple wildcard.
> ----
> # ./perf probe -d 'vfs*|malloc'
> Removed event: probe:vfs_read
> Removed event: probe_libc:malloc
> ----
>
> Changes in v4:
> - Removes patches which already merged.
> - [1/4] Update for the latest perf/core.
>

Thanks, all tested and applied,

- Arnaldo

Subject: Re: [PATCH perf/core v4 4/4] perf probe: Cleanup and consolidate command parsers

On 2015/05/05 11:29, Masami Hiramatsu wrote:
> To simplify the perf-probe command code, consolidate some
> similar functions and use command short-name for command
> classification, instead of separated booleans.

Oops, I've found my mistake on this patch. perf-probe without
any command but probe defs should imply "--add", but this patch
drops that.
I'll update this for v5 patch.

Thank you,

>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> ---
> tools/perf/builtin-probe.c | 105 ++++++++++++++++----------------------------
> 1 file changed, 37 insertions(+), 68 deletions(-)
>
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 08c9481..17870b8 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -48,14 +48,10 @@
>
> /* Session management structure */
> static struct {
> + int command; /* Command short_name */
> bool list_events;
> bool force_add;
> - bool show_lines;
> - bool show_vars;
> bool show_ext_vars;
> - bool show_funcs;
> - bool mod_events;
> - bool del_events;
> bool uprobes;
> bool quiet;
> bool target_used;
> @@ -175,44 +171,11 @@ static int parse_probe_event_argv(int argc, const char **argv)
>
> len += sprintf(&buf[len], "%s ", argv[i]);
> }
> - params.mod_events = true;
> ret = parse_probe_event(buf);
> free(buf);
> return ret;
> }
>
> -static int opt_add_probe_event(const struct option *opt __maybe_unused,
> - const char *str, int unset __maybe_unused)
> -{
> - if (str) {
> - params.mod_events = true;
> - return parse_probe_event(str);
> - } else
> - return 0;
> -}
> -
> -static int opt_del_probe_event(const struct option *opt __maybe_unused,
> - const char *str, int unset __maybe_unused)
> -{
> - if (str) {
> - params.del_events = true;
> - return params_add_filter(str);
> - }
> - return 0;
> -}
> -
> -static int opt_list_probe_event(const struct option *opt __maybe_unused,
> - const char *str, int unset)
> -{
> - if (!unset)
> - params.list_events = true;
> -
> - if (str)
> - return params_add_filter(str);
> -
> - return 0;
> -}
> -
> static int opt_set_target(const struct option *opt, const char *str,
> int unset __maybe_unused)
> {
> @@ -250,8 +213,10 @@ static int opt_set_target(const struct option *opt, const char *str,
> return ret;
> }
>
> +/* Command option callbacks */
> +
> #ifdef HAVE_DWARF_SUPPORT
> -static int opt_show_lines(const struct option *opt __maybe_unused,
> +static int opt_show_lines(const struct option *opt,
> const char *str, int unset __maybe_unused)
> {
> int ret = 0;
> @@ -259,19 +224,19 @@ static int opt_show_lines(const struct option *opt __maybe_unused,
> if (!str)
> return 0;
>
> - if (params.show_lines) {
> + if (params.command == 'L') {
> pr_warning("Warning: more than one --line options are"
> " detected. Only the first one is valid.\n");
> return 0;
> }
>
> - params.show_lines = true;
> + params.command = opt->short_name;
> ret = parse_line_range_desc(str, &params.line_range);
>
> return ret;
> }
>
> -static int opt_show_vars(const struct option *opt __maybe_unused,
> +static int opt_show_vars(const struct option *opt,
> const char *str, int unset __maybe_unused)
> {
> struct perf_probe_event *pev = &params.events[params.nevents];
> @@ -285,16 +250,27 @@ static int opt_show_vars(const struct option *opt __maybe_unused,
> pr_err(" Error: '--vars' doesn't accept arguments.\n");
> return -EINVAL;
> }
> - params.show_vars = true;
> + params.command = opt->short_name;
>
> return ret;
> }
> #endif
> -static int opt_show_funcs(const struct option *opt __maybe_unused,
> - const char *str, int unset)
> +static int opt_add_probe_event(const struct option *opt,
> + const char *str, int unset __maybe_unused)
> +{
> + if (str) {
> + params.command = opt->short_name;
> + return parse_probe_event(str);
> + }
> +
> + return 0;
> +}
> +
> +static int opt_set_filter_with_command(const struct option *opt,
> + const char *str, int unset)
> {
> if (!unset)
> - params.show_funcs = true;
> + params.command = opt->short_name;
>
> if (str)
> return params_add_filter(str);
> @@ -360,10 +336,10 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> OPT_BOOLEAN('q', "quiet", &params.quiet,
> "be quiet (do not show any mesages)"),
> OPT_CALLBACK_DEFAULT('l', "list", NULL, "[GROUP:]EVENT",
> - "list up probe events", opt_list_probe_event,
> - DEFAULT_LIST_FILTER),
> + "list up probe events",
> + opt_set_filter_with_command, DEFAULT_LIST_FILTER),
> OPT_CALLBACK('d', "del", NULL, "[GROUP:]EVENT", "delete a probe event.",
> - opt_del_probe_event),
> + opt_set_filter_with_command),
> OPT_CALLBACK('a', "add", NULL,
> #ifdef HAVE_DWARF_SUPPORT
> "[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT"
> @@ -412,7 +388,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> "Set how many probe points can be found for a probe."),
> OPT_CALLBACK_DEFAULT('F', "funcs", NULL, "[FILTER]",
> "Show potential probe-able functions.",
> - opt_show_funcs, DEFAULT_FUNC_FILTER),
> + opt_set_filter_with_command, DEFAULT_FUNC_FILTER),
> OPT_CALLBACK('\0', "filter", NULL,
> "[!]FILTER", "Set a filter (with --vars/funcs only)\n"
> "\t\t\t(default: \"" DEFAULT_VAR_FILTER "\" for --vars,\n"
> @@ -462,16 +438,13 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> if (params.max_probe_points == 0)
> params.max_probe_points = MAX_PROBES;
>
> - if ((!params.nevents && !params.del_events && !params.list_events &&
> - !params.show_lines && !params.show_funcs))
> - usage_with_options(probe_usage, options);
> -
> /*
> * Only consider the user's kernel image path if given.
> */
> symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
>
> - if (params.list_events) {
> + switch (params.command) {
> + case 'l':
> if (params.uprobes) {
> pr_warning(" Error: Don't use --list with --exec.\n");
> usage_with_options(probe_usage, options);
> @@ -480,24 +453,20 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> if (ret < 0)
> pr_err_with_code(" Error: Failed to show event list.", ret);
> return ret;
> - }
> - if (params.show_funcs) {
> + case 'F':
> ret = show_available_funcs(params.target, params.filter,
> params.uprobes);
> if (ret < 0)
> pr_err_with_code(" Error: Failed to show functions.", ret);
> return ret;
> - }
> -
> #ifdef HAVE_DWARF_SUPPORT
> - if (params.show_lines) {
> + case 'L':
> ret = show_line_range(&params.line_range, params.target,
> params.uprobes);
> if (ret < 0)
> pr_err_with_code(" Error: Failed to show lines.", ret);
> return ret;
> - }
> - if (params.show_vars) {
> + case 'V':
> if (!params.filter)
> params.filter = strfilter__new(DEFAULT_VAR_FILTER,
> NULL);
> @@ -510,18 +479,15 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> if (ret < 0)
> pr_err_with_code(" Error: Failed to show vars.", ret);
> return ret;
> - }
> #endif
> -
> - if (params.del_events) {
> + case 'd':
> ret = del_perf_probe_events(params.filter);
> if (ret < 0) {
> pr_err_with_code(" Error: Failed to delete events.", ret);
> return ret;
> }
> - }
> -
> - if (params.nevents) {
> + break;
> + case 'a':
> /* Ensure the last given target is used */
> if (params.target && !params.target_used) {
> pr_warning(" Error: -x/-m must follow the probe definitions.\n");
> @@ -535,6 +501,9 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> pr_err_with_code(" Error: Failed to add events.", ret);
> return ret;
> }
> + break;
> + default:
> + usage_with_options(probe_usage, options);
> }
> return 0;
> }
>
> --
> 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
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]

Subject: Re: [PATCH perf/core v4 2/4] perf probe: Accept filter argument for --funcs

On 2015/05/05 23:31, Arnaldo Carvalho de Melo wrote:
> Em Tue, May 05, 2015 at 11:29:50AM +0900, Masami Hiramatsu escreveu:
>> This allows user to pass the filter pattern directly to
>> --funcs option as below.
>> ----
>> # ./perf probe -F *kmalloc
>> __kmalloc
>> devm_kmalloc
>> mempool_kmalloc
>> sg_kmalloc
>> sock_kmalloc
>> ----
>
> Tested, works well, so I went to try and use it in another option:
>
> [root@ssdandy acme]# perf probe kmalloc*
> Probe point 'kmalloc*' not found.
> Error: Failed to add events.
> [root@ssdandy acme]#

Well, I'm already preparing it ;)

BTW, what patches have you applied? Can I update 4/4 or would I
better make another bugfix?

Thank you,

>
> 8-)
>
> - Arnaldo
>


--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]

2015-05-05 16:09:15

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH perf/core v4 4/4] perf probe: Cleanup and consolidate command parsers

Em Wed, May 06, 2015 at 12:06:16AM +0900, Masami Hiramatsu escreveu:
> On 2015/05/05 11:29, Masami Hiramatsu wrote:
> > To simplify the perf-probe command code, consolidate some
> > similar functions and use command short-name for command
> > classification, instead of separated booleans.
>
> Oops, I've found my mistake on this patch. perf-probe without
> any command but probe defs should imply "--add", but this patch
> drops that.
> I'll update this for v5 patch.

Humm, ok, so I'll drop this one from my perf/core branch now, to keep it
bisectable, ok?

- Arnaldo

> Thank you,
>
> >
> > Signed-off-by: Masami Hiramatsu <[email protected]>
> > ---
> > tools/perf/builtin-probe.c | 105 ++++++++++++++++----------------------------
> > 1 file changed, 37 insertions(+), 68 deletions(-)
> >
> > diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> > index 08c9481..17870b8 100644
> > --- a/tools/perf/builtin-probe.c
> > +++ b/tools/perf/builtin-probe.c
> > @@ -48,14 +48,10 @@
> >
> > /* Session management structure */
> > static struct {
> > + int command; /* Command short_name */
> > bool list_events;
> > bool force_add;
> > - bool show_lines;
> > - bool show_vars;
> > bool show_ext_vars;
> > - bool show_funcs;
> > - bool mod_events;
> > - bool del_events;
> > bool uprobes;
> > bool quiet;
> > bool target_used;
> > @@ -175,44 +171,11 @@ static int parse_probe_event_argv(int argc, const char **argv)
> >
> > len += sprintf(&buf[len], "%s ", argv[i]);
> > }
> > - params.mod_events = true;
> > ret = parse_probe_event(buf);
> > free(buf);
> > return ret;
> > }
> >
> > -static int opt_add_probe_event(const struct option *opt __maybe_unused,
> > - const char *str, int unset __maybe_unused)
> > -{
> > - if (str) {
> > - params.mod_events = true;
> > - return parse_probe_event(str);
> > - } else
> > - return 0;
> > -}
> > -
> > -static int opt_del_probe_event(const struct option *opt __maybe_unused,
> > - const char *str, int unset __maybe_unused)
> > -{
> > - if (str) {
> > - params.del_events = true;
> > - return params_add_filter(str);
> > - }
> > - return 0;
> > -}
> > -
> > -static int opt_list_probe_event(const struct option *opt __maybe_unused,
> > - const char *str, int unset)
> > -{
> > - if (!unset)
> > - params.list_events = true;
> > -
> > - if (str)
> > - return params_add_filter(str);
> > -
> > - return 0;
> > -}
> > -
> > static int opt_set_target(const struct option *opt, const char *str,
> > int unset __maybe_unused)
> > {
> > @@ -250,8 +213,10 @@ static int opt_set_target(const struct option *opt, const char *str,
> > return ret;
> > }
> >
> > +/* Command option callbacks */
> > +
> > #ifdef HAVE_DWARF_SUPPORT
> > -static int opt_show_lines(const struct option *opt __maybe_unused,
> > +static int opt_show_lines(const struct option *opt,
> > const char *str, int unset __maybe_unused)
> > {
> > int ret = 0;
> > @@ -259,19 +224,19 @@ static int opt_show_lines(const struct option *opt __maybe_unused,
> > if (!str)
> > return 0;
> >
> > - if (params.show_lines) {
> > + if (params.command == 'L') {
> > pr_warning("Warning: more than one --line options are"
> > " detected. Only the first one is valid.\n");
> > return 0;
> > }
> >
> > - params.show_lines = true;
> > + params.command = opt->short_name;
> > ret = parse_line_range_desc(str, &params.line_range);
> >
> > return ret;
> > }
> >
> > -static int opt_show_vars(const struct option *opt __maybe_unused,
> > +static int opt_show_vars(const struct option *opt,
> > const char *str, int unset __maybe_unused)
> > {
> > struct perf_probe_event *pev = &params.events[params.nevents];
> > @@ -285,16 +250,27 @@ static int opt_show_vars(const struct option *opt __maybe_unused,
> > pr_err(" Error: '--vars' doesn't accept arguments.\n");
> > return -EINVAL;
> > }
> > - params.show_vars = true;
> > + params.command = opt->short_name;
> >
> > return ret;
> > }
> > #endif
> > -static int opt_show_funcs(const struct option *opt __maybe_unused,
> > - const char *str, int unset)
> > +static int opt_add_probe_event(const struct option *opt,
> > + const char *str, int unset __maybe_unused)
> > +{
> > + if (str) {
> > + params.command = opt->short_name;
> > + return parse_probe_event(str);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int opt_set_filter_with_command(const struct option *opt,
> > + const char *str, int unset)
> > {
> > if (!unset)
> > - params.show_funcs = true;
> > + params.command = opt->short_name;
> >
> > if (str)
> > return params_add_filter(str);
> > @@ -360,10 +336,10 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> > OPT_BOOLEAN('q', "quiet", &params.quiet,
> > "be quiet (do not show any mesages)"),
> > OPT_CALLBACK_DEFAULT('l', "list", NULL, "[GROUP:]EVENT",
> > - "list up probe events", opt_list_probe_event,
> > - DEFAULT_LIST_FILTER),
> > + "list up probe events",
> > + opt_set_filter_with_command, DEFAULT_LIST_FILTER),
> > OPT_CALLBACK('d', "del", NULL, "[GROUP:]EVENT", "delete a probe event.",
> > - opt_del_probe_event),
> > + opt_set_filter_with_command),
> > OPT_CALLBACK('a', "add", NULL,
> > #ifdef HAVE_DWARF_SUPPORT
> > "[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT"
> > @@ -412,7 +388,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> > "Set how many probe points can be found for a probe."),
> > OPT_CALLBACK_DEFAULT('F', "funcs", NULL, "[FILTER]",
> > "Show potential probe-able functions.",
> > - opt_show_funcs, DEFAULT_FUNC_FILTER),
> > + opt_set_filter_with_command, DEFAULT_FUNC_FILTER),
> > OPT_CALLBACK('\0', "filter", NULL,
> > "[!]FILTER", "Set a filter (with --vars/funcs only)\n"
> > "\t\t\t(default: \"" DEFAULT_VAR_FILTER "\" for --vars,\n"
> > @@ -462,16 +438,13 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> > if (params.max_probe_points == 0)
> > params.max_probe_points = MAX_PROBES;
> >
> > - if ((!params.nevents && !params.del_events && !params.list_events &&
> > - !params.show_lines && !params.show_funcs))
> > - usage_with_options(probe_usage, options);
> > -
> > /*
> > * Only consider the user's kernel image path if given.
> > */
> > symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
> >
> > - if (params.list_events) {
> > + switch (params.command) {
> > + case 'l':
> > if (params.uprobes) {
> > pr_warning(" Error: Don't use --list with --exec.\n");
> > usage_with_options(probe_usage, options);
> > @@ -480,24 +453,20 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> > if (ret < 0)
> > pr_err_with_code(" Error: Failed to show event list.", ret);
> > return ret;
> > - }
> > - if (params.show_funcs) {
> > + case 'F':
> > ret = show_available_funcs(params.target, params.filter,
> > params.uprobes);
> > if (ret < 0)
> > pr_err_with_code(" Error: Failed to show functions.", ret);
> > return ret;
> > - }
> > -
> > #ifdef HAVE_DWARF_SUPPORT
> > - if (params.show_lines) {
> > + case 'L':
> > ret = show_line_range(&params.line_range, params.target,
> > params.uprobes);
> > if (ret < 0)
> > pr_err_with_code(" Error: Failed to show lines.", ret);
> > return ret;
> > - }
> > - if (params.show_vars) {
> > + case 'V':
> > if (!params.filter)
> > params.filter = strfilter__new(DEFAULT_VAR_FILTER,
> > NULL);
> > @@ -510,18 +479,15 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> > if (ret < 0)
> > pr_err_with_code(" Error: Failed to show vars.", ret);
> > return ret;
> > - }
> > #endif
> > -
> > - if (params.del_events) {
> > + case 'd':
> > ret = del_perf_probe_events(params.filter);
> > if (ret < 0) {
> > pr_err_with_code(" Error: Failed to delete events.", ret);
> > return ret;
> > }
> > - }
> > -
> > - if (params.nevents) {
> > + break;
> > + case 'a':
> > /* Ensure the last given target is used */
> > if (params.target && !params.target_used) {
> > pr_warning(" Error: -x/-m must follow the probe definitions.\n");
> > @@ -535,6 +501,9 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> > pr_err_with_code(" Error: Failed to add events.", ret);
> > return ret;
> > }
> > + break;
> > + default:
> > + usage_with_options(probe_usage, options);
> > }
> > return 0;
> > }
> >
> > --
> > 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
> Linux Technology Research Center, System Productivity Research Dept.
> Center for Technology Innovation - Systems Engineering
> Hitachi, Ltd., Research & Development Group
> E-mail: [email protected]

Subject: [PATCH perf/core v5] perf probe: Cleanup and consolidate command parsers

To simplify the perf-probe command code, consolidate some
similar functions and use command short-name for command
classification, instead of separated booleans.

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/builtin-probe.c | 110 +++++++++++++++++---------------------------
1 file changed, 42 insertions(+), 68 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 08c9481..53d475b 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -48,14 +48,10 @@

/* Session management structure */
static struct {
+ int command; /* Command short_name */
bool list_events;
bool force_add;
- bool show_lines;
- bool show_vars;
bool show_ext_vars;
- bool show_funcs;
- bool mod_events;
- bool del_events;
bool uprobes;
bool quiet;
bool target_used;
@@ -175,44 +171,11 @@ static int parse_probe_event_argv(int argc, const char **argv)

len += sprintf(&buf[len], "%s ", argv[i]);
}
- params.mod_events = true;
ret = parse_probe_event(buf);
free(buf);
return ret;
}

-static int opt_add_probe_event(const struct option *opt __maybe_unused,
- const char *str, int unset __maybe_unused)
-{
- if (str) {
- params.mod_events = true;
- return parse_probe_event(str);
- } else
- return 0;
-}
-
-static int opt_del_probe_event(const struct option *opt __maybe_unused,
- const char *str, int unset __maybe_unused)
-{
- if (str) {
- params.del_events = true;
- return params_add_filter(str);
- }
- return 0;
-}
-
-static int opt_list_probe_event(const struct option *opt __maybe_unused,
- const char *str, int unset)
-{
- if (!unset)
- params.list_events = true;
-
- if (str)
- return params_add_filter(str);
-
- return 0;
-}
-
static int opt_set_target(const struct option *opt, const char *str,
int unset __maybe_unused)
{
@@ -250,8 +213,10 @@ static int opt_set_target(const struct option *opt, const char *str,
return ret;
}

+/* Command option callbacks */
+
#ifdef HAVE_DWARF_SUPPORT
-static int opt_show_lines(const struct option *opt __maybe_unused,
+static int opt_show_lines(const struct option *opt,
const char *str, int unset __maybe_unused)
{
int ret = 0;
@@ -259,19 +224,19 @@ static int opt_show_lines(const struct option *opt __maybe_unused,
if (!str)
return 0;

- if (params.show_lines) {
+ if (params.command == 'L') {
pr_warning("Warning: more than one --line options are"
" detected. Only the first one is valid.\n");
return 0;
}

- params.show_lines = true;
+ params.command = opt->short_name;
ret = parse_line_range_desc(str, &params.line_range);

return ret;
}

-static int opt_show_vars(const struct option *opt __maybe_unused,
+static int opt_show_vars(const struct option *opt,
const char *str, int unset __maybe_unused)
{
struct perf_probe_event *pev = &params.events[params.nevents];
@@ -285,16 +250,27 @@ static int opt_show_vars(const struct option *opt __maybe_unused,
pr_err(" Error: '--vars' doesn't accept arguments.\n");
return -EINVAL;
}
- params.show_vars = true;
+ params.command = opt->short_name;

return ret;
}
#endif
-static int opt_show_funcs(const struct option *opt __maybe_unused,
- const char *str, int unset)
+static int opt_add_probe_event(const struct option *opt,
+ const char *str, int unset __maybe_unused)
+{
+ if (str) {
+ params.command = opt->short_name;
+ return parse_probe_event(str);
+ }
+
+ return 0;
+}
+
+static int opt_set_filter_with_command(const struct option *opt,
+ const char *str, int unset)
{
if (!unset)
- params.show_funcs = true;
+ params.command = opt->short_name;

if (str)
return params_add_filter(str);
@@ -360,10 +336,10 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
OPT_BOOLEAN('q', "quiet", &params.quiet,
"be quiet (do not show any mesages)"),
OPT_CALLBACK_DEFAULT('l', "list", NULL, "[GROUP:]EVENT",
- "list up probe events", opt_list_probe_event,
- DEFAULT_LIST_FILTER),
+ "list up probe events",
+ opt_set_filter_with_command, DEFAULT_LIST_FILTER),
OPT_CALLBACK('d', "del", NULL, "[GROUP:]EVENT", "delete a probe event.",
- opt_del_probe_event),
+ opt_set_filter_with_command),
OPT_CALLBACK('a', "add", NULL,
#ifdef HAVE_DWARF_SUPPORT
"[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT"
@@ -412,7 +388,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
"Set how many probe points can be found for a probe."),
OPT_CALLBACK_DEFAULT('F', "funcs", NULL, "[FILTER]",
"Show potential probe-able functions.",
- opt_show_funcs, DEFAULT_FUNC_FILTER),
+ opt_set_filter_with_command, DEFAULT_FUNC_FILTER),
OPT_CALLBACK('\0', "filter", NULL,
"[!]FILTER", "Set a filter (with --vars/funcs only)\n"
"\t\t\t(default: \"" DEFAULT_VAR_FILTER "\" for --vars,\n"
@@ -444,11 +420,16 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
pr_warning(" Error: '-' is not supported.\n");
usage_with_options(probe_usage, options);
}
+ if (params.command && params.command != 'a') {
+ pr_warning(" Error: another command except --add is set.\n");
+ usage_with_options(probe_usage, options);
+ }
ret = parse_probe_event_argv(argc, argv);
if (ret < 0) {
pr_err_with_code(" Error: Command Parse Error.", ret);
return ret;
}
+ params.command = 'a';
}

if (params.quiet) {
@@ -462,16 +443,13 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (params.max_probe_points == 0)
params.max_probe_points = MAX_PROBES;

- if ((!params.nevents && !params.del_events && !params.list_events &&
- !params.show_lines && !params.show_funcs))
- usage_with_options(probe_usage, options);
-
/*
* Only consider the user's kernel image path if given.
*/
symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);

- if (params.list_events) {
+ switch (params.command) {
+ case 'l':
if (params.uprobes) {
pr_warning(" Error: Don't use --list with --exec.\n");
usage_with_options(probe_usage, options);
@@ -480,24 +458,20 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (ret < 0)
pr_err_with_code(" Error: Failed to show event list.", ret);
return ret;
- }
- if (params.show_funcs) {
+ case 'F':
ret = show_available_funcs(params.target, params.filter,
params.uprobes);
if (ret < 0)
pr_err_with_code(" Error: Failed to show functions.", ret);
return ret;
- }
-
#ifdef HAVE_DWARF_SUPPORT
- if (params.show_lines) {
+ case 'L':
ret = show_line_range(&params.line_range, params.target,
params.uprobes);
if (ret < 0)
pr_err_with_code(" Error: Failed to show lines.", ret);
return ret;
- }
- if (params.show_vars) {
+ case 'V':
if (!params.filter)
params.filter = strfilter__new(DEFAULT_VAR_FILTER,
NULL);
@@ -510,18 +484,15 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (ret < 0)
pr_err_with_code(" Error: Failed to show vars.", ret);
return ret;
- }
#endif
-
- if (params.del_events) {
+ case 'd':
ret = del_perf_probe_events(params.filter);
if (ret < 0) {
pr_err_with_code(" Error: Failed to delete events.", ret);
return ret;
}
- }
-
- if (params.nevents) {
+ break;
+ case 'a':
/* Ensure the last given target is used */
if (params.target && !params.target_used) {
pr_warning(" Error: -x/-m must follow the probe definitions.\n");
@@ -535,6 +506,9 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
pr_err_with_code(" Error: Failed to add events.", ret);
return ret;
}
+ break;
+ default:
+ usage_with_options(probe_usage, options);
}
return 0;
}

Subject: Re: Re: [PATCH perf/core v4 4/4] perf probe: Cleanup and consolidate command parsers

On 2015/05/06 0:12, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 06, 2015 at 12:06:16AM +0900, Masami Hiramatsu escreveu:
>> On 2015/05/05 11:29, Masami Hiramatsu wrote:
>>> To simplify the perf-probe command code, consolidate some
>>> similar functions and use command short-name for command
>>> classification, instead of separated booleans.
>>
>> Oops, I've found my mistake on this patch. perf-probe without
>> any command but probe defs should imply "--add", but this patch
>> drops that.
>> I'll update this for v5 patch.
>
> Humm, ok, so I'll drop this one from my perf/core branch now, to keep it
> bisectable, ok?
>

Yes, I've sent the newer version of this patch.

Thank you!


> - Arnaldo
>
>> Thank you,
>>
>>>
>>> Signed-off-by: Masami Hiramatsu <[email protected]>
>>> ---
>>> tools/perf/builtin-probe.c | 105 ++++++++++++++++----------------------------
>>> 1 file changed, 37 insertions(+), 68 deletions(-)
>>>
>>> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
>>> index 08c9481..17870b8 100644
>>> --- a/tools/perf/builtin-probe.c
>>> +++ b/tools/perf/builtin-probe.c
>>> @@ -48,14 +48,10 @@
>>>
>>> /* Session management structure */
>>> static struct {
>>> + int command; /* Command short_name */
>>> bool list_events;
>>> bool force_add;
>>> - bool show_lines;
>>> - bool show_vars;
>>> bool show_ext_vars;
>>> - bool show_funcs;
>>> - bool mod_events;
>>> - bool del_events;
>>> bool uprobes;
>>> bool quiet;
>>> bool target_used;
>>> @@ -175,44 +171,11 @@ static int parse_probe_event_argv(int argc, const char **argv)
>>>
>>> len += sprintf(&buf[len], "%s ", argv[i]);
>>> }
>>> - params.mod_events = true;
>>> ret = parse_probe_event(buf);
>>> free(buf);
>>> return ret;
>>> }
>>>
>>> -static int opt_add_probe_event(const struct option *opt __maybe_unused,
>>> - const char *str, int unset __maybe_unused)
>>> -{
>>> - if (str) {
>>> - params.mod_events = true;
>>> - return parse_probe_event(str);
>>> - } else
>>> - return 0;
>>> -}
>>> -
>>> -static int opt_del_probe_event(const struct option *opt __maybe_unused,
>>> - const char *str, int unset __maybe_unused)
>>> -{
>>> - if (str) {
>>> - params.del_events = true;
>>> - return params_add_filter(str);
>>> - }
>>> - return 0;
>>> -}
>>> -
>>> -static int opt_list_probe_event(const struct option *opt __maybe_unused,
>>> - const char *str, int unset)
>>> -{
>>> - if (!unset)
>>> - params.list_events = true;
>>> -
>>> - if (str)
>>> - return params_add_filter(str);
>>> -
>>> - return 0;
>>> -}
>>> -
>>> static int opt_set_target(const struct option *opt, const char *str,
>>> int unset __maybe_unused)
>>> {
>>> @@ -250,8 +213,10 @@ static int opt_set_target(const struct option *opt, const char *str,
>>> return ret;
>>> }
>>>
>>> +/* Command option callbacks */
>>> +
>>> #ifdef HAVE_DWARF_SUPPORT
>>> -static int opt_show_lines(const struct option *opt __maybe_unused,
>>> +static int opt_show_lines(const struct option *opt,
>>> const char *str, int unset __maybe_unused)
>>> {
>>> int ret = 0;
>>> @@ -259,19 +224,19 @@ static int opt_show_lines(const struct option *opt __maybe_unused,
>>> if (!str)
>>> return 0;
>>>
>>> - if (params.show_lines) {
>>> + if (params.command == 'L') {
>>> pr_warning("Warning: more than one --line options are"
>>> " detected. Only the first one is valid.\n");
>>> return 0;
>>> }
>>>
>>> - params.show_lines = true;
>>> + params.command = opt->short_name;
>>> ret = parse_line_range_desc(str, &params.line_range);
>>>
>>> return ret;
>>> }
>>>
>>> -static int opt_show_vars(const struct option *opt __maybe_unused,
>>> +static int opt_show_vars(const struct option *opt,
>>> const char *str, int unset __maybe_unused)
>>> {
>>> struct perf_probe_event *pev = &params.events[params.nevents];
>>> @@ -285,16 +250,27 @@ static int opt_show_vars(const struct option *opt __maybe_unused,
>>> pr_err(" Error: '--vars' doesn't accept arguments.\n");
>>> return -EINVAL;
>>> }
>>> - params.show_vars = true;
>>> + params.command = opt->short_name;
>>>
>>> return ret;
>>> }
>>> #endif
>>> -static int opt_show_funcs(const struct option *opt __maybe_unused,
>>> - const char *str, int unset)
>>> +static int opt_add_probe_event(const struct option *opt,
>>> + const char *str, int unset __maybe_unused)
>>> +{
>>> + if (str) {
>>> + params.command = opt->short_name;
>>> + return parse_probe_event(str);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int opt_set_filter_with_command(const struct option *opt,
>>> + const char *str, int unset)
>>> {
>>> if (!unset)
>>> - params.show_funcs = true;
>>> + params.command = opt->short_name;
>>>
>>> if (str)
>>> return params_add_filter(str);
>>> @@ -360,10 +336,10 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>>> OPT_BOOLEAN('q', "quiet", &params.quiet,
>>> "be quiet (do not show any mesages)"),
>>> OPT_CALLBACK_DEFAULT('l', "list", NULL, "[GROUP:]EVENT",
>>> - "list up probe events", opt_list_probe_event,
>>> - DEFAULT_LIST_FILTER),
>>> + "list up probe events",
>>> + opt_set_filter_with_command, DEFAULT_LIST_FILTER),
>>> OPT_CALLBACK('d', "del", NULL, "[GROUP:]EVENT", "delete a probe event.",
>>> - opt_del_probe_event),
>>> + opt_set_filter_with_command),
>>> OPT_CALLBACK('a', "add", NULL,
>>> #ifdef HAVE_DWARF_SUPPORT
>>> "[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT"
>>> @@ -412,7 +388,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>>> "Set how many probe points can be found for a probe."),
>>> OPT_CALLBACK_DEFAULT('F', "funcs", NULL, "[FILTER]",
>>> "Show potential probe-able functions.",
>>> - opt_show_funcs, DEFAULT_FUNC_FILTER),
>>> + opt_set_filter_with_command, DEFAULT_FUNC_FILTER),
>>> OPT_CALLBACK('\0', "filter", NULL,
>>> "[!]FILTER", "Set a filter (with --vars/funcs only)\n"
>>> "\t\t\t(default: \"" DEFAULT_VAR_FILTER "\" for --vars,\n"
>>> @@ -462,16 +438,13 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>>> if (params.max_probe_points == 0)
>>> params.max_probe_points = MAX_PROBES;
>>>
>>> - if ((!params.nevents && !params.del_events && !params.list_events &&
>>> - !params.show_lines && !params.show_funcs))
>>> - usage_with_options(probe_usage, options);
>>> -
>>> /*
>>> * Only consider the user's kernel image path if given.
>>> */
>>> symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
>>>
>>> - if (params.list_events) {
>>> + switch (params.command) {
>>> + case 'l':
>>> if (params.uprobes) {
>>> pr_warning(" Error: Don't use --list with --exec.\n");
>>> usage_with_options(probe_usage, options);
>>> @@ -480,24 +453,20 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>>> if (ret < 0)
>>> pr_err_with_code(" Error: Failed to show event list.", ret);
>>> return ret;
>>> - }
>>> - if (params.show_funcs) {
>>> + case 'F':
>>> ret = show_available_funcs(params.target, params.filter,
>>> params.uprobes);
>>> if (ret < 0)
>>> pr_err_with_code(" Error: Failed to show functions.", ret);
>>> return ret;
>>> - }
>>> -
>>> #ifdef HAVE_DWARF_SUPPORT
>>> - if (params.show_lines) {
>>> + case 'L':
>>> ret = show_line_range(&params.line_range, params.target,
>>> params.uprobes);
>>> if (ret < 0)
>>> pr_err_with_code(" Error: Failed to show lines.", ret);
>>> return ret;
>>> - }
>>> - if (params.show_vars) {
>>> + case 'V':
>>> if (!params.filter)
>>> params.filter = strfilter__new(DEFAULT_VAR_FILTER,
>>> NULL);
>>> @@ -510,18 +479,15 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>>> if (ret < 0)
>>> pr_err_with_code(" Error: Failed to show vars.", ret);
>>> return ret;
>>> - }
>>> #endif
>>> -
>>> - if (params.del_events) {
>>> + case 'd':
>>> ret = del_perf_probe_events(params.filter);
>>> if (ret < 0) {
>>> pr_err_with_code(" Error: Failed to delete events.", ret);
>>> return ret;
>>> }
>>> - }
>>> -
>>> - if (params.nevents) {
>>> + break;
>>> + case 'a':
>>> /* Ensure the last given target is used */
>>> if (params.target && !params.target_used) {
>>> pr_warning(" Error: -x/-m must follow the probe definitions.\n");
>>> @@ -535,6 +501,9 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>>> pr_err_with_code(" Error: Failed to add events.", ret);
>>> return ret;
>>> }
>>> + break;
>>> + default:
>>> + usage_with_options(probe_usage, options);
>>> }
>>> return 0;
>>> }
>>>
>>> --
>>> 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
>> Linux Technology Research Center, System Productivity Research Dept.
>> Center for Technology Innovation - Systems Engineering
>> Hitachi, Ltd., Research & Development Group
>> E-mail: [email protected]
>


--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]

2015-05-05 16:10:55

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH perf/core v4 2/4] perf probe: Accept filter argument for --funcs

Em Wed, May 06, 2015 at 12:10:28AM +0900, Masami Hiramatsu escreveu:
> On 2015/05/05 23:31, Arnaldo Carvalho de Melo wrote:
> > Em Tue, May 05, 2015 at 11:29:50AM +0900, Masami Hiramatsu escreveu:
> >> This allows user to pass the filter pattern directly to
> >> --funcs option as below.
> >> ----
> >> # ./perf probe -F *kmalloc
> >> __kmalloc
> >> devm_kmalloc
> >> mempool_kmalloc
> >> sg_kmalloc
> >> sock_kmalloc
> >> ----
> >
> > Tested, works well, so I went to try and use it in another option:
> >
> > [root@ssdandy acme]# perf probe kmalloc*
> > Probe point 'kmalloc*' not found.
> > Error: Failed to add events.
> > [root@ssdandy acme]#
>
> Well, I'm already preparing it ;)
>
> BTW, what patches have you applied? Can I update 4/4 or would I
> better make another bugfix?


perf/core, that has 4/4 removed, waiting for that respin you mentioned,
i.e. --add should be the default when no commands informed.


> Thank you,
>
> >
> > 8-)
> >
> > - Arnaldo
> >
>
>
> --
> Masami HIRAMATSU
> Linux Technology Research Center, System Productivity Research Dept.
> Center for Technology Innovation - Systems Engineering
> Hitachi, Ltd., Research & Development Group
> E-mail: [email protected]

Subject: [tip:perf/core] perf tools: Improve strfilter to append additional rules

Commit-ID: 4e60a2caefd1920867a84b978abc1eac118de596
Gitweb: http://git.kernel.org/tip/4e60a2caefd1920867a84b978abc1eac118de596
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 24 Apr 2015 18:47:44 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 4 May 2015 12:43:53 -0300

perf tools: Improve strfilter to append additional rules

Add strfilter__or/and to append additional rules to existing strfilter.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/strfilter.c | 40 ++++++++++++++++++++++++++++++++++++++++
tools/perf/util/strfilter.h | 26 ++++++++++++++++++++++++++
2 files changed, 66 insertions(+)

diff --git a/tools/perf/util/strfilter.c b/tools/perf/util/strfilter.c
index 79a757a..f3429cd 100644
--- a/tools/perf/util/strfilter.c
+++ b/tools/perf/util/strfilter.c
@@ -170,6 +170,46 @@ struct strfilter *strfilter__new(const char *rules, const char **err)
return filter;
}

+static int strfilter__append(struct strfilter *filter, bool _or,
+ const char *rules, const char **err)
+{
+ struct strfilter_node *right, *root;
+ const char *ep = NULL;
+
+ if (!filter || !rules)
+ return -EINVAL;
+
+ right = strfilter_node__new(rules, &ep);
+ if (!right || *ep != '\0') {
+ if (err)
+ *err = ep;
+ goto error;
+ }
+ root = strfilter_node__alloc(_or ? OP_or : OP_and, filter->root, right);
+ if (!root) {
+ ep = NULL;
+ goto error;
+ }
+
+ filter->root = root;
+ return 0;
+
+error:
+ strfilter_node__delete(right);
+ return ep ? -EINVAL : -ENOMEM;
+}
+
+int strfilter__or(struct strfilter *filter, const char *rules, const char **err)
+{
+ return strfilter__append(filter, true, rules, err);
+}
+
+int strfilter__and(struct strfilter *filter, const char *rules,
+ const char **err)
+{
+ return strfilter__append(filter, false, rules, err);
+}
+
static bool strfilter_node__compare(struct strfilter_node *node,
const char *str)
{
diff --git a/tools/perf/util/strfilter.h b/tools/perf/util/strfilter.h
index fe611f3..d007cdc 100644
--- a/tools/perf/util/strfilter.h
+++ b/tools/perf/util/strfilter.h
@@ -29,6 +29,32 @@ struct strfilter {
struct strfilter *strfilter__new(const char *rules, const char **err);

/**
+ * strfilter__or - Append an additional rule by logical-or
+ * @filter: Original string filter
+ * @rules: Filter rule to be appended at left of the root of
+ * @filter by using logical-or.
+ * @err: Pointer which points an error detected on @rules
+ *
+ * Parse @rules and join it to the @filter by using logical-or.
+ * Return 0 if success, or return the error code.
+ */
+int strfilter__or(struct strfilter *filter,
+ const char *rules, const char **err);
+
+/**
+ * strfilter__add - Append an additional rule by logical-and
+ * @filter: Original string filter
+ * @rules: Filter rule to be appended at left of the root of
+ * @filter by using logical-and.
+ * @err: Pointer which points an error detected on @rules
+ *
+ * Parse @rules and join it to the @filter by using logical-and.
+ * Return 0 if success, or return the error code.
+ */
+int strfilter__and(struct strfilter *filter,
+ const char *rules, const char **err);
+
+/**
* strfilter__compare - compare given string and a string filter
* @filter: String filter
* @str: target string

Subject: [tip:perf/core] perf tools: Add strfilter__string to recover rules string

Commit-ID: 3f51972c599cf95702819bd06a7a5412c523ebfe
Gitweb: http://git.kernel.org/tip/3f51972c599cf95702819bd06a7a5412c523ebfe
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 24 Apr 2015 18:47:46 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 4 May 2015 12:43:54 -0300

perf tools: Add strfilter__string to recover rules string

Add strfilter__string to recover rules string from strfilter. This will
be good for debugging.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/strfilter.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/strfilter.h | 9 ++++++
2 files changed, 76 insertions(+)

diff --git a/tools/perf/util/strfilter.c b/tools/perf/util/strfilter.c
index f3429cd..bcae659 100644
--- a/tools/perf/util/strfilter.c
+++ b/tools/perf/util/strfilter.c
@@ -237,3 +237,70 @@ bool strfilter__compare(struct strfilter *filter, const char *str)
return false;
return strfilter_node__compare(filter->root, str);
}
+
+static int strfilter_node__sprint(struct strfilter_node *node, char *buf);
+
+/* sprint node in parenthesis if needed */
+static int strfilter_node__sprint_pt(struct strfilter_node *node, char *buf)
+{
+ int len;
+ int pt = node->r ? 2 : 0; /* don't need to check node->l */
+
+ if (buf && pt)
+ *buf++ = '(';
+ len = strfilter_node__sprint(node, buf);
+ if (len < 0)
+ return len;
+ if (buf && pt)
+ *(buf + len) = ')';
+ return len + pt;
+}
+
+static int strfilter_node__sprint(struct strfilter_node *node, char *buf)
+{
+ int len = 0, rlen;
+
+ if (!node || !node->p)
+ return -EINVAL;
+
+ switch (*node->p) {
+ case '|':
+ case '&':
+ len = strfilter_node__sprint_pt(node->l, buf);
+ if (len < 0)
+ return len;
+ case '!':
+ if (buf) {
+ *(buf + len++) = *node->p;
+ buf += len;
+ } else
+ len++;
+ rlen = strfilter_node__sprint_pt(node->r, buf);
+ if (rlen < 0)
+ return rlen;
+ len += rlen;
+ break;
+ default:
+ len = strlen(node->p);
+ if (buf)
+ strcpy(buf, node->p);
+ }
+
+ return len;
+}
+
+char *strfilter__string(struct strfilter *filter)
+{
+ int len;
+ char *ret = NULL;
+
+ len = strfilter_node__sprint(filter->root, NULL);
+ if (len < 0)
+ return NULL;
+
+ ret = malloc(len + 1);
+ if (ret)
+ strfilter_node__sprint(filter->root, ret);
+
+ return ret;
+}
diff --git a/tools/perf/util/strfilter.h b/tools/perf/util/strfilter.h
index d007cdc..cff5eda 100644
--- a/tools/perf/util/strfilter.h
+++ b/tools/perf/util/strfilter.h
@@ -71,4 +71,13 @@ bool strfilter__compare(struct strfilter *filter, const char *str);
*/
void strfilter__delete(struct strfilter *filter);

+/**
+ * strfilter__string - Reconstruct a rule string from filter
+ * @filter: String filter to reconstruct
+ *
+ * Reconstruct a rule string from @filter. This will be good for
+ * debug messages. Note that returning string must be freed afterward.
+ */
+char *strfilter__string(struct strfilter *filter);
+
#endif

Subject: [tip:perf/core] perf probe: Accept multiple filter options

Commit-ID: 96b55e39237b8bc92d8e6b96f896c106f2d39cf6
Gitweb: http://git.kernel.org/tip/96b55e39237b8bc92d8e6b96f896c106f2d39cf6
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 24 Apr 2015 18:47:48 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 4 May 2015 12:43:55 -0300

perf probe: Accept multiple filter options

Accept multiple filter options. Each filters are combined by logical-or.
E.g. --filter abc* --filter *def is same as --filter abc*|*def

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-probe.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 92dcce0..be17075 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -262,21 +262,25 @@ static int opt_set_filter(const struct option *opt __maybe_unused,
const char *str, int unset __maybe_unused)
{
const char *err;
+ int ret = 0;

if (str) {
pr_debug2("Set filter: %s\n", str);
- if (params.filter)
- strfilter__delete(params.filter);
- params.filter = strfilter__new(str, &err);
if (!params.filter) {
+ params.filter = strfilter__new(str, &err);
+ if (!params.filter)
+ ret = err ? -EINVAL : -ENOMEM;
+ } else
+ ret = strfilter__or(params.filter, str, &err);
+
+ if (ret == -EINVAL) {
pr_err("Filter parse error at %td.\n", err - str + 1);
pr_err("Source: \"%s\"\n", str);
pr_err(" %*c\n", (int)(err - str + 1), '^');
- return -EINVAL;
}
}

- return 0;
+ return ret;
}

static int init_params(void)

Subject: [tip:perf/core] perf probe: Accept filter argument for --list

Commit-ID: b6a896438b3275df434a8f99bee58292b31693bd
Gitweb: http://git.kernel.org/tip/b6a896438b3275df434a8f99bee58292b31693bd
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 24 Apr 2015 18:47:50 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 4 May 2015 12:43:56 -0300

perf probe: Accept filter argument for --list

Currently, perf-probe --list option ignores given event filter.
----
# ./perf probe -l vfs\*
probe:vfs_read (on vfs_read@ksrc/linux-3/fs/read_write.c)
probe_libc:malloc (on __libc_malloc@malloc/malloc.c in /usr/lib64/libc-2.17.so)
----

This changes --list option to accept the event filter argument as below.
----
# ./perf probe -l vfs\*
probe:vfs_read (on vfs_read@ksrc/linux-3/fs/read_write.c)
# ./perf probe -l \*libc:\*
probe_libc:malloc (on __libc_malloc@malloc/malloc.c in /usr/lib64/libc-2.17.so)
----

Signed-off-by: Masami Hiramatsu <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Documentation/perf-probe.txt | 6 +--
tools/perf/builtin-probe.c | 70 ++++++++++++++++++++++-----------
tools/perf/util/probe-event.c | 27 +++++++++++--
tools/perf/util/probe-event.h | 2 +-
4 files changed, 73 insertions(+), 32 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index a4a3cc7..d0feb8e 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -14,7 +14,7 @@ or
or
'perf probe' [options] --del='[GROUP:]EVENT' [...]
or
-'perf probe' --list
+'perf probe' --list[=[GROUP:]EVENT]
or
'perf probe' [options] --line='LINE'
or
@@ -66,8 +66,8 @@ OPTIONS
classes(e.g. [a-z], [!A-Z]).

-l::
---list::
- List up current probe events.
+--list[=[GROUP:]EVENT]::
+ List up current probe events. This can also accept filtering patterns of event names.

-L::
--line=::
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index be17075..feca316 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -44,6 +44,7 @@

#define DEFAULT_VAR_FILTER "!__k???tab_* & !__crc_*"
#define DEFAULT_FUNC_FILTER "!_*"
+#define DEFAULT_LIST_FILTER "*:*"

/* Session management structure */
static struct {
@@ -93,6 +94,28 @@ static int parse_probe_event(const char *str)
return ret;
}

+static int params_add_filter(const char *str)
+{
+ const char *err = NULL;
+ int ret = 0;
+
+ pr_debug2("Add filter: %s\n", str);
+ if (!params.filter) {
+ params.filter = strfilter__new(str, &err);
+ if (!params.filter)
+ ret = err ? -EINVAL : -ENOMEM;
+ } else
+ ret = strfilter__or(params.filter, str, &err);
+
+ if (ret == -EINVAL) {
+ pr_err("Filter parse error at %td.\n", err - str + 1);
+ pr_err("Source: \"%s\"\n", str);
+ pr_err(" %*c\n", (int)(err - str + 1), '^');
+ }
+
+ return ret;
+}
+
static int set_target(const char *ptr)
{
int found = 0;
@@ -180,6 +203,18 @@ static int opt_del_probe_event(const struct option *opt __maybe_unused,
return 0;
}

+static int opt_list_probe_event(const struct option *opt __maybe_unused,
+ const char *str, int unset)
+{
+ if (!unset)
+ params.list_events = true;
+
+ if (str)
+ return params_add_filter(str);
+
+ return 0;
+}
+
static int opt_set_target(const struct option *opt, const char *str,
int unset __maybe_unused)
{
@@ -261,26 +296,10 @@ static int opt_show_vars(const struct option *opt __maybe_unused,
static int opt_set_filter(const struct option *opt __maybe_unused,
const char *str, int unset __maybe_unused)
{
- const char *err;
- int ret = 0;
+ if (str)
+ return params_add_filter(str);

- if (str) {
- pr_debug2("Set filter: %s\n", str);
- if (!params.filter) {
- params.filter = strfilter__new(str, &err);
- if (!params.filter)
- ret = err ? -EINVAL : -ENOMEM;
- } else
- ret = strfilter__or(params.filter, str, &err);
-
- if (ret == -EINVAL) {
- pr_err("Filter parse error at %td.\n", err - str + 1);
- pr_err("Source: \"%s\"\n", str);
- pr_err(" %*c\n", (int)(err - str + 1), '^');
- }
- }
-
- return ret;
+ return 0;
}

static int init_params(void)
@@ -320,21 +339,22 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
"perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]",
"perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]",
"perf probe [<options>] --del '[GROUP:]EVENT' ...",
- "perf probe --list",
+ "perf probe --list [GROUP:]EVENT ...",
#ifdef HAVE_DWARF_SUPPORT
"perf probe [<options>] --line 'LINEDESC'",
"perf probe [<options>] --vars 'PROBEPOINT'",
#endif
"perf probe [<options>] --funcs",
NULL
-};
+ };
struct option options[] = {
OPT_INCR('v', "verbose", &verbose,
"be more verbose (show parsed arguments, etc)"),
OPT_BOOLEAN('q', "quiet", &params.quiet,
"be quiet (do not show any mesages)"),
- OPT_BOOLEAN('l', "list", &params.list_events,
- "list up current probe events"),
+ OPT_CALLBACK_DEFAULT('l', "list", NULL, "[GROUP:]EVENT",
+ "list up probe events", opt_list_probe_event,
+ DEFAULT_LIST_FILTER),
OPT_CALLBACK('d', "del", NULL, "[GROUP:]EVENT", "delete a probe event.",
opt_del_probe_event),
OPT_CALLBACK('a', "add", NULL,
@@ -448,7 +468,9 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
pr_warning(" Error: Don't use --list with --exec.\n");
usage_with_options(probe_usage, options);
}
- ret = show_perf_probe_events();
+ ret = show_perf_probe_events(params.filter);
+ strfilter__delete(params.filter);
+ params.filter = NULL;
if (ret < 0)
pr_err_with_code(" Error: Failed to show event list.", ret);
return ret;
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 416c10f..5995d81 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2146,7 +2146,23 @@ static int show_perf_probe_event(struct perf_probe_event *pev,
return ret;
}

-static int __show_perf_probe_events(int fd, bool is_kprobe)
+static bool filter_probe_trace_event(struct probe_trace_event *tev,
+ struct strfilter *filter)
+{
+ char tmp[128];
+
+ /* At first, check the event name itself */
+ if (strfilter__compare(filter, tev->event))
+ return true;
+
+ /* Next, check the combination of name and group */
+ if (e_snprintf(tmp, 128, "%s:%s", tev->group, tev->event) < 0)
+ return false;
+ return strfilter__compare(filter, tmp);
+}
+
+static int __show_perf_probe_events(int fd, bool is_kprobe,
+ struct strfilter *filter)
{
int ret = 0;
struct probe_trace_event tev;
@@ -2164,12 +2180,15 @@ static int __show_perf_probe_events(int fd, bool is_kprobe)
strlist__for_each(ent, rawlist) {
ret = parse_probe_trace_command(ent->s, &tev);
if (ret >= 0) {
+ if (!filter_probe_trace_event(&tev, filter))
+ goto next;
ret = convert_to_perf_probe_event(&tev, &pev,
is_kprobe);
if (ret >= 0)
ret = show_perf_probe_event(&pev,
tev.point.module);
}
+next:
clear_perf_probe_event(&pev);
clear_probe_trace_event(&tev);
if (ret < 0)
@@ -2181,7 +2200,7 @@ static int __show_perf_probe_events(int fd, bool is_kprobe)
}

/* List up current perf-probe events */
-int show_perf_probe_events(void)
+int show_perf_probe_events(struct strfilter *filter)
{
int kp_fd, up_fd, ret;

@@ -2193,7 +2212,7 @@ int show_perf_probe_events(void)

kp_fd = open_kprobe_events(false);
if (kp_fd >= 0) {
- ret = __show_perf_probe_events(kp_fd, true);
+ ret = __show_perf_probe_events(kp_fd, true, filter);
close(kp_fd);
if (ret < 0)
goto out;
@@ -2207,7 +2226,7 @@ int show_perf_probe_events(void)
}

if (up_fd >= 0) {
- ret = __show_perf_probe_events(up_fd, false);
+ ret = __show_perf_probe_events(up_fd, false, filter);
close(up_fd);
}
out:
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 180f142..ec13362 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -127,7 +127,7 @@ extern const char *kernel_get_module_path(const char *module);
extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
int max_probe_points, bool force_add);
extern int del_perf_probe_events(struct strlist *dellist);
-extern int show_perf_probe_events(void);
+extern int show_perf_probe_events(struct strfilter *filter);
extern int show_line_range(struct line_range *lr, const char *module,
bool user);
extern int show_available_vars(struct perf_probe_event *pevs, int npevs,

Subject: [tip:perf/core] perf probe: Allow to use filter on --del command

Commit-ID: 307a464b2342a502da492f0ada8cefd6ab7f63a7
Gitweb: http://git.kernel.org/tip/307a464b2342a502da492f0ada8cefd6ab7f63a7
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Tue, 5 May 2015 11:29:48 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 5 May 2015 18:13:02 -0300

perf probe: Allow to use filter on --del command

This makes perf-probe --del option to accept filter rules
not only simple glob pattern. This simplifies the code and
improve the flexibility.

E.g. if we remove 2 different pattern events, we need 2
-d options.
----
# ./perf probe -d vfs\* -d malloc
Removed event: probe_libc:malloc
Removed event: probe:vfs_read
----

This allows you to joint the 2 patterns with '|'.

----
# ./perf probe -d 'vfs*|malloc'
Removed event: probe:vfs_read
Removed event: probe_libc:malloc
----

Signed-off-by: Masami Hiramatsu <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-probe.c | 16 +++----
tools/perf/util/probe-event.c | 102 +++++++++++++-----------------------------
tools/perf/util/probe-event.h | 2 +-
3 files changed, 39 insertions(+), 81 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index feca316..1f41b4e 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -55,12 +55,12 @@ static struct {
bool show_ext_vars;
bool show_funcs;
bool mod_events;
+ bool del_events;
bool uprobes;
bool quiet;
bool target_used;
int nevents;
struct perf_probe_event events[MAX_PROBES];
- struct strlist *dellist;
struct line_range line_range;
char *target;
int max_probe_points;
@@ -195,10 +195,8 @@ static int opt_del_probe_event(const struct option *opt __maybe_unused,
const char *str, int unset __maybe_unused)
{
if (str) {
- params.mod_events = true;
- if (!params.dellist)
- params.dellist = strlist__new(true, NULL);
- strlist__add(params.dellist, str);
+ params.del_events = true;
+ return params_add_filter(str);
}
return 0;
}
@@ -313,8 +311,6 @@ static void cleanup_params(void)

for (i = 0; i < params.nevents; i++)
clear_perf_probe_event(params.events + i);
- if (params.dellist)
- strlist__delete(params.dellist);
line_range__clear(&params.line_range);
free(params.target);
if (params.filter)
@@ -454,7 +450,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (params.max_probe_points == 0)
params.max_probe_points = MAX_PROBES;

- if ((!params.nevents && !params.dellist && !params.list_events &&
+ if ((!params.nevents && !params.del_events && !params.list_events &&
!params.show_lines && !params.show_funcs))
usage_with_options(probe_usage, options);

@@ -514,8 +510,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
}
#endif

- if (params.dellist) {
- ret = del_perf_probe_events(params.dellist);
+ if (params.del_events) {
+ ret = del_perf_probe_events(params.filter);
if (ret < 0) {
pr_err_with_code(" Error: Failed to delete events.", ret);
return ret;
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 5995d81..abf5845 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2734,40 +2734,39 @@ error:
return ret;
}

-static int del_trace_probe_event(int fd, const char *buf,
- struct strlist *namelist)
+static int del_trace_probe_events(int fd, struct strfilter *filter,
+ struct strlist *namelist)
{
- struct str_node *ent, *n;
+ struct str_node *ent;
+ const char *p;
int ret = -ENOENT;

- if (strpbrk(buf, "*?")) { /* Glob-exp */
- strlist__for_each_safe(ent, n, namelist)
- if (strglobmatch(ent->s, buf)) {
- ret = __del_trace_probe_event(fd, ent);
- if (ret < 0)
- break;
- strlist__remove(namelist, ent);
- }
- } else {
- ent = strlist__find(namelist, buf);
- if (ent) {
+ if (!namelist)
+ return -ENOENT;
+
+ strlist__for_each(ent, namelist) {
+ p = strchr(ent->s, ':');
+ if ((p && strfilter__compare(filter, p + 1)) ||
+ strfilter__compare(filter, ent->s)) {
ret = __del_trace_probe_event(fd, ent);
- if (ret >= 0)
- strlist__remove(namelist, ent);
+ if (ret < 0)
+ break;
}
}

return ret;
}

-int del_perf_probe_events(struct strlist *dellist)
+int del_perf_probe_events(struct strfilter *filter)
{
- int ret = -1, ret2, ufd = -1, kfd = -1;
- char buf[128];
- const char *group, *event;
- char *p, *str;
- struct str_node *ent;
+ int ret, ret2, ufd = -1, kfd = -1;
struct strlist *namelist = NULL, *unamelist = NULL;
+ char *str = strfilter__string(filter);
+
+ if (!str)
+ return -EINVAL;
+
+ pr_debug("Delete filter: \'%s\'\n", str);

/* Get current event names */
kfd = open_kprobe_events(true);
@@ -2780,59 +2779,21 @@ int del_perf_probe_events(struct strlist *dellist)

if (kfd < 0 && ufd < 0) {
print_both_open_warning(kfd, ufd);
+ ret = kfd;
goto error;
}

- if (namelist == NULL && unamelist == NULL) {
- ret = -ENOENT;
+ ret = del_trace_probe_events(kfd, filter, namelist);
+ if (ret < 0 && ret != -ENOENT)
goto error;
- }
-
- strlist__for_each(ent, dellist) {
- str = strdup(ent->s);
- if (str == NULL) {
- ret = -ENOMEM;
- goto error;
- }
- pr_debug("Parsing: %s\n", str);
- p = strchr(str, ':');
- if (p) {
- group = str;
- *p = '\0';
- event = p + 1;
- } else {
- group = "*";
- event = str;
- }

- if (event && *event == '.')
- event++;
-
- ret = e_snprintf(buf, 128, "%s:%s", group, event);
- if (ret < 0) {
- pr_err("Failed to copy event.");
- free(str);
- goto error;
- }
-
- pr_debug("Group: %s, Event: %s\n", group, event);
- free(str);
-
- ret = ret2 = -ENOENT;
- if (namelist)
- ret = del_trace_probe_event(kfd, buf, namelist);
-
- if ((ret >= 0 || ret == -ENOENT) && unamelist)
- ret2 = del_trace_probe_event(ufd, buf, unamelist);
-
- /* Since we can remove probes which already removed, don't check it */
- if (ret == -ENOENT && ret2 == -ENOENT)
- pr_debug("Event \"%s\" does not exist.\n", buf);
- else if (ret < 0 || ret2 < 0) {
- if (ret >= 0)
- ret = ret2;
- break;
- }
+ ret2 = del_trace_probe_events(ufd, filter, unamelist);
+ if (ret2 < 0 && ret2 != -ENOENT)
+ ret = ret2;
+ else if (ret == -ENOENT && ret2 == -ENOENT) {
+ pr_debug("\"%s\" does not hit any event.\n", str);
+ /* Note that this is silently ignored */
+ ret = 0;
}

error:
@@ -2845,6 +2806,7 @@ error:
strlist__delete(unamelist);
close(ufd);
}
+ free(str);

return ret;
}
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index ec13362..e10aedc 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -126,7 +126,7 @@ extern const char *kernel_get_module_path(const char *module);

extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
int max_probe_points, bool force_add);
-extern int del_perf_probe_events(struct strlist *dellist);
+extern int del_perf_probe_events(struct strfilter *filter);
extern int show_perf_probe_events(struct strfilter *filter);
extern int show_line_range(struct line_range *lr, const char *module,
bool user);

Subject: [tip:perf/core] perf probe: Accept filter argument for --funcs

Commit-ID: 9f7811d08dcf7b3e900cbc0d8384b713a86b034f
Gitweb: http://git.kernel.org/tip/9f7811d08dcf7b3e900cbc0d8384b713a86b034f
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Tue, 5 May 2015 11:29:50 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 5 May 2015 18:13:04 -0300

perf probe: Accept filter argument for --funcs

This allows the user to pass the filter pattern directly to the --funcs
option as below:

----
# ./perf probe -F *kmalloc
__kmalloc
devm_kmalloc
mempool_kmalloc
sg_kmalloc
sock_kmalloc
----

We previously needed to use the --filter option for that.

Signed-off-by: Masami Hiramatsu <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Documentation/perf-probe.txt | 3 ++-
tools/perf/builtin-probe.c | 19 ++++++++++++++-----
2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index d0feb8e..a272f2e 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -84,9 +84,10 @@ OPTIONS
variables.

-F::
---funcs::
+--funcs[=FILTER]::
Show available functions in given module or kernel. With -x/--exec,
can also list functions in a user space executable / shared library.
+ This also can accept a FILTER rule argument.

--filter=FILTER::
(Only for --vars and --funcs) Set filter. FILTER is a combination of glob
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 1f41b4e..5a0e8f1 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -290,6 +290,17 @@ static int opt_show_vars(const struct option *opt __maybe_unused,
return ret;
}
#endif
+static int opt_show_funcs(const struct option *opt __maybe_unused,
+ const char *str, int unset)
+{
+ if (!unset)
+ params.show_funcs = true;
+
+ if (str)
+ return params_add_filter(str);
+
+ return 0;
+}

static int opt_set_filter(const struct option *opt __maybe_unused,
const char *str, int unset __maybe_unused)
@@ -399,8 +410,9 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
OPT__DRY_RUN(&probe_event_dry_run),
OPT_INTEGER('\0', "max-probes", &params.max_probe_points,
"Set how many probe points can be found for a probe."),
- OPT_BOOLEAN('F', "funcs", &params.show_funcs,
- "Show potential probe-able functions."),
+ OPT_CALLBACK_DEFAULT('F', "funcs", NULL, "[FILTER]",
+ "Show potential probe-able functions.",
+ opt_show_funcs, DEFAULT_FUNC_FILTER),
OPT_CALLBACK('\0', "filter", NULL,
"[!]FILTER", "Set a filter (with --vars/funcs only)\n"
"\t\t\t(default: \"" DEFAULT_VAR_FILTER "\" for --vars,\n"
@@ -472,9 +484,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
return ret;
}
if (params.show_funcs) {
- if (!params.filter)
- params.filter = strfilter__new(DEFAULT_FUNC_FILTER,
- NULL);
ret = show_available_funcs(params.target, params.filter,
params.uprobes);
strfilter__delete(params.filter);

Subject: [tip:perf/core] perf probe: Remove redundant cleanup of params.filter

Commit-ID: 3da166b7b5253aaa6b36410f5d4c4a996ee5915d
Gitweb: http://git.kernel.org/tip/3da166b7b5253aaa6b36410f5d4c4a996ee5915d
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Tue, 5 May 2015 11:29:52 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 5 May 2015 18:13:05 -0300

perf probe: Remove redundant cleanup of params.filter

Since params.filter will be released in cleanup_params, we don't need to
clear it in each command.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-probe.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 5a0e8f1..08c9481 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -477,8 +477,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
usage_with_options(probe_usage, options);
}
ret = show_perf_probe_events(params.filter);
- strfilter__delete(params.filter);
- params.filter = NULL;
if (ret < 0)
pr_err_with_code(" Error: Failed to show event list.", ret);
return ret;
@@ -486,8 +484,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (params.show_funcs) {
ret = show_available_funcs(params.target, params.filter,
params.uprobes);
- strfilter__delete(params.filter);
- params.filter = NULL;
if (ret < 0)
pr_err_with_code(" Error: Failed to show functions.", ret);
return ret;
@@ -511,8 +507,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
params.target,
params.filter,
params.show_ext_vars);
- strfilter__delete(params.filter);
- params.filter = NULL;
if (ret < 0)
pr_err_with_code(" Error: Failed to show vars.", ret);
return ret;

Subject: [tip:perf/core] perf probe: Cleanup and consolidate command parsers

Commit-ID: b1019d5e6e605190d008003a382407f23e19f807
Gitweb: http://git.kernel.org/tip/b1019d5e6e605190d008003a382407f23e19f807
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 6 May 2015 00:22:57 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 5 May 2015 18:13:07 -0300

perf probe: Cleanup and consolidate command parsers

To simplify the 'perf probe' command code, consolidate some similar
functions and use command short-name for command classification, instead
of separate booleans.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-probe.c | 110 +++++++++++++++++----------------------------
1 file changed, 42 insertions(+), 68 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 08c9481..53d475b 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -48,14 +48,10 @@

/* Session management structure */
static struct {
+ int command; /* Command short_name */
bool list_events;
bool force_add;
- bool show_lines;
- bool show_vars;
bool show_ext_vars;
- bool show_funcs;
- bool mod_events;
- bool del_events;
bool uprobes;
bool quiet;
bool target_used;
@@ -175,44 +171,11 @@ static int parse_probe_event_argv(int argc, const char **argv)

len += sprintf(&buf[len], "%s ", argv[i]);
}
- params.mod_events = true;
ret = parse_probe_event(buf);
free(buf);
return ret;
}

-static int opt_add_probe_event(const struct option *opt __maybe_unused,
- const char *str, int unset __maybe_unused)
-{
- if (str) {
- params.mod_events = true;
- return parse_probe_event(str);
- } else
- return 0;
-}
-
-static int opt_del_probe_event(const struct option *opt __maybe_unused,
- const char *str, int unset __maybe_unused)
-{
- if (str) {
- params.del_events = true;
- return params_add_filter(str);
- }
- return 0;
-}
-
-static int opt_list_probe_event(const struct option *opt __maybe_unused,
- const char *str, int unset)
-{
- if (!unset)
- params.list_events = true;
-
- if (str)
- return params_add_filter(str);
-
- return 0;
-}
-
static int opt_set_target(const struct option *opt, const char *str,
int unset __maybe_unused)
{
@@ -250,8 +213,10 @@ static int opt_set_target(const struct option *opt, const char *str,
return ret;
}

+/* Command option callbacks */
+
#ifdef HAVE_DWARF_SUPPORT
-static int opt_show_lines(const struct option *opt __maybe_unused,
+static int opt_show_lines(const struct option *opt,
const char *str, int unset __maybe_unused)
{
int ret = 0;
@@ -259,19 +224,19 @@ static int opt_show_lines(const struct option *opt __maybe_unused,
if (!str)
return 0;

- if (params.show_lines) {
+ if (params.command == 'L') {
pr_warning("Warning: more than one --line options are"
" detected. Only the first one is valid.\n");
return 0;
}

- params.show_lines = true;
+ params.command = opt->short_name;
ret = parse_line_range_desc(str, &params.line_range);

return ret;
}

-static int opt_show_vars(const struct option *opt __maybe_unused,
+static int opt_show_vars(const struct option *opt,
const char *str, int unset __maybe_unused)
{
struct perf_probe_event *pev = &params.events[params.nevents];
@@ -285,16 +250,27 @@ static int opt_show_vars(const struct option *opt __maybe_unused,
pr_err(" Error: '--vars' doesn't accept arguments.\n");
return -EINVAL;
}
- params.show_vars = true;
+ params.command = opt->short_name;

return ret;
}
#endif
-static int opt_show_funcs(const struct option *opt __maybe_unused,
- const char *str, int unset)
+static int opt_add_probe_event(const struct option *opt,
+ const char *str, int unset __maybe_unused)
+{
+ if (str) {
+ params.command = opt->short_name;
+ return parse_probe_event(str);
+ }
+
+ return 0;
+}
+
+static int opt_set_filter_with_command(const struct option *opt,
+ const char *str, int unset)
{
if (!unset)
- params.show_funcs = true;
+ params.command = opt->short_name;

if (str)
return params_add_filter(str);
@@ -360,10 +336,10 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
OPT_BOOLEAN('q', "quiet", &params.quiet,
"be quiet (do not show any mesages)"),
OPT_CALLBACK_DEFAULT('l', "list", NULL, "[GROUP:]EVENT",
- "list up probe events", opt_list_probe_event,
- DEFAULT_LIST_FILTER),
+ "list up probe events",
+ opt_set_filter_with_command, DEFAULT_LIST_FILTER),
OPT_CALLBACK('d', "del", NULL, "[GROUP:]EVENT", "delete a probe event.",
- opt_del_probe_event),
+ opt_set_filter_with_command),
OPT_CALLBACK('a', "add", NULL,
#ifdef HAVE_DWARF_SUPPORT
"[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT"
@@ -412,7 +388,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
"Set how many probe points can be found for a probe."),
OPT_CALLBACK_DEFAULT('F', "funcs", NULL, "[FILTER]",
"Show potential probe-able functions.",
- opt_show_funcs, DEFAULT_FUNC_FILTER),
+ opt_set_filter_with_command, DEFAULT_FUNC_FILTER),
OPT_CALLBACK('\0', "filter", NULL,
"[!]FILTER", "Set a filter (with --vars/funcs only)\n"
"\t\t\t(default: \"" DEFAULT_VAR_FILTER "\" for --vars,\n"
@@ -444,11 +420,16 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
pr_warning(" Error: '-' is not supported.\n");
usage_with_options(probe_usage, options);
}
+ if (params.command && params.command != 'a') {
+ pr_warning(" Error: another command except --add is set.\n");
+ usage_with_options(probe_usage, options);
+ }
ret = parse_probe_event_argv(argc, argv);
if (ret < 0) {
pr_err_with_code(" Error: Command Parse Error.", ret);
return ret;
}
+ params.command = 'a';
}

if (params.quiet) {
@@ -462,16 +443,13 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (params.max_probe_points == 0)
params.max_probe_points = MAX_PROBES;

- if ((!params.nevents && !params.del_events && !params.list_events &&
- !params.show_lines && !params.show_funcs))
- usage_with_options(probe_usage, options);
-
/*
* Only consider the user's kernel image path if given.
*/
symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);

- if (params.list_events) {
+ switch (params.command) {
+ case 'l':
if (params.uprobes) {
pr_warning(" Error: Don't use --list with --exec.\n");
usage_with_options(probe_usage, options);
@@ -480,24 +458,20 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (ret < 0)
pr_err_with_code(" Error: Failed to show event list.", ret);
return ret;
- }
- if (params.show_funcs) {
+ case 'F':
ret = show_available_funcs(params.target, params.filter,
params.uprobes);
if (ret < 0)
pr_err_with_code(" Error: Failed to show functions.", ret);
return ret;
- }
-
#ifdef HAVE_DWARF_SUPPORT
- if (params.show_lines) {
+ case 'L':
ret = show_line_range(&params.line_range, params.target,
params.uprobes);
if (ret < 0)
pr_err_with_code(" Error: Failed to show lines.", ret);
return ret;
- }
- if (params.show_vars) {
+ case 'V':
if (!params.filter)
params.filter = strfilter__new(DEFAULT_VAR_FILTER,
NULL);
@@ -510,18 +484,15 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (ret < 0)
pr_err_with_code(" Error: Failed to show vars.", ret);
return ret;
- }
#endif
-
- if (params.del_events) {
+ case 'd':
ret = del_perf_probe_events(params.filter);
if (ret < 0) {
pr_err_with_code(" Error: Failed to delete events.", ret);
return ret;
}
- }
-
- if (params.nevents) {
+ break;
+ case 'a':
/* Ensure the last given target is used */
if (params.target && !params.target_used) {
pr_warning(" Error: -x/-m must follow the probe definitions.\n");
@@ -535,6 +506,9 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
pr_err_with_code(" Error: Failed to add events.", ret);
return ret;
}
+ break;
+ default:
+ usage_with_options(probe_usage, options);
}
return 0;
}