Re: [RFC PATCH v1 09/14] perf stat: Extend CSV output linter to test advanced PMU and metric-only checks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] A redundant `if [ $skip_test -ne 1 ]` block was introduced instead of grouping the checks together.
- [Low] The commit message incorrectly references JSON layouts in a patch that strictly tests CSV output formatting.
--

commit 02bae29bcacfe6b2ac713a32abfb3c12b7c4b5c2
Author: Ian Rogers <irogers@google.com>

perf stat: Extend CSV output linter to test advanced PMU and metric-only checks

This commit extends the perf stat CSV output linter tests to run advanced
hardware PMU, topology-aware aggregation, and metric-only checks a second
time using the new CSV print flag.

> This guarantees that CSV outputs produced by the decoupled printing callbacks
> are verified and column-valid under advanced aggregation modes and metric-only
> JSON row-column layouts.

[Severity: Low]
This isn't a bug, but the commit message references testing JSON row-column
layouts. Since this patch strictly modifies stat+csv_output.sh to verify CSV
outputs, is the reference to JSON an accidental carryover from a different
testing patch?

> diff --git a/tools/perf/tests/shell/stat+csv_output.sh b/tools/perf/tests/shell/stat+csv_output.sh
> --- a/tools/perf/tests/shell/stat+csv_output.sh
> +++ b/tools/perf/tests/shell/stat+csv_output.sh
> @@ -103,6 +103,12 @@ then
>  	check_per_die "CSV (New API)" "$perf_cmd"
>  	check_per_socket "CSV (New API)" "$perf_cmd"
>  fi
> +if [ $skip_test -ne 1 ]
> +then
> +	check_per_cache_instance "CSV (New API)" "$perf_cmd"
> +	check_per_cluster "CSV (New API)" "$perf_cmd"
> +fi

[Severity: Low]
This isn't a bug, but could these new checks simply be appended inside the
existing conditional block immediately above this, rather than opening a
new identical conditional block?

> +check_metric_only "CSV (New API)" "$perf_cmd"
>  
>  cleanup
>  exit 0

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522223342.2393553-1-irogers@google.com?part=9