Date: Tue, 5 Apr 2016 02:16:11 +0200
From: Jiri Olsa <jolsa@redhat.com>
To: kan.liang@intel.com
Cc: acme@kernel.org, ak@linux.intel.com, alexander.shishkin@linux.intel.com,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] perf tools: Fix format value calculation
Message-ID: <20160405001611.GA9484@krava.local>
References: <1459775574-7595-1-git-send-email-kan.liang@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1459775574-7595-1-git-send-email-kan.liang@intel.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1674
Lines: 57

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
>