linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Revert "perf tools: Fix PMU term format max value calculation"
@ 2018-10-03  7:20 Jiri Olsa
  2018-10-03 14:34 ` [RFC] perf tools: Wrong filter_band* values in json calculation" Jiri Olsa
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Jiri Olsa @ 2018-10-03  7:20 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Andi Kleen, Kan Liang, lkml, Ingo Molnar, Namhyung Kim,
	Alexander Shishkin, Peter Zijlstra, Andi Kleen, Michael Petlan

This reverts commit ac0e2cd555373ae6f8f3a3ad3fbbf5b6d1e7aaaa.

Michael reported an issue with oversized terms values assignment
and I noticed there was actually a misunderstanding of the max
value check in the past.

The above commit's changelog says:

  If bit 21 is set, there is parsing issues as below.

    $ perf stat -a -e uncore_qpi_0/event=0x200002,umask=0x8/
    event syntax error: '..pi_0/event=0x200002,umask=0x8/'
                                      \___ value too big for format, maximum is 511

But there's no issue there, because the event value is distributed
along the value defined by the format. Even if the format defines
separated bit, the value is treated as a continual number, which
should follow the format definition.

In above case it's 9-bit value with last bit separated:
  $ cat uncore_qpi_0/format/event
  config:0-7,21

Hence the value 0x200002 is correctly reported as format violation,
because it exceeds 9 bits. It should have been 0x102 instead, which
sets the 9th bit - the bit 21 of the format.

  $ perf stat -vv -a -e uncore_qpi_0/event=0x102,umask=0x8/
  Using CPUID GenuineIntel-6-2D
  ...
  ------------------------------------------------------------
  perf_event_attr:
    type                             10
    size                             112
    config                           0x200802
    sample_type                      IDENTIFIER
  ...

Cc: Andi Kleen <ak@linux.intel.com>
Cc: Kan Liang <kan.liang@intel.com>
Reported-by: Michael Petlan <mpetlan@redhat.com>
Link: http://lkml.kernel.org/n/tip-icxq7a1r66lusm3ahaimekis@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/pmu.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index afd68524ffa9..7799788f662f 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -930,13 +930,14 @@ static void pmu_format_value(unsigned long *format, __u64 value, __u64 *v,
 
 static __u64 pmu_format_max_value(const unsigned long *format)
 {
-	__u64 w = 0;
-	int fbit;
-
-	for_each_set_bit(fbit, format, PERF_PMU_FORMAT_BITS)
-		w |= (1ULL << fbit);
+	int w;
 
-	return w;
+	w = bitmap_weight(format, PERF_PMU_FORMAT_BITS);
+	if (!w)
+		return 0;
+	if (w < 64)
+		return (1ULL << w) - 1;
+	return -1;
 }
 
 /*
-- 
2.17.1


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

* [RFC] perf tools: Wrong filter_band* values in json calculation"
  2018-10-03  7:20 [PATCH] Revert "perf tools: Fix PMU term format max value calculation" Jiri Olsa
@ 2018-10-03 14:34 ` Jiri Olsa
  2018-10-03 14:45   ` Andi Kleen
  2018-10-09  9:59 ` [PATCH] Revert "perf tools: Fix PMU term format max value calculation" Jiri Olsa
  2018-10-18  6:17 ` [tip:perf/urgent] " tip-bot for Jiri Olsa
  2 siblings, 1 reply; 11+ messages in thread
From: Jiri Olsa @ 2018-10-03 14:34 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Arnaldo Carvalho de Melo, Andi Kleen, Kan Liang, lkml,
	Ingo Molnar, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
	Andi Kleen, Michael Petlan, Jiri Olsa

On Wed, Oct 03, 2018 at 09:20:46AM +0200, Jiri Olsa wrote:
> This reverts commit ac0e2cd555373ae6f8f3a3ad3fbbf5b6d1e7aaaa.
> 
> Michael reported an issue with oversized terms values assignment
> and I noticed there was actually a misunderstanding of the max
> value check in the past.

hi,
this is just a follow up question.. Miachel's original report
was that he could not stat following event:

  $ perf stat -e unc_p_freq_ge_1200mhz_cycles -a -- ls
  event syntax error: '..e_1200mhz_cycles'
                                    \___ value too big for format, maximum is 255
  Run 'perf list' for a list of valid events

The event is unwrapped into:
  uncore_pcu/event=0xb,filter_band0=1200/

where filter_band0 format says it's one byte only:

  [root@intel-lizardhead-01 uncore_pcu]# cat format/filter_band0
  config1:0-7

while json files specifies bigger number:

        "Filter": "filter_band0=1200",

  [root@intel-lizardhead-01 uncore_pcu]# cat format/filter_band1
  config1:8-15
  [root@intel-lizardhead-01 uncore_pcu]# cat format/filter_band2
  config1:16-23
  [root@intel-lizardhead-01 uncore_pcu]# cat format/filter_band3
  config1:24-31

The reason seems to be that filter_band* values are
supposed to be in 100Mhz units.. it's stated in the
json help for the events, like:
  filter_band3=XXX, with XXX in 100Mhz units

Attached change and the original patch fix the problem
problem for me, but Intel guys might have already some
update on that.. and I have no way on checking on this
other than the help message I found.. hence the RFC ;-)

also there might be more of those..

note there's couple of changes that actually changed
the number completely, like:

-        "Filter": "edge=1,filter_band2=4000",
+        "Filter": "edge=1,filter_band2=30",

thanks,
jirka


---
 .../arch/x86/ivytown/uncore-power.json           | 16 ++++++++--------
 .../arch/x86/jaketown/uncore-power.json          | 16 ++++++++--------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json b/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json
index d40498f2cb1e..635c09fda1d9 100644
--- a/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json
+++ b/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json
@@ -188,7 +188,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xb",
         "EventName": "UNC_P_FREQ_GE_1200MHZ_CYCLES",
-        "Filter": "filter_band0=1200",
+        "Filter": "filter_band0=12",
         "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_1200mhz_cycles %",
         "PerPkg": "1",
@@ -199,7 +199,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xc",
         "EventName": "UNC_P_FREQ_GE_2000MHZ_CYCLES",
-        "Filter": "filter_band1=2000",
+        "Filter": "filter_band1=20",
         "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_2000mhz_cycles %",
         "PerPkg": "1",
@@ -210,7 +210,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xd",
         "EventName": "UNC_P_FREQ_GE_3000MHZ_CYCLES",
-        "Filter": "filter_band2=3000",
+        "Filter": "filter_band2=30",
         "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_3000mhz_cycles %",
         "PerPkg": "1",
@@ -221,7 +221,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xe",
         "EventName": "UNC_P_FREQ_GE_4000MHZ_CYCLES",
-        "Filter": "filter_band3=4000",
+        "Filter": "filter_band3=40",
         "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_4000mhz_cycles %",
         "PerPkg": "1",
@@ -232,7 +232,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xb",
         "EventName": "UNC_P_FREQ_GE_1200MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band0=1200",
+        "Filter": "edge=1,filter_band0=12",
         "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_1200mhz_cycles %",
         "PerPkg": "1",
@@ -243,7 +243,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xc",
         "EventName": "UNC_P_FREQ_GE_2000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band1=2000",
+        "Filter": "edge=1,filter_band1=20",
         "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_2000mhz_cycles %",
         "PerPkg": "1",
@@ -254,7 +254,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xd",
         "EventName": "UNC_P_FREQ_GE_3000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band2=4000",
+        "Filter": "edge=1,filter_band2=30",
         "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_3000mhz_cycles %",
         "PerPkg": "1",
@@ -265,7 +265,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xe",
         "EventName": "UNC_P_FREQ_GE_4000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band3=4000",
+        "Filter": "edge=1,filter_band3=40",
         "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_4000mhz_cycles %",
         "PerPkg": "1",
diff --git a/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json b/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json
index 16034bfd06dd..8755693d86c6 100644
--- a/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json
+++ b/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json
@@ -187,7 +187,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xb",
         "EventName": "UNC_P_FREQ_GE_1200MHZ_CYCLES",
-        "Filter": "filter_band0=1200",
+        "Filter": "filter_band0=12",
         "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_1200mhz_cycles %",
         "PerPkg": "1",
@@ -198,7 +198,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xc",
         "EventName": "UNC_P_FREQ_GE_2000MHZ_CYCLES",
-        "Filter": "filter_band1=2000",
+        "Filter": "filter_band1=20",
         "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_2000mhz_cycles %",
         "PerPkg": "1",
@@ -209,7 +209,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xd",
         "EventName": "UNC_P_FREQ_GE_3000MHZ_CYCLES",
-        "Filter": "filter_band2=3000",
+        "Filter": "filter_band2=30",
         "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_3000mhz_cycles %",
         "PerPkg": "1",
@@ -220,7 +220,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xe",
         "EventName": "UNC_P_FREQ_GE_4000MHZ_CYCLES",
-        "Filter": "filter_band3=4000",
+        "Filter": "filter_band3=40",
         "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_4000mhz_cycles %",
         "PerPkg": "1",
@@ -231,7 +231,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xb",
         "EventName": "UNC_P_FREQ_GE_1200MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band0=1200",
+        "Filter": "edge=1,filter_band0=12",
         "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_1200mhz_cycles %",
         "PerPkg": "1",
@@ -242,7 +242,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xc",
         "EventName": "UNC_P_FREQ_GE_2000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band1=2000",
+        "Filter": "edge=1,filter_band1=20",
         "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_2000mhz_cycles %",
         "PerPkg": "1",
@@ -253,7 +253,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xd",
         "EventName": "UNC_P_FREQ_GE_3000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band2=4000",
+        "Filter": "edge=1,filter_band2=30",
         "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_3000mhz_cycles %",
         "PerPkg": "1",
@@ -264,7 +264,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xe",
         "EventName": "UNC_P_FREQ_GE_4000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band3=4000",
+        "Filter": "edge=1,filter_band3=40",
         "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_4000mhz_cycles %",
         "PerPkg": "1",
-- 
2.17.1


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

* Re: [RFC] perf tools: Wrong filter_band* values in json calculation"
  2018-10-03 14:34 ` [RFC] perf tools: Wrong filter_band* values in json calculation" Jiri Olsa
@ 2018-10-03 14:45   ` Andi Kleen
  2018-10-09 10:01     ` Jiri Olsa
  0 siblings, 1 reply; 11+ messages in thread
From: Andi Kleen @ 2018-10-03 14:45 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Kan Liang, lkml, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Andi Kleen,
	Michael Petlan

> note there's couple of changes that actually changed
> the number completely, like:
> 
> -        "Filter": "edge=1,filter_band2=4000",
> +        "Filter": "edge=1,filter_band2=30",

Thanks. Looks good. I'll fix the scripts to generate the uncore events.

-Andi

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

* Re: [PATCH] Revert "perf tools: Fix PMU term format max value calculation"
  2018-10-03  7:20 [PATCH] Revert "perf tools: Fix PMU term format max value calculation" Jiri Olsa
  2018-10-03 14:34 ` [RFC] perf tools: Wrong filter_band* values in json calculation" Jiri Olsa
@ 2018-10-09  9:59 ` Jiri Olsa
  2018-10-09 13:52   ` Arnaldo Carvalho de Melo
  2018-10-18  6:17 ` [tip:perf/urgent] " tip-bot for Jiri Olsa
  2 siblings, 1 reply; 11+ messages in thread
From: Jiri Olsa @ 2018-10-09  9:59 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Andi Kleen, Kan Liang, lkml,
	Ingo Molnar, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
	Andi Kleen, Michael Petlan

ping

thanks,
jirka

On Wed, Oct 03, 2018 at 09:20:46AM +0200, Jiri Olsa wrote:
> This reverts commit ac0e2cd555373ae6f8f3a3ad3fbbf5b6d1e7aaaa.
> 
> Michael reported an issue with oversized terms values assignment
> and I noticed there was actually a misunderstanding of the max
> value check in the past.
> 
> The above commit's changelog says:
> 
>   If bit 21 is set, there is parsing issues as below.
> 
>     $ perf stat -a -e uncore_qpi_0/event=0x200002,umask=0x8/
>     event syntax error: '..pi_0/event=0x200002,umask=0x8/'
>                                       \___ value too big for format, maximum is 511
> 
> But there's no issue there, because the event value is distributed
> along the value defined by the format. Even if the format defines
> separated bit, the value is treated as a continual number, which
> should follow the format definition.
> 
> In above case it's 9-bit value with last bit separated:
>   $ cat uncore_qpi_0/format/event
>   config:0-7,21
> 
> Hence the value 0x200002 is correctly reported as format violation,
> because it exceeds 9 bits. It should have been 0x102 instead, which
> sets the 9th bit - the bit 21 of the format.
> 
>   $ perf stat -vv -a -e uncore_qpi_0/event=0x102,umask=0x8/
>   Using CPUID GenuineIntel-6-2D
>   ...
>   ------------------------------------------------------------
>   perf_event_attr:
>     type                             10
>     size                             112
>     config                           0x200802
>     sample_type                      IDENTIFIER
>   ...
> 
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Kan Liang <kan.liang@intel.com>
> Reported-by: Michael Petlan <mpetlan@redhat.com>
> Link: http://lkml.kernel.org/n/tip-icxq7a1r66lusm3ahaimekis@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/pmu.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index afd68524ffa9..7799788f662f 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -930,13 +930,14 @@ static void pmu_format_value(unsigned long *format, __u64 value, __u64 *v,
>  
>  static __u64 pmu_format_max_value(const unsigned long *format)
>  {
> -	__u64 w = 0;
> -	int fbit;
> -
> -	for_each_set_bit(fbit, format, PERF_PMU_FORMAT_BITS)
> -		w |= (1ULL << fbit);
> +	int w;
>  
> -	return w;
> +	w = bitmap_weight(format, PERF_PMU_FORMAT_BITS);
> +	if (!w)
> +		return 0;
> +	if (w < 64)
> +		return (1ULL << w) - 1;
> +	return -1;
>  }
>  
>  /*
> -- 
> 2.17.1
> 

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

* Re: [RFC] perf tools: Wrong filter_band* values in json calculation"
  2018-10-03 14:45   ` Andi Kleen
@ 2018-10-09 10:01     ` Jiri Olsa
  2018-10-09 21:18       ` Andi Kleen
  0 siblings, 1 reply; 11+ messages in thread
From: Jiri Olsa @ 2018-10-09 10:01 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Arnaldo Carvalho de Melo, Kan Liang, lkml, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Andi Kleen,
	Michael Petlan

On Wed, Oct 03, 2018 at 07:45:50AM -0700, Andi Kleen wrote:
> > note there's couple of changes that actually changed
> > the number completely, like:
> > 
> > -        "Filter": "edge=1,filter_band2=4000",
> > +        "Filter": "edge=1,filter_band2=30",
> 
> Thanks. Looks good. I'll fix the scripts to generate the uncore events.

hi,
any idea when you could post an update for this?

thanks,
jirka

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

* Re: [PATCH] Revert "perf tools: Fix PMU term format max value calculation"
  2018-10-09  9:59 ` [PATCH] Revert "perf tools: Fix PMU term format max value calculation" Jiri Olsa
@ 2018-10-09 13:52   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-09 13:52 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jiri Olsa, Andi Kleen, Kan Liang, lkml, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Andi Kleen,
	Michael Petlan

Em Tue, Oct 09, 2018 at 11:59:17AM +0200, Jiri Olsa escreveu:
> ping

oops, applied to perf/urgent, added a Fixes: line so that stable@ pick
this up.

- Arnaldo
 
> thanks,
> jirka
> 
> On Wed, Oct 03, 2018 at 09:20:46AM +0200, Jiri Olsa wrote:
> > This reverts commit ac0e2cd555373ae6f8f3a3ad3fbbf5b6d1e7aaaa.
> > 
> > Michael reported an issue with oversized terms values assignment
> > and I noticed there was actually a misunderstanding of the max
> > value check in the past.
> > 
> > The above commit's changelog says:
> > 
> >   If bit 21 is set, there is parsing issues as below.
> > 
> >     $ perf stat -a -e uncore_qpi_0/event=0x200002,umask=0x8/
> >     event syntax error: '..pi_0/event=0x200002,umask=0x8/'
> >                                       \___ value too big for format, maximum is 511
> > 
> > But there's no issue there, because the event value is distributed
> > along the value defined by the format. Even if the format defines
> > separated bit, the value is treated as a continual number, which
> > should follow the format definition.
> > 
> > In above case it's 9-bit value with last bit separated:
> >   $ cat uncore_qpi_0/format/event
> >   config:0-7,21
> > 
> > Hence the value 0x200002 is correctly reported as format violation,
> > because it exceeds 9 bits. It should have been 0x102 instead, which
> > sets the 9th bit - the bit 21 of the format.
> > 
> >   $ perf stat -vv -a -e uncore_qpi_0/event=0x102,umask=0x8/
> >   Using CPUID GenuineIntel-6-2D
> >   ...
> >   ------------------------------------------------------------
> >   perf_event_attr:
> >     type                             10
> >     size                             112
> >     config                           0x200802
> >     sample_type                      IDENTIFIER
> >   ...
> > 
> > Cc: Andi Kleen <ak@linux.intel.com>
> > Cc: Kan Liang <kan.liang@intel.com>
> > Reported-by: Michael Petlan <mpetlan@redhat.com>
> > Link: http://lkml.kernel.org/n/tip-icxq7a1r66lusm3ahaimekis@git.kernel.org
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/perf/util/pmu.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > index afd68524ffa9..7799788f662f 100644
> > --- a/tools/perf/util/pmu.c
> > +++ b/tools/perf/util/pmu.c
> > @@ -930,13 +930,14 @@ static void pmu_format_value(unsigned long *format, __u64 value, __u64 *v,
> >  
> >  static __u64 pmu_format_max_value(const unsigned long *format)
> >  {
> > -	__u64 w = 0;
> > -	int fbit;
> > -
> > -	for_each_set_bit(fbit, format, PERF_PMU_FORMAT_BITS)
> > -		w |= (1ULL << fbit);
> > +	int w;
> >  
> > -	return w;
> > +	w = bitmap_weight(format, PERF_PMU_FORMAT_BITS);
> > +	if (!w)
> > +		return 0;
> > +	if (w < 64)
> > +		return (1ULL << w) - 1;
> > +	return -1;
> >  }
> >  
> >  /*
> > -- 
> > 2.17.1
> > 

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

* Re: [RFC] perf tools: Wrong filter_band* values in json calculation"
  2018-10-09 10:01     ` Jiri Olsa
@ 2018-10-09 21:18       ` Andi Kleen
  2018-10-10  8:03         ` [PATCH] perf tools: Fix wrong filter_band* values for uncore events Jiri Olsa
  0 siblings, 1 reply; 11+ messages in thread
From: Andi Kleen @ 2018-10-09 21:18 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Kan Liang, lkml, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Andi Kleen,
	Michael Petlan

On Tue, Oct 09, 2018 at 12:01:44PM +0200, Jiri Olsa wrote:
> On Wed, Oct 03, 2018 at 07:45:50AM -0700, Andi Kleen wrote:
> > > note there's couple of changes that actually changed
> > > the number completely, like:
> > > 
> > > -        "Filter": "edge=1,filter_band2=4000",
> > > +        "Filter": "edge=1,filter_band2=30",
> > 
> > Thanks. Looks good. I'll fix the scripts to generate the uncore events.
> 
> hi,
> any idea when you could post an update for this?

Can just use your patch for the existing event lists. You can add

Acked-by: Andi Kleen <ak@linux.intel.com>

I was mainly worried about future updates, but it doesn't seem to be a problem.

-Andi

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

* [PATCH] perf tools: Fix wrong filter_band* values for uncore events
  2018-10-09 21:18       ` Andi Kleen
@ 2018-10-10  8:03         ` Jiri Olsa
  2018-10-10 12:58           ` Arnaldo Carvalho de Melo
  2018-10-18  6:17           ` [tip:perf/urgent] perf vendor events intel: " tip-bot for Jiri Olsa
  0 siblings, 2 replies; 11+ messages in thread
From: Jiri Olsa @ 2018-10-10  8:03 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Arnaldo Carvalho de Melo, Kan Liang, lkml, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Andi Kleen,
	Michael Petlan

On Tue, Oct 09, 2018 at 02:18:39PM -0700, Andi Kleen wrote:
> On Tue, Oct 09, 2018 at 12:01:44PM +0200, Jiri Olsa wrote:
> > On Wed, Oct 03, 2018 at 07:45:50AM -0700, Andi Kleen wrote:
> > > > note there's couple of changes that actually changed
> > > > the number completely, like:
> > > > 
> > > > -        "Filter": "edge=1,filter_band2=4000",
> > > > +        "Filter": "edge=1,filter_band2=30",
> > > 
> > > Thanks. Looks good. I'll fix the scripts to generate the uncore events.
> > 
> > hi,
> > any idea when you could post an update for this?
> 
> Can just use your patch for the existing event lists. You can add
> 
> Acked-by: Andi Kleen <ak@linux.intel.com>
> 
> I was mainly worried about future updates, but it doesn't seem to be a problem.

ok, attaching patch with full changelog

thanks,
jirka


---
Michael reported that he could not stat following event:

  $ perf stat -e unc_p_freq_ge_1200mhz_cycles -a -- ls
  event syntax error: '..e_1200mhz_cycles'
                                    \___ value too big for format, maximum is 255
  Run 'perf list' for a list of valid events

The event is unwrapped into:
  uncore_pcu/event=0xb,filter_band0=1200/

where filter_band0 format says it's one byte only:

  # cat uncore_pcu/format/filter_band0
  config1:0-7

while json files specifies bigger number:

  "Filter": "filter_band0=1200",

all the filter_band* formats show 1 byte width:

  # cat uncore_pcu/format/filter_band1
  config1:8-15
  # cat uncore_pcu/format/filter_band2
  config1:16-23
  # cat uncore_pcu/format/filter_band3
  config1:24-31

The reason of the issue is that filter_band* values are
supposed to be in 100Mhz units.. it's stated in the json
help for the events, like:

  filter_band3=XXX, with XXX in 100Mhz units

This patch divides the filter_band* values by 100,
plus there's couple of changes that actually change
the number completely, like:

-        "Filter": "edge=1,filter_band2=4000",
+        "Filter": "edge=1,filter_band2=30",

Reported-by: Michael Petlan <mpetlan@redhat.com>
Acked-by: Andi Kleen <ak@linux.intel.com>
Link: http://lkml.kernel.org/n/tip-ujl4yzkexf3i10fjw24maa9e@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../arch/x86/ivytown/uncore-power.json           | 16 ++++++++--------
 .../arch/x86/jaketown/uncore-power.json          | 16 ++++++++--------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json b/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json
index d40498f2cb1e..635c09fda1d9 100644
--- a/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json
+++ b/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json
@@ -188,7 +188,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xb",
         "EventName": "UNC_P_FREQ_GE_1200MHZ_CYCLES",
-        "Filter": "filter_band0=1200",
+        "Filter": "filter_band0=12",
         "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_1200mhz_cycles %",
         "PerPkg": "1",
@@ -199,7 +199,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xc",
         "EventName": "UNC_P_FREQ_GE_2000MHZ_CYCLES",
-        "Filter": "filter_band1=2000",
+        "Filter": "filter_band1=20",
         "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_2000mhz_cycles %",
         "PerPkg": "1",
@@ -210,7 +210,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xd",
         "EventName": "UNC_P_FREQ_GE_3000MHZ_CYCLES",
-        "Filter": "filter_band2=3000",
+        "Filter": "filter_band2=30",
         "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_3000mhz_cycles %",
         "PerPkg": "1",
@@ -221,7 +221,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xe",
         "EventName": "UNC_P_FREQ_GE_4000MHZ_CYCLES",
-        "Filter": "filter_band3=4000",
+        "Filter": "filter_band3=40",
         "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_4000mhz_cycles %",
         "PerPkg": "1",
@@ -232,7 +232,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xb",
         "EventName": "UNC_P_FREQ_GE_1200MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band0=1200",
+        "Filter": "edge=1,filter_band0=12",
         "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_1200mhz_cycles %",
         "PerPkg": "1",
@@ -243,7 +243,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xc",
         "EventName": "UNC_P_FREQ_GE_2000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band1=2000",
+        "Filter": "edge=1,filter_band1=20",
         "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_2000mhz_cycles %",
         "PerPkg": "1",
@@ -254,7 +254,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xd",
         "EventName": "UNC_P_FREQ_GE_3000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band2=4000",
+        "Filter": "edge=1,filter_band2=30",
         "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_3000mhz_cycles %",
         "PerPkg": "1",
@@ -265,7 +265,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xe",
         "EventName": "UNC_P_FREQ_GE_4000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band3=4000",
+        "Filter": "edge=1,filter_band3=40",
         "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_4000mhz_cycles %",
         "PerPkg": "1",
diff --git a/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json b/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json
index 16034bfd06dd..8755693d86c6 100644
--- a/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json
+++ b/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json
@@ -187,7 +187,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xb",
         "EventName": "UNC_P_FREQ_GE_1200MHZ_CYCLES",
-        "Filter": "filter_band0=1200",
+        "Filter": "filter_band0=12",
         "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_1200mhz_cycles %",
         "PerPkg": "1",
@@ -198,7 +198,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xc",
         "EventName": "UNC_P_FREQ_GE_2000MHZ_CYCLES",
-        "Filter": "filter_band1=2000",
+        "Filter": "filter_band1=20",
         "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_2000mhz_cycles %",
         "PerPkg": "1",
@@ -209,7 +209,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xd",
         "EventName": "UNC_P_FREQ_GE_3000MHZ_CYCLES",
-        "Filter": "filter_band2=3000",
+        "Filter": "filter_band2=30",
         "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_3000mhz_cycles %",
         "PerPkg": "1",
@@ -220,7 +220,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xe",
         "EventName": "UNC_P_FREQ_GE_4000MHZ_CYCLES",
-        "Filter": "filter_band3=4000",
+        "Filter": "filter_band3=40",
         "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_4000mhz_cycles %",
         "PerPkg": "1",
@@ -231,7 +231,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xb",
         "EventName": "UNC_P_FREQ_GE_1200MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band0=1200",
+        "Filter": "edge=1,filter_band0=12",
         "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_1200mhz_cycles %",
         "PerPkg": "1",
@@ -242,7 +242,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xc",
         "EventName": "UNC_P_FREQ_GE_2000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band1=2000",
+        "Filter": "edge=1,filter_band1=20",
         "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_2000mhz_cycles %",
         "PerPkg": "1",
@@ -253,7 +253,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xd",
         "EventName": "UNC_P_FREQ_GE_3000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band2=4000",
+        "Filter": "edge=1,filter_band2=30",
         "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_3000mhz_cycles %",
         "PerPkg": "1",
@@ -264,7 +264,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xe",
         "EventName": "UNC_P_FREQ_GE_4000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band3=4000",
+        "Filter": "edge=1,filter_band3=40",
         "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_4000mhz_cycles %",
         "PerPkg": "1",
-- 
2.17.1


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

* Re: [PATCH] perf tools: Fix wrong filter_band* values for uncore events
  2018-10-10  8:03         ` [PATCH] perf tools: Fix wrong filter_band* values for uncore events Jiri Olsa
@ 2018-10-10 12:58           ` Arnaldo Carvalho de Melo
  2018-10-18  6:17           ` [tip:perf/urgent] perf vendor events intel: " tip-bot for Jiri Olsa
  1 sibling, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-10 12:58 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Andi Kleen, Kan Liang, lkml, Ingo Molnar, Namhyung Kim,
	Alexander Shishkin, Peter Zijlstra, Andi Kleen, Michael Petlan

Em Wed, Oct 10, 2018 at 10:03:39AM +0200, Jiri Olsa escreveu:
> On Tue, Oct 09, 2018 at 02:18:39PM -0700, Andi Kleen wrote:
> > On Tue, Oct 09, 2018 at 12:01:44PM +0200, Jiri Olsa wrote:
> > > On Wed, Oct 03, 2018 at 07:45:50AM -0700, Andi Kleen wrote:
> > > > > note there's couple of changes that actually changed
> > > > > the number completely, like:
> > > > > 
> > > > > -        "Filter": "edge=1,filter_band2=4000",
> > > > > +        "Filter": "edge=1,filter_band2=30",
> > > > 
> > > > Thanks. Looks good. I'll fix the scripts to generate the uncore events.
> > > 
> > > hi,
> > > any idea when you could post an update for this?
> > 
> > Can just use your patch for the existing event lists. You can add
> > 
> > Acked-by: Andi Kleen <ak@linux.intel.com>
> > 
> > I was mainly worried about future updates, but it doesn't seem to be a problem.
> 
> ok, attaching patch with full changelog
> 
> thanks,
> jirka

Ok, queuing this up in perf/urgent.

Thanks,

- Arnaldo
 
> 
> ---
> Michael reported that he could not stat following event:
> 
>   $ perf stat -e unc_p_freq_ge_1200mhz_cycles -a -- ls
>   event syntax error: '..e_1200mhz_cycles'
>                                     \___ value too big for format, maximum is 255
>   Run 'perf list' for a list of valid events
> 
> The event is unwrapped into:
>   uncore_pcu/event=0xb,filter_band0=1200/
> 
> where filter_band0 format says it's one byte only:
> 
>   # cat uncore_pcu/format/filter_band0
>   config1:0-7
> 
> while json files specifies bigger number:
> 
>   "Filter": "filter_band0=1200",
> 
> all the filter_band* formats show 1 byte width:
> 
>   # cat uncore_pcu/format/filter_band1
>   config1:8-15
>   # cat uncore_pcu/format/filter_band2
>   config1:16-23
>   # cat uncore_pcu/format/filter_band3
>   config1:24-31
> 
> The reason of the issue is that filter_band* values are
> supposed to be in 100Mhz units.. it's stated in the json
> help for the events, like:
> 
>   filter_band3=XXX, with XXX in 100Mhz units
> 
> This patch divides the filter_band* values by 100,
> plus there's couple of changes that actually change
> the number completely, like:
> 
> -        "Filter": "edge=1,filter_band2=4000",
> +        "Filter": "edge=1,filter_band2=30",
> 
> Reported-by: Michael Petlan <mpetlan@redhat.com>
> Acked-by: Andi Kleen <ak@linux.intel.com>
> Link: http://lkml.kernel.org/n/tip-ujl4yzkexf3i10fjw24maa9e@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  .../arch/x86/ivytown/uncore-power.json           | 16 ++++++++--------
>  .../arch/x86/jaketown/uncore-power.json          | 16 ++++++++--------
>  2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json b/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json
> index d40498f2cb1e..635c09fda1d9 100644
> --- a/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json
> +++ b/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json
> @@ -188,7 +188,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xb",
>          "EventName": "UNC_P_FREQ_GE_1200MHZ_CYCLES",
> -        "Filter": "filter_band0=1200",
> +        "Filter": "filter_band0=12",
>          "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_1200mhz_cycles %",
>          "PerPkg": "1",
> @@ -199,7 +199,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xc",
>          "EventName": "UNC_P_FREQ_GE_2000MHZ_CYCLES",
> -        "Filter": "filter_band1=2000",
> +        "Filter": "filter_band1=20",
>          "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_2000mhz_cycles %",
>          "PerPkg": "1",
> @@ -210,7 +210,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xd",
>          "EventName": "UNC_P_FREQ_GE_3000MHZ_CYCLES",
> -        "Filter": "filter_band2=3000",
> +        "Filter": "filter_band2=30",
>          "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_3000mhz_cycles %",
>          "PerPkg": "1",
> @@ -221,7 +221,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xe",
>          "EventName": "UNC_P_FREQ_GE_4000MHZ_CYCLES",
> -        "Filter": "filter_band3=4000",
> +        "Filter": "filter_band3=40",
>          "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_4000mhz_cycles %",
>          "PerPkg": "1",
> @@ -232,7 +232,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xb",
>          "EventName": "UNC_P_FREQ_GE_1200MHZ_TRANSITIONS",
> -        "Filter": "edge=1,filter_band0=1200",
> +        "Filter": "edge=1,filter_band0=12",
>          "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_1200mhz_cycles %",
>          "PerPkg": "1",
> @@ -243,7 +243,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xc",
>          "EventName": "UNC_P_FREQ_GE_2000MHZ_TRANSITIONS",
> -        "Filter": "edge=1,filter_band1=2000",
> +        "Filter": "edge=1,filter_band1=20",
>          "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_2000mhz_cycles %",
>          "PerPkg": "1",
> @@ -254,7 +254,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xd",
>          "EventName": "UNC_P_FREQ_GE_3000MHZ_TRANSITIONS",
> -        "Filter": "edge=1,filter_band2=4000",
> +        "Filter": "edge=1,filter_band2=30",
>          "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_3000mhz_cycles %",
>          "PerPkg": "1",
> @@ -265,7 +265,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xe",
>          "EventName": "UNC_P_FREQ_GE_4000MHZ_TRANSITIONS",
> -        "Filter": "edge=1,filter_band3=4000",
> +        "Filter": "edge=1,filter_band3=40",
>          "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_4000mhz_cycles %",
>          "PerPkg": "1",
> diff --git a/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json b/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json
> index 16034bfd06dd..8755693d86c6 100644
> --- a/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json
> +++ b/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json
> @@ -187,7 +187,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xb",
>          "EventName": "UNC_P_FREQ_GE_1200MHZ_CYCLES",
> -        "Filter": "filter_band0=1200",
> +        "Filter": "filter_band0=12",
>          "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_1200mhz_cycles %",
>          "PerPkg": "1",
> @@ -198,7 +198,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xc",
>          "EventName": "UNC_P_FREQ_GE_2000MHZ_CYCLES",
> -        "Filter": "filter_band1=2000",
> +        "Filter": "filter_band1=20",
>          "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_2000mhz_cycles %",
>          "PerPkg": "1",
> @@ -209,7 +209,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xd",
>          "EventName": "UNC_P_FREQ_GE_3000MHZ_CYCLES",
> -        "Filter": "filter_band2=3000",
> +        "Filter": "filter_band2=30",
>          "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_3000mhz_cycles %",
>          "PerPkg": "1",
> @@ -220,7 +220,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xe",
>          "EventName": "UNC_P_FREQ_GE_4000MHZ_CYCLES",
> -        "Filter": "filter_band3=4000",
> +        "Filter": "filter_band3=40",
>          "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_4000mhz_cycles %",
>          "PerPkg": "1",
> @@ -231,7 +231,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xb",
>          "EventName": "UNC_P_FREQ_GE_1200MHZ_TRANSITIONS",
> -        "Filter": "edge=1,filter_band0=1200",
> +        "Filter": "edge=1,filter_band0=12",
>          "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_1200mhz_cycles %",
>          "PerPkg": "1",
> @@ -242,7 +242,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xc",
>          "EventName": "UNC_P_FREQ_GE_2000MHZ_TRANSITIONS",
> -        "Filter": "edge=1,filter_band1=2000",
> +        "Filter": "edge=1,filter_band1=20",
>          "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_2000mhz_cycles %",
>          "PerPkg": "1",
> @@ -253,7 +253,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xd",
>          "EventName": "UNC_P_FREQ_GE_3000MHZ_TRANSITIONS",
> -        "Filter": "edge=1,filter_band2=4000",
> +        "Filter": "edge=1,filter_band2=30",
>          "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_3000mhz_cycles %",
>          "PerPkg": "1",
> @@ -264,7 +264,7 @@
>          "Counter": "0,1,2,3",
>          "EventCode": "0xe",
>          "EventName": "UNC_P_FREQ_GE_4000MHZ_TRANSITIONS",
> -        "Filter": "edge=1,filter_band3=4000",
> +        "Filter": "edge=1,filter_band3=40",
>          "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
>          "MetricName": "freq_ge_4000mhz_cycles %",
>          "PerPkg": "1",
> -- 
> 2.17.1

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

* [tip:perf/urgent] Revert "perf tools: Fix PMU term format max value calculation"
  2018-10-03  7:20 [PATCH] Revert "perf tools: Fix PMU term format max value calculation" Jiri Olsa
  2018-10-03 14:34 ` [RFC] perf tools: Wrong filter_band* values in json calculation" Jiri Olsa
  2018-10-09  9:59 ` [PATCH] Revert "perf tools: Fix PMU term format max value calculation" Jiri Olsa
@ 2018-10-18  6:17 ` tip-bot for Jiri Olsa
  2 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Jiri Olsa @ 2018-10-18  6:17 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, ak, mpetlan, hpa, kan.liang, mingo, tglx, jolsa,
	namhyung, peterz, acme, alexander.shishkin

Commit-ID:  1b9caa10b31dda0866f4028e4bfb923fb6e4072f
Gitweb:     https://git.kernel.org/tip/1b9caa10b31dda0866f4028e4bfb923fb6e4072f
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Wed, 3 Oct 2018 09:20:46 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 9 Oct 2018 10:48:55 -0300

Revert "perf tools: Fix PMU term format max value calculation"

This reverts commit ac0e2cd555373ae6f8f3a3ad3fbbf5b6d1e7aaaa.

Michael reported an issue with oversized terms values assignment
and I noticed there was actually a misunderstanding of the max
value check in the past.

The above commit's changelog says:

  If bit 21 is set, there is parsing issues as below.

    $ perf stat -a -e uncore_qpi_0/event=0x200002,umask=0x8/
    event syntax error: '..pi_0/event=0x200002,umask=0x8/'
                                      \___ value too big for format, maximum is 511

But there's no issue there, because the event value is distributed
along the value defined by the format. Even if the format defines
separated bit, the value is treated as a continual number, which
should follow the format definition.

In above case it's 9-bit value with last bit separated:
  $ cat uncore_qpi_0/format/event
  config:0-7,21

Hence the value 0x200002 is correctly reported as format violation,
because it exceeds 9 bits. It should have been 0x102 instead, which
sets the 9th bit - the bit 21 of the format.

  $ perf stat -vv -a -e uncore_qpi_0/event=0x102,umask=0x8/
  Using CPUID GenuineIntel-6-2D
  ...
  ------------------------------------------------------------
  perf_event_attr:
    type                             10
    size                             112
    config                           0x200802
    sample_type                      IDENTIFIER
  ...

Reported-by: Michael Petlan <mpetlan@redhat.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Fixes: ac0e2cd55537 ("perf tools: Fix PMU term format max value calculation")
Link: http://lkml.kernel.org/r/20181003072046.29276-1-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/pmu.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index afd68524ffa9..7799788f662f 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -930,13 +930,14 @@ static void pmu_format_value(unsigned long *format, __u64 value, __u64 *v,
 
 static __u64 pmu_format_max_value(const unsigned long *format)
 {
-	__u64 w = 0;
-	int fbit;
-
-	for_each_set_bit(fbit, format, PERF_PMU_FORMAT_BITS)
-		w |= (1ULL << fbit);
+	int w;
 
-	return w;
+	w = bitmap_weight(format, PERF_PMU_FORMAT_BITS);
+	if (!w)
+		return 0;
+	if (w < 64)
+		return (1ULL << w) - 1;
+	return -1;
 }
 
 /*

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

* [tip:perf/urgent] perf vendor events intel: Fix wrong filter_band* values for uncore events
  2018-10-10  8:03         ` [PATCH] perf tools: Fix wrong filter_band* values for uncore events Jiri Olsa
  2018-10-10 12:58           ` Arnaldo Carvalho de Melo
@ 2018-10-18  6:17           ` tip-bot for Jiri Olsa
  1 sibling, 0 replies; 11+ messages in thread
From: tip-bot for Jiri Olsa @ 2018-10-18  6:17 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mpetlan, jolsa, tglx, linux-kernel, acme, kan.liang, peterz,
	jolsa, mingo, ak, namhyung, alexander.shishkin, hpa

Commit-ID:  94aafb74cee0002e2f2eb6dc5376f54d5951ab4d
Gitweb:     https://git.kernel.org/tip/94aafb74cee0002e2f2eb6dc5376f54d5951ab4d
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Wed, 10 Oct 2018 10:03:39 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 11 Oct 2018 11:13:23 -0300

perf vendor events intel: Fix wrong filter_band* values for uncore events

Michael reported that he could not stat following event:

  $ perf stat -e unc_p_freq_ge_1200mhz_cycles -a -- ls
  event syntax error: '..e_1200mhz_cycles'
                                    \___ value too big for format, maximum is 255
  Run 'perf list' for a list of valid events

The event is unwrapped into:

  uncore_pcu/event=0xb,filter_band0=1200/

where filter_band0 format says it's one byte only:

  # cat uncore_pcu/format/filter_band0
  config1:0-7

while JSON files specifies bigger number:

  "Filter": "filter_band0=1200",

all the filter_band* formats show 1 byte width:

  # cat uncore_pcu/format/filter_band1
  config1:8-15
  # cat uncore_pcu/format/filter_band2
  config1:16-23
  # cat uncore_pcu/format/filter_band3
  config1:24-31

The reason of the issue is that filter_band* values are supposed to be
in 100Mhz units.. it's stated in the JSON help for the events, like:

  filter_band3=XXX, with XXX in 100Mhz units

This patch divides the filter_band* values by 100, plus there's couple
of changes that actually change the number completely, like:

  -        "Filter": "edge=1,filter_band2=4000",
  +        "Filter": "edge=1,filter_band2=30",

Reported-by: Michael Petlan <mpetlan@redhat.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Andi Kleen <ak@linux.intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20181010080339.GB15790@krava
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json | 16 ++++++++--------
 .../perf/pmu-events/arch/x86/jaketown/uncore-power.json  | 16 ++++++++--------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json b/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json
index d40498f2cb1e..635c09fda1d9 100644
--- a/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json
+++ b/tools/perf/pmu-events/arch/x86/ivytown/uncore-power.json
@@ -188,7 +188,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xb",
         "EventName": "UNC_P_FREQ_GE_1200MHZ_CYCLES",
-        "Filter": "filter_band0=1200",
+        "Filter": "filter_band0=12",
         "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_1200mhz_cycles %",
         "PerPkg": "1",
@@ -199,7 +199,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xc",
         "EventName": "UNC_P_FREQ_GE_2000MHZ_CYCLES",
-        "Filter": "filter_band1=2000",
+        "Filter": "filter_band1=20",
         "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_2000mhz_cycles %",
         "PerPkg": "1",
@@ -210,7 +210,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xd",
         "EventName": "UNC_P_FREQ_GE_3000MHZ_CYCLES",
-        "Filter": "filter_band2=3000",
+        "Filter": "filter_band2=30",
         "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_3000mhz_cycles %",
         "PerPkg": "1",
@@ -221,7 +221,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xe",
         "EventName": "UNC_P_FREQ_GE_4000MHZ_CYCLES",
-        "Filter": "filter_band3=4000",
+        "Filter": "filter_band3=40",
         "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_4000mhz_cycles %",
         "PerPkg": "1",
@@ -232,7 +232,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xb",
         "EventName": "UNC_P_FREQ_GE_1200MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band0=1200",
+        "Filter": "edge=1,filter_band0=12",
         "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_1200mhz_cycles %",
         "PerPkg": "1",
@@ -243,7 +243,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xc",
         "EventName": "UNC_P_FREQ_GE_2000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band1=2000",
+        "Filter": "edge=1,filter_band1=20",
         "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_2000mhz_cycles %",
         "PerPkg": "1",
@@ -254,7 +254,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xd",
         "EventName": "UNC_P_FREQ_GE_3000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band2=4000",
+        "Filter": "edge=1,filter_band2=30",
         "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_3000mhz_cycles %",
         "PerPkg": "1",
@@ -265,7 +265,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xe",
         "EventName": "UNC_P_FREQ_GE_4000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band3=4000",
+        "Filter": "edge=1,filter_band3=40",
         "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_4000mhz_cycles %",
         "PerPkg": "1",
diff --git a/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json b/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json
index 16034bfd06dd..8755693d86c6 100644
--- a/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json
+++ b/tools/perf/pmu-events/arch/x86/jaketown/uncore-power.json
@@ -187,7 +187,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xb",
         "EventName": "UNC_P_FREQ_GE_1200MHZ_CYCLES",
-        "Filter": "filter_band0=1200",
+        "Filter": "filter_band0=12",
         "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_1200mhz_cycles %",
         "PerPkg": "1",
@@ -198,7 +198,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xc",
         "EventName": "UNC_P_FREQ_GE_2000MHZ_CYCLES",
-        "Filter": "filter_band1=2000",
+        "Filter": "filter_band1=20",
         "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_2000mhz_cycles %",
         "PerPkg": "1",
@@ -209,7 +209,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xd",
         "EventName": "UNC_P_FREQ_GE_3000MHZ_CYCLES",
-        "Filter": "filter_band2=3000",
+        "Filter": "filter_band2=30",
         "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_3000mhz_cycles %",
         "PerPkg": "1",
@@ -220,7 +220,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xe",
         "EventName": "UNC_P_FREQ_GE_4000MHZ_CYCLES",
-        "Filter": "filter_band3=4000",
+        "Filter": "filter_band3=40",
         "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_4000mhz_cycles %",
         "PerPkg": "1",
@@ -231,7 +231,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xb",
         "EventName": "UNC_P_FREQ_GE_1200MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band0=1200",
+        "Filter": "edge=1,filter_band0=12",
         "MetricExpr": "(UNC_P_FREQ_GE_1200MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_1200mhz_cycles %",
         "PerPkg": "1",
@@ -242,7 +242,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xc",
         "EventName": "UNC_P_FREQ_GE_2000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band1=2000",
+        "Filter": "edge=1,filter_band1=20",
         "MetricExpr": "(UNC_P_FREQ_GE_2000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_2000mhz_cycles %",
         "PerPkg": "1",
@@ -253,7 +253,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xd",
         "EventName": "UNC_P_FREQ_GE_3000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band2=4000",
+        "Filter": "edge=1,filter_band2=30",
         "MetricExpr": "(UNC_P_FREQ_GE_3000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_3000mhz_cycles %",
         "PerPkg": "1",
@@ -264,7 +264,7 @@
         "Counter": "0,1,2,3",
         "EventCode": "0xe",
         "EventName": "UNC_P_FREQ_GE_4000MHZ_TRANSITIONS",
-        "Filter": "edge=1,filter_band3=4000",
+        "Filter": "edge=1,filter_band3=40",
         "MetricExpr": "(UNC_P_FREQ_GE_4000MHZ_CYCLES / UNC_P_CLOCKTICKS) * 100.",
         "MetricName": "freq_ge_4000mhz_cycles %",
         "PerPkg": "1",

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

end of thread, other threads:[~2018-10-18  6:17 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-03  7:20 [PATCH] Revert "perf tools: Fix PMU term format max value calculation" Jiri Olsa
2018-10-03 14:34 ` [RFC] perf tools: Wrong filter_band* values in json calculation" Jiri Olsa
2018-10-03 14:45   ` Andi Kleen
2018-10-09 10:01     ` Jiri Olsa
2018-10-09 21:18       ` Andi Kleen
2018-10-10  8:03         ` [PATCH] perf tools: Fix wrong filter_band* values for uncore events Jiri Olsa
2018-10-10 12:58           ` Arnaldo Carvalho de Melo
2018-10-18  6:17           ` [tip:perf/urgent] perf vendor events intel: " tip-bot for Jiri Olsa
2018-10-09  9:59 ` [PATCH] Revert "perf tools: Fix PMU term format max value calculation" Jiri Olsa
2018-10-09 13:52   ` Arnaldo Carvalho de Melo
2018-10-18  6:17 ` [tip:perf/urgent] " 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).