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 <a...@linux.intel.com> > Cc: Kan Liang <kan.li...@intel.com> > Reported-by: Michael Petlan <mpet...@redhat.com> > Link: http://lkml.kernel.org/n/tip-icxq7a1r66lusm3ahaime...@git.kernel.org > Signed-off-by: Jiri Olsa <jo...@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 >
