Re: [PATCH v2 4/4] perf test: Update test for --for-each-cgroup option

[Namhyung Kim <namhyung@kernel.org>]

On Fri, Jul 10, 2026 at 11:52:00PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The `check_evlist_expand()` test fails to exercise the bug it intends to verify because it specifies `-e cpu-clock`.
> - [Medium] Bash pipeline semantics mask `perf stat` command failures, potentially causing false test passes.
> --
> 
> --- Patch [4]: [PATCH v2 4/4] perf test: Update test for --for-each-cgroup option ---
> commit b42b21fdf494fb61acde3f563ac29e9b49df9327
> Author: Namhyung Kim <namhyung@kernel.org>
> 
> perf test: Update test for --for-each-cgroup option
> 
> This commit updates the stat_bpf_counters_cgrp.sh test to simply check the
> number of output lines with and without the --for-each-cgroup option.
> 
> > diff --git a/tools/perf/tests/shell/stat_bpf_counters_cgrp.sh b/tools/perf/tests/shell/stat_bpf_counters_cgrp.sh
> > index ff2e06c408bc6..17b17b112169c 100755
> > --- a/tools/perf/tests/shell/stat_bpf_counters_cgrp.sh
> > +++ b/tools/perf/tests/shell/stat_bpf_counters_cgrp.sh
> [ ... ]
> > @@ -58,9 +58,24 @@ check_system_wide_counted()
> >  	fi
> >  }
> >  
> > +# Missing flags in evlist__clone() resulted in different output.
> > +# Just check the number of output lines for simple verification.
> > +check_evlist_expand()
> > +{
> > +	normal_output=$(perf stat -a -e cpu-clock -x, true 2>&1 | wc -l)
> > +	expand_output=$(perf stat -a -e cpu-clock -x, --bpf-counters --for-each-cgroup / true 2>&1 | wc -l)
> 
> [Severity: Medium]
> Will this actually test the bug with evlist__clone() missing flags?
> 
> By specifying -e cpu-clock, the loading of default events is disabled,
> meaning default_metricgroup and default_show_events are false. This prevents
> the duplication bug from occurring during evsel__clone(), rendering the test
> ineffective at catching the regression it was meant to verify.

Oops, that's right.  It should have this on top.

Thanks,
Namhyung


diff --git a/tools/perf/tests/shell/stat_bpf_counters_cgrp.sh b/tools/perf/tests/shell/stat_bpf_counters_cgrp.sh
index 17b17b112169c8ba..febe3af0644157f3 100755
--- a/tools/perf/tests/shell/stat_bpf_counters_cgrp.sh
+++ b/tools/perf/tests/shell/stat_bpf_counters_cgrp.sh
@@ -62,8 +62,8 @@ check_system_wide_counted()
 # Just check the number of output lines for simple verification.
 check_evlist_expand()
 {
-       normal_output=$(perf stat -a -e cpu-clock -x, true 2>&1 | wc -l)
-       expand_output=$(perf stat -a -e cpu-clock -x, --bpf-counters --for-each-cgroup / true 2>&1 | wc -l)
+       normal_output=$(perf stat -a -x, true 2>&1 | wc -l)
+       expand_output=$(perf stat -a -x, --bpf-counters --for-each-cgroup / true 2>&1 | wc -l)
        if [ "${normal_output}" != "${expand_output}" ]; then
                if [ "${verbose}" = "1" ]; then
                        echo "Normal output has ${normal_output} lines, but it now has ${expand_output}"