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

[åæéå / HIRAMATUïMASAMI]

>From: Namhyung Kim [mailto:namhyung@xxxxxxxxx] 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@xxxxxxxxxxx>

Thanks!

>Cc: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
>Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
>---
> 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