Hi Namhyung,
From: Namhyung Kim [mailto:namhy...@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:namhy...@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 <namhy...@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...@hitachi.com>
>Cc: Wang Nan <wangn...@huawei.com>
>Signed-off-by: Namhyung Kim <namhy...@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
