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 >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 >Cc: Wang Nan >Signed-off-by: Namhyung Kim >--- > 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++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I