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 <masami.hiramatsu...@hitachi.com>
> Reported-by: Naohiro Aota <na...@elisp.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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to