| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150612191200.GB6850@kernel.org>
Date: Fri, 12 Jun 2015 16:12:00 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Cc: Naohiro Aota <naota@...sp.net>,
Peter Zijlstra <peterz@...radead.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
David Ahern <dsahern@...il.com>, namhyung@...nel.org,
Jiri Olsa <jolsa@...hat.com>, Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH perf/core 1/4] [BUGFIX] perf probe: List probes in stdout
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@...c/linux-3/fs/read_write.c)
> -----
> With this fix,
> -----
> # perf probe -l > list
> # cat list
> probe:vfs_read (on vfs_read@...c/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@...ux/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@...c/linux-3/fs/read_write.c)
> probe:vfs_write (on vfs_write@...c/linux-3/fs/read_write.c)
> -----
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> Reported-by: Naohiro Aota <naota@...sp.net>
> ---
> 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists