[PATCH 0/2] perf lock contention: Assorted fixes

[PATCH 0/2] perf lock contention: Assorted fixes

[Ravi Bangoria]

o Patch 1 resolves a segmentation fault in both "perf lock report"
  and "perf lock contention".
o Patch 2 repairs the perf-lock unit test.

Ravi Bangoria (2):
  perf lock: Fix segfault due to missing kernel map
  perf test: Fix lock contention test

 tools/perf/builtin-lock.c                 |  2 ++
 tools/perf/tests/shell/lock_contention.sh | 14 +++++++++-----
 2 files changed, 11 insertions(+), 5 deletions(-)

-- 
2.51.0

[PATCH 1/2] perf lock: Fix segfault due to missing kernel map

[Ravi Bangoria]

Kernel maps are encoded in PERF_RECORD_MMAP2 samples but "perf lock
report" and "perf lock contention" do not process MMAP2 samples.
Because of that, machine->vmlinux_map stays NULL and any later
access triggers a segmentation fault. Fix it by adding ->mmap2()
callbacks.

Fixes: 53b00ff358dc ("perf record: Make --buildid-mmap the default")
Reported-by: Tycho Andersen (AMD) <tycho@kernel.org>
Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
Tested-by: Tycho Andersen (AMD) <tycho@kernel.org>
---
 tools/perf/builtin-lock.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 078634461df2..e8962c985d34 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -1867,6 +1867,7 @@ static int __cmd_report(bool display_info)
 	eops.sample		 = process_sample_event;
 	eops.comm		 = perf_event__process_comm;
 	eops.mmap		 = perf_event__process_mmap;
+	eops.mmap2		 = perf_event__process_mmap2;
 	eops.namespaces		 = perf_event__process_namespaces;
 	eops.tracing_data	 = perf_event__process_tracing_data;
 	session = perf_session__new(&data, &eops);
@@ -2023,6 +2024,7 @@ static int __cmd_contention(int argc, const char **argv)
 	eops.sample		 = process_sample_event;
 	eops.comm		 = perf_event__process_comm;
 	eops.mmap		 = perf_event__process_mmap;
+	eops.mmap2		 = perf_event__process_mmap2;
 	eops.tracing_data	 = perf_event__process_tracing_data;
 
 	perf_env__init(&host_env);
-- 
2.51.0

[PATCH 2/2] perf test: Fix lock contention test

[Ravi Bangoria]

Couple of independent fixes:
1. Wire in SIGSEGV handler that terminates the test with a failure code.
2. Use "--lock-cgroup" instead of "-g"; "-g" was proposed but never
   merged. See commit 4d1792d0a256 ("perf lock contention: Add
   --lock-cgroup option").
3. Call cleanup() on every normal exit so trap_cleanup() doesn't mistake
   it for an unexpected signal and emit a false-negative "Unexpected
   signal in main" message.

Before patch:
  # ./perf test -vv "lock contention"
   85: kernel lock contention analysis test:
  --- start ---
  test child forked, pid 610711
  Testing perf lock record and perf lock contention
  Testing perf lock contention --use-bpf
  Testing perf lock record and perf lock contention at the same time
  Testing perf lock contention --threads
  Testing perf lock contention --lock-addr
  Testing perf lock contention --lock-cgroup
  Unexpected signal in test_aggr_cgroup
  ---- end(0) ----
   85: kernel lock contention analysis test                            : Ok

After patch:
  # ./perf test -vv "lock contention"
   85: kernel lock contention analysis test:
  --- start ---
  test child forked, pid 602637
  Testing perf lock record and perf lock contention
  Testing perf lock contention --use-bpf
  Testing perf lock record and perf lock contention at the same time
  Testing perf lock contention --threads
  Testing perf lock contention --lock-addr
  Testing perf lock contention --lock-cgroup
  Testing perf lock contention --type-filter (w/ spinlock)
  Testing perf lock contention --lock-filter (w/ tasklist_lock)
  Testing perf lock contention --callstack-filter (w/ unix_stream)
  [Skip] Could not find 'unix_stream'
  Testing perf lock contention --callstack-filter with task aggregation
  [Skip] Could not find 'unix_stream'
  Testing perf lock contention --cgroup-filter
  Testing perf lock contention CSV output
  ---- end(0) ----
   85: kernel lock contention analysis test                            : Ok

Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
---
 tools/perf/tests/shell/lock_contention.sh | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/perf/tests/shell/lock_contention.sh b/tools/perf/tests/shell/lock_contention.sh
index 7248a74ca2a3..6dd90519f45c 100755
--- a/tools/perf/tests/shell/lock_contention.sh
+++ b/tools/perf/tests/shell/lock_contention.sh
@@ -13,15 +13,18 @@ cleanup() {
 	rm -f ${perfdata}
 	rm -f ${result}
 	rm -f ${errout}
-	trap - EXIT TERM INT
+	trap - EXIT TERM INT ERR
 }
 
 trap_cleanup() {
+	if (( $? == 139 )); then #SIGSEGV
+		err=1
+	fi
 	echo "Unexpected signal in ${FUNCNAME[1]}"
 	cleanup
 	exit ${err}
 }
-trap trap_cleanup EXIT TERM INT
+trap trap_cleanup EXIT TERM INT ERR
 
 check() {
 	if [ "$(id -u)" != 0 ]; then
@@ -145,7 +148,7 @@ test_aggr_cgroup()
 	fi
 
 	# the perf lock contention output goes to the stderr
-	perf lock con -a -b -g -E 1 -q -- perf bench sched messaging -p > /dev/null 2> ${result}
+	perf lock con -a -b --lock-cgroup -E 1 -q -- perf bench sched messaging -p > /dev/null 2> ${result}
 	if [ "$(cat "${result}" | wc -l)" != "1" ]; then
 		echo "[Fail] BPF result count is not 1:" "$(cat "${result}" | wc -l)"
 		err=1
@@ -271,7 +274,7 @@ test_cgroup_filter()
 		return
 	fi
 
-	perf lock con -a -b -g -E 1 -F wait_total -q -- perf bench sched messaging -p > /dev/null 2> ${result}
+	perf lock con -a -b --lock-cgroup -E 1 -F wait_total -q -- perf bench sched messaging -p > /dev/null 2> ${result}
 	if [ "$(cat "${result}" | wc -l)" != "1" ]; then
 		echo "[Fail] BPF result should have a cgroup result:" "$(cat "${result}")"
 		err=1
@@ -279,7 +282,7 @@ test_cgroup_filter()
 	fi
 
 	cgroup=$(cat "${result}" | awk '{ print $3 }')
-	perf lock con -a -b -g -E 1 -G "${cgroup}" -q -- perf bench sched messaging -p > /dev/null 2> ${result}
+	perf lock con -a -b --lock-cgroup -E 1 -G "${cgroup}" -q -- perf bench sched messaging -p > /dev/null 2> ${result}
 	if [ "$(cat "${result}" | wc -l)" != "1" ]; then
 		echo "[Fail] BPF result should have a result with cgroup filter:" "$(cat "${cgroup}")"
 		err=1
@@ -338,4 +341,5 @@ test_aggr_task_stack_filter
 test_cgroup_filter
 test_csv_output
 
+cleanup
 exit ${err}
-- 
2.51.0

Re: [PATCH 0/2] perf lock contention: Assorted fixes

[Ian Rogers]

On Thu, Nov 13, 2025 at 8:03 AM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>
> o Patch 1 resolves a segmentation fault in both "perf lock report"
>   and "perf lock contention".
> o Patch 2 repairs the perf-lock unit test.

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks!
Ian

> Ravi Bangoria (2):
>   perf lock: Fix segfault due to missing kernel map
>   perf test: Fix lock contention test
>
>  tools/perf/builtin-lock.c                 |  2 ++
>  tools/perf/tests/shell/lock_contention.sh | 14 +++++++++-----
>  2 files changed, 11 insertions(+), 5 deletions(-)
>
> --
> 2.51.0
>

Re: [PATCH 0/2] perf lock contention: Assorted fixes

[Arnaldo Carvalho de Melo]

On Thu, Nov 13, 2025 at 08:57:40AM -0800, Ian Rogers wrote:
> On Thu, Nov 13, 2025 at 8:03 AM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
> >
> > o Patch 1 resolves a segmentation fault in both "perf lock report"
> >   and "perf lock contention".
> > o Patch 2 repairs the perf-lock unit test.
 
> Reviewed-by: Ian Rogers <irogers@google.com>

Thanks, tested, reproduced the segfault and the fix for it, applied to
perf-tools,

- Arnaldo

Re: [PATCH 0/2] perf lock contention: Assorted fixes

[Namhyung Kim]

Hello,

On Thu, Nov 13, 2025 at 04:01:22PM +0000, Ravi Bangoria wrote:
> o Patch 1 resolves a segmentation fault in both "perf lock report"
>   and "perf lock contention".
> o Patch 2 repairs the perf-lock unit test.
> 
> Ravi Bangoria (2):
>   perf lock: Fix segfault due to missing kernel map
>   perf test: Fix lock contention test

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks for the fix!
Namhyung

> 
>  tools/perf/builtin-lock.c                 |  2 ++
>  tools/perf/tests/shell/lock_contention.sh | 14 +++++++++-----
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> -- 
> 2.51.0
>