From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753010AbbJTPNG (ORCPT <rfc822;w@1wt.eu>);
	Tue, 20 Oct 2015 11:13:06 -0400
Received: from mail.kernel.org ([198.145.29.136]:53274 "EHLO mail.kernel.org"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752512AbbJTPNB (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Tue, 20 Oct 2015 11:13:01 -0400
Date: Tue, 20 Oct 2015 12:12:55 -0300
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Wang Nan <wangnan0@huawei.com>
Cc: ast@plumgrid.com, brendan.d.gregg@gmail.com, a.p.zijlstra@chello.nl,
        daniel@iogearbox.net, dsahern@gmail.com, hekuang@huawei.com,
        jolsa@kernel.org, lizefan@huawei.com, masami.hiramatsu.pt@hitachi.com,
        namhyung@kernel.org, paulus@samba.org, linux-kernel@vger.kernel.org,
        pi3orama@163.com, xiakaixu@huawei.com
Subject: Re: [PATCH 03/31] perf tools: Enable passing bpf object file to
 --event
Message-ID: <20151020151255.GF5119@kernel.org>
References: <1444826502-49291-1-git-send-email-wangnan0@huawei.com>
 <1444826502-49291-4-git-send-email-wangnan0@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1444826502-49291-4-git-send-email-wangnan0@huawei.com>
X-Url: http://acmel.wordpress.com
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Em Wed, Oct 14, 2015 at 12:41:14PM +0000, Wang Nan escreveu:
> By introducing new rules in tools/perf/util/parse-events.[ly], this
> patch enables 'perf record --event bpf_file.o' to select events by an
> eBPF object file. It calls parse_events_load_bpf() to load that file,
> which uses bpf__prepare_load() and finally calls bpf_object__open() for
> the object files.
> 
> After applying this patch, commands like:
> 
>  # perf record --event foo.o sleep
> 
> become possible.

So, trying the above command I get almost perfect output:

  [root@felicio ~]# perf record --event foo.o sleep
  libbpf: failed to open foo.o: No such file or directory
  event syntax error: 'foo.o'
                       \___ BPF object file 'foo.o' is invalid

  (add -v to see detail)
  Run 'perf list' for a list of valid events

   Usage: perf record [<options>] [<command>]
      or: perf record [<options>] -- <command> [<options>]

      -e, --event <event>   event selector. use 'perf list' to list available events
  [root@felicio ~]# 


Good thing would be to not have any message from libbpf and the right error
message from the parser, i.e. the first three lines become these two:


  event syntax error: 'foo.o'
                       \___ BPF object file 'foo.o' not found.o

But that can be fixed up in an upcoming patch, so I am applying this one now in
my new attempt at processing this patchkit.

- Arnaldo

 
> However, at this point it is unable to link any useful things onto the
> evsel list because the creating of probe points and BPF program
> attaching have not been implemented.  Before real events are possible to
> be extracted, to avoid perf report error because of empty evsel list,
> this patch link a dummy evsel. The dummy event related code will be
> removed when probing and extracting code is ready.
> 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Alexei Starovoitov <ast@plumgrid.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.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/ebpf-6yw9eg0ej3l4jnqhinngkw86@git.kernel.org
> ---
>  tools/perf/perf.c              |  2 ++
>  tools/perf/util/Build          |  1 +
>  tools/perf/util/parse-events.c | 57 ++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/parse-events.h |  8 ++++++
>  tools/perf/util/parse-events.l |  3 +++
>  tools/perf/util/parse-events.y | 18 ++++++++++++-
>  6 files changed, 88 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index 5437134..3d4c7c0 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -15,6 +15,7 @@
>  #include "util/run-command.h"
>  #include "util/parse-events.h"
>  #include "util/parse-options.h"
> +#include "util/bpf-loader.h"
>  #include "util/debug.h"
>  #include <api/fs/tracing_path.h>
>  #include <pthread.h>
> @@ -385,6 +386,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
>  	status = p->fn(argc, argv, prefix);
>  	exit_browser(status);
>  	perf_env__exit(&perf_env);
> +	bpf__clear();
>  
>  	if (status)
>  		return status & 0xff;
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 9217119..591b3fe 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -87,6 +87,7 @@ libperf-$(CONFIG_AUXTRACE) += intel-bts.o
>  libperf-y += parse-branch-options.o
>  libperf-y += parse-regs-options.o
>  
> +libperf-$(CONFIG_LIBBPF) += bpf-loader.o
>  libperf-$(CONFIG_LIBELF) += symbol-elf.o
>  libperf-$(CONFIG_LIBELF) += probe-file.o
>  libperf-$(CONFIG_LIBELF) += probe-event.o
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 991bbd4..a02abd3 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -11,6 +11,7 @@
>  #include "symbol.h"
>  #include "cache.h"
>  #include "header.h"
> +#include "bpf-loader.h"
>  #include "debug.h"
>  #include <api/fs/tracing_path.h>
>  #include "parse-events-bison.h"
> @@ -529,6 +530,62 @@ static int add_tracepoint_multi_sys(struct list_head *list, int *idx,
>  	return ret;
>  }
>  
> +int parse_events_load_bpf_obj(struct parse_events_evlist *data,
> +			      struct list_head *list,
> +			      struct bpf_object *obj)
> +{
> +	int err;
> +	char errbuf[BUFSIZ];
> +
> +	if (IS_ERR(obj) || !obj) {
> +		snprintf(errbuf, sizeof(errbuf),
> +			 "Internal error: load bpf obj with NULL");
> +		err = -EINVAL;
> +		goto errout;
> +	}
> +
> +	/*
> +	 * Temporary add a dummy event here so we can check whether
> +	 * basic bpf loader works. Following patches will replace
> +	 * dummy event by useful evsels.
> +	 */
> +	return parse_events_add_numeric(data, list, PERF_TYPE_SOFTWARE,
> +					PERF_COUNT_SW_DUMMY, NULL);
> +errout:
> +	data->error->help = strdup("(add -v to see detail)");
> +	data->error->str = strdup(errbuf);
> +	return err;
> +}
> +
> +int parse_events_load_bpf(struct parse_events_evlist *data,
> +			  struct list_head *list,
> +			  char *bpf_file_name)
> +{
> +	struct bpf_object *obj;
> +
> +	obj = bpf__prepare_load(bpf_file_name);
> +	if (IS_ERR(obj) || !obj) {
> +		char errbuf[BUFSIZ];
> +		int err;
> +
> +		err = obj ? PTR_ERR(obj) : -EINVAL;
> +
> +		if (err == -ENOTSUP)
> +			snprintf(errbuf, sizeof(errbuf),
> +				 "BPF support is not compiled");
> +		else
> +			snprintf(errbuf, sizeof(errbuf),
> +				 "BPF object file '%s' is invalid",
> +				 bpf_file_name);
> +
> +		data->error->help = strdup("(add -v to see detail)");
> +		data->error->str = strdup(errbuf);
> +		return err;
> +	}
> +
> +	return parse_events_load_bpf_obj(data, list, obj);
> +}
> +
>  static int
>  parse_breakpoint_type(const char *type, struct perf_event_attr *attr)
>  {
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index f13d3cc..fbb16c7 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -121,6 +121,14 @@ int parse_events_add_tracepoint(struct list_head *list, int *idx,
>  				char *sys, char *event,
>  				struct parse_events_error *error,
>  				struct list_head *head_config);
> +int parse_events_load_bpf(struct parse_events_evlist *data,
> +			  struct list_head *list,
> +			  char *bpf_file_name);
> +/* Provide this function for perf test */
> +struct bpf_object;
> +int parse_events_load_bpf_obj(struct parse_events_evlist *data,
> +			      struct list_head *list,
> +			      struct bpf_object *obj);
>  int parse_events_add_numeric(struct parse_events_evlist *data,
>  			     struct list_head *list,
>  			     u32 type, u64 config,
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index be24457..5e5d31a 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -115,6 +115,7 @@ do {							\
>  group		[^,{}/]*[{][^}]*[}][^,{}/]*
>  event_pmu	[^,{}/]+[/][^/]*[/][^,{}/]*
>  event		[^,{}/]+
> +bpf_object	.*\.(o|bpf)
>  
>  num_dec		[0-9]+
>  num_hex		0x[a-fA-F0-9]+
> @@ -159,6 +160,7 @@ modifier_bp	[rwx]{1,3}
>  		}
>  
>  {event_pmu}	|
> +{bpf_object}	|
>  {event}		{
>  			BEGIN(INITIAL);
>  			REWIND(1);
> @@ -264,6 +266,7 @@ r{num_raw_hex}		{ return raw(yyscanner); }
>  {num_hex}		{ return value(yyscanner, 16); }
>  
>  {modifier_event}	{ return str(yyscanner, PE_MODIFIER_EVENT); }
> +{bpf_object}		{ return str(yyscanner, PE_BPF_OBJECT); }
>  {name}			{ return pmu_str_check(yyscanner); }
>  "/"			{ BEGIN(config); return '/'; }
>  -			{ return '-'; }
> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index ae6af26..497f19b 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -42,6 +42,7 @@ static inc_group_count(struct list_head *list,
>  %token PE_VALUE PE_VALUE_SYM_HW PE_VALUE_SYM_SW PE_RAW PE_TERM
>  %token PE_EVENT_NAME
>  %token PE_NAME
> +%token PE_BPF_OBJECT
>  %token PE_MODIFIER_EVENT PE_MODIFIER_BP
>  %token PE_NAME_CACHE_TYPE PE_NAME_CACHE_OP_RESULT
>  %token PE_PREFIX_MEM PE_PREFIX_RAW PE_PREFIX_GROUP
> @@ -53,6 +54,7 @@ static inc_group_count(struct list_head *list,
>  %type <num> PE_RAW
>  %type <num> PE_TERM
>  %type <str> PE_NAME
> +%type <str> PE_BPF_OBJECT
>  %type <str> PE_NAME_CACHE_TYPE
>  %type <str> PE_NAME_CACHE_OP_RESULT
>  %type <str> PE_MODIFIER_EVENT
> @@ -70,6 +72,7 @@ static inc_group_count(struct list_head *list,
>  %type <tracepoint_name> tracepoint_name
>  %type <head> event_legacy_numeric
>  %type <head> event_legacy_raw
> +%type <head> event_bpf_file
>  %type <head> event_def
>  %type <head> event_mod
>  %type <head> event_name
> @@ -203,7 +206,8 @@ event_def: event_pmu |
>  	   event_legacy_mem |
>  	   event_legacy_tracepoint sep_dc |
>  	   event_legacy_numeric sep_dc |
> -	   event_legacy_raw sep_dc
> +	   event_legacy_raw sep_dc |
> +	   event_bpf_file
>  
>  event_pmu:
>  PE_NAME '/' event_config '/'
> @@ -449,6 +453,18 @@ PE_RAW
>  	$$ = list;
>  }
>  
> +event_bpf_file:
> +PE_BPF_OBJECT
> +{
> +	struct parse_events_evlist *data = _data;
> +	struct parse_events_error *error = data->error;
> +	struct list_head *list;
> +
> +	ALLOC_LIST(list);
> +	ABORT_ON(parse_events_load_bpf(data, list, $1));
> +	$$ = list;
> +}
> +
>  start_terms: event_config
>  {
>  	struct parse_events_terms *data = _data;
> -- 
> 1.8.3.4