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

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

[Masami Hiramatsu]

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: masami.hiramatsu.pt@hitachi.com

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

[Masami Hiramatsu]

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

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 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

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

[tip-bot for Masami Hiramatsu]

Commit-ID:  4e60a2caefd1920867a84b978abc1eac118de596
Gitweb:     http://git.kernel.org/tip/4e60a2caefd1920867a84b978abc1eac118de596
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 24 Apr 2015 18:47:44 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <masami.hiramatsu.pt@hitachi.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150424094744.23967.97614.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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

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

[Masami Hiramatsu]

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

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 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

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

[tip-bot for Masami Hiramatsu]

Commit-ID:  3f51972c599cf95702819bd06a7a5412c523ebfe
Gitweb:     http://git.kernel.org/tip/3f51972c599cf95702819bd06a7a5412c523ebfe
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 24 Apr 2015 18:47:46 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <masami.hiramatsu.pt@hitachi.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150424094746.23967.52434.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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

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

[Masami Hiramatsu]

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 <masami.hiramatsu.pt@hitachi.com>
---
 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)

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

[tip-bot for Masami Hiramatsu]

Commit-ID:  96b55e39237b8bc92d8e6b96f896c106f2d39cf6
Gitweb:     http://git.kernel.org/tip/96b55e39237b8bc92d8e6b96f896c106f2d39cf6
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 24 Apr 2015 18:47:48 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <masami.hiramatsu.pt@hitachi.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150424094748.23967.63355.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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)

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

[Masami Hiramatsu]

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 <masami.hiramatsu.pt@hitachi.com>
---
 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,

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

[tip-bot for Masami Hiramatsu]

Commit-ID:  b6a896438b3275df434a8f99bee58292b31693bd
Gitweb:     http://git.kernel.org/tip/b6a896438b3275df434a8f99bee58292b31693bd
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 24 Apr 2015 18:47:50 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <masami.hiramatsu.pt@hitachi.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150424094750.23967.53868.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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,

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

[Masami Hiramatsu]

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 <masami.hiramatsu.pt@hitachi.com>
---
 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);

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

[Arnaldo Carvalho de Melo]

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 <naveen.n.rao@linux.vnet.ibm.com>
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

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

[Masami Hiramatsu]

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 <naveen.n.rao@linux.vnet.ibm.com>
> 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: masami.hiramatsu.pt@hitachi.com

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

[Arnaldo Carvalho de Melo]

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 <naveen.n.rao@linux.vnet.ibm.com>
> > 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

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

[Masami Hiramatsu]

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: masami.hiramatsu.pt@hitachi.com

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

[Masami Hiramatsu]

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 <masami.hiramatsu.pt@hitachi.com>
---
 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);

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

[tip-bot for Masami Hiramatsu]

Commit-ID:  307a464b2342a502da492f0ada8cefd6ab7f63a7
Gitweb:     http://git.kernel.org/tip/307a464b2342a502da492f0ada8cefd6ab7f63a7
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Tue, 5 May 2015 11:29:48 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <masami.hiramatsu.pt@hitachi.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150505022948.23399.4197.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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);

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

[Masami Hiramatsu]

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 <masami.hiramatsu.pt@hitachi.com>
---
 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);

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

[Arnaldo Carvalho de Melo]

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

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

[Masami Hiramatsu]

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: masami.hiramatsu.pt@hitachi.com

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

[Arnaldo Carvalho de Melo]

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: masami.hiramatsu.pt@hitachi.com

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

[tip-bot for Masami Hiramatsu]

Commit-ID:  9f7811d08dcf7b3e900cbc0d8384b713a86b034f
Gitweb:     http://git.kernel.org/tip/9f7811d08dcf7b3e900cbc0d8384b713a86b034f
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Tue, 5 May 2015 11:29:50 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <masami.hiramatsu.pt@hitachi.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150505022950.23399.22435.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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);

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

[Masami Hiramatsu]

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

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 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;

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

[tip-bot for Masami Hiramatsu]

Commit-ID:  3da166b7b5253aaa6b36410f5d4c4a996ee5915d
Gitweb:     http://git.kernel.org/tip/3da166b7b5253aaa6b36410f5d4c4a996ee5915d
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Tue, 5 May 2015 11:29:52 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <masami.hiramatsu.pt@hitachi.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150505022952.23399.58072.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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;

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

[Masami Hiramatsu]

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 <masami.hiramatsu.pt@hitachi.com>
---
 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;
 }

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

[Masami Hiramatsu]

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 <masami.hiramatsu.pt@hitachi.com>
> ---
>  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 majordomo@vger.kernel.org
> 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: masami.hiramatsu.pt@hitachi.com

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

[Arnaldo Carvalho de Melo]

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 <masami.hiramatsu.pt@hitachi.com>
> > ---
> >  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 majordomo@vger.kernel.org
> > 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: masami.hiramatsu.pt@hitachi.com

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

[Masami Hiramatsu]

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 <masami.hiramatsu.pt@hitachi.com>
---
 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;
 }

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

[tip-bot for Masami Hiramatsu]

Commit-ID:  b1019d5e6e605190d008003a382407f23e19f807
Gitweb:     http://git.kernel.org/tip/b1019d5e6e605190d008003a382407f23e19f807
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Wed, 6 May 2015 00:22:57 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <masami.hiramatsu.pt@hitachi.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150505152257.18790.41548.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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;
 }

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

[Masami Hiramatsu]

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 <masami.hiramatsu.pt@hitachi.com>
>>> ---
>>>  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 majordomo@vger.kernel.org
>>> 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: masami.hiramatsu.pt@hitachi.com
> 


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com

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

[Arnaldo Carvalho de Melo]

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

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

[Masami Hiramatsu]

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 <masami.hiramatsu.pt@hitachi.com>
---
 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);

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

[Masami Hiramatsu]

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

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 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;

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

[Masami Hiramatsu]

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 <masami.hiramatsu.pt@hitachi.com>
---
 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;
 }

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

[Masami Hiramatsu]

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: masami.hiramatsu.pt@hitachi.com