Re: [PATCH 14/31] perf tools: Suppress probing messages when probing by BPF loading

[Namhyung Kim <namhyung@kernel.org>]

On Sat, Aug 29, 2015 at 04:21:48AM +0000, Wang Nan wrote:
> This patch suppresses message output by add_perf_probe_events() and
> del_perf_probe_events() if they are triggered by BPF loading. Before
> this patch, when using 'perf record' with BPF object/source as event
> selector, following message will be output:
> 
>      Added new event:
>            perf_bpf_probe:lock_page_ret (on __lock_page%return)
>         You can now use it in all perf tools, such as:
> 	            perf record -e perf_bpf_probe:lock_page_ret -aR sleep 1
>      ...
>      Removed event: perf_bpf_probe:lock_page_ret
> 
> Which is misleading, especially 'use it in all perf tools' because they
> will be removed after 'pref record' exit.
> 
> In this patch, a 'silent' field is appended into probe_conf to control
> output. bpf__{,un}probe() set it to true when calling
> {add,del}_perf_probe_events().

I think that printing those messages should be done in cmd_probe()
rather than add/del_perf_probe_events()..

Thanks,
Namhyung


> 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Alexei Starovoitov <ast@plumgrid.com>
> Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: He Kuang <hekuang@huawei.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Kaixu Xia <xiakaixu@huawei.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Zefan Li <lizefan@huawei.com>
> Cc: pi3orama@163.com
> Link: http://lkml.kernel.org/n/1440151770-129878-12-git-send-email-wangnan0@huawei.com
> ---
>  tools/perf/util/bpf-loader.c  |  6 ++++++
>  tools/perf/util/probe-event.c | 17 ++++++++++++-----
>  tools/perf/util/probe-event.h |  1 +
>  tools/perf/util/probe-file.c  |  5 ++++-
>  4 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index c3bc0a8..77eeb99 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -188,6 +188,7 @@ static bool is_probed;
>  int bpf__unprobe(void)
>  {
>  	struct strfilter *delfilter;
> +	bool old_silent = probe_conf.silent;
>  	int ret;
>  
>  	if (!is_probed)
> @@ -199,7 +200,9 @@ int bpf__unprobe(void)
>  		return -ENOMEM;
>  	}
>  
> +	probe_conf.silent = true;
>  	ret = del_perf_probe_events(delfilter);
> +	probe_conf.silent = old_silent;
>  	strfilter__delete(delfilter);
>  	if (ret < 0 && is_probed)
>  		pr_debug("Error: failed to delete events: %s\n",
> @@ -215,6 +218,7 @@ int bpf__probe(void)
>  	struct bpf_object *obj, *tmp;
>  	struct bpf_program *prog;
>  	struct perf_probe_event *pevs;
> +	bool old_silent = probe_conf.silent;
>  
>  	pevs = calloc(MAX_PROBES, sizeof(pevs[0]));
>  	if (!pevs)
> @@ -235,9 +239,11 @@ int bpf__probe(void)
>  		}
>  	}
>  
> +	probe_conf.silent = true;
>  	probe_conf.max_probes = MAX_PROBES;
>  	/* Let add_perf_probe_events generates probe_trace_event (tevs) */
>  	err = add_perf_probe_events(pevs, nr_events, false);
> +	probe_conf.silent = old_silent;
>  
>  	/* add_perf_probe_events return negative when fail */
>  	if (err < 0) {
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 57a7bae..e720913 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -52,7 +52,9 @@
>  #define PERFPROBE_GROUP "probe"
>  
>  bool probe_event_dry_run;	/* Dry run flag */
> -struct probe_conf probe_conf;
> +struct probe_conf probe_conf = {
> +	.silent = false,
> +};
>  
>  #define semantic_error(msg ...) pr_err("Semantic error :" msg)
>  
> @@ -2192,10 +2194,12 @@ static int show_perf_probe_event(const char *group, const char *event,
>  
>  	ret = perf_probe_event__sprintf(group, event, pev, module, &buf);
>  	if (ret >= 0) {
> -		if (use_stdout)
> +		if (use_stdout && !probe_conf.silent)
>  			printf("%s\n", buf.buf);
> -		else
> +		else if (!probe_conf.silent)
>  			pr_info("%s\n", buf.buf);
> +		else
> +			pr_debug("%s\n", buf.buf);
>  	}
>  	strbuf_release(&buf);
>  
> @@ -2418,7 +2422,10 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
>  	}
>  
>  	ret = 0;
> -	pr_info("Added new event%s\n", (ntevs > 1) ? "s:" : ":");
> +	if (!probe_conf.silent)
> +		pr_info("Added new event%s\n", (ntevs > 1) ? "s:" : ":");
> +	else
> +		pr_debug("Added new event%s\n", (ntevs > 1) ? "s:" : ":");
>  	for (i = 0; i < ntevs; i++) {
>  		tev = &tevs[i];
>  		/* Skip if the symbol is out of .text or blacklisted */
> @@ -2454,7 +2461,7 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
>  		warn_uprobe_event_compat(tev);
>  
>  	/* Note that it is possible to skip all events because of blacklist */
> -	if (ret >= 0 && event) {
> +	if (ret >= 0 && event && !probe_conf.silent) {
>  		/* Show how to use the event. */
>  		pr_info("\nYou can now use it in all perf tools, such as:\n\n");
>  		pr_info("\tperf record -e %s:%s -aR sleep 1\n\n", group, event);
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index 915f0d8..3ab9c3e 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -13,6 +13,7 @@ struct probe_conf {
>  	bool	force_add;
>  	bool	no_inlines;
>  	int	max_probes;
> +	bool	silent;
>  };
>  extern struct probe_conf probe_conf;
>  extern bool probe_event_dry_run;
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index bbb2437..db7bd4c 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -267,7 +267,10 @@ static int __del_trace_probe_event(int fd, struct str_node *ent)
>  		goto error;
>  	}
>  
> -	pr_info("Removed event: %s\n", ent->s);
> +	if (!probe_conf.silent)
> +		pr_info("Removed event: %s\n", ent->s);
> +	else
> +		pr_debug("Removed event: %s\n", ent->s);
>  	return 0;
>  error:
>  	pr_warning("Failed to delete event: %s\n",
> -- 
> 2.1.0
>