Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1370857 > unrolled thread

[PATCH 1/1] perf tools: Fix format value calculation

Started bykan.liang@intel.com
First post2016-04-04 22:30 +0200
Last post2016-04-05 05:20 +0200
Articles 3 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 1/1] perf tools: Fix format value calculation kan.liang@intel.com - 2016-04-04 22:30 +0200
    Re: [PATCH 1/1] perf tools: Fix format value calculation Jiri Olsa <jolsa@redhat.com> - 2016-04-05 02:20 +0200
      RE: [PATCH 1/1] perf tools: Fix format value calculation "Liang, Kan" <kan.liang@intel.com> - 2016-04-05 05:20 +0200

#1370857 — [PATCH 1/1] perf tools: Fix format value calculation

Fromkan.liang@intel.com
Date2016-04-04 22:30 +0200
Subject[PATCH 1/1] perf tools: Fix format value calculation
Message-ID<rkjih-dT-3@gated-at.bofh.it>
From: Kan Liang <kan.liang@intel.com>

The calculation of format value also rely on the continuity of the
format. However, uncore event format is not continuous.
E.g. The bit 21 as qpi event is lost.

perf stat -a -e uncore_qpi_0/event=0x200038,config1=0x1C00,
config2=0x3FE00/ -vvv
------------------------------------------------------------
perf_event_attr:
  type                             10
  size                             112
  config                           0x38



This patch checks the bit according to the bit position.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/util/pmu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index bf34468..47c096c 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -586,14 +586,14 @@ __u64 perf_pmu__format_bits(struct list_head *formats, const char *name)
 static void pmu_format_value(unsigned long *format, __u64 value, __u64 *v,
 			     bool zero)
 {
-	unsigned long fbit, vbit;
+	unsigned long fbit;
 
-	for (fbit = 0, vbit = 0; fbit < PERF_PMU_FORMAT_BITS; fbit++) {
+	for (fbit = 0; fbit < PERF_PMU_FORMAT_BITS; fbit++) {
 
 		if (!test_bit(fbit, format))
 			continue;
 
-		if (value & (1llu << vbit++))
+		if (value & (1llu << fbit))
 			*v |= (1llu << fbit);
 		else if (zero)
 			*v &= ~(1llu << fbit);
-- 
2.5.5

[toc] | [next] | [standalone]


#1371062

FromJiri Olsa <jolsa@redhat.com>
Date2016-04-05 02:20 +0200
Message-ID<rkmSS-2Qr-1@gated-at.bofh.it>
In reply to#1370857
On Mon, Apr 04, 2016 at 06:12:54AM -0700, kan.liang@intel.com wrote:
> From: Kan Liang <kan.liang@intel.com>
> 
> The calculation of format value also rely on the continuity of the
> format. However, uncore event format is not continuous.
> E.g. The bit 21 as qpi event is lost.
> 
> perf stat -a -e uncore_qpi_0/event=0x200038,config1=0x1C00,
> config2=0x3FE00/ -vvv
> ------------------------------------------------------------
> perf_event_attr:
>   type                             10
>   size                             112
>   config                           0x38

could you please share the event's format?

would be great to have some simple automated test for this one..

thanks,
jirka

> 
> 
> 
> This patch checks the bit according to the bit position.
> 
> Signed-off-by: Kan Liang <kan.liang@intel.com>
> ---
>  tools/perf/util/pmu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index bf34468..47c096c 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -586,14 +586,14 @@ __u64 perf_pmu__format_bits(struct list_head *formats, const char *name)
>  static void pmu_format_value(unsigned long *format, __u64 value, __u64 *v,
>  			     bool zero)
>  {
> -	unsigned long fbit, vbit;
> +	unsigned long fbit;
>  
> -	for (fbit = 0, vbit = 0; fbit < PERF_PMU_FORMAT_BITS; fbit++) {
> +	for (fbit = 0; fbit < PERF_PMU_FORMAT_BITS; fbit++) {
>  
>  		if (!test_bit(fbit, format))
>  			continue;
>  
> -		if (value & (1llu << vbit++))
> +		if (value & (1llu << fbit))
>  			*v |= (1llu << fbit);
>  		else if (zero)
>  			*v &= ~(1llu << fbit);
> -- 
> 2.5.5
> 

[toc] | [prev] | [next] | [standalone]


#1371157

From"Liang, Kan" <kan.liang@intel.com>
Date2016-04-05 05:20 +0200
Message-ID<rkpH4-50J-9@gated-at.bofh.it>
In reply to#1371062
> On Mon, Apr 04, 2016 at 06:12:54AM -0700, kan.liang@intel.com wrote:
> > From: Kan Liang <kan.liang@intel.com>
> >
> > The calculation of format value also rely on the continuity of the
> > format. However, uncore event format is not continuous.
> > E.g. The bit 21 as qpi event is lost.
> >
> > perf stat -a -e uncore_qpi_0/event=0x200038,config1=0x1C00,
> > config2=0x3FE00/ -vvv
> > ------------------------------------------------------------
> > perf_event_attr:
> >   type                             10
> >   size                             112
> >   config                           0x38
> 
> could you please share the event's format?
>

cat /sys/devices/uncore_qpi_0/format/event
config:0-7,21
 
> would be great to have some simple automated test for this one..
>

It looks there is a test case in perf test. 
7: Test perf pmu format parsing
But it looks there are some issues for the test case.

The test format with config is 
	{ "krava01", "config:0-1,62-63\n", },
	{ "krava02", "config:10-17\n", },
	{ "krava03", "config:5\n", },
The test input is
	{
		.config    = (char *) "krava01",
		.val.num   = 15,
		.type_val  = PARSE_EVENTS__TERM_TYPE_NUM,
		.type_term = PARSE_EVENTS__TERM_TYPE_USER,
	},
	{
		.config    = (char *) "krava02",
		.val.num   = 170,
		.type_val  = PARSE_EVENTS__TERM_TYPE_NUM,
		.type_term = PARSE_EVENTS__TERM_TYPE_USER,
	},
	{
		.config    = (char *) "krava03",
		.val.num   = 1,
		.type_val  = PARSE_EVENTS__TERM_TYPE_NUM,
		.type_term = PARSE_EVENTS__TERM_TYPE_USER,
	},

The input value of "krava01" is 15 (0xf). The format of "krava01" is "config:0-1,62-63\n".
Apparently, the input has wrong format. But it looks the test case doesn't think so. 
Also, at the end of the test case, it expects attr.config == 0xc00000000002a823.
I think it doesn't make sense either. 

Any thoughts?

Thanks,
Kan

> thanks,
> jirka
> 
> >
> >
> >
> > This patch checks the bit according to the bit position.
> >
> > Signed-off-by: Kan Liang <kan.liang@intel.com>
> > ---
> >  tools/perf/util/pmu.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index
> > bf34468..47c096c 100644
> > --- a/tools/perf/util/pmu.c
> > +++ b/tools/perf/util/pmu.c
> > @@ -586,14 +586,14 @@ __u64 perf_pmu__format_bits(struct list_head
> > *formats, const char *name)  static void pmu_format_value(unsigned long
> *format, __u64 value, __u64 *v,
> >  			     bool zero)
> >  {
> > -	unsigned long fbit, vbit;
> > +	unsigned long fbit;
> >
> > -	for (fbit = 0, vbit = 0; fbit < PERF_PMU_FORMAT_BITS; fbit++) {
> > +	for (fbit = 0; fbit < PERF_PMU_FORMAT_BITS; fbit++) {
> >
> >  		if (!test_bit(fbit, format))
> >  			continue;
> >
> > -		if (value & (1llu << vbit++))
> > +		if (value & (1llu << fbit))
> >  			*v |= (1llu << fbit);
> >  		else if (zero)
> >  			*v &= ~(1llu << fbit);
> > --
> > 2.5.5
> >

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web