From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753392AbbIJGoQ (ORCPT ); Thu, 10 Sep 2015 02:44:16 -0400 Received: from mail-pa0-f44.google.com ([209.85.220.44]:36847 "EHLO mail-pa0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750843AbbIJGoN (ORCPT ); Thu, 10 Sep 2015 02:44:13 -0400 Date: Thu, 10 Sep 2015 15:40:55 +0900 From: Namhyung Kim To: =?utf-8?B?5bmz5p2+6ZuF5bezIC8gSElSQU1BVFXvvIxNQVNBTUk=?= Cc: "Wangnan (F)" , Arnaldo Carvalho de Melo , Ingo Molnar , Peter Zijlstra , Jiri Olsa , LKML , "pi3orama@163.com" Subject: Re: Re: [PATCH v2 1/5] perf probe: Split add_perf_probe_events() Message-ID: <20150910064055.GA18425@danjae.kornet> References: <1441368963-11565-1-git-send-email-namhyung@kernel.org> <55EBEF99.8010506@huawei.com> <20150910022325.GA32720@danjae.kornet> <50399556C9727B4D88A595C8584AAB375252011D@GSjpTKYDCembx32.service.hitachi.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <50399556C9727B4D88A595C8584AAB375252011D@GSjpTKYDCembx32.service.hitachi.net> User-Agent: Mutt/1.5.23+102 (2ca89bed6448) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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