All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org, Jiri Olsa <jolsa@kernel.org>,
	David Ahern <dsahern@gmail.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: [PATCH 10/18] perf tools: Fail on using multiple bits long terms without value
Date: Mon, 20 Feb 2017 16:08:31 -0300	[thread overview]
Message-ID: <20170220190839.3928-11-acme@kernel.org> (raw)
In-Reply-To: <20170220190839.3928-1-acme@kernel.org>

From: Jiri Olsa <jolsa@kernel.org>

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 <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1487340058-10496-4-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.c |  2 ++
 tools/perf/util/parse-events.h |  2 ++
 tools/perf/util/parse-events.y | 14 +++++++-------
 tools/perf/util/pmu.c          | 13 +++++++++++--
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 984d99a8fdc5..67a8aebc67ab 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -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_;
@@ -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,
 	};
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index da246a3ddb69..1af6a267c21b 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -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;
@@ -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,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index a14b47ab3879..30f018ea1370 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -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,
@@ -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);
@@ -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;
 }
 |
@@ -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;
 }
 |
@@ -591,7 +591,7 @@ 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;
 }
 |
@@ -599,7 +599,7 @@ 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;
 }
 |
@@ -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;
 }
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 49bfee0e3d9e..63cb46cb9b0f 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -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",
-- 
2.9.3

  parent reply	other threads:[~2017-02-20 19:12 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-20 19:08 [GIT PULL 00/18] perf/core improvements and fixes Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 01/18] tools perf scripting python: clang doesn't have -spec, remove it Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 02/18] perf python: Filter out -specs=/a/b/c from the python binding cc options Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 03/18] tools lib traceevent: It's preempt not prempt Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 04/18] perf session: Fix DEBUG=1 build with clang Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 05/18] perf cpumap: Add cpu__max_present_cpu() Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 06/18] perf header: Make build_cpu_topology skip offline/absent CPUs Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 07/18] perf tools: Replace _SC_NPROCESSORS_CONF with max_present_cpu in cpu_topology_map Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 08/18] perf build: Add special fixdep cleaning rule Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 09/18] perf tools: Move new_term arguments into struct parse_events_term template Arnaldo Carvalho de Melo
2017-02-20 19:08 ` Arnaldo Carvalho de Melo [this message]
2017-02-20 19:08 ` [PATCH 11/18] perf stat: Add -a as default target Arnaldo Carvalho de Melo
2017-02-20 19:15   ` Borislav Petkov
2017-02-20 19:55     ` Arnaldo Carvalho de Melo
2017-02-20 19:59       ` Borislav Petkov
2017-02-21  7:49         ` Jiri Olsa
2017-02-20 19:08 ` [PATCH 12/18] perf record: " Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 13/18] perf utils: Add perf_quiet_option() Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 14/18] perf utils: Check verbose flag properly Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 15/18] perf report: Add -q/--quiet option Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 16/18] perf diff: " Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 17/18] perf annotate: " Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 18/18] perf record: Honor --quiet option properly Arnaldo Carvalho de Melo
2017-02-21  8:08 ` [GIT PULL 00/18] perf/core improvements and fixes Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170220190839.3928-11-acme@kernel.org \
    --to=acme@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@redhat.com \
    --cc=dsahern@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.