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

[John Garry]

On 10/08/2023 20:27, Ian Rogers wrote:
> 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.
> >
> > https://urldefense.com/v3/__http://lists.infradead.org/pipermail/linux-arm-kernel/2023-June/844874.html__;!!ACWV5N9M2RV99hQ!It9EhKK4s2uBUJyQvLg-ruUfENAA6Sw7TWVo_hF8XmFoQ6q565iYafTnN-yoBNh3EQ1IFa2tHknUjNHs5dI$
> >
> > > 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://urldefense.com/v3/__https://lore.kernel.org/linux-perf-users/9e851e2a-26c7-ba78-cb20-be4337b2916a@xxxxxxxxxx/__;!!ACWV5N9M2RV99hQ!It9EhKK4s2uBUJyQvLg-ruUfENAA6Sw7TWVo_hF8XmFoQ6q565iYafTnN-yoBNh3EQ1IFa2tHknU_t28TiE$
> > >
> > > 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>

Hi Arnaldo,

Can you consider applying this patch along with 
https://lore.kernel.org/linux-perf-users/CAP-5=fX2uE=B_Vb90nn5EV0mw+AJBpjDecP9w29OUn=j7HKPPg@xxxxxxxxxxxxxx/

I think that we should expect another version of that series with 
changes elsewhere.

Thanks,
John

> 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