Re: [PATCH 4/4] perf tests c2c: Add function view stdio coverage

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Fri, Aug 21, 2026 at 05:58:03AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The newly added `test_c2c_function_report` test fails spuriously because it incorrectly assumes `perfdata` is valid when `err` is 0. If the workload in `test_c2c_record_report` fails, it returns early but leaves `err=0`. This causes `test_c2c_function_report` to process an invalid or missing `perfdata` file, failing its diagnostic checks and setting `err=1` (test failure).
> --

Can you please take a look at this one? Namhyung Acked the series and
the other patches are clean so after this gets addressed I'll merge it.

Thanks,

- Arnaldo
 
> --- Patch [4]: [PATCH 4/4] perf tests c2c: Add function view stdio coverage ---
> commit 32c977aa30c2c6186f39077056298ef3b53307dd
> Author: Jiebin Sun <jiebin.sun@intel.com>
> 
> perf tests c2c: Add function view stdio coverage
> 
> Exercise the function view without driving a terminal now that it has
> a stdio path. Keep the existing datasym record/report coverage and make a
> separate recording of the contended locks used by the futex hash benchmark.
> 
> > diff --git a/tools/perf/tests/shell/c2c.sh b/tools/perf/tests/shell/c2c.sh
> > index f5f223cbf9cc7..cf76359d19ccd 100755
> > --- a/tools/perf/tests/shell/c2c.sh
> > +++ b/tools/perf/tests/shell/c2c.sh
> [ ... ]
> > @@ -58,6 +63,114 @@ test_c2c_record_report() {
> >  	echo "c2c record and report test [Success]"
> >  }
> >  
> > +test_c2c_function_report() {
> > +	echo "c2c function stdio report test"
> > +
> > +	if perf c2c report -i "${perfdata}" --function -c pid > "${perfout}" 2>&1 ; then
> > +		echo "c2c function stdio report test [Failed: report accepted missing iaddr]"
> > +		err=1
> > +		return
> > +	fi
> [ ... ]
> >  test_c2c_record_report
> > +if [ "${err}" -eq 0 ]; then
> > +	test_c2c_function_report
> > +fi
> >  cleanup
> >  exit $err
> 
> [Severity: Medium]
> Will this lead to a spurious test failure if the record workload fails?
> 
> In test_c2c_record_report(), if the 'perf test -w datasym 1' workload fails,
> it returns early without modifying err, leaving err=0:
> 
> 	if ! perf c2c record -o "${perfdata}" -- perf test -w datasym 1 > /dev/null 2>&1 ; then
> 		echo "c2c record and report test [Skipped: perf c2c record failed during workload]"
> 		return
> 	fi
> 
> Because err remains 0, test_c2c_function_report() will conditionally execute
> and attempt to use the missing or invalid perfdata file. This will cause
> test_c2c_function_report() to fail its diagnostic checks and set err=1.
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787283281.git.jiebin.sun@intel.com?part=4