[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZONgPy7pK7Qc7Cc6@kernel.org>
Date: Mon, 21 Aug 2023 10:01:51 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: John Garry <john.g.garry@...cle.com>
Cc: Ian Rogers <irogers@...gle.com>,
Ilkka Koskinen <ilkka@...amperecomputing.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf jevents: Raise exception for no definition of a
arch std event
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@...amperecomputing.com> 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@oracle.com/__;!!ACWV5N9M2RV99hQ!It9EhKK4s2uBUJyQvLg-ruUfENAA6Sw7TWVo_hF8XmFoQ6q565iYafTnN-yoBNh3EQ1IFa2tHknU_t28TiE$
> > > >
> > > > Signed-off-by: John Garry <john.g.garry@...cle.com>
> > >
> > > Thanks for the patch! I quickly tested it and it worked as expected. Just
> > > in case this is needed:
> > >
> > > Tested-by: Ilkka Koskinen <ilkka@...amperecomputing.com>
> >
> > Reviewed-by: Ian Rogers <irogers@...gle.com>
>
> Hi Arnaldo,
>
> Can you consider applying this patch along with https://lore.kernel.org/linux-perf-users/CAP-5=fX2uE=B_Vb90nn5EV0mw+AJBpjDecP9w29OUn=j7HKPPg@mail.gmail.com/
>
> 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
Powered by blists - more mailing lists