* [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.