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). What about this? >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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/