mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: TAKANO Mitsuhiro <takano32@gmail.com>
To: James Clark <james.clark@linaro.org>
Cc: linux-perf-users@vger.kernel.org,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Ian Rogers <irogers@google.com>,
	John Garry <john.garry@linux.dev>,
	Ilkka Koskinen <ilkka@os.amperecomputing.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] perf vendor events arm64 AmpereOneX: Fix misspelled PublicDescription keys
Date: Tue, 10 Nov 2026 19:42:05 +0000	[thread overview]
Message-ID: <179433972506.1746408.6665119010100304383@gmail.com> (raw)
In-Reply-To: <bf6925f6-613f-4e20-8491-2a99183f34fb@linaro.org>

On 2026-11-09 10:24, James Clark wrote:

> Shouldn't jevents enforce some kind of schema? This seems like a bug in
> the generator that it's allowed. And we have all the json files in one
> repo, so we can fix everything when adding strict validation.

You are right, and there is a patch for it now:

https://lore.kernel.org/linux-perf-users/20261110193239.1741014-1-takano32@gmail.com/

It is not a schema, and I don't think it should be one yet.  It checks
nothing about types, required fields or value ranges.  It reports the keys
that jevents.py never asks for, when the spelling is a near miss for a
name it does ask for.  The failure here is that "PublicDescrition" was
never read, so the field was dead in 16 objects and nothing in the build or
in the tables said so.  What makes it cost something is the AMD tables:
"BriefDescript6ion" is the only description those three objects have, so
the generated table carries an empty text field for ls_inef_sw_pref.all on
Zen 4, Zen 5 and Zen 6.  In your table the output is unchanged, because the
value repeats BriefDescription and jevents.py drops the long description
when the two are equal - which is exactly why the typo could sit there
unnoticed.

I did not go the schema route because a table may legitimately carry a
field for another reader: the Intel tables carry Counter, CountersNumFixed,
CountersNumGeneric and Experimental, none of which jevents.py reads, and
Experimental is read by metric.py.  A schema, or an allow list inside
jevents.py, would mean asking every vendor that adds such a field to edit
perf's Python first.  So the set of names jevents.py reads is the only
list, and a key below the similarity cutoff is left alone.  It will not
catch everything -- a key that is a word of its own, or a misspelling that
lands on another existing key, gets through -- which is why I would rather
not present it as validation.

It reads the whole tree, not just the architecture being built: the typo is
cheapest to fix by whoever reads that table next, and that person may be
building something else -- this arm64 one shows up in a run over the whole
tree from an x86 box.  The exit status is 0 for a clean tree, 1 when
something was reported and 2 when a table could not be read.  make turns
both failures into its own status, so a script that has to tell them apart
should run pmu-events/jevents.py directly; for a person it is

  make -C tools/perf jevents-check

which reads the whole tree, and JEVENTS_CHECK_ARCH=x86 reads x86 plus the
common and test tables every build reads.  I kept it out of `make check`,
which is the sparse run over the C sources and needs a configured tree and a
C parser, and out of perf test as well: tests/pmu-events.c compares what it
generates from testarch/testcpu against a fixed list, and a key jevents.py
never read leaves no trace in the generated C, so a runtime test has
nothing to assert on.

It is not wired into the build: I had a version that ran it from
pmu-events/Build on every build and dropped it, because a report about a
table this build does not use would fail the build for everyone.  It costs
nothing when it does not run, since the tracking that makes a report
possible is switched off unless the check asked for it.

The two misspellings it finds were posted separately: the arm64 one is
this thread, the AMD one is

https://lore.kernel.org/linux-perf-users/20261108184600.3810059-1-takano32@gmail.com/

The generated tables are byte for byte the same as before on x86, arm64 and
the whole tree.

Written with Hermes Agent, an LLM assistant; the author reviewed it and ran
the checks it describes.


      reply	other threads:[~2026-11-10 19:42 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-08 18:38 TAKANO Mitsuhiro
2026-11-09 10:24 ` James Clark
2026-11-10 19:42   ` TAKANO Mitsuhiro [this message]

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=179433972506.1746408.6665119010100304383@gmail.com \
    --to=takano32@gmail.com \
    --cc=acme@kernel.org \
    --cc=ilkka@os.amperecomputing.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=john.garry@linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=namhyung@kernel.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®