[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <dd4bc3fe-5b8e-6e64-bcec-29263df43086@oracle.com>
Date: Mon, 21 Aug 2023 11:46:20 +0100
From: John Garry <john.g.garry@...cle.com>
To: Ian Rogers <irogers@...gle.com>,
Ilkka Koskinen <ilkka@...amperecomputing.com>
Cc: acme@...nel.org, 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
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.
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
>>>
>>>
Powered by blists - more mailing lists