Re: [PATCH v2 3/3] perf jevents: Check the JSON keys that jevents.py never reads

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] perf jevents: module-scope variable e in __main__ breaks
  pylint W0621
- [Low] perf jevents: empty tasks check in main() skips arch std errors
  and arch=none
--

commit 75a86d8bd9af5645c902739baea7b69e539c8eea
Author: TAKANO Mitsuhiro <takano32@gmail.com>

perf jevents: Check the JSON keys that jevents.py never reads

This commit adds a --check mode to jevents.py, along with a make target and
shell test, to detect unread JSON keys that look like misspellings of known
field names and metrics missing a MetricExpr.

> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> index 844fa82887bbf..d31f8e2fd6d3f 100755
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py

[ ... ]

> @@ -1497,14 +1696,104 @@ model must include the implementor such as "arm/cortex-a34".''',
> +  if not archs and _args.arch != 'none':
> +    raise IOError(f'Missing architecture directory \'{_args.arch}\'')
> +
> +  archs.sort()
> +  import concurrent.futures
> +  tasks = []
> +  def collect_json(parents: Sequence[str], item: os.DirEntry) -> None:
> +    if len(parents) == 0:
> +      return
> +    if item.is_file() and item.name.endswith('.json') and not item.name.endswith('metricgroups.json'):
> +      tasks.append((item.path, get_topic(item.name)))
> +
> +  failed: List[Tuple[str, Exception]] = []
> +  try:
> +    for arch in archs:
> +      arch_path = f'{_args.starting_dir}/{arch}'
> +      preprocess_arch_std_files(arch_path, failed if _args.check else None)
> +      ftw(arch_path, [], collect_json)
> +
> +    if not tasks:
> +      print(f'jevents: error: no event table under \'{_args.starting_dir}\' '
> +            f'matched arch \'{_args.arch}\'', file=sys.stderr)
> +      sys.exit(2 if _args.check else 1)

[Severity: Low]
Does exiting when tasks is empty skip reporting errors and mistyped keys
collected by preprocess_arch_std_files() in --check mode?

collect_json() only adds JSON files in subdirectories (len(parents) > 0) to
tasks, whereas preprocess_arch_std_files() reads top-level JSON files into
_json_cache and records parse errors in failed.  If an architecture directory
contains only top-level standard event files, main() exits here before
printing failed or running _report_mistyped_keys() and
_report_metrics_without_expr().  Also, in build mode with _args.arch ==
'none' on a directory without common or test subdirectories, this exits with
status 1 instead of generating the empty C tables that the _args.arch !=
'none' guard above allows.

[ ... ]

> @@ -1607,4 +1867,16 @@ struct pmu_table_entry {
>      _args.output_string_file.close()
>  
>  if __name__ == '__main__':
> -  main()
> +  try:
> +    main()
> +  except SystemExit:
> +    raise
> +  except KeyboardInterrupt:
> +    raise
> +  except BaseException as e:
> +    # A build should see the traceback; a check gives its caller a status,
> +    # and 2 is the one that says the tool could not do its job.
> +    if _args is not None and getattr(_args, 'check', False):
> +      print(f'jevents: error: {e}', file=sys.stderr)
> +      sys.exit(2)
> +    raise

[Severity: Medium]
Does binding e in the module-level __main__ block break the default perf
build when pylint is enabled?

Because if __name__ == '__main__': runs at module scope, binding e here
causes pylint to report W0621 (redefined-outer-name) on every function and
lambda in jevents.py that uses a local variable or parameter named e, such
as add_events_table_entries(), failing the pylint-pmu-events build target in
tools/perf/Makefile.perf.

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