From: TAKANO Mitsuhiro <takano32@gmail.com>
To: linux-perf-users@vger.kernel.org,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Namhyung Kim <namhyung@kernel.org>
Cc: Ian Rogers <irogers@google.com>,
linux-kernel@vger.kernel.org,
TAKANO Mitsuhiro <takano32@gmail.com>,
James Clark <james.clark@linaro.org>,
John Garry <john.garry@linux.dev>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Mark Rutland <mark.rutland@arm.com>
Subject: [PATCH v2 1/3] perf jevents: Name the metric when a MetricExpr is missing
Date: Wed, 11 Nov 2026 02:34:46 +0000 [thread overview]
Message-ID: <20261111023448.1970467-2-takano32@gmail.com> (raw)
In-Reply-To: <20261111023448.1970467-1-takano32@gmail.com>
jevents.py reads each field it wants out of a JSON object by name, so a
MetricExpr key that is missing or spelled another way leaves the object
with a MetricName and no expression. Two different failures followed,
neither of which named the metric: the rewrite in _read_json_events_impl()
raised AttributeError on the None, and a metric that got that far reached
print_pending_metrics(), whose assert fired on it.
Raise in the reader instead, naming the metric, before either of them can
happen. The check is not inside the '-' test that decides whether a metric
takes part in the rewrite: a name with a dash in it can lose its
expression the same way.
Two smaller changes on the same path:
- An architecture standard table that does not parse now carries the
reason in its message ("Failure processing '<file>' in '<dir>':
<cause>"). The traceback used to end with the file name alone, and
only the chained exception said why.
- A table that does not parse is reported on stderr with a "jevents:
error:" prefix instead of on stdout, where it was mixed with the
generated output.
And the output file is opened after the tables have been read, and not by
argparse, so a build that fails while reading them leaves a file the
caller named as it was instead of truncating it to the header.
Signed-off-by: TAKANO Mitsuhiro <takano32@gmail.com>
---
tools/perf/pmu-events/jevents.py | 35 ++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index 080e04b51b8c..86060a228b75 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -460,11 +460,17 @@ def _read_json_events_impl(path: str, topic: str) -> Sequence[JsonEvent]:
with open(path, encoding='utf-8') as f:
events = json.load(f, object_hook=JsonEvent)
except BaseException:
- print(f"Exception processing {path}")
+ print(f'jevents: error: Exception processing {path}', file=sys.stderr)
raise
metrics: list[Tuple[str, str, metric.Expression]] = []
for event in events:
event.topic = topic
+ if event.metric_name and event.metric_expr is None:
+ # A MetricName without a MetricExpr leaves nothing to put in the
+ # generated tables, whether or not the name has a dash, so the build
+ # must not go on. Name the metric instead of letting the table printer
+ # assert on the None.
+ raise RuntimeError(f'Metric {event.metric_name!r} has no MetricExpr')
if event.metric_name and '-' not in event.metric_name:
metrics.append((event.pmu, event.metric_name, event.metric_expr))
updates = metric.RewriteMetricsInTermsOfOthers(metrics)
@@ -493,7 +499,7 @@ def preprocess_arch_std_files(archpath: str) -> None:
if event.metric_name:
_arch_std_events[event.metric_name.lower()] = event
except Exception as e:
- raise RuntimeError(f'Failure processing \'{item.name}\' in \'{archpath}\'') from e
+ raise RuntimeError(f'Failure processing \'{item.name}\' in \'{archpath}\': {e}') from e
def add_events_table_entries(item: os.DirEntry, topic: str) -> None:
@@ -1490,12 +1496,29 @@ such as "arm/cortex-a34".''',
type=dir_path,
help='Root of tree containing architecture directories containing json files'
)
- ap.add_argument(
- 'output_file', type=argparse.FileType('w', encoding='utf-8'), nargs='?', default=sys.stdout)
- ap.add_argument(
- 'output_string_file', type=argparse.FileType('w', encoding='utf-8'), nargs='?', default=None)
+ ap.add_argument('output_file', nargs='?', default=None,
+ help='file to write the tables to, default standard output')
+ ap.add_argument('output_string_file', nargs='?', default=None,
+ help='file to write big_c_string to when it is to be kept '
+ 'apart from the tables')
_args = ap.parse_args()
+ # Opened here, and not by argparse, so that a failure before the tables are
+ # written cannot empty a file the caller named.
+ if _args.output_file in (None, '-'):
+ _args.output_file = sys.stdout
+ else:
+ try:
+ _args.output_file = open(_args.output_file, 'w', encoding='utf-8')
+ except OSError as e:
+ ap.error(f'argument output_file: {e}')
+ if _args.output_string_file == '-':
+ _args.output_string_file = sys.stdout
+ elif _args.output_string_file is not None:
+ try:
+ _args.output_string_file = open(_args.output_string_file, 'w', encoding='utf-8')
+ except OSError as e:
+ ap.error(f'argument output_string_file: {e}')
_args.output_file.write(f"""/* SPDX-License-Identifier: GPL-2.0 */
/* THIS FILE WAS AUTOGENERATED BY `jevents.py arch={_args.arch} model={_args.model}` ! */
""")
--
2.53.0
next prev parent reply other threads:[~2026-11-11 2:34 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-10 22:22 [PATCH 0/3] perf jevents: Report the JSON keys that jevents.py never reads TAKANO Mitsuhiro
2026-11-10 22:22 ` [PATCH 1/3] perf jevents: Name the metric when a MetricExpr is missing TAKANO Mitsuhiro
2026-11-10 22:22 ` [PATCH 2/3] perf jevents: Make the model argument optional TAKANO Mitsuhiro
2026-11-10 22:22 ` [PATCH 3/3] perf jevents: Check the JSON keys that jevents.py never reads TAKANO Mitsuhiro
2026-11-11 2:34 ` [PATCH v2 0/3] perf jevents: Report " TAKANO Mitsuhiro
2026-11-11 2:34 ` TAKANO Mitsuhiro [this message]
2026-11-11 2:34 ` [PATCH v2 2/3] perf jevents: Make the model argument optional TAKANO Mitsuhiro
2026-11-11 2:34 ` [PATCH v2 3/3] perf jevents: Check the JSON keys that jevents.py never reads TAKANO Mitsuhiro
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261111023448.1970467-2-takano32@gmail.com \
--to=takano32@gmail.com \
--cc=acme@kernel.org \
--cc=irogers@google.com \
--cc=james.clark@linaro.org \
--cc=john.garry@linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®