Re: [PATCH 4/4] perf tests c2c: Add function view stdio coverage
| Newsgroups | org.kernel.vger.linux-perf-users |
|---|---|
| Message-ID | <[email protected]> |
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). -- --- Patch [4]: [PATCH 4/4] perf tests c2c: Add function view stdio coverage --- commit 32c977aa30c2c6186f39077056298ef3b53307dd Author: Jiebin Sun <[email protected]> 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/[email protected]?part=4