* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).