Re: [PATCH v6 3/7] perf jevents: Support more event fields
From: Ian Rogers <irogers@google.com>
Date: 2023-08-14 22:33:15
Also in:
linux-doc, linux-perf-users, lkml
On Mon, Aug 7, 2023 at 12:51 AM Jing Zhang [off-list ref] wrote:
The usual event descriptions are "event=xxx" or "config=xxx", while the event descriptions of CMN are "type=xxx, eventid=xxx" or more complex.
I found this difficult to read in relation to the code. Perhaps: The previous code assumes an event has either an "event=" or "config" field at the beginning. For CMN neither of these may be present, as an event is typically "type=xx,eventid=xxx". I think the use of the name "type" here is unfortunate. It conflicts with the PMU's type as defined in perf_event_attr. In general I think the jevents.py code needs improving, the event_fields dictionary is convoluted, we shouldn't be afraid to change the event json for example to get rid of things like ExtSel, we should really ensure the formats in the events are valid for the PMU they are for.
$cat /sys/bus/event_source/devices/arm_cmn_0/events/hnf_cache_fill type=0x5,eventid=0x3 When adding aliases for events described as "event=xxx" or "config=xxx", EventCode or ConfigCode can be used in the JSON files to describe the events. But "eventid=xxx, type=xxx" cannot be supported at present. If EventCode and ConfigCode is not added in the alias JSON file, the event description will add "event=0" by default. So, even if the event field is added to supplement "eventid=xxx" and "type=xxx", the final parsing result will be "event=0, eventid=xxx, type=xxx". Therefore, when EventCode and ConfigCode are missing in JSON, "event=0" is no longer added by default. EventIdCode and Type are added to the event field, and ConfigCode is moved into the event_field array which can also guarantee its original function.
A useful test can be to build with JEVENTS_ARCH=all and confirm the before and after change generated pmu-events.c is the same.
quoted hunk ↗ jump to hunk
Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com> --- tools/perf/pmu-events/jevents.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py index f57a8f2..9c0f63a 100755 --- a/tools/perf/pmu-events/jevents.py +++ b/tools/perf/pmu-events/jevents.py@@ -275,12 +275,6 @@ class JsonEvent: } return table[unit] if unit in table else f'uncore_{unit.lower()}' - eventcode = 0 - if 'EventCode' in jd: - eventcode = int(jd['EventCode'].split(',', 1)[0], 0) - if 'ExtSel' in jd: - eventcode |= int(jd['ExtSel']) << 8 - configcode = int(jd['ConfigCode'], 0) if 'ConfigCode' in jd else None self.name = jd['EventName'].lower() if 'EventName' in jd else None self.topic = '' self.compat = jd.get('Compat')@@ -317,7 +311,15 @@ class JsonEvent: if precise and self.desc and '(Precise Event)' not in self.desc: extra_desc += ' (Must be precise)' if precise == '2' else (' (Precise ' 'event)') - event = f'config={llx(configcode)}' if configcode is not None else f'event={llx(eventcode)}' + eventcode = None + if 'EventCode' in jd: + eventcode = int(jd['EventCode'].split(',', 1)[0], 0) + if 'ExtSel' in jd: + if eventcode is None: + eventcode = int(jd['ExtSel']) << 8 + else: + eventcode |= int(jd['ExtSel']) << 8 + event = f'event={llx(eventcode)}' if eventcode is not None else None event_fields = [ ('AnyThread', 'any='), ('PortMask', 'ch_mask='),@@ -327,10 +329,13 @@ class JsonEvent: ('Invert', 'inv='), ('SampleAfterValue', 'period='), ('UMask', 'umask='), + ('ConfigCode', 'config='),
This loses the int and potential base conversion of ConfigCode. Clearly the code was taking care to maintain this behavior so I suspect this change has broken something. JEVENTS_ARCH=all should reveal the answer.
+ ('Type', 'type='),
+ ('EventIdCode', 'eventid='),
]
for key, value in event_fields:
if key in jd and jd[key] != '0':
- event += ',' + value + jd[key]
+ event = event + ',' + value + jd[key] if event is not None else value + jd[key]
Perhaps initialize event above to the empty string then:
if key in jd and jd[key] != '0':
if event:
event += ','
event += value + jd[key]
Thanks,
Ian
if filter:
event += f',{filter}'
if msr:
--
1.8.3.1_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel