Re: [PATCH] Revert "perf tools: Fix PMU term format max value calculation"

From: Arnaldo Carvalho de Melo
Date: Tue Oct 09 2018 - 09:52:31 EST


Em Tue, Oct 09, 2018 at 11:59:17AM +0200, Jiri Olsa escreveu:
> ping

oops, applied to perf/urgent, added a Fixes: line so that stable@ pick
this up.

- Arnaldo

> 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 <ak@xxxxxxxxxxxxxxx>
> > Cc: Kan Liang <kan.liang@xxxxxxxxx>
> > Reported-by: Michael Petlan <mpetlan@xxxxxxxxxx>
> > Link: http://lkml.kernel.org/n/tip-icxq7a1r66lusm3ahaimekis@xxxxxxxxxxxxxx
> > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> > ---
> > 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
> >