Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752334AbbFLTMR (ORCPT ); Fri, 12 Jun 2015 15:12:17 -0400 Received: from mail.kernel.org ([198.145.29.136]:35837 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750980AbbFLTMQ (ORCPT ); Fri, 12 Jun 2015 15:12:16 -0400 Date: Fri, 12 Jun 2015 16:12:00 -0300 From: Arnaldo Carvalho de Melo To: Masami Hiramatsu Cc: Naohiro Aota , Peter Zijlstra , Linux Kernel Mailing List , David Ahern , namhyung@kernel.org, Jiri Olsa , Ingo Molnar Subject: Re: [PATCH perf/core 1/4] [BUGFIX] perf probe: List probes in stdout Message-ID: <20150612191200.GB6850@kernel.org> References: <20150612050806.20548.12371.stgit@localhost.localdomain> <20150612050813.20548.16385.stgit@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150612050813.20548.16385.stgit@localhost.localdomain> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4995 Lines: 152 Em Fri, Jun 12, 2015 at 02:08:13PM +0900, Masami Hiramatsu escreveu: > Since commit 5e17b28f1e24 ("perf probe: Add --quiet option to > suppress output result message") have replaced printf with pr_info, > perf probe -l outputs its result in stderr. However, that is not > what the commit expected. > e.g. > ----- > # perf probe -l > /dev/null > probe:vfs_read (on vfs_read@ksrc/linux-3/fs/read_write.c) > ----- > With this fix, > ----- > # perf probe -l > list > # cat list > probe:vfs_read (on vfs_read@ksrc/linux-3/fs/read_write.c) Nope, with this fix just applied: [acme@zoo linux]$ rm -f ~/bin/perf;hash -r;type perf;rm -rf /tmp/build/perf ; mkdir -p /tmp/build/perf ; make -C tools/perf O=/tmp/build/perf install-bin > /dev/null; type perf bash: type: perf: not found PERF_VERSION = 4.1.rc5.g3f046e perf is /home/acme/bin/perf [acme@zoo linux]$ sudo ~acme/bin/perf probe -l > /tmp/bla probe:SYSC_perf_event_open (on SYSC_perf_event_open@linux/kernel/events/core.c) [acme@zoo linux]$ cat /tmp/bla [acme@zoo linux]$ git log --oneline | head -1 3f046ed885e1 perf probe: List probes in stdout acme@zoo linux]$ I see there is one place where this 'use_stdout' thing is always 'false', is that the problem? Do we really need to have this complexity? Why not plain use fprintf(stdout)? This is not a informative message, this is the command output, right? - Arnaldo > ----- > Of course, --quiet(-q) still works on --add/--del. > ----- > # perf probe -q vfs_write > # perf probe -l > probe:vfs_read (on vfs_read@ksrc/linux-3/fs/read_write.c) > probe:vfs_write (on vfs_write@ksrc/linux-3/fs/read_write.c) > ----- > > Signed-off-by: Masami Hiramatsu > Reported-by: Naohiro Aota > --- > tools/perf/util/probe-event.c | 49 +++++++++++++++++++++++++++++------------ > 1 file changed, 35 insertions(+), 14 deletions(-) > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index d4cf50b..710830c 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -2126,9 +2126,9 @@ kprobe_blacklist__find_by_address(struct list_head *blacklist, > return NULL; > } > > -/* Show an event */ > -static int show_perf_probe_event(struct perf_probe_event *pev, > - const char *module) > +static int perf_probe_event__sprintf(struct perf_probe_event *pev, > + const char *module, > + struct strbuf *result) > { > int i, ret; > char buf[128]; > @@ -2141,27 +2141,47 @@ static int show_perf_probe_event(struct perf_probe_event *pev, > > ret = e_snprintf(buf, 128, "%s:%s", pev->group, pev->event); > if (ret < 0) > - return ret; > + goto out; > > - pr_info(" %-20s (on %s", buf, place); > + strbuf_addf(result, " %-20s (on %s", buf, place); > if (module) > - pr_info(" in %s", module); > + strbuf_addf(result, " in %s", module); > > if (pev->nargs > 0) { > - pr_info(" with"); > + strbuf_addstr(result, " with"); > for (i = 0; i < pev->nargs; i++) { > ret = synthesize_perf_probe_arg(&pev->args[i], > buf, 128); > if (ret < 0) > - break; > - pr_info(" %s", buf); > + goto out; > + strbuf_addf(result, " %s", buf); > } > } > - pr_info(")\n"); > + strbuf_addch(result, ')'); > +out: > free(place); > return ret; > } > > +/* Show an event */ > +static int show_perf_probe_event(struct perf_probe_event *pev, > + const char *module, bool use_stdout) > +{ > + struct strbuf buf = STRBUF_INIT; > + int ret; > + > + ret = perf_probe_event__sprintf(pev, module, &buf); > + if (ret >= 0) { > + if (use_stdout) > + printf("%s\n", buf.buf); > + else > + pr_info("%s\n", buf.buf); > + } > + strbuf_release(&buf); > + > + return ret; > +} > + > static bool filter_probe_trace_event(struct probe_trace_event *tev, > struct strfilter *filter) > { > @@ -2200,9 +2220,10 @@ static int __show_perf_probe_events(int fd, bool is_kprobe, > goto next; > ret = convert_to_perf_probe_event(&tev, &pev, > is_kprobe); > - if (ret >= 0) > - ret = show_perf_probe_event(&pev, > - tev.point.module); > + if (ret < 0) > + goto next; > + ret = show_perf_probe_event(&pev, tev.point.module, > + false); > } > next: > clear_perf_probe_event(&pev); > @@ -2463,7 +2484,7 @@ static int __add_probe_trace_events(struct perf_probe_event *pev, > group = pev->group; > pev->event = tev->event; > pev->group = tev->group; > - show_perf_probe_event(pev, tev->point.module); > + show_perf_probe_event(pev, tev->point.module, true); > /* Trick here - restore current event/group */ > pev->event = (char *)event; > pev->group = (char *)group; > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/