lkml.org 
[lkml]   [2015]   [Sep]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: Re: [PATCH v2 1/5] perf probe: Split add_perf_probe_events()
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


\
 
 \ /
  Last update: 2015-09-10 09:01    [W:0.066 / U:0.252 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site