Re: [PATCH v10 1/1] perf jevents: Check the keys of the JSON tables

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] perf test: jevents_check.sh never checks in-tree PMU JSON
  tables
--

commit 1b41788b6e87acfa8066606100a1198d73e6bf8c
Author: TAKANO Mitsuhiro <takano32@gmail.com>

perf jevents: Check the keys of the JSON tables

This commit adds a --check option to jevents.py to report unknown keys in
PMU event JSON tables and adds a shell test for it under perf test.

> diff --git a/tools/perf/tests/shell/jevents_check.sh b/tools/perf/tests/shell/jevents_check.sh
> new file mode 100755
> index 0000000000000..b596f8bc509d9
> --- /dev/null
> +++ b/tools/perf/tests/shell/jevents_check.sh
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +# jevents_check: an unknown key in a table is reported (no_mlock)
> +#
> +# A key that is spelled differently is dropped without a word, so the keys of
> +# the tables are checked against the list jevents.py reads.
> +
> +jevents=$(dirname "$0")/../../pmu-events/jevents.py
> +python=${PYTHON:-python3}
> +
> +# An installed perf has the test but not the generator, and a python that
> +# cannot run the generator is not a failure of the check.
> +[ -r "${jevents}" ] || exit 2
> +command -v "${python}" > /dev/null || exit 2
> +
> +tmp=$(mktemp -d)
> +trap 'rm -rf "${tmp}"' EXIT
> +tables="${tmp}/arch/x86/model"
> +mkdir -p "${tables}"
> +
> +# Known keys pass.
> +printf '[{"EventName": "a", "EventCode": "0x1"}]\n' > "${tables}/events.json"
> +"${python}" "${jevents}" --check "${tmp}/arch" > /dev/null || exit 1
> +
> +# A key the generator does not read is reported.
> +printf '[{"EventName": "a", "NotAKey": "x"}]\n' > "${tables}/events.json"
> +"${python}" "${jevents}" --check "${tmp}/arch" > /dev/null && exit 1

[Severity: Medium]
Does jevents_check.sh ever run jevents.py --check on the in-tree tables under
tools/perf/pmu-events/arch?  The test only runs --check against synthetic
one-line JSON files in ${tmp}/arch, and nothing in the build invokes
jevents.py --check either, so perf test passes without validating the
in-tree JSON tables or checking that _TABLE_KEYS stays in sync with them.

Also, the NotAKey test treats any non-zero exit status as success rather
than checking specifically for exit status 1, so an unexpected script or
argument error (exit status 2) would also pass that check.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113032927.634183-1-takano32@gmail.com?part=1