Skip to content

Commit

Permalink
perf tools: Fail on using multiple bits long terms without value
Browse files Browse the repository at this point in the history
Currently we allow not to specify value for numeric terms and we set
them to value 1. This was originaly meant just for single bit terms to
allow user to type:

  $ perf record -e 'cpu/cpu-cycles,any'

instead of:

  $ perf record -e 'cpu/cpu-cycles,any=1'

However it works also for multi bits terms like:

  $ perf record -e 'cpu/event/' ls
  ...
  $ perf evlist -v
  ..., config: 0x1, ...

After discussion with Peter we decided making such term usage to fail,
like:

  $ perf record -e 'cpu/event/' ls
  event syntax error: 'cpu/event/'
                       \___ no value assigned for term
  ...

Signed-off-by: Jiri Olsa <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
  • Loading branch information
olsajiri authored and acmel committed Feb 17, 2017
1 parent 67b49b3 commit 99e7138
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 9 deletions.
2 changes: 2 additions & 0 deletions tools/perf/util/parse-events.c
Original file line number Diff line number Diff line change
Expand Up @@ -2349,6 +2349,7 @@ static int new_term(struct parse_events_term **_term,

int parse_events_term__num(struct parse_events_term **term,
int type_term, char *config, u64 num,
bool no_value,
void *loc_term_, void *loc_val_)
{
YYLTYPE *loc_term = loc_term_;
Expand All @@ -2358,6 +2359,7 @@ int parse_events_term__num(struct parse_events_term **term,
.type_val = PARSE_EVENTS__TERM_TYPE_NUM,
.type_term = type_term,
.config = config,
.no_value = no_value,
.err_term = loc_term ? loc_term->first_column : 0,
.err_val = loc_val ? loc_val->first_column : 0,
};
Expand Down
2 changes: 2 additions & 0 deletions tools/perf/util/parse-events.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ struct parse_events_term {
int type_term;
struct list_head list;
bool used;
bool no_value;

/* error string indexes for within parsed string */
int err_term;
Expand Down Expand Up @@ -122,6 +123,7 @@ void parse_events__shrink_config_terms(void);
int parse_events__is_hardcoded_term(struct parse_events_term *term);
int parse_events_term__num(struct parse_events_term **term,
int type_term, char *config, u64 num,
bool novalue,
void *loc_term, void *loc_val);
int parse_events_term__str(struct parse_events_term **term,
int type_term, char *config, char *str,
Expand Down
14 changes: 7 additions & 7 deletions tools/perf/util/parse-events.y
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ PE_KERNEL_PMU_EVENT sep_dc
if (!strcasecmp(alias->name, $1)) {
ALLOC_LIST(head);
ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
$1, 1, &@1, NULL));
$1, 1, false, &@1, NULL));
list_add_tail(&term->list, head);

if (!parse_events_add_pmu(data, list,
Expand Down Expand Up @@ -282,7 +282,7 @@ PE_PMU_EVENT_PRE '-' PE_PMU_EVENT_SUF sep_dc

ALLOC_LIST(head);
ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
&pmu_name, 1, &@1, NULL));
&pmu_name, 1, false, &@1, NULL));
list_add_tail(&term->list, head);

ALLOC_LIST(list);
Expand Down Expand Up @@ -548,7 +548,7 @@ PE_NAME '=' PE_VALUE
struct parse_events_term *term;

ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
$1, $3, &@1, &@3));
$1, $3, false, &@1, &@3));
$$ = term;
}
|
Expand All @@ -566,7 +566,7 @@ PE_NAME
struct parse_events_term *term;

ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
$1, 1, &@1, NULL));
$1, 1, true, &@1, NULL));
$$ = term;
}
|
Expand All @@ -591,15 +591,15 @@ PE_TERM '=' PE_VALUE
{
struct parse_events_term *term;

ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, $3, &@1, &@3));
ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, $3, false, &@1, &@3));
$$ = term;
}
|
PE_TERM
{
struct parse_events_term *term;

ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, 1, &@1, NULL));
ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, 1, true, &@1, NULL));
$$ = term;
}
|
Expand All @@ -620,7 +620,7 @@ PE_NAME array '=' PE_VALUE
struct parse_events_term *term;

ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
$1, $4, &@1, &@4));
$1, $4, false, &@1, &@4));
term->array = $2;
$$ = term;
}
Expand Down
13 changes: 11 additions & 2 deletions tools/perf/util/pmu.c
Original file line number Diff line number Diff line change
Expand Up @@ -834,9 +834,18 @@ static int pmu_config_term(struct list_head *formats,
* Either directly use a numeric term, or try to translate string terms
* using event parameters.
*/
if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM) {
if (term->no_value &&
bitmap_weight(format->bits, PERF_PMU_FORMAT_BITS) > 1) {
if (err) {
err->idx = term->err_val;
err->str = strdup("no value assigned for term");
}
return -EINVAL;
}

val = term->val.num;
else if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) {
} else if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) {
if (strcmp(term->val.str, "?")) {
if (verbose) {
pr_info("Invalid sysfs entry %s=%s\n",
Expand Down

0 comments on commit 99e7138

Please sign in to comment.