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

[John Garry <john.g.garry@oracle.com>]

On 10/08/2023 20:27, Ian Rogers wrote:
> On Wed, Aug 9, 2023 at 10:11 PM Ilkka Koskinen
> <ilkka@os.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@oracle.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@os.amperecomputing.com>
> 
> Reviewed-by: Ian Rogers <irogers@google.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
>>>
>>>