All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Andi Kleen <ak@linux.intel.com>, Kan Liang <kan.liang@intel.com>,
	lkml <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Andi Kleen <andi@firstfloor.org>,
	Michael Petlan <mpetlan@redhat.com>
Subject: Re: [PATCH] perf tools: Fix wrong filter_band* values for uncore events
Date: Wed, 10 Oct 2018 09:58:16 -0300	[thread overview]
Message-ID: <20181010125816.GJ10775@kernel.org> (raw)
In-Reply-To: <20181010080339.GB15790@krava>

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

  reply	other threads:[~2018-10-10 12:58 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181010125816.GJ10775@kernel.org \
    --to=acme@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=andi@firstfloor.org \
    --cc=jolsa@redhat.com \
    --cc=kan.liang@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mpetlan@redhat.com \
    --cc=namhyung@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.