perf tools: Force uncore events to system wide monitoring

PATCH In review Jiri Olsa 5 participants 10 messages Feb 24, 2017 – Mar 7, 2017 perf events

lore

Share & tools

lore

Jiri Olsa

author reviewer

2017-02-24 10:17 UTC #0 lore raw

PATCH 1/1 perf tools: Force uncore events to system wide monitoring

Adding system_wide flag to uncore pmu objects and passing this flag along to the their events.

Making system wide (-a) the default option if no target was specified and one of following conditions is met:

- there's no workload specified (current behaviour)
- there is workload specified but all requested events are system wide events

Mixed events core/uncore with workload:

  $ perf stat -e 'uncore_cbox_0/clockticks/,cycles' sleep 1

Performance counter stats for 'sleep 1':

     <not supported>      uncore_cbox_0/clockticks/
             980,489      cycles

         1.000897406 seconds time elapsed

Uncore event with workload:

  $ perf stat -e 'uncore_cbox_0/clockticks/' sleep 1

Performance counter stats for 'system wide':

281,473,897,192,670 uncore_cbox_0/clockticks/

         1.000833784 seconds time elapsed

Signed-off-by Jiri Olsa <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Adrian Hunter <[email protected]>

Cc Borislav Petkov <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

---
 tools/perf/arch/x86/util/pmu.c |  6 +++++-
 tools/perf/builtin-stat.c      | 32 +++++++++++++++++++++++++++++---
 tools/perf/util/parse-events.c |  1 +
 tools/perf/util/pmu.h          |  1 +
 4 files changed, 36 insertions(+), 4 deletions(-)

tools/perf/arch/x86/util/pmu.c +5 -1

index 79fe07158d00..654290f87a19 100644
@@ -11,8 +11,12 @@ struct perf_event_attr *perf_pmu__get_default_config(struct perf_pmu *pmu __mayb
 #ifdef HAVE_AUXTRACE_SUPPORT
 	if (!strcmp(pmu->name, INTEL_PT_PMU_NAME))
 		return intel_pt_pmu_default_config(pmu);
-	if (!strcmp(pmu->name, INTEL_BTS_PMU_NAME))
+	if (!strcmp(pmu->name, INTEL_BTS_PMU_NAME)) {
 		pmu->selectable = true;
+		return NULL;
+	}
 #endif
+	/* All uncore PMUs are monitored system wide. */
+	pmu->system_wide = !strncmp(pmu->name, "uncore", 6);
 	return NULL;
 }

tools/perf/builtin-stat.c +29 -3

index 13b54999ad79..c32c7fa6f092 100644
@@ -2339,6 +2339,34 @@ static int __cmd_report(int argc, const char **argv)
 	return 0;
 }
 
+static void setup_system_wide(int forks)
+{
+	/*
+	 * Make system wide (-a) the default target if
+	 * no target was specified and one of following
+	 * conditions is met:
+	 *
+	 *   - there's no workload specified
+	 *   - there is workload specified but all requested
+	 *     events are system wide events
+	 */
+	if (!target__none(&target))
+		return;
+
+	if (!forks)
+		target.system_wide = true;
+	else {
+		struct perf_evsel *counter;
+
+		evlist__for_each_entry(evsel_list, counter) {
+			if (!counter->system_wide)
+				return;
+		}
+
+		target.system_wide = true;
+	}
+}
+
 int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	const char * const stat_usage[] = {

@@ -2445,9 +2473,7 @@ 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))
-		target.system_wide = true;
+	setup_system_wide(argc);
 
 	if (run_count < 0) {
 		pr_err("Run count must be a positive number\n");

tools/perf/util/parse-events.c +1 -0

index 67a8aebc67ab..8b06b21f1bbc 100644
@@ -1254,6 +1254,7 @@ int parse_events_add_pmu(struct parse_events_evlist *data,
 		evsel->scale = info.scale;
 		evsel->per_pkg = info.per_pkg;
 		evsel->snapshot = info.snapshot;
+		evsel->system_wide = pmu->system_wide;
 	}
 
 	return evsel ? 0 : -ENOMEM;

tools/perf/util/pmu.h +1 -0

index 00852ddc7741..0ce3ffacbf92 100644
@@ -21,6 +21,7 @@ struct perf_pmu {
 	char *name;
 	__u32 type;
 	bool selectable;
+	bool system_wide;
 	struct perf_event_attr *default_config;
 	struct cpu_map *cpus;
 	struct list_head format;  /* HEAD struct perf_pmu_format -> list */

Signature · 1 line

2.7.4

Arnaldo Carvalho de Melo

maintainer

2017-02-24 13:35 UTC #1 lore raw

Em Fri, Feb 24, 2017 at 11:17:14AM +0100, Jiri Olsa escreveu:

Quoted 2 lines

Adding system_wide flag to uncore pmu objects and passing this flag along to the their events.

Boris, ack? Tested-by?

- Arnaldo

Quoted 134 lines

Making system wide (-a) the default option if no target was specified and one of following conditions is met:

- there's no workload specified (current behaviour)
- there is workload specified but all requested events are system wide events

Mixed events core/uncore with workload:

  $ perf stat -e 'uncore_cbox_0/clockticks/,cycles' sleep 1

Performance counter stats for 'sleep 1':

     <not supported>      uncore_cbox_0/clockticks/
             980,489      cycles

         1.000897406 seconds time elapsed

Uncore event with workload:

  $ perf stat -e 'uncore_cbox_0/clockticks/' sleep 1

Performance counter stats for 'system wide':

281,473,897,192,670 uncore_cbox_0/clockticks/

         1.000833784 seconds time elapsed

Signed-off-by Jiri Olsa <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Adrian Hunter <[email protected]>

Cc Borislav Petkov <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

---
 tools/perf/arch/x86/util/pmu.c |  6 +++++-
 tools/perf/builtin-stat.c      | 32 +++++++++++++++++++++++++++++---
 tools/perf/util/parse-events.c |  1 +
 tools/perf/util/pmu.h          |  1 +
 4 files changed, 36 insertions(+), 4 deletions(-)

tools/perf/arch/x86/util/pmu.c +5 -1

index 79fe07158d00..654290f87a19 100644
@@ -11,8 +11,12 @@ struct perf_event_attr *perf_pmu__get_default_config(struct perf_pmu *pmu __mayb
 #ifdef HAVE_AUXTRACE_SUPPORT
 	if (!strcmp(pmu->name, INTEL_PT_PMU_NAME))
 		return intel_pt_pmu_default_config(pmu);
-	if (!strcmp(pmu->name, INTEL_BTS_PMU_NAME))
+	if (!strcmp(pmu->name, INTEL_BTS_PMU_NAME)) {
 		pmu->selectable = true;
+		return NULL;
+	}
 #endif
+	/* All uncore PMUs are monitored system wide. */
+	pmu->system_wide = !strncmp(pmu->name, "uncore", 6);
 	return NULL;
 }

tools/perf/builtin-stat.c +29 -3

index 13b54999ad79..c32c7fa6f092 100644
@@ -2339,6 +2339,34 @@ static int __cmd_report(int argc, const char **argv)
 	return 0;
 }
 
+static void setup_system_wide(int forks)
+{
+	/*
+	 * Make system wide (-a) the default target if
+	 * no target was specified and one of following
+	 * conditions is met:
+	 *
+	 *   - there's no workload specified
+	 *   - there is workload specified but all requested
+	 *     events are system wide events
+	 */
+	if (!target__none(&target))
+		return;
+
+	if (!forks)
+		target.system_wide = true;
+	else {
+		struct perf_evsel *counter;
+
+		evlist__for_each_entry(evsel_list, counter) {
+			if (!counter->system_wide)
+				return;
+		}
+
+		target.system_wide = true;
+	}
+}
+
 int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	const char * const stat_usage[] = {

@@ -2445,9 +2473,7 @@ 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))
-		target.system_wide = true;
+	setup_system_wide(argc);
 
 	if (run_count < 0) {
 		pr_err("Run count must be a positive number\n");

tools/perf/util/parse-events.c +1 -0

index 67a8aebc67ab..8b06b21f1bbc 100644
@@ -1254,6 +1254,7 @@ int parse_events_add_pmu(struct parse_events_evlist *data,
 		evsel->scale = info.scale;
 		evsel->per_pkg = info.per_pkg;
 		evsel->snapshot = info.snapshot;
+		evsel->system_wide = pmu->system_wide;
 	}
 
 	return evsel ? 0 : -ENOMEM;

tools/perf/util/pmu.h +1 -0

index 00852ddc7741..0ce3ffacbf92 100644
@@ -21,6 +21,7 @@ struct perf_pmu {
 	char *name;
 	__u32 type;
 	bool selectable;
+	bool system_wide;
 	struct perf_event_attr *default_config;
 	struct cpu_map *cpus;
 	struct list_head format;  /* HEAD struct perf_pmu_format -> list */

Signature · 1 line

2.7.4

Jiri Olsa

2017-02-24 14:00 UTC #2 lore raw

Quoted 5 lines · Arnaldo Carvalho de Melo

Em Fri, Feb 24, 2017 at 11:17:14AM +0100, Jiri Olsa escreveu:
Quoted 2 lines
Adding system_wide flag to uncore pmu objects and passing this flag along to the their events.
Boris, ack? Tested-by?

actualy we discussed that on irc and I need to send new version ;-)

jirka

Jiri Olsa

2017-02-27 09:29 UTC #3 lore raw

Quoted 8 lines · Jiri Olsa

Quoted 5 lines · Arnaldo Carvalho de Melo
Em Fri, Feb 24, 2017 at 11:17:14AM +0100, Jiri Olsa escreveu:
Quoted 2 lines
Adding system_wide flag to uncore pmu objects and passing this flag along to the their events.
Boris, ack? Tested-by?
actualy we discussed that on irc and I need to send new version ;-)

Boris,
would you mind testing new version?

thanks,
jirka

---

tools/perf/builtin-stat.c +29 -3

index f4f555a67e9b..1320352cda04 100644
@@ -2350,6 +2350,34 @@ static int __cmd_report(int argc, const char **argv)
 	return 0;
 }
 
+static void setup_system_wide(int forks)
+{
+	/*
+	 * Make system wide (-a) the default target if
+	 * no target was specified and one of following
+	 * conditions is met:
+	 *
+	 *   - there's no workload specified
+	 *   - there is workload specified but all requested
+	 *     events are system wide events
+	 */
+	if (!target__none(&target))
+		return;
+
+	if (!forks)
+		target.system_wide = true;
+	else {
+		struct perf_evsel *counter;
+
+		evlist__for_each_entry(evsel_list, counter) {
+			if (!counter->system_wide)
+				return;
+		}
+
+		target.system_wide = true;
+	}
+}
+
 int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	const char * const stat_usage[] = {

@@ -2456,9 +2484,7 @@ 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))
-		target.system_wide = true;
+	setup_system_wide(argc);
 
 	if (run_count < 0) {
 		pr_err("Run count must be a positive number\n");

tools/perf/util/parse-events.c +3 -2

index 67a8aebc67ab..54355d3caf09 100644
@@ -316,8 +316,9 @@ __add_event(struct list_head *list, int *idx,
 		return NULL;
 
 	(*idx)++;
-	evsel->cpus     = cpu_map__get(cpus);
-	evsel->own_cpus = cpu_map__get(cpus);
+	evsel->cpus        = cpu_map__get(cpus);
+	evsel->own_cpus    = cpu_map__get(cpus);
+	evsel->system_wide = !!cpus;
 
 	if (name)
 		evsel->name = strdup(name);

Borislav Petkov

2017-02-27 11:40 UTC #4 lore raw

Quoted 1 line · Jiri Olsa

would you mind testing new version?

Looks ok to me.

Thanks.

[boris@pd: ~/kernel/linux/tools/perf> ./perf stat -v -e amd_nb/event=0xe0,umask=0x1f/ sleep 1 Using CPUID AuthenticAMD-21-2
amd_nb/event=0xe0,umask=0x1f/: 977574 1002133816 1002133816

Performance counter stats for 'system wide':

           977,574      amd_nb/event=0xe0,umask=0x1f/                                   

       1.002162083 seconds time elapsed

[boris@pd: ~/kernel/linux/tools/perf> ./perf stat -v -e amd_nb/event=0xe0,umask=0x1f/,cycles sleep 1 Using CPUID AuthenticAMD-21-2
Warning:
amd_nb/event=0xe0,umask=0x1f/ event is not supported by the kernel.
failed to read counter amd_nb/event=0xe0,umask=0x1f/
amd_nb/event=0xe0,umask=0x1f/: 0 0 0
cycles: 1132814 754123 754123

Performance counter stats for 'sleep 1':

<not supported> amd_nb/event=0xe0,umask=0x1f/

         1,132,814      cycles                                                      

       1.001673257 seconds time elapsed

[boris@pd: ~/kernel/linux/tools/perf> ./perf stat -v -e amd_nb/event=0xe0,umask=0x1f/,amd_nb/event=0xe1,umask=0x3/ sleep 1 Using CPUID AuthenticAMD-21-2
amd_nb/event=0xe0,umask=0x1f/: 1002677 1001767669 1001767669
amd_nb/event=0xe1,umask=0x3/: 0 1001764647 1001764647

Performance counter stats for 'system wide':

         1,002,677      amd_nb/event=0xe0,umask=0x1f/                                   
                 0      amd_nb/event=0xe1,umask=0x3/                                   

       1.001811216 seconds time elapsed

[boris@pd: ~/kernel/linux/tools/perf> ./perf stat -v -e amd_nb/event=0xe0,umask=0x1f/,amd_nb/event=0xe1,umask=0x3/,amd_nb/event=0xe2,umask=0x3/ sleep 1 Using CPUID AuthenticAMD-21-2
amd_nb/event=0xe0,umask=0x1f/: 216137 1001633975 1001633975
amd_nb/event=0xe1,umask=0x3/: 0 1001635564 1001635564
amd_nb/event=0xe2,umask=0x3/: 1084188 1001637368 1001637368

Performance counter stats for 'system wide':

           216,137      amd_nb/event=0xe0,umask=0x1f/                                   
                 0      amd_nb/event=0xe1,umask=0x3/                                   
         1,084,188      amd_nb/event=0xe2,umask=0x3/

       1.001687549 seconds time elapsed

[boris@pd: ~/kernel/linux/tools/perf> ./perf stat -v -a -e amd_nb/event=0xe0,umask=0x1f/,amd_nb/event=0xe1,umask=0x3/,amd_nb/event=0xe2,umask=0x3/,cycles sleep 1 Using CPUID AuthenticAMD-21-2
amd_nb/event=0xe0,umask=0x1f/: 226857 1001760122 1001760122
amd_nb/event=0xe1,umask=0x3/: 0 1001757534 1001757534
amd_nb/event=0xe2,umask=0x3/: 1117142 1001753602 1001753602
cycles: 109689123 8014364579 8014364579

Performance counter stats for 'system wide':

           226,857      amd_nb/event=0xe0,umask=0x1f/                                   
                 0      amd_nb/event=0xe1,umask=0x3/                                   
         1,117,142      amd_nb/event=0xe2,umask=0x3/                                   
       109,689,123      cycles

       1.001970476 seconds time elapsed

Signature · 4 lines

Regards/Gruss,
    Boris.

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

Jiri Olsa

2017-02-27 09:57 UTC #5 lore raw

PATCH v2 1/1 perf tools: Force uncore events to system wide monitoring

Quoted 6 lines · Borislav Petkov

Quoted 1 line · Jiri Olsa
would you mind testing new version?
Looks ok to me.
Thanks.

thanks a lot, full patch attached

jirka

---
Making system wide (-a) the default option if no target was specified and one of following conditions is met:

- there's no workload specified (current behaviour)
- there is workload specified but all requested events are system wide events

Mixed events core/uncore with workload:

  $ perf stat -e 'uncore_cbox_0/clockticks/,cycles' sleep 1

Performance counter stats for 'sleep 1':

     <not supported>      uncore_cbox_0/clockticks/
             980,489      cycles

         1.000897406 seconds time elapsed

Uncore event with workload:

  $ perf stat -e 'uncore_cbox_0/clockticks/' sleep 1

Performance counter stats for 'system wide':

281,473,897,192,670 uncore_cbox_0/clockticks/

         1.000833784 seconds time elapsed

Signed-off-by Jiri Olsa <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Adrian Hunter <[email protected]>

Cc Borislav Petkov <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

---
 tools/perf/builtin-stat.c      | 32 +++++++++++++++++++++++++++++---
 tools/perf/util/parse-events.c |  5 +++--
 2 files changed, 32 insertions(+), 5 deletions(-)

tools/perf/builtin-stat.c +29 -3

index f4f555a67e9b..1320352cda04 100644
@@ -2350,6 +2350,34 @@ static int __cmd_report(int argc, const char **argv)
 	return 0;
 }
 
+static void setup_system_wide(int forks)
+{
+	/*
+	 * Make system wide (-a) the default target if
+	 * no target was specified and one of following
+	 * conditions is met:
+	 *
+	 *   - there's no workload specified
+	 *   - there is workload specified but all requested
+	 *     events are system wide events
+	 */
+	if (!target__none(&target))
+		return;
+
+	if (!forks)
+		target.system_wide = true;
+	else {
+		struct perf_evsel *counter;
+
+		evlist__for_each_entry(evsel_list, counter) {
+			if (!counter->system_wide)
+				return;
+		}
+
+		target.system_wide = true;
+	}
+}
+
 int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	const char * const stat_usage[] = {

@@ -2456,9 +2484,7 @@ 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))
-		target.system_wide = true;
+	setup_system_wide(argc);
 
 	if (run_count < 0) {
 		pr_err("Run count must be a positive number\n");

tools/perf/util/parse-events.c +3 -2

index 67a8aebc67ab..54355d3caf09 100644
@@ -316,8 +316,9 @@ __add_event(struct list_head *list, int *idx,
 		return NULL;
 
 	(*idx)++;
-	evsel->cpus     = cpu_map__get(cpus);
-	evsel->own_cpus = cpu_map__get(cpus);
+	evsel->cpus        = cpu_map__get(cpus);
+	evsel->own_cpus    = cpu_map__get(cpus);
+	evsel->system_wide = !!cpus;
 
 	if (name)
 		evsel->name = strdup(name);

Signature · 1 line

2.7.4

Arnaldo Carvalho de Melo

maintainer

2017-03-01 21:47 UTC #6 lore raw

Em Mon, Feb 27, 2017 at 10:48:18AM +0100, Jiri Olsa escreveu:

Quoted 11 lines · Borislav Petkov

Quoted 6 lines · Jiri Olsa
Quoted 1 line
would you mind testing new version?
Looks ok to me.
Thanks.
thanks a lot, full patch attached
jirka

With it I can't build perf anymore, as I use perf with all make invocations, so that I can catch when the tool stops working for users more quickly, like in this case:

[acme@jouet linux]$ perf stat -e cycles usleep 1

Performance counter stats for 'usleep 1':

             653,232      cycles:u                                                    

         0.005507594 seconds time elapsed

[acme@jouet linux]$ perf stat usleep 1
Error:
You may not have permission to collect system-wide stats.

Consider tweaking /proc/sys/kernel/perf_event_paranoid, which controls use of the performance events system by unprivileged users (without CAP_SYS_ADMIN).

The current value is 2:

-1: Allow use of (almost) all events by all users

Quoted 3 lines

= 0: Disallow raw tracepoint access by users without CAP_IOC_LOCK = 1: Disallow CPU event access by users without CAP_SYS_ADMIN = 2: Disallow kernel profiling by users without CAP_SYS_ADMIN

To make this setting permanent, edit /etc/sysctl.conf too, e.g.:

kernel.perf_event_paranoid = -1

[acme@jouet linux]$

------------------------------------------------

If I revert your patch thins are back working:

[acme@jouet linux]$ perf stat -e cycles usleep 1

Performance counter stats for 'usleep 1':

           459,396      cycles:u                                                    

       0.001159874 seconds time elapsed

[acme@jouet linux]$ perf stat usleep 1

Performance counter stats for 'usleep 1':

          2.022049      task-clock:u (msec)       #    0.181 CPUs utilized          
                 0      context-switches:u        #    0.000 K/sec                  
                 0      cpu-migrations:u          #    0.000 K/sec                  
                48      page-faults:u             #    0.024 M/sec                  
         2,177,301      cycles:u                  #    1.077 GHz                    
           245,106      instructions:u            #    0.11  insn per cycle         
            48,000      branches:u                #   23.738 M/sec                  
             3,936      branch-misses:u           #    8.20% of all branches

       0.011181563 seconds time elapsed

[acme@jouet linux]$

---------------------------------------------

Seems related to the auto :u done when the user can't do kernel profiling, etc.

Re-applying your patch, to triple check:

[acme@jouet linux]$ perf stat usleep 1 2>&1 | head -2 Error:
You may not have permission to collect system-wide stats. [acme@jouet linux]$

------------------------------------------------

Waiting for v3 :-)

- Arnaldo

Quoted 108 lines

---
Making system wide (-a) the default option if no target was specified and one of following conditions is met:

- there's no workload specified (current behaviour)
- there is workload specified but all requested events are system wide events

Mixed events core/uncore with workload:

  $ perf stat -e 'uncore_cbox_0/clockticks/,cycles' sleep 1

Performance counter stats for 'sleep 1':

     <not supported>      uncore_cbox_0/clockticks/
             980,489      cycles

         1.000897406 seconds time elapsed

Uncore event with workload:

  $ perf stat -e 'uncore_cbox_0/clockticks/' sleep 1

Performance counter stats for 'system wide':

281,473,897,192,670 uncore_cbox_0/clockticks/

         1.000833784 seconds time elapsed

Signed-off-by Jiri Olsa <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Adrian Hunter <[email protected]>

Cc Borislav Petkov <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

---
 tools/perf/builtin-stat.c      | 32 +++++++++++++++++++++++++++++---
 tools/perf/util/parse-events.c |  5 +++--
 2 files changed, 32 insertions(+), 5 deletions(-)

tools/perf/builtin-stat.c +29 -3

index f4f555a67e9b..1320352cda04 100644
@@ -2350,6 +2350,34 @@ static int __cmd_report(int argc, const char **argv)
 	return 0;
 }
 
+static void setup_system_wide(int forks)
+{
+	/*
+	 * Make system wide (-a) the default target if
+	 * no target was specified and one of following
+	 * conditions is met:
+	 *
+	 *   - there's no workload specified
+	 *   - there is workload specified but all requested
+	 *     events are system wide events
+	 */
+	if (!target__none(&target))
+		return;
+
+	if (!forks)
+		target.system_wide = true;
+	else {
+		struct perf_evsel *counter;
+
+		evlist__for_each_entry(evsel_list, counter) {
+			if (!counter->system_wide)
+				return;
+		}
+
+		target.system_wide = true;
+	}
+}
+
 int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	const char * const stat_usage[] = {

@@ -2456,9 +2484,7 @@ 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))
-		target.system_wide = true;
+	setup_system_wide(argc);
 
 	if (run_count < 0) {
 		pr_err("Run count must be a positive number\n");

tools/perf/util/parse-events.c +3 -2

index 67a8aebc67ab..54355d3caf09 100644
@@ -316,8 +316,9 @@ __add_event(struct list_head *list, int *idx,
 		return NULL;
 
 	(*idx)++;
-	evsel->cpus     = cpu_map__get(cpus);
-	evsel->own_cpus = cpu_map__get(cpus);
+	evsel->cpus        = cpu_map__get(cpus);
+	evsel->own_cpus    = cpu_map__get(cpus);
+	evsel->system_wide = !!cpus;
 
 	if (name)
 		evsel->name = strdup(name);

Signature · 1 line

2.7.4

Jiri Olsa

2017-03-02 07:55 UTC #7 lore raw

On Wed, Mar 01, 2017 at 06:36:39PM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

Quoted 23 lines

[acme@jouet linux]$ perf stat usleep 1

Performance counter stats for 'usleep 1':

          2.022049      task-clock:u (msec)       #    0.181 CPUs utilized          
                 0      context-switches:u        #    0.000 K/sec                  
                 0      cpu-migrations:u          #    0.000 K/sec                  
                48      page-faults:u             #    0.024 M/sec                  
         2,177,301      cycles:u                  #    1.077 GHz                    
           245,106      instructions:u            #    0.11  insn per cycle         
            48,000      branches:u                #   23.738 M/sec                  
             3,936      branch-misses:u           #    8.20% of all branches

       0.011181563 seconds time elapsed

[acme@jouet linux]$

---------------------------------------------

Seems related to the auto :u done when the user can't do kernel profiling, etc.

Re-applying your patch, to triple check:

Quoted 8 lines

[acme@jouet linux]$ perf stat usleep 1 2>&1 | head -2 Error:
You may not have permission to collect system-wide stats. [acme@jouet linux]$
------------------------------------------------
Waiting for v3 :-)

oh my... I'll check

thanks,
jirka

Jiri Olsa

2017-03-02 08:09 UTC #8 lore raw

Quoted 32 lines · Jiri Olsa

On Wed, Mar 01, 2017 at 06:36:39PM -0300, Arnaldo Carvalho de Melo wrote:
SNIP
Quoted 28 lines
[acme@jouet linux]$ perf stat usleep 1
Performance counter stats for 'usleep 1':
          2.022049      task-clock:u (msec)       #    0.181 CPUs utilized          
                 0      context-switches:u        #    0.000 K/sec                  
                 0      cpu-migrations:u          #    0.000 K/sec                  
                48      page-faults:u             #    0.024 M/sec                  
         2,177,301      cycles:u                  #    1.077 GHz                    
           245,106      instructions:u            #    0.11  insn per cycle         
            48,000      branches:u                #   23.738 M/sec                  
             3,936      branch-misses:u           #    8.20% of all branches        
       0.011181563 seconds time elapsed
[acme@jouet linux]$
---------------------------------------------
Seems related to the auto :u done when the user can't do kernel profiling, etc.
Re-applying your patch, to triple check:
[acme@jouet linux]$ perf stat usleep 1 2>&1 | head -2 Error:
You may not have permission to collect system-wide stats. [acme@jouet linux]$

Quoted 5 lines

Quoted 3 lines
------------------------------------------------
Waiting for v3 :-)
oh my... I'll check

could you please try with attched change

thanks,
jirka

---

tools/perf/builtin-stat.c +2 -1

index 1320352cda04..f53f449d864d 100644
@@ -2374,7 +2374,8 @@ static void setup_system_wide(int forks)
 				return;
 		}
 
-		target.system_wide = true;
+		if (evsel_list->nr_entries)
+			target.system_wide = true;
 	}
 }

tip-bot for Jiri Olsa

merge bot

2017-03-07 08:32 UTC #9 lore raw

In reply to Jiri Olsa

perf tools: Force uncore events to system wide monitoring

Commit-ID:  e3ba76deef23064fc272424b86b506cd80b04fc5
Gitweb:     http://git.kernel.org/tip/e3ba76deef23064fc272424b86b506cd80b04fc5
Author:     Jiri Olsa <[email protected]>
AuthorDate: Mon, 27 Feb 2017 10:48:18 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Fri, 3 Mar 2017 19:07:19 -0300

perf tools: Force uncore events to system wide monitoring

Make system wide (-a) the default option if no target was specified and one of following conditions is met:

- there's no workload specified (current behaviour)
- there is workload specified but all requested events are system wide ones

Mixed events core/uncore with workload:

  $ perf stat -e 'uncore_cbox_0/clockticks/,cycles' sleep 1

Performance counter stats for 'sleep 1':

     <not supported>      uncore_cbox_0/clockticks/
             980,489      cycles

         1.000897406 seconds time elapsed

Uncore event with workload:

  $ perf stat -e 'uncore_cbox_0/clockticks/' sleep 1

Performance counter stats for 'system wide':

281,473,897,192,670 uncore_cbox_0/clockticks/

         1.000833784 seconds time elapsed

Committer note:

When testing I realized the default case for !root, i.e. no events passed via -e, was broke by v2 of this patch, reported and after a patch provided by Jiri it is back working:

[acme@jouet linux]$ perf stat usleep 1

Performance counter stats for 'usleep 1':

         0.401335      task-clock:u (msec)     #   0.297 CPUs utilized
                0      context-switches:u      #   0.000 K/sec
                0      cpu-migrations:u        #   0.000 K/sec
               48      page-faults:u           #   0.120 M/sec
          458,146      cycles:u                #   1.142 GHz
          245,113      instructions:u          #   0.54  insn per cycle
           47,991      branches:u              # 119.578 M/sec
            4,022      branch-misses:u         #   8.38% of all branches

      0.001350029 seconds time elapsed

[acme@jouet linux]$

Suggested-and-Tested-by Borislav Petkov <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

Tested-by Arnaldo Carvalho de Melo <[email protected]>

Cc Adrian Hunter <[email protected]>

Cc David Ahern <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/20170227094818.GA12764@krava open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/builtin-stat.c      | 33 ++++++++++++++++++++++++++++++---
 tools/perf/util/parse-events.c |  5 +++--
 2 files changed, 33 insertions(+), 5 deletions(-)

tools/perf/builtin-stat.c +30 -3

index f4f555a..f53f449 100644
@@ -2350,6 +2350,35 @@ static int __cmd_report(int argc, const char **argv)
 	return 0;
 }
 
+static void setup_system_wide(int forks)
+{
+	/*
+	 * Make system wide (-a) the default target if
+	 * no target was specified and one of following
+	 * conditions is met:
+	 *
+	 *   - there's no workload specified
+	 *   - there is workload specified but all requested
+	 *     events are system wide events
+	 */
+	if (!target__none(&target))
+		return;
+
+	if (!forks)
+		target.system_wide = true;
+	else {
+		struct perf_evsel *counter;
+
+		evlist__for_each_entry(evsel_list, counter) {
+			if (!counter->system_wide)
+				return;
+		}
+
+		if (evsel_list->nr_entries)
+			target.system_wide = true;
+	}
+}
+
 int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	const char * const stat_usage[] = {

@@ -2456,9 +2485,7 @@ 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))
-		target.system_wide = true;
+	setup_system_wide(argc);
 
 	if (run_count < 0) {
 		pr_err("Run count must be a positive number\n");

tools/perf/util/parse-events.c +3 -2

index 67a8aeb..54355d3 100644
@@ -316,8 +316,9 @@ __add_event(struct list_head *list, int *idx,
 		return NULL;
 
 	(*idx)++;
-	evsel->cpus     = cpu_map__get(cpus);
-	evsel->own_cpus = cpu_map__get(cpus);
+	evsel->cpus        = cpu_map__get(cpus);
+	evsel->own_cpus    = cpu_map__get(cpus);
+	evsel->system_wide = !!cpus;
 
 	if (name)
 		evsel->name = strdup(name);