Re: [PATCH v1] perf tests: Restore -p flag to lock contention test

[Namhyung Kim <namhyung@kernel.org>]

On Wed, Jul 08, 2026 at 05:59:47PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Adding the `-p` (pipe) flag to the global workload unconditionally disables the stack filter tests.
> --
> 
> --- Patch [1]: [PATCH v1] perf tests: Restore -p flag to lock contention test ---
> commit b4bb7054950dea7123c601308622ddcd596c94cf
> Author: Ian Rogers <irogers@google.com>
> 
> perf tests: Restore -p flag to lock contention test
> 
> This commit restores the -p flag to the perf bench sched messaging workload
> used by the lock contention test. By defaulting to pipes instead of
> socketpairs, it ensures architectures like s390 generate sufficient lock
> events for the BPF trace filtering logic to pass.
> 
> > diff --git a/tools/perf/tests/shell/lock_contention.sh b/tools/perf/tests/shell/lock_contention.sh
> > index ba598370be738..5df9a0cd48a34 100755
> > --- a/tools/perf/tests/shell/lock_contention.sh
> > +++ b/tools/perf/tests/shell/lock_contention.sh
> > @@ -11,7 +11,8 @@ errout=$(mktemp /tmp/__perf_test.errout.XXXXX)
> >  
> >  # Workload to generate lock contention.
> >  # Using 1 group (-g 1) keeps runtime low while generating sufficient lock events.
> > -msg_workload="perf bench sched messaging -g 1"
> > +# We include -p (pipes) because socketpairs don't generate enough lock events on s390.
> > +msg_workload="perf bench sched messaging -g 1 -p"
> 
> [Severity: Medium]
> Will adding the -p flag cause test_stack_filter() and 
> test_aggr_task_stack_filter() to unconditionally skip?
> 
> These tests explicitly verify filtering behavior by searching for unix_stream 
> locks in tools/perf/tests/shell/lock_contention.sh:
> 
> test_stack_filter()
> {
> 	echo "Testing perf lock contention --callstack-filter (w/ unix_stream)"
> 	perf lock contention -i ${perfdata} -v -q 2> ${result}
> 	if [ "$(grep -c unix_stream "${result}")" = "0" ]; then
> 		echo "[Skip] Could not find 'unix_stream'"
> 		return
> 	fi
> 
> Since pipes use pipe locks instead of AF_UNIX stream socket locks 
> (unix_stream), the shared perfdata profile will no longer contain these locks. 
> 
> Does this mean we lose test suite coverage for the --callstack-filter option 
> across all architectures because the grep will always return 0?

I think it's a valid concern but it didn't come from this change.
Maybe we need to check different symbols like "pipe_".

Thanks,
Namhyung