All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf tools: Force uncore events to system wide monitoring
@ 2017-02-24 10:17 Jiri Olsa
  2017-02-24 13:32 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 10+ messages in thread
From: Jiri Olsa @ 2017-02-24 10:17 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Peter Zijlstra, Adrian Hunter, Borislav Petkov,
	lkml, Ingo Molnar, David Ahern

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

Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Link: http://lkml.kernel.org/n/tip-rh8kybg6xdadfrw6x6uj37i2@git.kernel.org
---
 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(-)

diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c
index 79fe07158d00..654290f87a19 100644
--- a/tools/perf/arch/x86/util/pmu.c
+++ b/tools/perf/arch/x86/util/pmu.c
@@ -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;
 }
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 13b54999ad79..c32c7fa6f092 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -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");
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 67a8aebc67ab..8b06b21f1bbc 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -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;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 00852ddc7741..0ce3ffacbf92 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -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 */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] perf tools: Force uncore events to system wide monitoring
  2017-02-24 10:17 [PATCH] perf tools: Force uncore events to system wide monitoring Jiri Olsa
@ 2017-02-24 13:32 ` Arnaldo Carvalho de Melo
  2017-02-24 14:00   ` Jiri Olsa
  0 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-02-24 13:32 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Namhyung Kim, Peter Zijlstra, Adrian Hunter, Borislav Petkov,
	lkml, Ingo Molnar, David Ahern

Em Fri, Feb 24, 2017 at 11:17:14AM +0100, Jiri Olsa escreveu:
> Adding system_wide flag to uncore pmu objects and passing
> this flag along to the their events.

Boris, ack? Tested-by?

- Arnaldo
 
> 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
> 
> Suggested-by: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Link: http://lkml.kernel.org/n/tip-rh8kybg6xdadfrw6x6uj37i2@git.kernel.org
> ---
>  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(-)
> 
> diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c
> index 79fe07158d00..654290f87a19 100644
> --- a/tools/perf/arch/x86/util/pmu.c
> +++ b/tools/perf/arch/x86/util/pmu.c
> @@ -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;
>  }
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 13b54999ad79..c32c7fa6f092 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -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");
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 67a8aebc67ab..8b06b21f1bbc 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -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;
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index 00852ddc7741..0ce3ffacbf92 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -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 */
> -- 
> 2.7.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] perf tools: Force uncore events to system wide monitoring
  2017-02-24 13:32 ` Arnaldo Carvalho de Melo
@ 2017-02-24 14:00   ` Jiri Olsa
  2017-02-27  9:28     ` Jiri Olsa
  0 siblings, 1 reply; 10+ messages in thread
From: Jiri Olsa @ 2017-02-24 14:00 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Peter Zijlstra, Adrian Hunter,
	Borislav Petkov, lkml, Ingo Molnar, David Ahern

On Fri, Feb 24, 2017 at 10:32:22AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 24, 2017 at 11:17:14AM +0100, Jiri Olsa escreveu:
> > 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] perf tools: Force uncore events to system wide monitoring
  2017-02-24 14:00   ` Jiri Olsa
@ 2017-02-27  9:28     ` Jiri Olsa
  2017-02-27  9:44       ` Borislav Petkov
  0 siblings, 1 reply; 10+ messages in thread
From: Jiri Olsa @ 2017-02-27  9:28 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Borislav Petkov
  Cc: Jiri Olsa, Namhyung Kim, Peter Zijlstra, Adrian Hunter, lkml,
	Ingo Molnar, David Ahern

On Fri, Feb 24, 2017 at 03:00:03PM +0100, Jiri Olsa wrote:
> On Fri, Feb 24, 2017 at 10:32:22AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Feb 24, 2017 at 11:17:14AM +0100, Jiri Olsa escreveu:
> > > 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


---
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f4f555a67e9b..1320352cda04 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -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");
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 67a8aebc67ab..54355d3caf09 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -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);

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] perf tools: Force uncore events to system wide monitoring
  2017-02-27  9:28     ` Jiri Olsa
@ 2017-02-27  9:44       ` Borislav Petkov
  2017-02-27  9:48         ` [PATCHv2] " Jiri Olsa
  0 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2017-02-27  9:44 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim,
	Peter Zijlstra, Adrian Hunter, lkml, Ingo Molnar, David Ahern

On Mon, Feb 27, 2017 at 10:28:59AM +0100, Jiri Olsa wrote:
> 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

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCHv2] perf tools: Force uncore events to system wide monitoring
  2017-02-27  9:44       ` Borislav Petkov
@ 2017-02-27  9:48         ` Jiri Olsa
  2017-03-01 21:36           ` Arnaldo Carvalho de Melo
  2017-03-07  8:22           ` [tip:perf/core] " tip-bot for Jiri Olsa
  0 siblings, 2 replies; 10+ messages in thread
From: Jiri Olsa @ 2017-02-27  9:48 UTC (permalink / raw)
  To: Borislav Petkov, Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Peter Zijlstra, Adrian Hunter, lkml,
	Ingo Molnar, David Ahern

On Mon, Feb 27, 2017 at 10:44:13AM +0100, Borislav Petkov wrote:
> On Mon, Feb 27, 2017 at 10:28:59AM +0100, Jiri Olsa wrote:
> > 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

Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Link: http://lkml.kernel.org/n/tip-rh8kybg6xdadfrw6x6uj37i2@git.kernel.org
---
 tools/perf/builtin-stat.c      | 32 +++++++++++++++++++++++++++++---
 tools/perf/util/parse-events.c |  5 +++--
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f4f555a67e9b..1320352cda04 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -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");
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 67a8aebc67ab..54355d3caf09 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -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);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCHv2] perf tools: Force uncore events to system wide monitoring
  2017-02-27  9:48         ` [PATCHv2] " Jiri Olsa
@ 2017-03-01 21:36           ` Arnaldo Carvalho de Melo
  2017-03-02  7:44             ` Jiri Olsa
  2017-03-07  8:22           ` [tip:perf/core] " tip-bot for Jiri Olsa
  1 sibling, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-03-01 21:36 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Borislav Petkov, Jiri Olsa, Namhyung Kim, Peter Zijlstra,
	Adrian Hunter, lkml, Ingo Molnar, David Ahern

Em Mon, Feb 27, 2017 at 10:48:18AM +0100, Jiri Olsa escreveu:
> On Mon, Feb 27, 2017 at 10:44:13AM +0100, Borislav Petkov wrote:
> > On Mon, Feb 27, 2017 at 10:28:59AM +0100, Jiri Olsa wrote:
> > > 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
  >= 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


 
> 
> ---
> 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
> 
> Suggested-by: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Link: http://lkml.kernel.org/n/tip-rh8kybg6xdadfrw6x6uj37i2@git.kernel.org
> ---
>  tools/perf/builtin-stat.c      | 32 +++++++++++++++++++++++++++++---
>  tools/perf/util/parse-events.c |  5 +++--
>  2 files changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index f4f555a67e9b..1320352cda04 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -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");
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 67a8aebc67ab..54355d3caf09 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -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);
> -- 
> 2.7.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCHv2] perf tools: Force uncore events to system wide monitoring
  2017-03-01 21:36           ` Arnaldo Carvalho de Melo
@ 2017-03-02  7:44             ` Jiri Olsa
  2017-03-02  8:08               ` Jiri Olsa
  0 siblings, 1 reply; 10+ messages in thread
From: Jiri Olsa @ 2017-03-02  7:44 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Borislav Petkov, Jiri Olsa, Namhyung Kim, Peter Zijlstra,
	Adrian Hunter, lkml, Ingo Molnar, David Ahern

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

SNIP

>   [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 :-)

oh my... I'll check

thanks,
jirka

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCHv2] perf tools: Force uncore events to system wide monitoring
  2017-03-02  7:44             ` Jiri Olsa
@ 2017-03-02  8:08               ` Jiri Olsa
  0 siblings, 0 replies; 10+ messages in thread
From: Jiri Olsa @ 2017-03-02  8:08 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Borislav Petkov, Jiri Olsa, Namhyung Kim, Peter Zijlstra,
	Adrian Hunter, lkml, Ingo Molnar, David Ahern

On Thu, Mar 02, 2017 at 08:44:10AM +0100, Jiri Olsa wrote:
> On Wed, Mar 01, 2017 at 06:36:39PM -0300, Arnaldo Carvalho de Melo wrote:
> 
> SNIP
> 
> >   [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 :-)
> 
> oh my... I'll check

could you please try with attched change

thanks,
jirka


---
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 1320352cda04..f53f449d864d 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -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;
 	}
 }
 

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [tip:perf/core] perf tools: Force uncore events to system wide monitoring
  2017-02-27  9:48         ` [PATCHv2] " Jiri Olsa
  2017-03-01 21:36           ` Arnaldo Carvalho de Melo
@ 2017-03-07  8:22           ` tip-bot for Jiri Olsa
  1 sibling, 0 replies; 10+ messages in thread
From: tip-bot for Jiri Olsa @ 2017-03-07  8:22 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, bp, mingo, jolsa, acme, namhyung, dsahern, hpa,
	adrian.hunter, linux-kernel, a.p.zijlstra, jolsa

Commit-ID:  e3ba76deef23064fc272424b86b506cd80b04fc5
Gitweb:     http://git.kernel.org/tip/e3ba76deef23064fc272424b86b506cd80b04fc5
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Mon, 27 Feb 2017 10:48:18 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <bp@alien8.de>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
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/20170227094818.GA12764@krava
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c      | 33 ++++++++++++++++++++++++++++++---
 tools/perf/util/parse-events.c |  5 +++--
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f4f555a..f53f449 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -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");
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 67a8aeb..54355d3 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -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);

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-03-07  8:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-24 10:17 [PATCH] perf tools: Force uncore events to system wide monitoring Jiri Olsa
2017-02-24 13:32 ` Arnaldo Carvalho de Melo
2017-02-24 14:00   ` Jiri Olsa
2017-02-27  9:28     ` Jiri Olsa
2017-02-27  9:44       ` Borislav Petkov
2017-02-27  9:48         ` [PATCHv2] " Jiri Olsa
2017-03-01 21:36           ` Arnaldo Carvalho de Melo
2017-03-02  7:44             ` Jiri Olsa
2017-03-02  8:08               ` Jiri Olsa
2017-03-07  8:22           ` [tip:perf/core] " tip-bot for Jiri Olsa

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.