[PATCH 1/5] perf probe: Split add_perf_probe_events()

[PATCH 1/5] perf probe: Split add_perf_probe_events()

[Namhyung Kim]

The add_perf_probe_events() does 3 things:

 1. convert all perf events to trace events
 2. add all trace events to kernel
 3. cleanup all trace events

But sometimes we need to do something with the trace events.  So split
the funtion into three, so that it can access intermediate trace events
via struct __event_package if needed.

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/probe-event.c | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index eb5f18b75402..2c762f41e7a5 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2765,9 +2765,10 @@ struct __event_package {
 	int				ntevs;
 };
 
-int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
+static int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs,
+				     struct __event_package **ppkgs)
 {
-	int i, j, ret;
+	int i, ret;
 	struct __event_package *pkgs;
 
 	ret = 0;
@@ -2792,12 +2793,21 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 		ret  = convert_to_probe_trace_events(pkgs[i].pev,
 						     &pkgs[i].tevs);
 		if (ret < 0)
-			goto end;
+			return ret;
 		pkgs[i].ntevs = ret;
 	}
 	/* This just release blacklist only if allocated */
 	kprobe_blacklist__release();
 
+	*ppkgs = pkgs;
+
+	return 0;
+}
+
+static int apply_perf_probe_events(struct __event_package *pkgs, int npevs)
+{
+	int i, ret = 0;
+
 	/* Loop 2: add all events */
 	for (i = 0; i < npevs; i++) {
 		ret = __add_probe_trace_events(pkgs[i].pev, pkgs[i].tevs,
@@ -2806,7 +2816,16 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 		if (ret < 0)
 			break;
 	}
-end:
+	return ret;
+}
+
+static void cleanup_perf_probe_events(struct __event_package *pkgs, int npevs)
+{
+	int i, j;
+
+	if (pkgs == NULL)
+		return;
+
 	/* Loop 3: cleanup and free trace events  */
 	for (i = 0; i < npevs; i++) {
 		for (j = 0; j < pkgs[i].ntevs; j++)
@@ -2815,6 +2834,18 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 	}
 	free(pkgs);
 	exit_symbol_maps();
+}
+
+int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
+{
+	int ret;
+	struct __event_package *pkgs = NULL;
+
+	ret = convert_perf_probe_events(pevs, npevs, &pkgs);
+	if (ret == 0)
+		ret = apply_perf_probe_events(pkgs, npevs);
+
+	cleanup_perf_probe_events(pkgs, npevs);
 
 	return ret;
 }
-- 
2.5.0

[PATCH 2/5] perf probe: Attach trace_probe_event with perf_probe_event

[Namhyung Kim]

This patch drops struct __event_package structure.  Instead, it adds
trace_probe_event into 'struct perf_probe_event'.

trace_probe_event information gives further patches a chance to access
actual probe points and actual arguments.  Using them, perf probe can
get whole list of added probes and print them at once.

Other users like upcoming bpf_loader will be able to attach one bpf
program to different probing points of an inline functions (which has
multiple probing points) and glob functions.  Moreover, by reading
arguments information, bpf code for reading those arguments can be
generated.

Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Wang Nan <wangnan0@huawei.com>
[namhyung: extract necessary part from the existing patch]
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/probe-event.c | 57 +++++++++++++------------------------------
 tools/perf/util/probe-event.h |  5 ++++
 2 files changed, 22 insertions(+), 40 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 2c762f41e7a5..0d3a051b9202 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2759,59 +2759,39 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
 	return find_probe_trace_events_from_map(pev, tevs);
 }
 
-struct __event_package {
-	struct perf_probe_event		*pev;
-	struct probe_trace_event	*tevs;
-	int				ntevs;
-};
-
-static int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs,
-				     struct __event_package **ppkgs)
+int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 {
 	int i, ret;
-	struct __event_package *pkgs;
-
-	ret = 0;
-	pkgs = zalloc(sizeof(struct __event_package) * npevs);
-
-	if (pkgs == NULL)
-		return -ENOMEM;
 
 	ret = init_symbol_maps(pevs->uprobes);
-	if (ret < 0) {
-		free(pkgs);
+	if (ret < 0)
 		return ret;
-	}
 
 	/* Loop 1: convert all events */
 	for (i = 0; i < npevs; i++) {
-		pkgs[i].pev = &pevs[i];
 		/* Init kprobe blacklist if needed */
-		if (!pkgs[i].pev->uprobes)
+		if (!pevs[i].uprobes)
 			kprobe_blacklist__init();
 		/* Convert with or without debuginfo */
-		ret  = convert_to_probe_trace_events(pkgs[i].pev,
-						     &pkgs[i].tevs);
+		ret  = convert_to_probe_trace_events(&pevs[i], &pevs[i].tevs);
 		if (ret < 0)
 			return ret;
-		pkgs[i].ntevs = ret;
+		pevs[i].ntevs = ret;
 	}
 	/* This just release blacklist only if allocated */
 	kprobe_blacklist__release();
 
-	*ppkgs = pkgs;
-
 	return 0;
 }
 
-static int apply_perf_probe_events(struct __event_package *pkgs, int npevs)
+int apply_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 {
 	int i, ret = 0;
 
 	/* Loop 2: add all events */
 	for (i = 0; i < npevs; i++) {
-		ret = __add_probe_trace_events(pkgs[i].pev, pkgs[i].tevs,
-					       pkgs[i].ntevs,
+		ret = __add_probe_trace_events(&pevs[i], pevs[i].tevs,
+					       pevs[i].ntevs,
 					       probe_conf.force_add);
 		if (ret < 0)
 			break;
@@ -2819,33 +2799,30 @@ static int apply_perf_probe_events(struct __event_package *pkgs, int npevs)
 	return ret;
 }
 
-static void cleanup_perf_probe_events(struct __event_package *pkgs, int npevs)
+void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 {
 	int i, j;
 
-	if (pkgs == NULL)
-		return;
-
 	/* Loop 3: cleanup and free trace events  */
 	for (i = 0; i < npevs; i++) {
-		for (j = 0; j < pkgs[i].ntevs; j++)
-			clear_probe_trace_event(&pkgs[i].tevs[j]);
-		zfree(&pkgs[i].tevs);
+		for (j = 0; j < pevs[i].ntevs; j++)
+			clear_probe_trace_event(&pevs[i].tevs[j]);
+		zfree(&pevs[i].tevs);
+		pevs[i].ntevs = 0;
 	}
-	free(pkgs);
+
 	exit_symbol_maps();
 }
 
 int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 {
 	int ret;
-	struct __event_package *pkgs = NULL;
 
-	ret = convert_perf_probe_events(pevs, npevs, &pkgs);
+	ret = convert_perf_probe_events(pevs, npevs);
 	if (ret == 0)
-		ret = apply_perf_probe_events(pkgs, npevs);
+		ret = apply_perf_probe_events(pevs, npevs);
 
-	cleanup_perf_probe_events(pkgs, npevs);
+	cleanup_perf_probe_events(pevs, npevs);
 
 	return ret;
 }
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 6e7ec68a4aa8..70c327bd61de 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -87,6 +87,8 @@ struct perf_probe_event {
 	bool			uprobes;	/* Uprobe event flag */
 	char			*target;	/* Target binary */
 	struct perf_probe_arg	*args;	/* Arguments */
+	struct probe_trace_event *tevs;
+	int			ntevs;
 };
 
 /* Line range */
@@ -138,6 +140,9 @@ extern void line_range__clear(struct line_range *lr);
 extern int line_range__init(struct line_range *lr);
 
 extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs);
+extern int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs);
+extern int apply_perf_probe_events(struct perf_probe_event *pevs, int npevs);
+extern void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs);
 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,
-- 
2.5.0

[PATCH 3/5] perf probe: Move print logic into cmd_probe()

[Namhyung Kim]

Showing actual trace event when adding perf events is only needed in
perf probe command.  But the add functionality itself can be used by
other places.  So move the printing code into the cmd_probe().

Also it combines the output if more than one event is added.

Before:
  $ sudo perf probe -a do_fork -a do_exit
  Added new event:
  probe:do_fork        (on do_fork)

  You can now use it in all perf tools, such as:

      perf record -e probe:do_fork -aR sleep 1

  Added new events:
  probe:do_exit        (on do_exit)
  probe:do_exit_1      (on do_exit)

  You can now use it in all perf tools, such as:

      perf record -e probe:do_exit_1 -aR sleep 1

After:
  $ sudo perf probe -a do_fork -a do_exit
  Added new events:
  probe:do_fork        (on do_fork)
  probe:do_exit        (on do_exit)
  probe:do_exit_1      (on do_exit)

  You can now use it in all perf tools, such as:

      perf record -e probe:do_exit_1 -aR sleep 1

Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-probe.c    | 48 ++++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/probe-event.c | 22 +++-----------------
 tools/perf/util/probe-event.h |  3 +++
 3 files changed, 53 insertions(+), 20 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index b81cec33b4b2..b8cf6cb7e1bf 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -311,6 +311,52 @@ static void pr_err_with_code(const char *msg, int err)
 	pr_err("\n");
 }
 
+static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs)
+{
+	int ret;
+	int i, k;
+	const char *event = NULL, *group = NULL;
+
+	ret = convert_perf_probe_events(pevs, npevs);
+	if (ret < 0)
+		goto out_cleanup;
+
+	ret = apply_perf_probe_events(pevs, npevs);
+	if (ret < 0)
+		goto out_cleanup;
+
+	for (i = k = 0; i < npevs; i++)
+		k += pevs[i].ntevs;
+
+	pr_info("Added new event%s\n", (k > 1) ? "s:" : ":");
+	for (i = 0; i < npevs; i++) {
+		struct perf_probe_event *pev = &pevs[i];
+
+		for (k = 0; k < pev->ntevs; k++) {
+			struct probe_trace_event *tev = &pev->tevs[k];
+
+			/* We use tev's name for showing new events */
+			show_perf_probe_event(tev->group, tev->event, pev,
+					      tev->point.module, false);
+
+			/* Save the last valid name */
+			event = tev->event;
+			group = tev->group;
+		}
+	}
+
+	/* Note that it is possible to skip all events because of blacklist */
+	if (event) {
+		/* Show how to use the event. */
+		pr_info("\nYou can now use it in all perf tools, such as:\n\n");
+		pr_info("\tperf record -e %s:%s -aR sleep 1\n\n", group, event);
+	}
+
+out_cleanup:
+	cleanup_perf_probe_events(pevs, npevs);
+	return ret;
+}
+
 static int
 __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 {
@@ -496,7 +542,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 			usage_with_options(probe_usage, options);
 		}
 
-		ret = add_perf_probe_events(params.events, params.nevents);
+		ret = perf_add_probe_events(params.events, params.nevents);
 		if (ret < 0) {
 			pr_err_with_code("  Error: Failed to add events.", ret);
 			return ret;
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 0d3a051b9202..01b9a5bd9449 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2180,9 +2180,9 @@ static int perf_probe_event__sprintf(const char *group, const char *event,
 }
 
 /* Show an event */
-static int show_perf_probe_event(const char *group, const char *event,
-				 struct perf_probe_event *pev,
-				 const char *module, bool use_stdout)
+int show_perf_probe_event(const char *group, const char *event,
+			  struct perf_probe_event *pev,
+			  const char *module, bool use_stdout)
 {
 	struct strbuf buf = STRBUF_INIT;
 	int ret;
@@ -2399,7 +2399,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 {
 	int i, fd, ret;
 	struct probe_trace_event *tev = NULL;
-	const char *event = NULL, *group = NULL;
 	struct strlist *namelist;
 
 	fd = probe_file__open(PF_FL_RW | (pev->uprobes ? PF_FL_UPROBE : 0));
@@ -2415,7 +2414,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 	}
 
 	ret = 0;
-	pr_info("Added new event%s\n", (ntevs > 1) ? "s:" : ":");
 	for (i = 0; i < ntevs; i++) {
 		tev = &tevs[i];
 		/* Skip if the symbol is out of .text or blacklisted */
@@ -2432,13 +2430,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 		if (ret < 0)
 			break;
 
-		/* We use tev's name for showing new events */
-		show_perf_probe_event(tev->group, tev->event, pev,
-				      tev->point.module, false);
-		/* Save the last valid name */
-		event = tev->event;
-		group = tev->group;
-
 		/*
 		 * Probes after the first probe which comes from same
 		 * user input are always allowed to add suffix, because
@@ -2450,13 +2441,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 	if (ret == -EINVAL && pev->uprobes)
 		warn_uprobe_event_compat(tev);
 
-	/* Note that it is possible to skip all events because of blacklist */
-	if (ret >= 0 && event) {
-		/* Show how to use the event. */
-		pr_info("\nYou can now use it in all perf tools, such as:\n\n");
-		pr_info("\tperf record -e %s:%s -aR sleep 1\n\n", group, event);
-	}
-
 	strlist__delete(namelist);
 close_out:
 	close(fd);
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 70c327bd61de..610f743671e1 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -144,6 +144,9 @@ extern int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs);
 extern int apply_perf_probe_events(struct perf_probe_event *pevs, int npevs);
 extern void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs);
 extern int del_perf_probe_events(struct strfilter *filter);
+extern int show_perf_probe_event(const char *group, const char *event,
+				 struct perf_probe_event *pev,
+				 const char *module, bool use_stdout);
 extern int show_perf_probe_events(struct strfilter *filter);
 extern int show_line_range(struct line_range *lr, const char *module,
 			   bool user);
-- 
2.5.0

[PATCH 4/5] perf probe: Split del_perf_probe_events()

[Namhyung Kim]

The del_perf_probe_events() does 2 things:

1. find existing events which match to filter
2. delete such trace events from kernel

But sometimes we need to do something with the trace events.  So split
the funtion into two, so that it can access intermediate trace events
name using strlist if needed.

Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/probe-file.c | 40 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index bbb243717ec8..f00b0df56dfe 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -275,7 +275,8 @@ static int __del_trace_probe_event(int fd, struct str_node *ent)
 	return ret;
 }
 
-int probe_file__del_events(int fd, struct strfilter *filter)
+static int probe_file__get_events(int fd, struct strfilter *filter,
+				  struct strlist *plist)
 {
 	struct strlist *namelist;
 	struct str_node *ent;
@@ -290,12 +291,43 @@ int probe_file__del_events(int fd, struct strfilter *filter)
 		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)
-				break;
+			strlist__add(plist, ent->s);
+			ret = 0;
 		}
 	}
 	strlist__delete(namelist);
 
 	return ret;
 }
+
+static int probe_file__del_strlist(int fd, struct strlist *namelist)
+{
+	int ret = 0;
+	struct str_node *ent;
+
+	strlist__for_each(ent, namelist) {
+		ret = __del_trace_probe_event(fd, ent);
+		if (ret < 0)
+			break;
+	}
+	return ret;
+}
+
+int probe_file__del_events(int fd, struct strfilter *filter)
+{
+	struct strlist *namelist;
+	int ret;
+
+	namelist = strlist__new(NULL, NULL);
+	if (!namelist)
+		return -ENOMEM;
+
+	ret = probe_file__get_events(fd, filter, namelist);
+	if (ret < 0)
+		return ret;
+
+	ret = probe_file__del_strlist(fd, namelist);
+	strlist__delete(namelist);
+
+	return ret;
+}
-- 
2.5.0

[PATCH 5/5] perf probe: Print deleted events in cmd_probe()

[Namhyung Kim]

Showing actual trace event when deleteing perf events is only needed in
perf probe command.  But the add functionality itself can be used by
other places.  So move the printing code into the cmd_probe().

The output is not changed.

Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-probe.c    | 62 ++++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/probe-event.c |  5 ----
 tools/perf/util/probe-event.h |  5 ++++
 tools/perf/util/probe-file.c  |  7 +++--
 4 files changed, 69 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index b8cf6cb7e1bf..ee2c46d8353e 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -41,6 +41,7 @@
 #include "util/parse-options.h"
 #include "util/probe-finder.h"
 #include "util/probe-event.h"
+#include "util/probe-file.h"
 
 #define DEFAULT_VAR_FILTER "!__k???tab_* & !__crc_*"
 #define DEFAULT_FUNC_FILTER "!_*"
@@ -357,6 +358,65 @@ static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs)
 	return ret;
 }
 
+static int perf_del_probe_events(struct strfilter *filter)
+{
+	int ret, ret2, ufd = -1, kfd = -1;
+	char *str = strfilter__string(filter);
+	struct strlist *klist = NULL, *ulist = NULL;
+	struct str_node *ent;
+
+	if (!str)
+		return -EINVAL;
+
+	pr_debug("Delete filter: \'%s\'\n", str);
+
+	/* Get current event names */
+	ret = probe_file__open_both(&kfd, &ufd, PF_FL_RW);
+	if (ret < 0)
+		goto out;
+
+	klist = strlist__new(NULL, NULL);
+	if (!klist)
+		return -ENOMEM;
+
+	ret = probe_file__get_events(kfd, filter, klist);
+	if (ret == 0) {
+		strlist__for_each(ent, klist)
+			pr_info("Removed event: %s\n", ent->s);
+
+		ret = probe_file__del_strlist(kfd, klist);
+		if (ret < 0)
+			goto error;
+	}
+
+	ret2 = probe_file__get_events(ufd, filter, ulist);
+	if (ret2 == 0) {
+		strlist__for_each(ent, ulist)
+			pr_info("Removed event: %s\n", ent->s);
+
+		ret2 = probe_file__del_strlist(ufd, ulist);
+		if (ret2 < 0)
+			goto error;
+	}
+
+	if (ret == -ENOENT && ret2 == -ENOENT)
+		pr_debug("\"%s\" does not hit any event.\n", str);
+		/* Note that this is silently ignored */
+	ret = 0;
+
+error:
+	if (kfd >= 0)
+		close(kfd);
+	if (ufd >= 0)
+		close(ufd);
+out:
+	strlist__delete(klist);
+	strlist__delete(ulist);
+	free(str);
+
+	return ret;
+}
+
 static int
 __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 {
@@ -529,7 +589,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		return ret;
 #endif
 	case 'd':
-		ret = del_perf_probe_events(params.filter);
+		ret = perf_del_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 01b9a5bd9449..3da9e1c792fa 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2819,8 +2819,6 @@ int del_perf_probe_events(struct strfilter *filter)
 	if (!str)
 		return -EINVAL;
 
-	pr_debug("Delete filter: \'%s\'\n", str);
-
 	/* Get current event names */
 	ret = probe_file__open_both(&kfd, &ufd, PF_FL_RW);
 	if (ret < 0)
@@ -2835,9 +2833,6 @@ int del_perf_probe_events(struct strfilter *filter)
 		ret = ret2;
 		goto error;
 	}
-	if (ret == -ENOENT && ret2 == -ENOENT)
-		pr_debug("\"%s\" does not hit any event.\n", str);
-		/* Note that this is silently ignored */
 	ret = 0;
 
 error:
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 610f743671e1..d491cf69f91b 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -144,6 +144,11 @@ extern int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs);
 extern int apply_perf_probe_events(struct perf_probe_event *pevs, int npevs);
 extern void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs);
 extern int del_perf_probe_events(struct strfilter *filter);
+
+extern int probe_file__get_events(int fd, struct strfilter *filter,
+				  struct strlist *plist);
+extern int probe_file__del_strlist(int fd, struct strlist *namelist);
+
 extern int show_perf_probe_event(const char *group, const char *event,
 				 struct perf_probe_event *pev,
 				 const char *module, bool use_stdout);
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index f00b0df56dfe..38c0a62039cc 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -267,7 +267,6 @@ static int __del_trace_probe_event(int fd, struct str_node *ent)
 		goto error;
 	}
 
-	pr_info("Removed event: %s\n", ent->s);
 	return 0;
 error:
 	pr_warning("Failed to delete event: %s\n",
@@ -275,8 +274,8 @@ static int __del_trace_probe_event(int fd, struct str_node *ent)
 	return ret;
 }
 
-static int probe_file__get_events(int fd, struct strfilter *filter,
-				  struct strlist *plist)
+int probe_file__get_events(int fd, struct strfilter *filter,
+			   struct strlist *plist)
 {
 	struct strlist *namelist;
 	struct str_node *ent;
@@ -300,7 +299,7 @@ static int probe_file__get_events(int fd, struct strfilter *filter,
 	return ret;
 }
 
-static int probe_file__del_strlist(int fd, struct strlist *namelist)
+int probe_file__del_strlist(int fd, struct strlist *namelist)
 {
 	int ret = 0;
 	struct str_node *ent;
-- 
2.5.0

RE: [PATCH 2/5] perf probe: Attach trace_probe_event with perf_probe_event

[平松雅巳 / HIRAMATU，MASAMI]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 5832 bytes --]

> From: Namhyung Kim [mailto:namhyung@kernel.org]
> 
> This patch drops struct __event_package structure.  Instead, it adds
> trace_probe_event into 'struct perf_probe_event'.
> 
> trace_probe_event information gives further patches a chance to access
> actual probe points and actual arguments.  Using them, perf probe can
> get whole list of added probes and print them at once.
> 
> Other users like upcoming bpf_loader will be able to attach one bpf
> program to different probing points of an inline functions (which has
> multiple probing points) and glob functions.  Moreover, by reading
> arguments information, bpf code for reading those arguments can be
> generated.

Looks good to me.

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

Thanks!

> 
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> [namhyung: extract necessary part from the existing patch]
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/probe-event.c | 57 +++++++++++++------------------------------
>  tools/perf/util/probe-event.h |  5 ++++
>  2 files changed, 22 insertions(+), 40 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 2c762f41e7a5..0d3a051b9202 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2759,59 +2759,39 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
>  	return find_probe_trace_events_from_map(pev, tevs);
>  }
> 
> -struct __event_package {
> -	struct perf_probe_event		*pev;
> -	struct probe_trace_event	*tevs;
> -	int				ntevs;
> -};
> -
> -static int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs,
> -				     struct __event_package **ppkgs)
> +int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs)
>  {
>  	int i, ret;
> -	struct __event_package *pkgs;
> -
> -	ret = 0;
> -	pkgs = zalloc(sizeof(struct __event_package) * npevs);
> -
> -	if (pkgs == NULL)
> -		return -ENOMEM;
> 
>  	ret = init_symbol_maps(pevs->uprobes);
> -	if (ret < 0) {
> -		free(pkgs);
> +	if (ret < 0)
>  		return ret;
> -	}
> 
>  	/* Loop 1: convert all events */
>  	for (i = 0; i < npevs; i++) {
> -		pkgs[i].pev = &pevs[i];
>  		/* Init kprobe blacklist if needed */
> -		if (!pkgs[i].pev->uprobes)
> +		if (!pevs[i].uprobes)
>  			kprobe_blacklist__init();
>  		/* Convert with or without debuginfo */
> -		ret  = convert_to_probe_trace_events(pkgs[i].pev,
> -						     &pkgs[i].tevs);
> +		ret  = convert_to_probe_trace_events(&pevs[i], &pevs[i].tevs);
>  		if (ret < 0)
>  			return ret;
> -		pkgs[i].ntevs = ret;
> +		pevs[i].ntevs = ret;
>  	}
>  	/* This just release blacklist only if allocated */
>  	kprobe_blacklist__release();
> 
> -	*ppkgs = pkgs;
> -
>  	return 0;
>  }
> 
> -static int apply_perf_probe_events(struct __event_package *pkgs, int npevs)
> +int apply_perf_probe_events(struct perf_probe_event *pevs, int npevs)
>  {
>  	int i, ret = 0;
> 
>  	/* Loop 2: add all events */
>  	for (i = 0; i < npevs; i++) {
> -		ret = __add_probe_trace_events(pkgs[i].pev, pkgs[i].tevs,
> -					       pkgs[i].ntevs,
> +		ret = __add_probe_trace_events(&pevs[i], pevs[i].tevs,
> +					       pevs[i].ntevs,
>  					       probe_conf.force_add);
>  		if (ret < 0)
>  			break;
> @@ -2819,33 +2799,30 @@ static int apply_perf_probe_events(struct __event_package *pkgs, int npevs)
>  	return ret;
>  }
> 
> -static void cleanup_perf_probe_events(struct __event_package *pkgs, int npevs)
> +void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs)
>  {
>  	int i, j;
> 
> -	if (pkgs == NULL)
> -		return;
> -
>  	/* Loop 3: cleanup and free trace events  */
>  	for (i = 0; i < npevs; i++) {
> -		for (j = 0; j < pkgs[i].ntevs; j++)
> -			clear_probe_trace_event(&pkgs[i].tevs[j]);
> -		zfree(&pkgs[i].tevs);
> +		for (j = 0; j < pevs[i].ntevs; j++)
> +			clear_probe_trace_event(&pevs[i].tevs[j]);
> +		zfree(&pevs[i].tevs);
> +		pevs[i].ntevs = 0;
>  	}
> -	free(pkgs);
> +
>  	exit_symbol_maps();
>  }
> 
>  int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
>  {
>  	int ret;
> -	struct __event_package *pkgs = NULL;
> 
> -	ret = convert_perf_probe_events(pevs, npevs, &pkgs);
> +	ret = convert_perf_probe_events(pevs, npevs);
>  	if (ret == 0)
> -		ret = apply_perf_probe_events(pkgs, npevs);
> +		ret = apply_perf_probe_events(pevs, npevs);
> 
> -	cleanup_perf_probe_events(pkgs, npevs);
> +	cleanup_perf_probe_events(pevs, npevs);
> 
>  	return ret;
>  }
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index 6e7ec68a4aa8..70c327bd61de 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -87,6 +87,8 @@ struct perf_probe_event {
>  	bool			uprobes;	/* Uprobe event flag */
>  	char			*target;	/* Target binary */
>  	struct perf_probe_arg	*args;	/* Arguments */
> +	struct probe_trace_event *tevs;
> +	int			ntevs;
>  };
> 
>  /* Line range */
> @@ -138,6 +140,9 @@ extern void line_range__clear(struct line_range *lr);
>  extern int line_range__init(struct line_range *lr);
> 
>  extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> +extern int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> +extern int apply_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> +extern void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs);
>  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,
> --
> 2.5.0

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH 5/5] perf probe: Print deleted events in cmd_probe()

[平松雅巳 / HIRAMATU，MASAMI]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1415 bytes --]

> From: Namhyung Kim [mailto:namhyung@kernel.org]
> 
> Showing actual trace event when deleteing perf events is only needed in
> perf probe command.  But the add functionality itself can be used by
> other places.  So move the printing code into the cmd_probe().
> 
> The output is not changed.
> 

OK, I just have a comment below.

> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index 610f743671e1..d491cf69f91b 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -144,6 +144,11 @@ extern int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs);
>  extern int apply_perf_probe_events(struct perf_probe_event *pevs, int npevs);
>  extern void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs);
>  extern int del_perf_probe_events(struct strfilter *filter);
> +
> +extern int probe_file__get_events(int fd, struct strfilter *filter,
> +				  struct strlist *plist);
> +extern int probe_file__del_strlist(int fd, struct strlist *namelist);

These should be moved into probe-file.h.

> +
>  extern int show_perf_probe_event(const char *group, const char *event,
>  				 struct perf_probe_event *pev,
>  				 const char *module, bool use_stdout);

Thanks!

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH 3/5] perf probe: Move print logic into cmd_probe()

[平松雅巳 / HIRAMATU，MASAMI]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 6868 bytes --]

> From: Namhyung Kim [mailto:namhyung@kernel.org]
> 
> Showing actual trace event when adding perf events is only needed in
> perf probe command.  But the add functionality itself can be used by
> other places.  So move the printing code into the cmd_probe().
> 
> Also it combines the output if more than one event is added.
> 
> Before:
>   $ sudo perf probe -a do_fork -a do_exit
>   Added new event:
>   probe:do_fork        (on do_fork)
> 
>   You can now use it in all perf tools, such as:
> 
>       perf record -e probe:do_fork -aR sleep 1
> 
>   Added new events:
>   probe:do_exit        (on do_exit)
>   probe:do_exit_1      (on do_exit)
> 
>   You can now use it in all perf tools, such as:
> 
>       perf record -e probe:do_exit_1 -aR sleep 1
> 
> After:
>   $ sudo perf probe -a do_fork -a do_exit
>   Added new events:
>   probe:do_fork        (on do_fork)
>   probe:do_exit        (on do_exit)
>   probe:do_exit_1      (on do_exit)
> 
>   You can now use it in all perf tools, such as:
> 
>       perf record -e probe:do_exit_1 -aR sleep 1
> 

Looks good to me :)

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

Thanks!

> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/builtin-probe.c    | 48 ++++++++++++++++++++++++++++++++++++++++++-
>  tools/perf/util/probe-event.c | 22 +++-----------------
>  tools/perf/util/probe-event.h |  3 +++
>  3 files changed, 53 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index b81cec33b4b2..b8cf6cb7e1bf 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -311,6 +311,52 @@ static void pr_err_with_code(const char *msg, int err)
>  	pr_err("\n");
>  }
> 
> +static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs)
> +{
> +	int ret;
> +	int i, k;
> +	const char *event = NULL, *group = NULL;
> +
> +	ret = convert_perf_probe_events(pevs, npevs);
> +	if (ret < 0)
> +		goto out_cleanup;
> +
> +	ret = apply_perf_probe_events(pevs, npevs);
> +	if (ret < 0)
> +		goto out_cleanup;
> +
> +	for (i = k = 0; i < npevs; i++)
> +		k += pevs[i].ntevs;
> +
> +	pr_info("Added new event%s\n", (k > 1) ? "s:" : ":");
> +	for (i = 0; i < npevs; i++) {
> +		struct perf_probe_event *pev = &pevs[i];
> +
> +		for (k = 0; k < pev->ntevs; k++) {
> +			struct probe_trace_event *tev = &pev->tevs[k];
> +
> +			/* We use tev's name for showing new events */
> +			show_perf_probe_event(tev->group, tev->event, pev,
> +					      tev->point.module, false);
> +
> +			/* Save the last valid name */
> +			event = tev->event;
> +			group = tev->group;
> +		}
> +	}
> +
> +	/* Note that it is possible to skip all events because of blacklist */
> +	if (event) {
> +		/* Show how to use the event. */
> +		pr_info("\nYou can now use it in all perf tools, such as:\n\n");
> +		pr_info("\tperf record -e %s:%s -aR sleep 1\n\n", group, event);
> +	}
> +
> +out_cleanup:
> +	cleanup_perf_probe_events(pevs, npevs);
> +	return ret;
> +}
> +
>  static int
>  __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>  {
> @@ -496,7 +542,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>  			usage_with_options(probe_usage, options);
>  		}
> 
> -		ret = add_perf_probe_events(params.events, params.nevents);
> +		ret = perf_add_probe_events(params.events, params.nevents);
>  		if (ret < 0) {
>  			pr_err_with_code("  Error: Failed to add events.", ret);
>  			return ret;
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 0d3a051b9202..01b9a5bd9449 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2180,9 +2180,9 @@ static int perf_probe_event__sprintf(const char *group, const char *event,
>  }
> 
>  /* Show an event */
> -static int show_perf_probe_event(const char *group, const char *event,
> -				 struct perf_probe_event *pev,
> -				 const char *module, bool use_stdout)
> +int show_perf_probe_event(const char *group, const char *event,
> +			  struct perf_probe_event *pev,
> +			  const char *module, bool use_stdout)
>  {
>  	struct strbuf buf = STRBUF_INIT;
>  	int ret;
> @@ -2399,7 +2399,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
>  {
>  	int i, fd, ret;
>  	struct probe_trace_event *tev = NULL;
> -	const char *event = NULL, *group = NULL;
>  	struct strlist *namelist;
> 
>  	fd = probe_file__open(PF_FL_RW | (pev->uprobes ? PF_FL_UPROBE : 0));
> @@ -2415,7 +2414,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
>  	}
> 
>  	ret = 0;
> -	pr_info("Added new event%s\n", (ntevs > 1) ? "s:" : ":");
>  	for (i = 0; i < ntevs; i++) {
>  		tev = &tevs[i];
>  		/* Skip if the symbol is out of .text or blacklisted */
> @@ -2432,13 +2430,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
>  		if (ret < 0)
>  			break;
> 
> -		/* We use tev's name for showing new events */
> -		show_perf_probe_event(tev->group, tev->event, pev,
> -				      tev->point.module, false);
> -		/* Save the last valid name */
> -		event = tev->event;
> -		group = tev->group;
> -
>  		/*
>  		 * Probes after the first probe which comes from same
>  		 * user input are always allowed to add suffix, because
> @@ -2450,13 +2441,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
>  	if (ret == -EINVAL && pev->uprobes)
>  		warn_uprobe_event_compat(tev);
> 
> -	/* Note that it is possible to skip all events because of blacklist */
> -	if (ret >= 0 && event) {
> -		/* Show how to use the event. */
> -		pr_info("\nYou can now use it in all perf tools, such as:\n\n");
> -		pr_info("\tperf record -e %s:%s -aR sleep 1\n\n", group, event);
> -	}
> -
>  	strlist__delete(namelist);
>  close_out:
>  	close(fd);
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index 70c327bd61de..610f743671e1 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -144,6 +144,9 @@ extern int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs);
>  extern int apply_perf_probe_events(struct perf_probe_event *pevs, int npevs);
>  extern void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs);
>  extern int del_perf_probe_events(struct strfilter *filter);
> +extern int show_perf_probe_event(const char *group, const char *event,
> +				 struct perf_probe_event *pev,
> +				 const char *module, bool use_stdout);
>  extern int show_perf_probe_events(struct strfilter *filter);
>  extern int show_line_range(struct line_range *lr, const char *module,
>  			   bool user);
> --
> 2.5.0

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH 4/5] perf probe: Split del_perf_probe_events()

[平松雅巳 / HIRAMATU，MASAMI]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2536 bytes --]

> From: Namhyung Kim [mailto:namhyung@kernel.org]
> 
> The del_perf_probe_events() does 2 things:
> 
> 1. find existing events which match to filter
> 2. delete such trace events from kernel
> 
> But sometimes we need to do something with the trace events.  So split
> the funtion into two, so that it can access intermediate trace events
> name using strlist if needed.
> 

Ok, but I think it might be better merged to 5/5.

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

Thanks,


> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/probe-file.c | 40 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index bbb243717ec8..f00b0df56dfe 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -275,7 +275,8 @@ static int __del_trace_probe_event(int fd, struct str_node *ent)
>  	return ret;
>  }
> 
> -int probe_file__del_events(int fd, struct strfilter *filter)
> +static int probe_file__get_events(int fd, struct strfilter *filter,
> +				  struct strlist *plist)
>  {
>  	struct strlist *namelist;
>  	struct str_node *ent;
> @@ -290,12 +291,43 @@ int probe_file__del_events(int fd, struct strfilter *filter)
>  		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)
> -				break;
> +			strlist__add(plist, ent->s);
> +			ret = 0;
>  		}
>  	}
>  	strlist__delete(namelist);
> 
>  	return ret;
>  }
> +
> +static int probe_file__del_strlist(int fd, struct strlist *namelist)
> +{
> +	int ret = 0;
> +	struct str_node *ent;
> +
> +	strlist__for_each(ent, namelist) {
> +		ret = __del_trace_probe_event(fd, ent);
> +		if (ret < 0)
> +			break;
> +	}
> +	return ret;
> +}
> +
> +int probe_file__del_events(int fd, struct strfilter *filter)
> +{
> +	struct strlist *namelist;
> +	int ret;
> +
> +	namelist = strlist__new(NULL, NULL);
> +	if (!namelist)
> +		return -ENOMEM;
> +
> +	ret = probe_file__get_events(fd, filter, namelist);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = probe_file__del_strlist(fd, namelist);
> +	strlist__delete(namelist);
> +
> +	return ret;
> +}
> --
> 2.5.0

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 2/5] perf probe: Attach trace_probe_event with perf_probe_event

[Namhyung Kim]

On Fri, Sep 04, 2015 at 11:36:56AM +0000, 平松雅巳 / HIRAMATU，MASAMI wrote:
> > From: Namhyung Kim [mailto:namhyung@kernel.org]

Oh, this patch is came from Wang Nan.  I missed to update the author,
will change in v2.


> > 
> > This patch drops struct __event_package structure.  Instead, it adds
> > trace_probe_event into 'struct perf_probe_event'.
> > 
> > trace_probe_event information gives further patches a chance to access
> > actual probe points and actual arguments.  Using them, perf probe can
> > get whole list of added probes and print them at once.
> > 
> > Other users like upcoming bpf_loader will be able to attach one bpf
> > program to different probing points of an inline functions (which has
> > multiple probing points) and glob functions.  Moreover, by reading
> > arguments information, bpf code for reading those arguments can be
> > generated.
> 
> Looks good to me.
> 
> Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

Thanks
Namhyung

Re: [PATCH 5/5] perf probe: Print deleted events in cmd_probe()

[Namhyung Kim]

On Fri, Sep 04, 2015 at 11:46:45AM +0000, 平松雅巳 / HIRAMATU，MASAMI wrote:
> > From: Namhyung Kim [mailto:namhyung@kernel.org]
> > 
> > Showing actual trace event when deleteing perf events is only needed in
> > perf probe command.  But the add functionality itself can be used by
> > other places.  So move the printing code into the cmd_probe().
> > 
> > The output is not changed.
> > 
> 
> OK, I just have a comment below.
> 
> > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> > index 610f743671e1..d491cf69f91b 100644
> > --- a/tools/perf/util/probe-event.h
> > +++ b/tools/perf/util/probe-event.h
> > @@ -144,6 +144,11 @@ extern int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> >  extern int apply_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> >  extern void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> >  extern int del_perf_probe_events(struct strfilter *filter);
> > +
> > +extern int probe_file__get_events(int fd, struct strfilter *filter,
> > +				  struct strlist *plist);
> > +extern int probe_file__del_strlist(int fd, struct strlist *namelist);
> 
> These should be moved into probe-file.h.

Right, will change!

Thanks,
Namhyung


> 
> > +
> >  extern int show_perf_probe_event(const char *group, const char *event,
> >  				 struct perf_probe_event *pev,
> >  				 const char *module, bool use_stdout);
> 
> Thanks!
> 

Re: [PATCH 4/5] perf probe: Split del_perf_probe_events()

[Namhyung Kim]

On Fri, Sep 04, 2015 at 11:52:33AM +0000, 平松雅巳 / HIRAMATU，MASAMI wrote:
> > From: Namhyung Kim [mailto:namhyung@kernel.org]
> > 
> > The del_perf_probe_events() does 2 things:
> > 
> > 1. find existing events which match to filter
> > 2. delete such trace events from kernel
> > 
> > But sometimes we need to do something with the trace events.  So split
> > the funtion into two, so that it can access intermediate trace events
> > name using strlist if needed.
> > 
> 
> Ok, but I think it might be better merged to 5/5.

I'm fine with merging them.  Arnaldo, I'll keep it separate now just
in case, but you can merge them if you want.

Thanks,
Namhyung


> 
> Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> 
> Thanks,
> 
> 
> > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/util/probe-file.c | 40 ++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 36 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> > index bbb243717ec8..f00b0df56dfe 100644
> > --- a/tools/perf/util/probe-file.c
> > +++ b/tools/perf/util/probe-file.c
> > @@ -275,7 +275,8 @@ static int __del_trace_probe_event(int fd, struct str_node *ent)
> >  	return ret;
> >  }
> > 
> > -int probe_file__del_events(int fd, struct strfilter *filter)
> > +static int probe_file__get_events(int fd, struct strfilter *filter,
> > +				  struct strlist *plist)
> >  {
> >  	struct strlist *namelist;
> >  	struct str_node *ent;
> > @@ -290,12 +291,43 @@ int probe_file__del_events(int fd, struct strfilter *filter)
> >  		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)
> > -				break;
> > +			strlist__add(plist, ent->s);
> > +			ret = 0;
> >  		}
> >  	}
> >  	strlist__delete(namelist);
> > 
> >  	return ret;
> >  }
> > +
> > +static int probe_file__del_strlist(int fd, struct strlist *namelist)
> > +{
> > +	int ret = 0;
> > +	struct str_node *ent;
> > +
> > +	strlist__for_each(ent, namelist) {
> > +		ret = __del_trace_probe_event(fd, ent);
> > +		if (ret < 0)
> > +			break;
> > +	}
> > +	return ret;
> > +}
> > +
> > +int probe_file__del_events(int fd, struct strfilter *filter)
> > +{
> > +	struct strlist *namelist;
> > +	int ret;
> > +
> > +	namelist = strlist__new(NULL, NULL);
> > +	if (!namelist)
> > +		return -ENOMEM;
> > +
> > +	ret = probe_file__get_events(fd, filter, namelist);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = probe_file__del_strlist(fd, namelist);
> > +	strlist__delete(namelist);
> > +
> > +	return ret;
> > +}
> > --
> > 2.5.0
>