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

From: Arnaldo Carvalho de Melo
Date: Fri Oct 23 2015 - 12:51:42 EST

Next message: Mathieu Poirier: "[PATCH] coresight: checking for NULL string in coresight_name_match()"
Previous message: kbuild test robot: "Re: [RFC] hwmon: ina2xx: port to using remap, improve bandwidth."
In reply to: Wang Nan: "[PATCH] perf tools: Enable pre-event inherit setting by config terms"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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@xxxxxxxxxx>
> Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Cc: Alexei Starovoitov <ast@xxxxxxxxxxxx>
> Cc: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
> Cc: Li Zefan <lizefan@xxxxxxxxxx>
> Cc: pi3orama@xxxxxxx
> Link: http://lkml.kernel.org/n/ebpf-2rrcnl73ih4o1qiu6t7bdxls@xxxxxxxxxxxxxx
> ---
>
> 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@xxxxxxxxxx
>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Mathieu Poirier: "[PATCH] coresight: checking for NULL string in coresight_name_match()"
Previous message: kbuild test robot: "Re: [RFC] hwmon: ina2xx: port to using remap, improve bandwidth."
In reply to: Wang Nan: "[PATCH] perf tools: Enable pre-event inherit setting by config terms"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]