Re: [PATCH] perf tools: Enable pre-event inherit setting by config terms

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Fri, Oct 23, 2015 at 09:23:22AM +0000, Wang Nan escreveu:
> This patch allows perf record setting event's attr.inherit bit by
> config terms like:
> 
>  # perf record -e cycles/no-inherit/ ...

I understand that the 'perf record' option is called --no-inherit, but
can we support both 'inherit' and 'no-inherit' here to avoid the double
negation:

	perf record -e cycles/no-inherit=0/

that would be more clear as:

	perf record -e cycles/inherit/

?

- Arnaldo
 
> So user can control inherit bit for each event separately.
> 
> In following example, a.out fork()s in main then do some complex
> CPU intensive computations in both of its children.
> 
> Basic result with and without inherit:
> 
>  # perf record -e cycles -e instructions ./a.out
>  [ perf record: Woken up 9 times to write data ]
>  [ perf record: Captured and wrote 2.205 MB perf.data (47920 samples) ]
>  # perf report --stdio
>  # ...
>  # Samples: 23K of event 'cycles'
>  # Event count (approx.): 23641752891
>  ...
>  # Samples: 24K of event 'instructions'
>  # Event count (approx.): 30428312415
> 
> 
>  # perf record -i -e cycles -e instructions ./a.out
>  [ perf record: Woken up 5 times to write data ]
>  [ perf record: Captured and wrote 1.111 MB perf.data (24019 samples) ]
>  ...
>  # Samples: 12K of event 'cycles'
>  # Event count (approx.): 11699501775
>  ...
>  # Samples: 12K of event 'instructions'
>  # Event count (approx.): 15058023559
> 
> Cancel inherit for one event when globally enable:
> 
>  # perf record -e cycles/no-inherit/ -e instructions ./a.out
>  [ perf record: Woken up 7 times to write data ]
>  [ perf record: Captured and wrote 1.660 MB perf.data (36004 samples) ]
>  ...
>  # Samples: 12K of event 'cycles/no-inherit/'
>  # Event count (approx.): 11895759282
>  ...
>  # Samples: 24K of event 'instructions'
>  # Event count (approx.): 30668000441
> 
> Enable inherit for one event when globally disable:
> 
>  # perf record -i -e cycles/no-inherit=0/ -e instructions ./a.out
>  [ perf record: Woken up 7 times to write data ]
>  [ perf record: Captured and wrote 1.654 MB perf.data (35868 samples) ]
>  ...
>  # Samples: 23K of event 'cycles/no-inherit=0/'
>  # Event count (approx.): 23285400229
>  ...
>  # Samples: 11K of event 'instructions'
>  # Event count (approx.): 14969050259
> 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Alexei Starovoitov <ast@plumgrid.com>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Li Zefan <lizefan@huawei.com>
> Cc: pi3orama@163.com
> Link: http://lkml.kernel.org/n/ebpf-2rrcnl73ih4o1qiu6t7bdxls@git.kernel.org
> ---
> 
> In [1], Alexei enforces BPF fd array that only no-inherit event can be
> used. Unfortunately, inherit is the default setting for perf and can
> only be turned off globally. We need this patch to support BPF PMU
> reading feature support for perf so we can insert a no-inherit perf
> event into fd array while other perf events still inherit.
> 
> [1] http://lkml.kernel.org/r/1445559014-4667-1-git-send-email-ast@kernel.org
> 
> ---
>  tools/perf/util/evsel.c        | 9 +++++++++
>  tools/perf/util/evsel.h        | 2 ++
>  tools/perf/util/parse-events.c | 7 +++++++
>  tools/perf/util/parse-events.h | 1 +
>  tools/perf/util/parse-events.l | 1 +
>  5 files changed, 20 insertions(+)
> 
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index ab05fa5..5566b16 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -652,6 +652,15 @@ static void apply_config_terms(struct perf_evsel *evsel,
>  		case PERF_EVSEL__CONFIG_TERM_STACK_USER:
>  			dump_size = term->val.stack_user;
>  			break;
> +		case PERF_EVSEL__CONFIG_TERM_NOINHERIT:
> +			/*
> +			 * attr->inherit should has already been set by
> +			 * perf_evsel__config. If user explicitly set
> +			 * inherit using config terms, override global
> +			 * opt->no_inherit setting.
> +			 */
> +			attr->inherit = !term->val.no_inherit;
> +			break;
>  		default:
>  			break;
>  		}
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 02a5fed..28dbac1 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -43,6 +43,7 @@ enum {
>  	PERF_EVSEL__CONFIG_TERM_TIME,
>  	PERF_EVSEL__CONFIG_TERM_CALLGRAPH,
>  	PERF_EVSEL__CONFIG_TERM_STACK_USER,
> +	PERF_EVSEL__CONFIG_TERM_NOINHERIT,
>  	PERF_EVSEL__CONFIG_TERM_MAX,
>  };
>  
> @@ -55,6 +56,7 @@ struct perf_evsel_config_term {
>  		bool	time;
>  		char	*callgraph;
>  		u64	stack_user;
> +		u64	no_inherit;
>  	} val;
>  };
>  
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 06ff8d6..478aecc 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -740,6 +740,9 @@ do {									   \
>  	case PARSE_EVENTS__TERM_TYPE_STACKSIZE:
>  		CHECK_TYPE_VAL(NUM);
>  		break;
> +	case PARSE_EVENTS__TERM_TYPE_NOINHERIT:
> +		CHECK_TYPE_VAL(NUM);
> +		break;
>  	case PARSE_EVENTS__TERM_TYPE_NAME:
>  		CHECK_TYPE_VAL(STR);
>  		break;
> @@ -775,6 +778,7 @@ static int config_term_tracepoint(struct perf_event_attr *attr,
>  	switch (term->type_term) {
>  	case PARSE_EVENTS__TERM_TYPE_CALLGRAPH:
>  	case PARSE_EVENTS__TERM_TYPE_STACKSIZE:
> +	case PARSE_EVENTS__TERM_TYPE_NOINHERIT:
>  		return config_term_common(attr, term, err);
>  	default:
>  		if (err) {
> @@ -838,6 +842,9 @@ do {								\
>  		case PARSE_EVENTS__TERM_TYPE_STACKSIZE:
>  			ADD_CONFIG_TERM(STACK_USER, stack_user, term->val.num);
>  			break;
> +		case PARSE_EVENTS__TERM_TYPE_NOINHERIT:
> +			ADD_CONFIG_TERM(NOINHERIT, no_inherit, term->val.num);
> +			break;
>  		default:
>  			break;
>  		}
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index fbb16c7..70cf07f 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -67,6 +67,7 @@ enum {
>  	PARSE_EVENTS__TERM_TYPE_TIME,
>  	PARSE_EVENTS__TERM_TYPE_CALLGRAPH,
>  	PARSE_EVENTS__TERM_TYPE_STACKSIZE,
> +	PARSE_EVENTS__TERM_TYPE_NOINHERIT
>  };
>  
>  struct parse_events_term {
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 5e5d31a..64d3ba8 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -189,6 +189,7 @@ branch_type		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE
>  time			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_TIME); }
>  call-graph		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CALLGRAPH); }
>  stack-size		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_STACKSIZE); }
> +no-inherit		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_NOINHERIT); }
>  ,			{ return ','; }
>  "/"			{ BEGIN(INITIAL); return '/'; }
>  {name_minus}		{ return str(yyscanner, PE_NAME); }
> -- 
> 1.8.3.4