Re: [PATCH] perf jevents: Raise exception for no definition of a arch std event

[Ian Rogers]

On Wed, Aug 9, 2023 at 10:11 PM Ilkka Koskinen
<ilkka@xxxxxxxxxxxxxxxxxxxxxx> wrote:
>
>
> Hi John,
>
> On Mon, 7 Aug 2023, John Garry wrote:
> > Recently Ilkka reported that the JSONs for the AmpereOne arm64-based
> > platform included a dud event which referenced a non-existent arch std
> > event [0].
>
> I wish I had found the bug in my patch a long time ago but, in fact, it
> was Dave Kleikamp who initially pointed it out to me and figured out the
> difference between jevents.c and jevents.py when porting the patch to 5.15
> kernel.
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2023-June/844874.html
>
> >
> > Previously in the times of jevents.c, we would raise an exception for this.
> >
> > This is still invalid, even though the current code just ignores such an
> > event.
> >
> > Re-introduce code to raise an exception for when no definition exists to
> > help catch as many invalid JSONs as possible.
> >
> > [0] https://lore.kernel.org/linux-perf-users/9e851e2a-26c7-ba78-cb20-be4337b2916a@xxxxxxxxxx/
> >
> > Signed-off-by: John Garry <john.g.garry@xxxxxxxxxx>
>
> Thanks for the patch! I quickly tested it and it worked as expected. Just
> in case this is needed:
>
> Tested-by: Ilkka Koskinen <ilkka@xxxxxxxxxxxxxxxxxxxxxx>

Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks,
Ian

> Cheers, Ilkka
>
> > ---
> > Please do not apply before [0], above.
> >
> > diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> > index 8cd561aa606a..98cccc3fcbbd 100755
> > --- a/tools/perf/pmu-events/jevents.py
> > +++ b/tools/perf/pmu-events/jevents.py
> > @@ -347,12 +347,15 @@ class JsonEvent:
> >       if self.desc and not self.desc.endswith('. '):
> >         self.desc += '. '
> >       self.desc = (self.desc if self.desc else '') + ('Unit: ' + self.pmu + ' ')
> > -    if arch_std and arch_std.lower() in _arch_std_events:
> > -      event = _arch_std_events[arch_std.lower()].event
> > -      # Copy from the architecture standard event to self for undefined fields.
> > -      for attr, value in _arch_std_events[arch_std.lower()].__dict__.items():
> > -        if hasattr(self, attr) and not getattr(self, attr):
> > -          setattr(self, attr, value)
> > +    if arch_std:
> > +      if arch_std.lower() in _arch_std_events:
> > +        event = _arch_std_events[arch_std.lower()].event
> > +        # Copy from the architecture standard event to self for undefined fields.
> > +        for attr, value in _arch_std_events[arch_std.lower()].__dict__.items():
> > +          if hasattr(self, attr) and not getattr(self, attr):
> > +            setattr(self, attr, value)
> > +      else:
> > +        raise argparse.ArgumentTypeError('Cannot find arch std event:', arch_std)
> >
> >     self.event = real_event(self.name, event)
> >
> > --
> > 2.35.3
> >
> >