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

[Arnaldo Carvalho de Melo]

Em Mon, Aug 21, 2023 at 11:46:20AM +0100, John Garry escreveu:
> 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.

Done, thanks for the ping.

- Arnaldo
 
> 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
> > > > 
> > > > 
> 

-- 

- Arnaldo