[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABPqkBRyEmouSfJC=PYzgC8hTPmSzODVC81swnZvLPm3EG2vcQ@mail.gmail.com>
Date: Wed, 4 Apr 2012 21:50:12 -0700
From: Stephane Eranian <eranian@...gle.com>
To: Jiri Olsa <jolsa@...hat.com>
Cc: acme@...hat.com, a.p.zijlstra@...llo.nl, mingo@...e.hu,
paulus@...ba.org, cjashfor@...ux.vnet.ibm.com, fweisbec@...il.com,
linux-kernel@...r.kernel.org, David Ahern <dsahern@...il.com>
Subject: Re: [PATCH 5/8] perf, tool: Add pmu event parse support for
branch_sample_type values
On Wed, Apr 4, 2012 at 1:21 PM, Jiri Olsa <jolsa@...hat.com> wrote:
> Adding a support to specify branch_type as a hardcoded term
> inside the pmu event definition.
>
> It is possible to specify pmu event like:
> "cpu/config=1,branch_type=hv|any_ret|1,config2=2/u"
>
> Following string values could be used as value for branch_type:
> u (PERF_SAMPLE_BRANCH_USER)
> k (PERF_SAMPLE_BRANCH_KERNEL)
> hv (PERF_SAMPLE_BRANCH_HV)
> any (PERF_SAMPLE_BRANCH_ANY)
> any_call (PERF_SAMPLE_BRANCH_ANY_CALL)
> any_ret (PERF_SAMPLE_BRANCH_ANY_RETURN)
> ind_call (PERF_SAMPLE_BRANCH_IND_CALL)
>
> Also a number could be specified as value.
>
Although it would be nice to have, the current kernel sampling
buffer layout + perf would not be able to parse the RECORD_SAMPLE
if you were to sample different things for different events:
perf record -e
cpu/event=0xc0,umask=1:branch_type=any/,cpu/event=0x3c,umask=0x1/
Perf report/annotate would not be able to distinguish a RECORD_SAMPLE
generated by the first or the second event. That's because the RECORD_SAMPLE
fixed header does not contain enough info to determine which event caused the
record to be generated. You need to event ID to decode the sample. The event ID
gives you the attr struct which gives you the attr->sample_type which drives the
layout of the RECORD_SAMPLE variable size body. The event ID is currently
saved "somewhere" in the variable-size body of the sample. You have a chicken
and egg problem here.
Next week, I will post a patch to extend the existing format to support sampling
on multiple events with different sample_type bitmasks.
> CC: Stephane Eranian <eranian@...gle.com>
>
> Signed-off-by: Jiri Olsa <jolsa@...hat.com>
> ---
> tools/perf/builtin-record.c | 31 +-----------
> tools/perf/builtin-test.c | 27 +++++++++++
> tools/perf/util/parse-events.c | 100 ++++++++++++++++++++++++++++++++++++++--
> tools/perf/util/parse-events.h | 1 +
> 4 files changed, 127 insertions(+), 32 deletions(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index be4e1ee..87fc68d 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -641,27 +641,6 @@ out_delete_session:
> return err;
> }
>
> -#define BRANCH_OPT(n, m) \
> - { .name = n, .mode = (m) }
> -
> -#define BRANCH_END { .name = NULL }
> -
> -struct branch_mode {
> - const char *name;
> - int mode;
> -};
> -
> -static const struct branch_mode branch_modes[] = {
> - BRANCH_OPT("u", PERF_SAMPLE_BRANCH_USER),
> - BRANCH_OPT("k", PERF_SAMPLE_BRANCH_KERNEL),
> - BRANCH_OPT("hv", PERF_SAMPLE_BRANCH_HV),
> - BRANCH_OPT("any", PERF_SAMPLE_BRANCH_ANY),
> - BRANCH_OPT("any_call", PERF_SAMPLE_BRANCH_ANY_CALL),
> - BRANCH_OPT("any_ret", PERF_SAMPLE_BRANCH_ANY_RETURN),
> - BRANCH_OPT("ind_call", PERF_SAMPLE_BRANCH_IND_CALL),
> - BRANCH_END
> -};
> -
> static int
> parse_branch_stack(const struct option *opt, const char *str, int unset)
> {
> @@ -671,7 +650,6 @@ parse_branch_stack(const struct option *opt, const char *str, int unset)
> PERF_SAMPLE_BRANCH_HV)
>
> uint64_t *mode = (uint64_t *)opt->value;
> - const struct branch_mode *br;
> char *s, *os = NULL, *p;
> int ret = -1;
>
> @@ -692,21 +670,18 @@ parse_branch_stack(const struct option *opt, const char *str, int unset)
> return -1;
>
> for (;;) {
> + u64 bmode;
> p = strchr(s, ',');
> if (p)
> *p = '\0';
>
> - for (br = branch_modes; br->name; br++) {
> - if (!strcasecmp(s, br->name))
> - break;
> - }
> - if (!br->name) {
> + if (parse_events__branch_type(&bmode, s)) {
> ui__warning("unknown branch filter %s,"
> " check man page\n", s);
> goto error;
> }
>
> - *mode |= br->mode;
> + *mode |= bmode;
>
> if (!p)
> break;
> diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
> index d522cf4..2e4dc0a 100644
> --- a/tools/perf/builtin-test.c
> +++ b/tools/perf/builtin-test.c
> @@ -1008,6 +1008,29 @@ static int test__checkevent_pmu_name(struct perf_evlist *evlist)
> return 0;
> }
>
> +static int test__checkevent_pmu_branch_type(struct perf_evlist *evlist)
> +{
> + struct perf_evsel *evsel = list_entry(evlist->entries.next,
> + struct perf_evsel, node);
> +
> + TEST_ASSERT_VAL("wrong number of entries", 1 == evlist->nr_entries);
> + TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->attr.type);
> + TEST_ASSERT_VAL("wrong config", 1 == evsel->attr.config);
> + TEST_ASSERT_VAL("wrong config2", 2 == evsel->attr.config2);
> +
> +#define BT (PERF_SAMPLE_BRANCH_USER | \
> + PERF_SAMPLE_BRANCH_HV | \
> + PERF_SAMPLE_BRANCH_ANY_RETURN)
> +
> + TEST_ASSERT_VAL("wrong sample_type",
> + PERF_SAMPLE_BRANCH_STACK && evsel->attr.sample_type);
> + TEST_ASSERT_VAL("wrong branch_type",
> + BT == evsel->attr.branch_sample_type);
> +
> +#undef BT
> + return 0;
> +}
> +
> static struct test__event_st {
> const char *name;
> __u32 type;
> @@ -1117,6 +1140,10 @@ static struct test__event_st {
> .name = "cpu/config=1,name=krava/u,cpu/config=2/u",
> .check = test__checkevent_pmu_name,
> },
> + {
> + .name = "cpu/config=1,branch_type=hv|any_ret|1,config2=2/u",
> + .check = test__checkevent_pmu_branch_type,
> + },
> };
>
> #define TEST__EVENTS_CNT (sizeof(test__events) / sizeof(struct test__event_st))
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index c349cf3..076b1f8 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -590,6 +590,99 @@ int parse_events_add_breakpoint(struct list_head *list, int *idx,
> return add_event(list, idx, &attr, name);
> }
>
> +#define BRANCH_OPT(n, m) { .name = n, .mode = (m) }
> +#define BRANCH_END { .name = NULL }
> +
> +struct branch_mode {
> + const char *name;
> + int mode;
> +};
> +
> +static const struct branch_mode branch_modes[] = {
> + BRANCH_OPT("u", PERF_SAMPLE_BRANCH_USER),
> + BRANCH_OPT("k", PERF_SAMPLE_BRANCH_KERNEL),
> + BRANCH_OPT("hv", PERF_SAMPLE_BRANCH_HV),
> + BRANCH_OPT("any", PERF_SAMPLE_BRANCH_ANY),
> + BRANCH_OPT("any_call", PERF_SAMPLE_BRANCH_ANY_CALL),
> + BRANCH_OPT("any_ret", PERF_SAMPLE_BRANCH_ANY_RETURN),
> + BRANCH_OPT("ind_call", PERF_SAMPLE_BRANCH_IND_CALL),
> + BRANCH_END
> +};
> +
> +int parse_events__branch_type(u64 *_type, char *str)
> +{
> + const struct branch_mode *br;
> +
> + for (br = branch_modes; br->name; br++) {
> + if (!strcasecmp(str, br->name)) {
> + *_type = br->mode;
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +
> +typedef int (process_term_list_cb)(struct parse_events__term_elem *elem,
> + void *arg);
> +
> +static int branch_type_cb(struct parse_events__term_elem *elem, void *arg)
> +{
> + u64 *_type = arg;
> + u64 type;
> +
> + switch (elem->type_val) {
> + case PARSE_EVENTS__TERM_TYPE_NUM:
> + type = elem->val.num;
> + break;
> + case PARSE_EVENTS__TERM_TYPE_STR:
> + if (parse_events__branch_type(&type, elem->val.str))
> + return -EINVAL;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + *_type |= type;
> + return 0;
> +}
> +
> +static int process_term_list(struct parse_events__term *term,
> + process_term_list_cb cb, void *arg)
> +{
> + struct parse_events__term_elem *elem;
> +
> + list_for_each_entry(elem, &term->val.list, list)
> + if (cb(elem, arg))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int branch_type(struct parse_events__term *term, u64 *_type)
> +{
> + u64 type = 0;
> +
> + switch (term->type_val) {
> + case PARSE_EVENTS__TERM_TYPE_NUM:
> + type = term->val.num;
> + break;
> + case PARSE_EVENTS__TERM_TYPE_STR:
> + if (parse_events__branch_type(&type, term->val.str))
> + return -EINVAL;
> + break;
> + case PARSE_EVENTS__TERM_TYPE_LIST:
> + if (process_term_list(term, branch_type_cb, &type))
> + return -EINVAL;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + *_type = type;
> + return 0;
> +}
> +
> static int config_term(struct perf_event_attr *attr,
> struct parse_events__term *term)
> {
> @@ -617,10 +710,9 @@ do { \
> attr->sample_period = term->val.num;
> break;
> case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE:
> - /*
> - * TODO uncomment when the field is available
> - * attr->branch_sample_type = term->val.num;
> - */
> + attr->sample_type |= PERF_SAMPLE_BRANCH_STACK;
> + if (branch_type(term, (u64 *)&attr->branch_sample_type))
> + return -EINVAL;
> break;
> default:
> return -EINVAL;
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index cabcbc1..8bb673d 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -83,6 +83,7 @@ int parse_events__term_list(struct parse_events__term **_term,
> struct list_head *list);
> int parse_events__term_elem(struct parse_events__term_elem **_e,
> int type_val, char *str, long num);
> +int parse_events__branch_type(u64 *_type, char *str);
> void parse_events__free_terms(struct list_head *terms);
> int parse_events_modifier(struct list_head *list __used, char *str __used);
> int parse_events_add_tracepoint(struct list_head *list, int *idx,
> --
> 1.7.7.6
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists