Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1583425 > unrolled thread

[PATCH 0/5] perf tools: Few fixes

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] perf tools: Few fixes Jiri Olsa <jolsa@kernel.org> - 2017-02-17 15:10 +0100
    [PATCH 3/5] perf tools: Fail on using multiple bits long terms without value Jiri Olsa <jolsa@kernel.org> - 2017-02-17 15:10 +0100
      [tip:perf/urgent] perf tools: Fail on using multiple bits long  terms without value tip-bot for Jiri Olsa <tipbot@zytor.com> - 2017-02-21 09:20 +0100
    [PATCH 5/5] perf record: Add -a as a default target Jiri Olsa <jolsa@kernel.org> - 2017-02-17 15:10 +0100
      Re: [PATCH 5/5] perf record: Add -a as a default target Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-02-17 15:30 +0100
        Re: [PATCH 5/5] perf record: Add -a as a default target Jiri Olsa <jolsa@redhat.com> - 2017-02-17 18:10 +0100
          [tip:perf/urgent] perf record: Add -a as default target tip-bot for Jiri Olsa <tipbot@zytor.com> - 2017-02-21 09:20 +0100
    [PATCH 4/5] perf stat: Add -a as a default target Jiri Olsa <jolsa@kernel.org> - 2017-02-17 15:10 +0100
      Re: [PATCH 4/5] perf stat: Add -a as a default target Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-02-17 15:30 +0100
        Re: [PATCH 4/5] perf stat: Add -a as a default target Jiri Olsa <jolsa@redhat.com> - 2017-02-17 15:40 +0100
          Re: [PATCH 4/5] perf stat: Add -a as a default target Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-02-17 15:50 +0100
            Re: [PATCH 4/5] perf stat: Add -a as a default target Jiri Olsa <jolsa@redhat.com> - 2017-02-17 15:50 +0100
            [PATCHv2 4/5] perf stat: Add -a as a default target Jiri Olsa <jolsa@redhat.com> - 2017-02-17 18:10 +0100
              Re: [PATCHv2 4/5] perf stat: Add -a as a default target Boris Petkov <bp@alien8.de> - 2017-02-17 19:20 +0100
                Re: [PATCHv2 4/5] perf stat: Add -a as a default target Borislav Petkov <bp@alien8.de> - 2017-02-18 19:00 +0100
                  Re: [PATCHv2 4/5] perf stat: Add -a as a default target Jiri Olsa <jolsa@redhat.com> - 2017-02-20 08:20 +0100
                    Re: [PATCHv2 4/5] perf stat: Add -a as a default target Borislav Petkov <bp@alien8.de> - 2017-02-20 21:40 +0100
                      Re: [PATCHv2 4/5] perf stat: Add -a as a default target Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-02-20 22:30 +0100
                        Re: [PATCHv2 4/5] perf stat: Add -a as a default target Borislav Petkov <bp@alien8.de> - 2017-02-20 23:50 +0100
                          Re: [PATCHv2 4/5] perf stat: Add -a as a default target Jiri Olsa <jolsa@redhat.com> - 2017-02-21 09:00 +0100
                            Re: [PATCHv2 4/5] perf stat: Add -a as a default target Jiri Olsa <jolsa@redhat.com> - 2017-02-21 12:10 +0100
                              Re: [PATCHv2 4/5] perf stat: Add -a as a default target Borislav Petkov <bp@alien8.de> - 2017-02-21 12:30 +0100
                                Re: [PATCHv2 4/5] perf stat: Add -a as a default target Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-02-21 14:40 +0100
                                  Re: [PATCHv2 4/5] perf stat: Add -a as a default target Borislav Petkov <bp@alien8.de> - 2017-02-21 15:10 +0100
                                    Re: [PATCHv2 4/5] perf stat: Add -a as a default target Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-02-21 15:30 +0100
              [tip:perf/urgent] perf stat: Add -a as default target tip-bot for Jiri Olsa <tipbot@zytor.com> - 2017-02-21 09:20 +0100
    [PATCH 1/5] perf build: Add special fixdep cleaning rule Jiri Olsa <jolsa@kernel.org> - 2017-02-17 15:10 +0100
      [tip:perf/urgent] perf build: Add special fixdep cleaning rule tip-bot for Jiri Olsa <tipbot@zytor.com> - 2017-02-21 09:20 +0100

Page 1 of 2  [1] 2  Next page →

#1583425 — [PATCH 0/5] perf tools: Few fixes

hi,
sending some assorted fixes.

Available also here:
  git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  perf/fixes

thanks,
jirka

---
Jiri Olsa (5):
      perf build: Add special fixdep cleaning rule
      perf tools: Move new_term arguments into struct parse_events_term template
      perf tools: Fail on using multiple bits long terms without value
      perf stat: Add -a as a default target
      perf record: Add -a as a default target

 tools/build/Makefile                     |  4 ++--
 tools/build/Makefile.include             |  3 +++
 tools/perf/Documentation/perf-record.txt |  2 +-
 tools/perf/Documentation/perf-stat.txt   |  2 +-
 tools/perf/Makefile.perf                 |  4 ++--
 tools/perf/builtin-record.c              |  6 ++++--
 tools/perf/builtin-stat.c                |  6 ++++--
 tools/perf/util/parse-events.c           | 71 +++++++++++++++++++++++++++++++++++++++++------------------------------
 tools/perf/util/parse-events.h           |  2 ++
 tools/perf/util/parse-events.y           | 14 +++++++-------
 tools/perf/util/pmu.c                    | 13 +++++++++++--
 11 files changed, 78 insertions(+), 49 deletions(-)

[toc] | [next] | [standalone]

#1583426 — [PATCH 3/5] perf tools: Fail on using multiple bits long terms without value

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 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/n/tip-5vdq8gfbsva97vwrfmjl5p6f@git.kernel.org
---
 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.7.4

[toc] | [prev] | [next] | [standalone]

#1585111 — [tip:perf/urgent] perf tools: Fail on using multiple bits long terms without value

Commit-ID:  99e7138eb7897aa0ccc6661173ae2d7e79721e05
Gitweb:     http://git.kernel.org/tip/99e7138eb7897aa0ccc6661173ae2d7e79721e05
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Fri, 17 Feb 2017 15:00:56 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 17 Feb 2017 17:28:22 -0300

perf tools: Fail on using multiple bits long terms without value

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 984d99a..67a8aeb 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 da246a3..1af6a26 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 a14b47a..30f018e 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 49bfee0..63cb46c 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",

[toc] | [prev] | [next] | [standalone]

#1583428 — [PATCH 5/5] perf record: Add -a as a default target

Adding system wide (-a) option as a default target
if non is specified.

  # perf record
  Warning: No target specified, setting system-wide collection (-a).
  ...

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/n/tip-tq11of2qlz8kxpxzva05d54l@git.kernel.org
---
 tools/perf/Documentation/perf-record.txt | 2 +-
 tools/perf/builtin-record.c              | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 27256bc68eda..b16003ec14a7 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -157,7 +157,7 @@ OPTIONS
 
 -a::
 --all-cpus::
-        System-wide collection from all CPUs.
+        System-wide collection from all CPUs (default if no target is specified).
 
 -p::
 --pid=::
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 6cd6776052e7..dab23f6b10f3 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1677,8 +1677,10 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	argc = parse_options(argc, argv, record_options, record_usage,
 			    PARSE_OPT_STOP_AT_NON_OPTION);
-	if (!argc && target__none(&rec->opts.target))
-		usage_with_options(record_usage, record_options);
+	if (!argc && target__none(&rec->opts.target)) {
+		pr_warning("Warning: No target specified, setting system-wide collection (-a).\n");
+		rec->opts.target.system_wide = true;
+	}
 
 	if (nr_cgroups && !rec->opts.target.system_wide) {
 		usage_with_options_msg(record_usage, record_options,
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1583469 — Re: [PATCH 5/5] perf record: Add -a as a default target

Em Fri, Feb 17, 2017 at 03:00:58PM +0100, Jiri Olsa escreveu:
> Adding system wide (-a) option as a default target
> if non is specified.
> 
>   # perf record
>   Warning: No target specified, setting system-wide collection (-a).

Ditto, this warning will get annoying after a while...

>   ...
> 
> 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/n/tip-tq11of2qlz8kxpxzva05d54l@git.kernel.org
> ---
>  tools/perf/Documentation/perf-record.txt | 2 +-
>  tools/perf/builtin-record.c              | 6 ++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index 27256bc68eda..b16003ec14a7 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -157,7 +157,7 @@ OPTIONS
>  
>  -a::
>  --all-cpus::
> -        System-wide collection from all CPUs.
> +        System-wide collection from all CPUs (default if no target is specified).
>  
>  -p::
>  --pid=::
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 6cd6776052e7..dab23f6b10f3 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1677,8 +1677,10 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
>  
>  	argc = parse_options(argc, argv, record_options, record_usage,
>  			    PARSE_OPT_STOP_AT_NON_OPTION);
> -	if (!argc && target__none(&rec->opts.target))
> -		usage_with_options(record_usage, record_options);
> +	if (!argc && target__none(&rec->opts.target)) {
> +		pr_warning("Warning: No target specified, setting system-wide collection (-a).\n");
> +		rec->opts.target.system_wide = true;
> +	}
>  
>  	if (nr_cgroups && !rec->opts.target.system_wide) {
>  		usage_with_options_msg(record_usage, record_options,
> -- 
> 2.7.4

[toc] | [prev] | [next] | [standalone]

#1583600 — Re: [PATCH 5/5] perf record: Add -a as a default target

On Fri, Feb 17, 2017 at 11:28:14AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 17, 2017 at 03:00:58PM +0100, Jiri Olsa escreveu:
> > Adding system wide (-a) option as a default target
> > if non is specified.
> > 
> >   # perf record
> >   Warning: No target specified, setting system-wide collection (-a).
> 
> Ditto, this warning will get annoying after a while...

ok, posting the change with no warning

jirka

---
Adding system wide (-a) option as a default target
if non is specified.

Running 'perf record' will now collect system wide data.

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/n/tip-tq11of2qlz8kxpxzva05d54l@git.kernel.org
---
 tools/perf/Documentation/perf-record.txt | 2 +-
 tools/perf/builtin-record.c              | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 27256bc68eda..b16003ec14a7 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -157,7 +157,7 @@ OPTIONS
 
 -a::
 --all-cpus::
-        System-wide collection from all CPUs.
+        System-wide collection from all CPUs (default if no target is specified).
 
 -p::
 --pid=::
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 6cd6776052e7..b87bbef73394 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1677,8 +1677,10 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	argc = parse_options(argc, argv, record_options, record_usage,
 			    PARSE_OPT_STOP_AT_NON_OPTION);
+
+	/* Make system wide (-a) the default target. */
 	if (!argc && target__none(&rec->opts.target))
-		usage_with_options(record_usage, record_options);
+		rec->opts.target.system_wide = true;
 
 	if (nr_cgroups && !rec->opts.target.system_wide) {
 		usage_with_options_msg(record_usage, record_options,
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1585105 — [tip:perf/urgent] perf record: Add -a as default target

Commit-ID:  483635a9d0802d5ffbe402ceac5b93ddb2acb138
Gitweb:     http://git.kernel.org/tip/483635a9d0802d5ffbe402ceac5b93ddb2acb138
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Fri, 17 Feb 2017 18:00:18 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 17 Feb 2017 17:32:38 -0300

perf record: Add -a as default target

Running 'perf record' with no target (-a, -p, -t, etc) will now collect
system wide data.

Commiter notes:

Testing it:

  [root@jouet ~]# perf record
  ^C[ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 1.351 MB perf.data (366 samples) ]
  #

is equivalent to:

  # perf record -a
  ^C[ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 1.411 MB perf.data (978 samples) ]
  #

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20170217170018.GA15389@krava
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-record.txt | 2 +-
 tools/perf/builtin-record.c              | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 27256bc..b16003e 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -157,7 +157,7 @@ OPTIONS
 
 -a::
 --all-cpus::
-        System-wide collection from all CPUs.
+        System-wide collection from all CPUs (default if no target is specified).
 
 -p::
 --pid=::
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 6cd6776..b87bbef 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1677,8 +1677,10 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	argc = parse_options(argc, argv, record_options, record_usage,
 			    PARSE_OPT_STOP_AT_NON_OPTION);
+
+	/* Make system wide (-a) the default target. */
 	if (!argc && target__none(&rec->opts.target))
-		usage_with_options(record_usage, record_options);
+		rec->opts.target.system_wide = true;
 
 	if (nr_cgroups && !rec->opts.target.system_wide) {
 		usage_with_options_msg(record_usage, record_options,

[toc] | [prev] | [next] | [standalone]

#1583432 — [PATCH 4/5] perf stat: Add -a as a default target

Boris asked for default -a option in case we monitor
only uncore events. While implementing that I thought
it might be actually useful to make it overall default.

  # perf stat
  Warning: No target specified, setting system-wide collection (-a).
  ...

Requested-by: Borislav Petkov <bp@alien8.de>
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>
Cc: Borislav Petkov <bp@alien8.de>
Link: http://lkml.kernel.org/n/tip-tq11of2qlz8kxpxzva05d54l@git.kernel.org
---
 tools/perf/Documentation/perf-stat.txt | 2 +-
 tools/perf/builtin-stat.c              | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index d96ccd4844df..aecf2a87e7d6 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -63,7 +63,7 @@ report::
 
 -a::
 --all-cpus::
-        system-wide collection from all CPUs
+        system-wide collection from all CPUs (default if no target is specified)
 
 -c::
 --scale::
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f28719178b51..0d09ec7029d3 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2445,8 +2445,10 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
 	} else if (big_num_opt == 0) /* User passed --no-big-num */
 		big_num = false;
 
-	if (!argc && target__none(&target))
-		usage_with_options(stat_usage, stat_options);
+	if (!argc && target__none(&target)) {
+		pr_warning("Warning: No target specified, setting system-wide collection (-a).\n");
+		target.system_wide = true;
+	}
 
 	if (run_count < 0) {
 		pr_err("Run count must be a positive number\n");
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1583472 — Re: [PATCH 4/5] perf stat: Add -a as a default target

Em Fri, Feb 17, 2017 at 03:00:57PM +0100, Jiri Olsa escreveu:
> Boris asked for default -a option in case we monitor
> only uncore events. While implementing that I thought
> it might be actually useful to make it overall default.
> 
>   # perf stat
>   Warning: No target specified, setting system-wide collection (-a).

Humm, would be interesting to disable this after a few warnings? Just
one?

BTW, this is how 'perf trace' works since day one, i.e. no target means
system wide syscall tracing.

- Arnaldo

>   ...
> 
> Requested-by: Borislav Petkov <bp@alien8.de>
> 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>
> Cc: Borislav Petkov <bp@alien8.de>
> Link: http://lkml.kernel.org/n/tip-tq11of2qlz8kxpxzva05d54l@git.kernel.org
> ---
>  tools/perf/Documentation/perf-stat.txt | 2 +-
>  tools/perf/builtin-stat.c              | 6 ++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
> index d96ccd4844df..aecf2a87e7d6 100644
> --- a/tools/perf/Documentation/perf-stat.txt
> +++ b/tools/perf/Documentation/perf-stat.txt
> @@ -63,7 +63,7 @@ report::
>  
>  -a::
>  --all-cpus::
> -        system-wide collection from all CPUs
> +        system-wide collection from all CPUs (default if no target is specified)
>  
>  -c::
>  --scale::
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index f28719178b51..0d09ec7029d3 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -2445,8 +2445,10 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
>  	} else if (big_num_opt == 0) /* User passed --no-big-num */
>  		big_num = false;
>  
> -	if (!argc && target__none(&target))
> -		usage_with_options(stat_usage, stat_options);
> +	if (!argc && target__none(&target)) {
> +		pr_warning("Warning: No target specified, setting system-wide collection (-a).\n");
> +		target.system_wide = true;
> +	}
>  
>  	if (run_count < 0) {
>  		pr_err("Run count must be a positive number\n");
> -- 
> 2.7.4

[toc] | [prev] | [next] | [standalone]

#1583482 — Re: [PATCH 4/5] perf stat: Add -a as a default target

On Fri, Feb 17, 2017 at 11:27:47AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 17, 2017 at 03:00:57PM +0100, Jiri Olsa escreveu:
> > Boris asked for default -a option in case we monitor
> > only uncore events. While implementing that I thought
> > it might be actually useful to make it overall default.
> > 
> >   # perf stat
> >   Warning: No target specified, setting system-wide collection (-a).
> 
> Humm, would be interesting to disable this after a few warnings? Just
> one?

not sure it's good idea to keep the count of that somewhere..
how about i make the warning smaller ;-)

  # perf stat
  Forced system wide target.
  ...

> 
> BTW, this is how 'perf trace' works since day one, i.e. no target means
> system wide syscall tracing.

or we could omit the warning completely as probably perf trace does

thanks,
jirka

[toc] | [prev] | [next] | [standalone]

#1583490 — Re: [PATCH 4/5] perf stat: Add -a as a default target

Em Fri, Feb 17, 2017 at 03:33:27PM +0100, Jiri Olsa escreveu:
> On Fri, Feb 17, 2017 at 11:27:47AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Feb 17, 2017 at 03:00:57PM +0100, Jiri Olsa escreveu:
> > > Boris asked for default -a option in case we monitor
> > > only uncore events. While implementing that I thought
> > > it might be actually useful to make it overall default.

> > >   # perf stat
> > >   Warning: No target specified, setting system-wide collection (-a).

> > Humm, would be interesting to disable this after a few warnings? Just
> > one?
 
> not sure it's good idea to keep the count of that somewhere..
> how about i make the warning smaller ;-)
 
>   # perf stat
>   Forced system wide target.
>   ...
 
> > BTW, this is how 'perf trace' works since day one, i.e. no target means
> > system wide syscall tracing.
 
> or we could omit the warning completely as probably perf trace does

I think that we should have some note on the Documentation (have you
added it?) and be done with it.

Another thing possiblity my mind, print that at the end? like:

perf record
^C[ perf record: Woken up 1 times to write data - system wide samples ]
[ perf record: Captured and wrote 1.738 MB perf.data (7565 samples) ]

----------

Then people will thing, hey, so now it does systemwide samples when I
pass no target, I don't have anymore to type _three_ keys! cool! :-)

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1583493 — Re: [PATCH 4/5] perf stat: Add -a as a default target

On Fri, Feb 17, 2017 at 11:41:28AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 17, 2017 at 03:33:27PM +0100, Jiri Olsa escreveu:
> > On Fri, Feb 17, 2017 at 11:27:47AM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Fri, Feb 17, 2017 at 03:00:57PM +0100, Jiri Olsa escreveu:
> > > > Boris asked for default -a option in case we monitor
> > > > only uncore events. While implementing that I thought
> > > > it might be actually useful to make it overall default.
> 
> > > >   # perf stat
> > > >   Warning: No target specified, setting system-wide collection (-a).
> 
> > > Humm, would be interesting to disable this after a few warnings? Just
> > > one?
>  
> > not sure it's good idea to keep the count of that somewhere..
> > how about i make the warning smaller ;-)
>  
> >   # perf stat
> >   Forced system wide target.
> >   ...
>  
> > > BTW, this is how 'perf trace' works since day one, i.e. no target means
> > > system wide syscall tracing.
>  
> > or we could omit the warning completely as probably perf trace does
> 
> I think that we should have some note on the Documentation (have you
> added it?) and be done with it.
> 
> Another thing possiblity my mind, print that at the end? like:
> 
> perf record
> ^C[ perf record: Woken up 1 times to write data - system wide samples ]
> [ perf record: Captured and wrote 1.738 MB perf.data (7565 samples) ]
> 
> ----------
> 
> Then people will thing, hey, so now it does systemwide samples when I
> pass no target, I don't have anymore to type _three_ keys! cool! :-)

ok, I'll check on that and send new version for the last 2 patches

thanks,
jirka

[toc] | [prev] | [next] | [standalone]

#1583596 — [PATCHv2 4/5] perf stat: Add -a as a default target

On Fri, Feb 17, 2017 at 11:41:28AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 17, 2017 at 03:33:27PM +0100, Jiri Olsa escreveu:
> > On Fri, Feb 17, 2017 at 11:27:47AM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Fri, Feb 17, 2017 at 03:00:57PM +0100, Jiri Olsa escreveu:
> > > > Boris asked for default -a option in case we monitor
> > > > only uncore events. While implementing that I thought
> > > > it might be actually useful to make it overall default.
> 
> > > >   # perf stat
> > > >   Warning: No target specified, setting system-wide collection (-a).
> 
> > > Humm, would be interesting to disable this after a few warnings? Just
> > > one?
>  
> > not sure it's good idea to keep the count of that somewhere..
> > how about i make the warning smaller ;-)
>  
> >   # perf stat
> >   Forced system wide target.
> >   ...
>  
> > > BTW, this is how 'perf trace' works since day one, i.e. no target means
> > > system wide syscall tracing.
>  
> > or we could omit the warning completely as probably perf trace does
> 
> I think that we should have some note on the Documentation (have you
> added it?) and be done with it.
> 
> Another thing possiblity my mind, print that at the end? like:
> 
> perf record
> ^C[ perf record: Woken up 1 times to write data - system wide samples ]
> [ perf record: Captured and wrote 1.738 MB perf.data (7565 samples) ]
> 
> ----------
> 
> Then people will thing, hey, so now it does systemwide samples when I
> pass no target, I don't have anymore to type _three_ keys! cool! :-)

ok, posting the change with no warning

jirka

---
Boris asked for default -a option in case we monitor
only uncore events. While implementing that I thought
it might be actually useful to make it overall default.

Running 'perf stat' will now collect system wide data.

Requested-by: Borislav Petkov <bp@alien8.de>
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/n/tip-tq11of2qlz8kxpxzva05d54l@git.kernel.org
---
 tools/perf/Documentation/perf-stat.txt | 2 +-
 tools/perf/builtin-stat.c              | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index d96ccd4844df..aecf2a87e7d6 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -63,7 +63,7 @@ report::
 
 -a::
 --all-cpus::
-        system-wide collection from all CPUs
+        system-wide collection from all CPUs (default if no target is specified)
 
 -c::
 --scale::
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f28719178b51..70ed4f9d014e 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2445,8 +2445,9 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
 	} else if (big_num_opt == 0) /* User passed --no-big-num */
 		big_num = false;
 
+	/* Make system wide (-a) the default target. */
 	if (!argc && target__none(&target))
-		usage_with_options(stat_usage, stat_options);
+		target.system_wide = true;
 
 	if (run_count < 0) {
 		pr_err("Run count must be a positive number\n");
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1583633 — Re: [PATCHv2 4/5] perf stat: Add -a as a default target

On February 17, 2017 6:00:34 PM GMT+01:00, Jiri Olsa <jolsa@redhat.com> wrote:
>---
>Boris asked for default -a option in case we monitor
>only uncore events. While implementing that I thought
>it might be actually useful to make it overall default.
>
>Running 'perf stat' will now collect system wide data.
>
>Requested-by: Borislav Petkov <bp@alien8.de>
>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/n/tip-tq11of2qlz8kxpxzva05d54l@git.kernel.org

LGTM.

Acked-by: me

Thanks.

-- 
Sent from a small device: formatting sux and brevity is inevitable. 

[toc] | [prev] | [next] | [standalone]

#1583940 — Re: [PATCHv2 4/5] perf stat: Add -a as a default target

On Fri, Feb 17, 2017 at 06:48:13PM +0100, Boris Petkov wrote:
> LGTM.
> 
> Acked-by: me

Well, it looks good but actually trying it is a different story. For
example:

$ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/ sleep 1

still says <not supported> because argc is not 0.

So how about the below diff instead?

$ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/

without args dumps the usage message and

$ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/ sleep 1

actually does the system-wide thing:

 Performance counter stats for 'system wide':

           196,469      amd_nb/event=0xe0,umask=0x1f/

       1.001815180 seconds time elapsed

Hmmm?

 tools/perf/Documentation/perf-stat.txt | 2 +-
 tools/perf/builtin-stat.c              | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

Index: linux/tools/perf/Documentation/perf-stat.txt
===================================================================
--- linux.orig/tools/perf/Documentation/perf-stat.txt	2017-02-18 18:38:23.751960730 +0100
+++ linux/tools/perf/Documentation/perf-stat.txt	2017-02-18 18:38:23.743960730 +0100
@@ -63,7 +63,7 @@ report::
 
 -a::
 --all-cpus::
-        system-wide collection from all CPUs
+        system-wide collection from all CPUs (default if no target is specified)
 
 -c::
 --scale::
Index: linux/tools/perf/builtin-stat.c
===================================================================
--- linux.orig/tools/perf/builtin-stat.c	2017-02-18 18:38:23.751960730 +0100
+++ linux/tools/perf/builtin-stat.c	2017-02-18 18:48:33.531959828 +0100
@@ -2445,8 +2445,12 @@ int cmd_stat(int argc, const char **argv
 	} else if (big_num_opt == 0) /* User passed --no-big-num */
 		big_num = false;
 
-	if (!argc && target__none(&target))
-		usage_with_options(stat_usage, stat_options);
+	/* Make system wide (-a) the default target. */
+	if (target__none(&target)) {
+		if (!argc)
+			usage_with_options(stat_usage, stat_options);
+		target.system_wide = true;
+	}
 
 	if (run_count < 0) {
 		pr_err("Run count must be a positive number\n");


-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1584373 — Re: [PATCHv2 4/5] perf stat: Add -a as a default target

On Sat, Feb 18, 2017 at 06:52:25PM +0100, Borislav Petkov wrote:
> On Fri, Feb 17, 2017 at 06:48:13PM +0100, Boris Petkov wrote:
> > LGTM.
> > 
> > Acked-by: me
> 
> Well, it looks good but actually trying it is a different story. For
> example:
> 
> $ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/ sleep 1
> 
> still says <not supported> because argc is not 0.
> 
> So how about the below diff instead?
> 
> $ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/
> 
> without args dumps the usage message and
> 
> $ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/ sleep 1
> 
> actually does the system-wide thing:
> 
>  Performance counter stats for 'system wide':
> 
>            196,469      amd_nb/event=0xe0,umask=0x1f/
> 
>        1.001815180 seconds time elapsed
> 
> Hmmm?

ugh, I thought it was too easy ;-)

it looks good to me

thanks,
jirka

[toc] | [prev] | [next] | [standalone]

#1584892 — Re: [PATCHv2 4/5] perf stat: Add -a as a default target

Btw, I received your mail just now - probably greylisting...

On Mon, Feb 20, 2017 at 10:44:33AM -0300, Arnaldo Carvalho de Melo wrote:
> Isn't this confusing, i.e. people runnin 'tool workload' can be lead to
> think that the events reported took place just when the workload was
> running, i.e. on the same cpu and while it was being scheduled?

That's a good point.

> I understand the desire to avoid asking people to use -a, i.e. if it
> only makes sense as system wide, hey, do it as system wide, but can't
> this be confusing?

Well, I did

tool workload

and it said <not supported>. Now, if I'm the only one to stare puzzled
at this and wonder why it says "not supported", then sure, I know now
that I should use -a.

But if other users are as confused as me, you probably want to tell them
to try -a too, no?

IOW, we probably could extend my other patch which says that people
should try to disable the HW NMI watchdog to say "try using -a for
uncore-only events" when it detects <not supported>.

Thoughts?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1584918 — Re: [PATCHv2 4/5] perf stat: Add -a as a default target

Em Mon, Feb 20, 2017 at 09:31:49PM +0100, Borislav Petkov escreveu:
> Btw, I received your mail just now - probably greylisting...
> 
> On Mon, Feb 20, 2017 at 10:44:33AM -0300, Arnaldo Carvalho de Melo wrote:
> > Isn't this confusing, i.e. people runnin 'tool workload' can be lead to
> > think that the events reported took place just when the workload was
> > running, i.e. on the same cpu and while it was being scheduled?
> 
> That's a good point.
> 
> > I understand the desire to avoid asking people to use -a, i.e. if it
> > only makes sense as system wide, hey, do it as system wide, but can't
> > this be confusing?
> 
> Well, I did
> 
> tool workload
> 
> and it said <not supported>. Now, if I'm the only one to stare puzzled

Well, this one should be read (and written in the tool output as):

<not supported in workload only mode, try system wide, using -a>

> at this and wonder why it says "not supported", then sure, I know now
> that I should use -a.
> 
> But if other users are as confused as me, you probably want to tell them
> to try -a too, no?
> 
> IOW, we probably could extend my other patch which says that people
> should try to disable the HW NMI watchdog to say "try using -a for
> uncore-only events" when it detects <not supported>.

Right, the ENOTSUPP in this case needs to be properly expanded into
something meaningful, as suggested above.

> 
> Thoughts?
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1584949 — Re: [PATCHv2 4/5] perf stat: Add -a as a default target

On Mon, Feb 20, 2017 at 06:22:54PM -0300, Arnaldo Carvalho de Melo wrote:
> Well, this one should be read (and written in the tool output as):
> 
> <not supported in workload only mode, try system wide, using -a>

Do you want to change that CNTR_NOT_SUPPORTED string unconditionally to
something like above?

Because perf_evsel.supported seems like it means that counter is not
supported but not necessarily only because of the missing -a for an
uncore event, AFAICT. I could be wrong.

> Right, the ENOTSUPP in this case needs to be properly expanded into
> something meaningful, as suggested above.

I dumped errno in __run_perf_stat():

./perf stat -v -e amd_nb/event=0xe0,umask=0x1f/ sleep 1
Using CPUID AuthenticAMD-21-2
Warning:
amd_nb/event=0xe0,umask=0x1f/ event is not supported by the kernel: 22.

It is -EINVAL and the syscall returns -EINVAL in bunch of places so I'm
guessing this might not be a good way to match the retval to the proper
error message.

Peterz said something about scanning all events supplied by -e and if
all are uncore, to set -a automatically. Can we do that?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1585095 — Re: [PATCHv2 4/5] perf stat: Add -a as a default target

On Mon, Feb 20, 2017 at 11:47:16PM +0100, Borislav Petkov wrote:
> On Mon, Feb 20, 2017 at 06:22:54PM -0300, Arnaldo Carvalho de Melo wrote:
> > Well, this one should be read (and written in the tool output as):
> > 
> > <not supported in workload only mode, try system wide, using -a>
> 
> Do you want to change that CNTR_NOT_SUPPORTED string unconditionally to
> something like above?
> 
> Because perf_evsel.supported seems like it means that counter is not
> supported but not necessarily only because of the missing -a for an
> uncore event, AFAICT. I could be wrong.
> 
> > Right, the ENOTSUPP in this case needs to be properly expanded into
> > something meaningful, as suggested above.
> 
> I dumped errno in __run_perf_stat():
> 
> ./perf stat -v -e amd_nb/event=0xe0,umask=0x1f/ sleep 1
> Using CPUID AuthenticAMD-21-2
> Warning:
> amd_nb/event=0xe0,umask=0x1f/ event is not supported by the kernel: 22.
> 
> It is -EINVAL and the syscall returns -EINVAL in bunch of places so I'm
> guessing this might not be a good way to match the retval to the proper
> error message.
> 
> Peterz said something about scanning all events supplied by -e and if
> all are uncore, to set -a automatically. Can we do that?

right, so that's different from what we actually did.. ;-)

I'll check on this one.. might not be as straight forward,
because some uncore events might have already cpumask limit

jirka

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web