All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/5] perf probe: Split add_perf_probe_events()
@ 2015-09-04 12:15 Namhyung Kim
  2015-09-04 12:16 ` [PATCH v2 2/5] perf probe: Attach trace_probe_event with perf_probe_event Namhyung Kim
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Namhyung Kim @ 2015-09-04 12:15 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Masami Hiramatsu,
	Wang Nan, pi3orama

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 @@ end:
 	}
 	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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 2/5] perf probe: Attach trace_probe_event with perf_probe_event
  2015-09-04 12:15 [PATCH v2 1/5] perf probe: Split add_perf_probe_events() Namhyung Kim
@ 2015-09-04 12:16 ` Namhyung Kim
  2015-09-08 14:39   ` [tip:perf/core] perf probe: Link trace_probe_event into perf_probe_event tip-bot for Wang Nan
  2015-09-04 12:16 ` [PATCH v2 3/5] perf probe: Move print logic into cmd_probe() Namhyung Kim
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2015-09-04 12:16 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Masami Hiramatsu,
	Wang Nan, pi3orama

From: Wang Nan <wangnan0@huawei.com>

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.

Acked-by: 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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 3/5] perf probe: Move print logic into cmd_probe()
  2015-09-04 12:15 [PATCH v2 1/5] perf probe: Split add_perf_probe_events() Namhyung Kim
  2015-09-04 12:16 ` [PATCH v2 2/5] perf probe: Attach trace_probe_event with perf_probe_event Namhyung Kim
@ 2015-09-04 12:16 ` Namhyung Kim
  2015-09-08 14:39   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2015-09-04 12:16 ` [PATCH v2 4/5] perf probe: Split del_perf_probe_events() Namhyung Kim
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2015-09-04 12:16 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Masami Hiramatsu,
	Wang Nan, pi3orama

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

Acked-by: 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 @@ out:
 }
 
 /* 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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 4/5] perf probe: Split del_perf_probe_events()
  2015-09-04 12:15 [PATCH v2 1/5] perf probe: Split add_perf_probe_events() Namhyung Kim
  2015-09-04 12:16 ` [PATCH v2 2/5] perf probe: Attach trace_probe_event with perf_probe_event Namhyung Kim
  2015-09-04 12:16 ` [PATCH v2 3/5] perf probe: Move print logic into cmd_probe() Namhyung Kim
@ 2015-09-04 12:16 ` Namhyung Kim
  2015-09-08 14:40   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2015-09-04 12:16 ` [PATCH v2 5/5] perf probe: Print deleted events in cmd_probe() Namhyung Kim
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2015-09-04 12:16 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Masami Hiramatsu,
	Wang Nan, pi3orama

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.

Acked-by: 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 @@ error:
 	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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 5/5] perf probe: Print deleted events in cmd_probe()
  2015-09-04 12:15 [PATCH v2 1/5] perf probe: Split add_perf_probe_events() Namhyung Kim
                   ` (2 preceding siblings ...)
  2015-09-04 12:16 ` [PATCH v2 4/5] perf probe: Split del_perf_probe_events() Namhyung Kim
@ 2015-09-04 12:16 ` Namhyung Kim
  2015-09-07  1:12   ` 平松雅巳 / HIRAMATU,MASAMI
  2015-09-08 14:40   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2015-09-06  7:47 ` [PATCH v2 1/5] perf probe: Split add_perf_probe_events() Wangnan (F)
  2015-09-08 14:39 ` [tip:perf/core] " tip-bot for Namhyung Kim
  5 siblings, 2 replies; 18+ messages in thread
From: Namhyung Kim @ 2015-09-04 12:16 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Masami Hiramatsu,
	Wang Nan, pi3orama

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 |  1 +
 tools/perf/util/probe-file.c  |  7 +++--
 tools/perf/util/probe-file.h  |  4 +++
 5 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 @@ out_cleanup:
 	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..9bcea36359f2 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -144,6 +144,7 @@ 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);
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 @@ error:
 	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;
diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index ada94a242a17..18ac9cf51c34 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -14,5 +14,9 @@ struct strlist *probe_file__get_namelist(int fd);
 struct strlist *probe_file__get_rawlist(int fd);
 int probe_file__add_event(int fd, struct probe_trace_event *tev);
 int probe_file__del_events(int fd, struct strfilter *filter);
+int probe_file__get_events(int fd, struct strfilter *filter,
+				  struct strlist *plist);
+int probe_file__del_strlist(int fd, struct strlist *namelist);
+
 
 #endif
-- 
2.5.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 1/5] perf probe: Split add_perf_probe_events()
  2015-09-04 12:15 [PATCH v2 1/5] perf probe: Split add_perf_probe_events() Namhyung Kim
                   ` (3 preceding siblings ...)
  2015-09-04 12:16 ` [PATCH v2 5/5] perf probe: Print deleted events in cmd_probe() Namhyung Kim
@ 2015-09-06  7:47 ` Wangnan (F)
  2015-09-08  1:53   ` Namhyung Kim
  2015-09-10  2:23   ` Namhyung Kim
  2015-09-08 14:39 ` [tip:perf/core] " tip-bot for Namhyung Kim
  5 siblings, 2 replies; 18+ messages in thread
From: Wangnan (F) @ 2015-09-06  7:47 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, Masami Hiramatsu, pi3orama

Hi Namhyung,

Thanks for this patchset.

Could you plase have a look at patch 5/27 and 6/27 in my newest pull 
request?
These 2 patches utilize new probing API to create probe point and collect
probe_trace_events. I'm not very sure I fully understand your design 
principle,
especially the cleanup part, because I can see different functions 
dealing with
cleanup:

cleanup_perf_probe_events
del_perf_probe_events
clear_perf_probe_event
clear_probe_trace_event

But non of them works perfectly for me.

In bpf_prog_priv__clear() function of 6/27, I copied some code from
cleanup_perf_probe_events(), because I think when destroying bpf programs,
the probe_trace_events should also be cleanuped, but we don't need call
exit_symbol_maps() many times, because we are in 'perf record', and not
sure whether other parts of perf need symbol maps. Otherwise I think 
directly
calling cleanup_perf_probe_events() sould be better.

You can find patch from:

http://lkml.kernel.org/n/1441523623-152703-6-git-send-email-wangnan0@huawei.com

http://lkml.kernel.org/n/1441523623-152703-7-git-send-email-wangnan0@huawei.com

Thank you.

On 2015/9/4 20:15, Namhyung Kim wrote:
> 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 @@ end:
>   	}
>   	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;
>   }



^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v2 5/5] perf probe: Print deleted events in cmd_probe()
  2015-09-04 12:16 ` [PATCH v2 5/5] perf probe: Print deleted events in cmd_probe() Namhyung Kim
@ 2015-09-07  1:12   ` 平松雅巳 / HIRAMATU,MASAMI
  2015-09-08 14:40   ` [tip:perf/core] " tip-bot for Namhyung Kim
  1 sibling, 0 replies; 18+ messages in thread
From: 平松雅巳 / HIRAMATU,MASAMI @ 2015-09-07  1:12 UTC (permalink / raw)
  To: 'Namhyung Kim', Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Wang Nan, pi3orama

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

>From: Namhyung Kim [mailto:namhyung@gmail.com] On Behalf Of 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.
>

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    | 62 ++++++++++++++++++++++++++++++++++++++++++-
> tools/perf/util/probe-event.c |  5 ----
> tools/perf/util/probe-event.h |  1 +
> tools/perf/util/probe-file.c  |  7 +++--
> tools/perf/util/probe-file.h  |  4 +++
> 5 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 @@ out_cleanup:
> 	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..9bcea36359f2 100644
>--- a/tools/perf/util/probe-event.h
>+++ b/tools/perf/util/probe-event.h
>@@ -144,6 +144,7 @@ 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);
>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 @@ error:
> 	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;
>diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
>index ada94a242a17..18ac9cf51c34 100644
>--- a/tools/perf/util/probe-file.h
>+++ b/tools/perf/util/probe-file.h
>@@ -14,5 +14,9 @@ struct strlist *probe_file__get_namelist(int fd);
> struct strlist *probe_file__get_rawlist(int fd);
> int probe_file__add_event(int fd, struct probe_trace_event *tev);
> int probe_file__del_events(int fd, struct strfilter *filter);
>+int probe_file__get_events(int fd, struct strfilter *filter,
>+				  struct strlist *plist);
>+int probe_file__del_strlist(int fd, struct strlist *namelist);
>+
>
> #endif
>--
>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¥

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 1/5] perf probe: Split add_perf_probe_events()
  2015-09-06  7:47 ` [PATCH v2 1/5] perf probe: Split add_perf_probe_events() Wangnan (F)
@ 2015-09-08  1:53   ` Namhyung Kim
  2015-09-10  2:23   ` Namhyung Kim
  1 sibling, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2015-09-08  1:53 UTC (permalink / raw)
  To: Wangnan (F)
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, Masami Hiramatsu, pi3orama

On Sun, Sep 06, 2015 at 03:47:37PM +0800, Wangnan (F) wrote:
> Hi Namhyung,

Hi,

I'm off until Wednesday.  I'll be able to take a look at it on
Thursday.

Thanks,
Namhyung


> 
> Thanks for this patchset.
> 
> Could you plase have a look at patch 5/27 and 6/27 in my newest pull
> request?
> These 2 patches utilize new probing API to create probe point and collect
> probe_trace_events. I'm not very sure I fully understand your design
> principle,
> especially the cleanup part, because I can see different functions dealing
> with
> cleanup:
> 
> cleanup_perf_probe_events
> del_perf_probe_events
> clear_perf_probe_event
> clear_probe_trace_event
> 
> But non of them works perfectly for me.
> 
> In bpf_prog_priv__clear() function of 6/27, I copied some code from
> cleanup_perf_probe_events(), because I think when destroying bpf programs,
> the probe_trace_events should also be cleanuped, but we don't need call
> exit_symbol_maps() many times, because we are in 'perf record', and not
> sure whether other parts of perf need symbol maps. Otherwise I think
> directly
> calling cleanup_perf_probe_events() sould be better.
> 
> You can find patch from:
> 
> http://lkml.kernel.org/n/1441523623-152703-6-git-send-email-wangnan0@huawei.com
> 
> http://lkml.kernel.org/n/1441523623-152703-7-git-send-email-wangnan0@huawei.com
> 
> Thank you.
> 
> On 2015/9/4 20:15, Namhyung Kim wrote:
> >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 @@ end:
> >  	}
> >  	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;
> >  }
> 
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [tip:perf/core] perf probe: Split add_perf_probe_events()
  2015-09-04 12:15 [PATCH v2 1/5] perf probe: Split add_perf_probe_events() Namhyung Kim
                   ` (4 preceding siblings ...)
  2015-09-06  7:47 ` [PATCH v2 1/5] perf probe: Split add_perf_probe_events() Wangnan (F)
@ 2015-09-08 14:39 ` tip-bot for Namhyung Kim
  5 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Namhyung Kim @ 2015-09-08 14:39 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: masami.hiramatsu.pt, mingo, tglx, wangnan0, hpa, linux-kernel,
	jolsa, a.p.zijlstra, namhyung, acme

Commit-ID:  844dffa598b55fca6a83a856214fc0cfc3da24e5
Gitweb:     http://git.kernel.org/tip/844dffa598b55fca6a83a856214fc0cfc3da24e5
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Fri, 4 Sep 2015 21:15:59 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 4 Sep 2015 12:33:02 -0300

perf probe: Split add_perf_probe_events()

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.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1441368963-11565-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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 eb5f18b..2c762f4 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 @@ end:
 	}
 	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;
 }

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [tip:perf/core] perf probe: Link trace_probe_event into perf_probe_event
  2015-09-04 12:16 ` [PATCH v2 2/5] perf probe: Attach trace_probe_event with perf_probe_event Namhyung Kim
@ 2015-09-08 14:39   ` tip-bot for Wang Nan
  0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Wang Nan @ 2015-09-08 14:39 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: wangnan0, linux-kernel, tglx, masami.hiramatsu.pt, a.p.zijlstra,
	hpa, mingo, acme, jolsa, namhyung

Commit-ID:  12fae5ef6dc6031ffcf4dffc6be5a16080e7dd7d
Gitweb:     http://git.kernel.org/tip/12fae5ef6dc6031ffcf4dffc6be5a16080e7dd7d
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Fri, 4 Sep 2015 21:16:00 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 4 Sep 2015 12:34:23 -0300

perf probe: Link trace_probe_event into perf_probe_event

This patch drops struct __event_package structure.  Instead, it adds a
'struct trace_probe_event' pointer to 'struct perf_probe_event'.

The trace_probe_event information gives further patches a chance to
access actual probe points and actual arguments.

Using them, 'perf probe' can get the whole list of added probes and
print them at once.

Other users like the upcoming bpf_loader will be able to attach one bpf
program to different probing points of an inline function (which has
multiple probing points) and glob functions.

Moreover, by reading the arguments information, bpf code for reading
those arguments can be generated.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1441368963-11565-2-git-send-email-namhyung@kernel.org
[namhyung: extract necessary part from the existing patch]
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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 2c762f4..0d3a051 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 6e7ec68..70c327b 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,

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [tip:perf/core] perf probe: Move print logic into cmd_probe()
  2015-09-04 12:16 ` [PATCH v2 3/5] perf probe: Move print logic into cmd_probe() Namhyung Kim
@ 2015-09-08 14:39   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Namhyung Kim @ 2015-09-08 14:39 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, wangnan0, namhyung, masami.hiramatsu.pt, a.p.zijlstra,
	tglx, hpa, jolsa, acme, linux-kernel

Commit-ID:  b02137cc6550c1fa28e9a7c943a79fe6e4c4378d
Gitweb:     http://git.kernel.org/tip/b02137cc6550c1fa28e9a7c943a79fe6e4c4378d
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Fri, 4 Sep 2015 21:16:01 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 4 Sep 2015 12:37:17 -0300

perf probe: Move print logic into cmd_probe()

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

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1441368963-11565-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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 b81cec3..b8cf6cb 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 0d3a051..01b9a5b 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2180,9 +2180,9 @@ out:
 }
 
 /* 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 70c327b..610f743 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);

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [tip:perf/core] perf probe: Split del_perf_probe_events()
  2015-09-04 12:16 ` [PATCH v2 4/5] perf probe: Split del_perf_probe_events() Namhyung Kim
@ 2015-09-08 14:40   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Namhyung Kim @ 2015-09-08 14:40 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: masami.hiramatsu.pt, acme, namhyung, linux-kernel, a.p.zijlstra,
	hpa, mingo, tglx, wangnan0, jolsa

Commit-ID:  e7895e422e4da63daedacad0a2ed1a5a6b502d66
Gitweb:     http://git.kernel.org/tip/e7895e422e4da63daedacad0a2ed1a5a6b502d66
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Fri, 4 Sep 2015 21:16:02 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 4 Sep 2015 12:43:19 -0300

perf probe: Split del_perf_probe_events()

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.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1441368963-11565-4-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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 bbb2437..f00b0df 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -275,7 +275,8 @@ error:
 	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;
+}

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [tip:perf/core] perf probe: Print deleted events in cmd_probe()
  2015-09-04 12:16 ` [PATCH v2 5/5] perf probe: Print deleted events in cmd_probe() Namhyung Kim
  2015-09-07  1:12   ` 平松雅巳 / HIRAMATU,MASAMI
@ 2015-09-08 14:40   ` tip-bot for Namhyung Kim
  1 sibling, 0 replies; 18+ messages in thread
From: tip-bot for Namhyung Kim @ 2015-09-08 14:40 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, acme, a.p.zijlstra, linux-kernel, wangnan0, namhyung,
	mingo, masami.hiramatsu.pt, jolsa, hpa

Commit-ID:  e607f1426b584f2bd3f688a2d416baf963251e7a
Gitweb:     http://git.kernel.org/tip/e607f1426b584f2bd3f688a2d416baf963251e7a
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Fri, 4 Sep 2015 21:16:03 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 4 Sep 2015 12:43:44 -0300

perf probe: Print deleted events in cmd_probe()

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.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1441368963-11565-5-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-probe.c    | 62 ++++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/probe-event.c |  5 ----
 tools/perf/util/probe-event.h |  1 +
 tools/perf/util/probe-file.c  |  7 +++--
 tools/perf/util/probe-file.h  |  4 +++
 5 files changed, 69 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index b8cf6cb..ee2c46d 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 @@ out_cleanup:
 	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 01b9a5b..3da9e1c 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 610f743..9bcea36 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -144,6 +144,7 @@ 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);
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index f00b0df..38c0a62 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 @@ error:
 	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;
diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index ada94a2..18ac9cf 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -14,5 +14,9 @@ struct strlist *probe_file__get_namelist(int fd);
 struct strlist *probe_file__get_rawlist(int fd);
 int probe_file__add_event(int fd, struct probe_trace_event *tev);
 int probe_file__del_events(int fd, struct strfilter *filter);
+int probe_file__get_events(int fd, struct strfilter *filter,
+				  struct strlist *plist);
+int probe_file__del_strlist(int fd, struct strlist *namelist);
+
 
 #endif

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 1/5] perf probe: Split add_perf_probe_events()
  2015-09-06  7:47 ` [PATCH v2 1/5] perf probe: Split add_perf_probe_events() Wangnan (F)
  2015-09-08  1:53   ` Namhyung Kim
@ 2015-09-10  2:23   ` Namhyung Kim
  2015-09-10  5:00     ` 平松雅巳 / HIRAMATU,MASAMI
  1 sibling, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2015-09-10  2:23 UTC (permalink / raw)
  To: Wangnan (F)
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, Masami Hiramatsu, pi3orama

On Sun, Sep 06, 2015 at 03:47:37PM +0800, Wangnan (F) wrote:
> Hi Namhyung,

Hi,

> 
> Thanks for this patchset.
> 
> Could you plase have a look at patch 5/27 and 6/27 in my newest pull
> request?
> These 2 patches utilize new probing API to create probe point and collect
> probe_trace_events. I'm not very sure I fully understand your design
> principle,
> especially the cleanup part, because I can see different functions dealing
> with
> cleanup:
> 
> cleanup_perf_probe_events
> del_perf_probe_events
> clear_perf_probe_event
> clear_probe_trace_event
> 
> But non of them works perfectly for me.

The cleanup_perf_probe_events() is just to keep the existing logic as
long as possible.  But I think it needs to call
clear_perf_probe_event().

The del_perf_probe_events() uses strfilter, but I think it can be
problematic if other instances or users are using similar events at
the same time.

So for your case, IMHO it'd better keeping the perf/trace events after
probing and reusing the events for unprobing.  I'll take a look at it.


> 
> In bpf_prog_priv__clear() function of 6/27, I copied some code from
> cleanup_perf_probe_events(), because I think when destroying bpf programs,
> the probe_trace_events should also be cleanuped, but we don't need call
> exit_symbol_maps() many times, because we are in 'perf record', and not
> sure whether other parts of perf need symbol maps. Otherwise I think
> directly
> calling cleanup_perf_probe_events() sould be better.

Yeah, I also think exit_symbol_maps() should not be a part of the
cleanup.  I'll send a patch soon.


> 
> You can find patch from:
> 
> http://lkml.kernel.org/n/1441523623-152703-6-git-send-email-wangnan0@huawei.com
> 
> http://lkml.kernel.org/n/1441523623-152703-7-git-send-email-wangnan0@huawei.com

Thanks for your work!
Namhyung

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: Re: [PATCH v2 1/5] perf probe: Split add_perf_probe_events()
  2015-09-10  2:23   ` Namhyung Kim
@ 2015-09-10  5:00     ` 平松雅巳 / HIRAMATU,MASAMI
  2015-09-10  6:40       ` Namhyung Kim
  0 siblings, 1 reply; 18+ messages in thread
From: 平松雅巳 / HIRAMATU,MASAMI @ 2015-09-10  5:00 UTC (permalink / raw)
  To: 'Namhyung Kim', Wangnan (F)
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, pi3orama

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

>From: Namhyung Kim [mailto:namhyung@gmail.com] On Behalf Of Namhyung Kim
>
>On Sun, Sep 06, 2015 at 03:47:37PM +0800, Wangnan (F) wrote:
>> Hi Namhyung,
>
>Hi,
>
>>
>> Thanks for this patchset.
>>
>> Could you plase have a look at patch 5/27 and 6/27 in my newest pull
>> request?
>> These 2 patches utilize new probing API to create probe point and collect
>> probe_trace_events. I'm not very sure I fully understand your design
>> principle,
>> especially the cleanup part, because I can see different functions dealing
>> with
>> cleanup:
>>
>> cleanup_perf_probe_events

This is for clearing an array of probe events.

>> del_perf_probe_events

This is not for cleanup, but for removing probes in the kernel.

>> clear_perf_probe_event
>> clear_probe_trace_event

These are the cleanup each event. Ah, right, since now perf_probe_event has probe_trace_events,
clear_perf_probe_event has to call clear_probe_trace_event.

>>
>> But non of them works perfectly for me.
>
>The cleanup_perf_probe_events() is just to keep the existing logic as
>long as possible.  But I think it needs to call
>clear_perf_probe_event().
>
>The del_perf_probe_events() uses strfilter, but I think it can be
>problematic if other instances or users are using similar events at
>the same time.

Yeah, since perf probe doesn't lock the ftrace, there should be a
timing bug, but it can be fixed easily by ignoring -ENOENT. :) 

>So for your case, IMHO it'd better keeping the perf/trace events after
>probing and reusing the events for unprobing.  I'll take a look at it.
>
>
>>
>> In bpf_prog_priv__clear() function of 6/27, I copied some code from
>> cleanup_perf_probe_events(), because I think when destroying bpf programs,
>> the probe_trace_events should also be cleanuped, but we don't need call
>> exit_symbol_maps() many times, because we are in 'perf record', and not
>> sure whether other parts of perf need symbol maps. Otherwise I think
>> directly
>> calling cleanup_perf_probe_events() sould be better.
>
>Yeah, I also think exit_symbol_maps() should not be a part of the
>cleanup.  I'll send a patch soon.

OK.


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¥

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Re: [PATCH v2 1/5] perf probe: Split add_perf_probe_events()
  2015-09-10  5:00     ` 平松雅巳 / HIRAMATU,MASAMI
@ 2015-09-10  6:40       ` Namhyung Kim
  2015-09-10  8:10         ` 平松雅巳 / HIRAMATU,MASAMI
  0 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2015-09-10  6:40 UTC (permalink / raw)
  To: 平松雅巳 / HIRAMATU,MASAMI
  Cc: Wangnan (F),
	Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, pi3orama

Hi Masami,

On Thu, Sep 10, 2015 at 05:00:07AM +0000, 平松雅巳 / HIRAMATU,MASAMI wrote:
> >From: Namhyung Kim [mailto:namhyung@gmail.com] On Behalf Of Namhyung Kim
> >The del_perf_probe_events() uses strfilter, but I think it can be
> >problematic if other instances or users are using similar events at
> >the same time.
> 
> Yeah, since perf probe doesn't lock the ftrace, there should be a
> timing bug, but it can be fixed easily by ignoring -ENOENT. :) 

By ignoring -ENOENT?  Are you saying that there's a race between two
deleters?  Yes, of course, but I think that the bug will hit an adder
and a deleter especially if automatic probing is used (by eBPF and/or
SDT recording).

What about this?


>From 45dba35cb0f5fa1b2e78fec8c05faf5e9a1b200e Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung@kernel.org>
Date: Thu, 10 Sep 2015 15:25:28 +0900
Subject: [PATCH] perf probe: Support deleting trace events directly

Currently del_perf_probe_events() deletes events which match to a given
filter.  But it might have a timing bug when other users also set probes
with similar names.  So it'd be better deleting our events directly
rather than pattern matching.

Since the del_perf_probe_events() has no user at this time, change it to
receive perf_probe_event's.  It is more consistent to other APIs as well.

Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/probe-event.c | 30 +++++++++++-------------------
 tools/perf/util/probe-event.h |  2 +-
 tools/perf/util/probe-file.c  | 24 ++++++++++++++++++++++++
 tools/perf/util/probe-file.h  |  1 +
 4 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 2b78e8f19b45..88d7ef87ab99 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2810,37 +2810,29 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 	return ret;
 }
 
-int del_perf_probe_events(struct strfilter *filter)
+int del_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 {
-	int ret, ret2, ufd = -1, kfd = -1;
-	char *str = strfilter__string(filter);
+	int i, j, ret, ret2;
+	int ufd = -1, kfd = -1;
 
-	if (!str)
-		return -EINVAL;
-
-	/* Get current event names */
 	ret = probe_file__open_both(&kfd, &ufd, PF_FL_RW);
 	if (ret < 0)
-		goto out;
+		return ret;
 
-	ret = probe_file__del_events(kfd, filter);
-	if (ret < 0 && ret != -ENOENT)
-		goto error;
+	for (i = 0; i < npevs; i++) {
+		int fd = pevs[i].uprobes ? ufd : kfd;
 
-	ret2 = probe_file__del_events(ufd, filter);
-	if (ret2 < 0 && ret2 != -ENOENT) {
-		ret = ret2;
-		goto error;
+		for (j = 0; j < pevs[i].ntevs; j++) {
+			ret2 = probe_file__del_trace_event(fd, &pevs[i].tevs[j]);
+			if (ret == 0)
+				ret = ret2;
+		}
 	}
-	ret = 0;
 
-error:
 	if (kfd >= 0)
 		close(kfd);
 	if (ufd >= 0)
 		close(ufd);
-out:
-	free(str);
 
 	return ret;
 }
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index ba926c30f8cd..cb58b981cc92 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -145,7 +145,7 @@ 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 del_perf_probe_events(struct perf_probe_event *pevs, int npevs);
 
 extern int show_perf_probe_event(const char *group, const char *event,
 				 struct perf_probe_event *pev,
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 89dbeb92c68e..334b7b75b55b 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -319,3 +319,27 @@ int probe_file__del_events(int fd, struct strfilter *filter)
 
 	return ret;
 }
+
+int probe_file__del_trace_event(int fd, struct probe_trace_event *tev)
+{
+	char buf[128];
+	int ret;
+
+	/* Convert from perf-probe event to trace-probe event */
+	ret = e_snprintf(buf, 128, "-:%s/%s", tev->group, tev->event);
+	if (ret < 0)
+		goto error;
+
+	pr_debug("Writing event: %s\n", buf);
+	ret = write(fd, buf, strlen(buf));
+	if (ret < 0) {
+		ret = -errno;
+		goto error;
+	}
+
+	return 0;
+error:
+	pr_warning("Failed to delete event: %s\n",
+		   strerror_r(-ret, buf, sizeof(buf)));
+	return ret;
+}
diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index 18ac9cf51c34..02515ea12f1e 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -17,6 +17,7 @@ int probe_file__del_events(int fd, struct strfilter *filter);
 int probe_file__get_events(int fd, struct strfilter *filter,
 				  struct strlist *plist);
 int probe_file__del_strlist(int fd, struct strlist *namelist);
+int probe_file__del_trace_event(int fd, struct probe_trace_event *tev);
 
 
 #endif
-- 
2.5.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: Re: [PATCH v2 1/5] perf probe: Split add_perf_probe_events()
  2015-09-10  6:40       ` Namhyung Kim
@ 2015-09-10  8:10         ` 平松雅巳 / HIRAMATU,MASAMI
  2015-09-11 16:29           ` Namhyung Kim
  0 siblings, 1 reply; 18+ messages in thread
From: 平松雅巳 / HIRAMATU,MASAMI @ 2015-09-10  8:10 UTC (permalink / raw)
  To: 'Namhyung Kim'
  Cc: Wangnan (F),
	Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, pi3orama

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

Hi Namhyung,

From: Namhyung Kim [mailto:namhyung@gmail.com] On Behalf Of Namhyung Kim
>
>Hi Masami,
>
>On Thu, Sep 10, 2015 at 05:00:07AM +0000, 平松雅巳 / HIRAMATU,MASAMI wrote:
>> >From: Namhyung Kim [mailto:namhyung@gmail.com] On Behalf Of Namhyung Kim
>> >The del_perf_probe_events() uses strfilter, but I think it can be
>> >problematic if other instances or users are using similar events at
>> >the same time.
>>
>> Yeah, since perf probe doesn't lock the ftrace, there should be a
>> timing bug, but it can be fixed easily by ignoring -ENOENT. :)
>
>By ignoring -ENOENT?  Are you saying that there's a race between two
>deleters?  Yes, of course, but I think that the bug will hit an adder
>and a deleter especially if automatic probing is used (by eBPF and/or
>SDT recording).

So, I don't think we need the automatic event removing. Instead, I'd like to
suggest to keep it on the list.

>What about this?

Since probe events are identified only by its name, strfilter still works.
You can remove specific event without any wildcard.

Thanks,


>From 45dba35cb0f5fa1b2e78fec8c05faf5e9a1b200e Mon Sep 17 00:00:00 2001
>From: Namhyung Kim <namhyung@kernel.org>
>Date: Thu, 10 Sep 2015 15:25:28 +0900
>Subject: [PATCH] perf probe: Support deleting trace events directly
>
>Currently del_perf_probe_events() deletes events which match to a given
>filter.  But it might have a timing bug when other users also set probes
>with similar names.  So it'd be better deleting our events directly
>rather than pattern matching.
>
>Since the del_perf_probe_events() has no user at this time, change it to
>receive perf_probe_event's.  It is more consistent to other APIs as well.
>
>Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>Cc: Wang Nan <wangnan0@huawei.com>
>Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>---
> tools/perf/util/probe-event.c | 30 +++++++++++-------------------
> tools/perf/util/probe-event.h |  2 +-
> tools/perf/util/probe-file.c  | 24 ++++++++++++++++++++++++
> tools/perf/util/probe-file.h  |  1 +
> 4 files changed, 37 insertions(+), 20 deletions(-)
>
>diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>index 2b78e8f19b45..88d7ef87ab99 100644
>--- a/tools/perf/util/probe-event.c
>+++ b/tools/perf/util/probe-event.c
>@@ -2810,37 +2810,29 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
> 	return ret;
> }
>
>-int del_perf_probe_events(struct strfilter *filter)
>+int del_perf_probe_events(struct perf_probe_event *pevs, int npevs)
> {
>-	int ret, ret2, ufd = -1, kfd = -1;
>-	char *str = strfilter__string(filter);
>+	int i, j, ret, ret2;
>+	int ufd = -1, kfd = -1;
>
>-	if (!str)
>-		return -EINVAL;
>-
>-	/* Get current event names */
> 	ret = probe_file__open_both(&kfd, &ufd, PF_FL_RW);
> 	if (ret < 0)
>-		goto out;
>+		return ret;
>
>-	ret = probe_file__del_events(kfd, filter);
>-	if (ret < 0 && ret != -ENOENT)
>-		goto error;
>+	for (i = 0; i < npevs; i++) {
>+		int fd = pevs[i].uprobes ? ufd : kfd;
>
>-	ret2 = probe_file__del_events(ufd, filter);
>-	if (ret2 < 0 && ret2 != -ENOENT) {
>-		ret = ret2;
>-		goto error;
>+		for (j = 0; j < pevs[i].ntevs; j++) {
>+			ret2 = probe_file__del_trace_event(fd, &pevs[i].tevs[j]);
>+			if (ret == 0)
>+				ret = ret2;
>+		}
> 	}
>-	ret = 0;
>
>-error:
> 	if (kfd >= 0)
> 		close(kfd);
> 	if (ufd >= 0)
> 		close(ufd);
>-out:
>-	free(str);
>
> 	return ret;
> }
>diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
>index ba926c30f8cd..cb58b981cc92 100644
>--- a/tools/perf/util/probe-event.h
>+++ b/tools/perf/util/probe-event.h
>@@ -145,7 +145,7 @@ 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 del_perf_probe_events(struct perf_probe_event *pevs, int npevs);
>
> extern int show_perf_probe_event(const char *group, const char *event,
> 				 struct perf_probe_event *pev,
>diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
>index 89dbeb92c68e..334b7b75b55b 100644
>--- a/tools/perf/util/probe-file.c
>+++ b/tools/perf/util/probe-file.c
>@@ -319,3 +319,27 @@ int probe_file__del_events(int fd, struct strfilter *filter)
>
> 	return ret;
> }
>+
>+int probe_file__del_trace_event(int fd, struct probe_trace_event *tev)
>+{
>+	char buf[128];
>+	int ret;
>+
>+	/* Convert from perf-probe event to trace-probe event */
>+	ret = e_snprintf(buf, 128, "-:%s/%s", tev->group, tev->event);
>+	if (ret < 0)
>+		goto error;
>+
>+	pr_debug("Writing event: %s\n", buf);
>+	ret = write(fd, buf, strlen(buf));
>+	if (ret < 0) {
>+		ret = -errno;
>+		goto error;
>+	}
>+
>+	return 0;
>+error:
>+	pr_warning("Failed to delete event: %s\n",
>+		   strerror_r(-ret, buf, sizeof(buf)));
>+	return ret;
>+}
>diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
>index 18ac9cf51c34..02515ea12f1e 100644
>--- a/tools/perf/util/probe-file.h
>+++ b/tools/perf/util/probe-file.h
>@@ -17,6 +17,7 @@ int probe_file__del_events(int fd, struct strfilter *filter);
> int probe_file__get_events(int fd, struct strfilter *filter,
> 				  struct strlist *plist);
> int probe_file__del_strlist(int fd, struct strlist *namelist);
>+int probe_file__del_trace_event(int fd, struct probe_trace_event *tev);
>
>
> #endif
>--
>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¥

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Re: [PATCH v2 1/5] perf probe: Split add_perf_probe_events()
  2015-09-10  8:10         ` 平松雅巳 / HIRAMATU,MASAMI
@ 2015-09-11 16:29           ` Namhyung Kim
  0 siblings, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2015-09-11 16:29 UTC (permalink / raw)
  To: 平松雅巳 / HIRAMATU,MASAMI
  Cc: Wangnan (F),
	Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
	LKML, pi3orama

On Thu, Sep 10, 2015 at 08:10:16AM +0000, 平松雅巳 / HIRAMATU,MASAMI wrote:
> Hi Namhyung,
> 
> From: Namhyung Kim [mailto:namhyung@gmail.com] On Behalf Of Namhyung Kim
> >
> >Hi Masami,
> >
> >On Thu, Sep 10, 2015 at 05:00:07AM +0000, 平松雅巳 / HIRAMATU,MASAMI wrote:
> >> >From: Namhyung Kim [mailto:namhyung@gmail.com] On Behalf Of Namhyung Kim
> >> >The del_perf_probe_events() uses strfilter, but I think it can be
> >> >problematic if other instances or users are using similar events at
> >> >the same time.
> >>
> >> Yeah, since perf probe doesn't lock the ftrace, there should be a
> >> timing bug, but it can be fixed easily by ignoring -ENOENT. :)
> >
> >By ignoring -ENOENT?  Are you saying that there's a race between two
> >deleters?  Yes, of course, but I think that the bug will hit an adder
> >and a deleter especially if automatic probing is used (by eBPF and/or
> >SDT recording).
> 
> So, I don't think we need the automatic event removing. Instead, I'd like to
> suggest to keep it on the list.

But why?  Do you want reuse the probes for next record session?

I think if something is generated automatically, it should be removed
automatically..

Thanks,
Namhyung

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-09-11 16:32 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-04 12:15 [PATCH v2 1/5] perf probe: Split add_perf_probe_events() Namhyung Kim
2015-09-04 12:16 ` [PATCH v2 2/5] perf probe: Attach trace_probe_event with perf_probe_event Namhyung Kim
2015-09-08 14:39   ` [tip:perf/core] perf probe: Link trace_probe_event into perf_probe_event tip-bot for Wang Nan
2015-09-04 12:16 ` [PATCH v2 3/5] perf probe: Move print logic into cmd_probe() Namhyung Kim
2015-09-08 14:39   ` [tip:perf/core] " tip-bot for Namhyung Kim
2015-09-04 12:16 ` [PATCH v2 4/5] perf probe: Split del_perf_probe_events() Namhyung Kim
2015-09-08 14:40   ` [tip:perf/core] " tip-bot for Namhyung Kim
2015-09-04 12:16 ` [PATCH v2 5/5] perf probe: Print deleted events in cmd_probe() Namhyung Kim
2015-09-07  1:12   ` 平松雅巳 / HIRAMATU,MASAMI
2015-09-08 14:40   ` [tip:perf/core] " tip-bot for Namhyung Kim
2015-09-06  7:47 ` [PATCH v2 1/5] perf probe: Split add_perf_probe_events() Wangnan (F)
2015-09-08  1:53   ` Namhyung Kim
2015-09-10  2:23   ` Namhyung Kim
2015-09-10  5:00     ` 平松雅巳 / HIRAMATU,MASAMI
2015-09-10  6:40       ` Namhyung Kim
2015-09-10  8:10         ` 平松雅巳 / HIRAMATU,MASAMI
2015-09-11 16:29           ` Namhyung Kim
2015-09-08 14:39 ` [tip:perf/core] " tip-bot for Namhyung Kim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.