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

[Liang, Kan]

> On Mon, Apr 04, 2016 at 06:12:54AM -0700, kan.liang@xxxxxxxxx wrote:
> > From: Kan Liang <kan.liang@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> > ---
> >  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
> >