[PATCH] perf lock contention: Allow 'mmap_lock' in -L/--lock-filter

[PATCH] perf lock contention: Allow 'mmap_lock' in -L/--lock-filter

[Namhyung Kim]

The -L/--lock-filter option is to specify target locks by name or
address.  It's basically for global locks where name or address is known
and fixed.  But 'mmap_lock' is a per-process lock so it cannot be used
for the -L option.

  $ sudo perf lock con -ab -L mmap_lock
  ignore unknown symbol: mmap_lock
  libbpf: map 'addr_filter': failed to create: -EINVAL
  libbpf: failed to load BPF skeleton 'lock_contention_bpf': -EINVAL
  Failed to load lock-contention BPF skeleton
  lock contention BPF setup failed

However, it's still a common source of contention especially in a large
process so we want to use it for the -L/--lock-filter option.  As there
is check_lock_type() to check mmap_lock at runtime, let's used it to
filter mmap_locks as a special case.

Of course, this only works with -b/--use-bpf option.

  $ sudo perf lock con -b -L mmap_lock -- perf bench mem mmap -f demand -t 2
  # Running 'mem/mmap' benchmark:
  # function 'demand' (Demand loaded mmap())
  # Copying 1MB bytes ...

         2.679184 GB/sec/thread	( +-   1.78% )
   contended   total wait     max wait     avg wait         type   caller

           1     15.22 us     15.22 us     15.22 us      rwsem:W   __vm_munmap+0x7e
           1      7.72 us      7.72 us      7.72 us      rwsem:R   lock_mm_and_find_vma+0x97

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/shell/lock_contention.sh      | 11 +++++++++++
 tools/perf/util/bpf_lock_contention.c          | 12 +++++++++++-
 tools/perf/util/bpf_skel/lock_contention.bpf.c | 11 +++++++++++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/lock_contention.sh b/tools/perf/tests/shell/lock_contention.sh
index 6dd90519f45cec1d..52e8b9db9fbd8844 100755
--- a/tools/perf/tests/shell/lock_contention.sh
+++ b/tools/perf/tests/shell/lock_contention.sh
@@ -208,6 +208,17 @@ test_lock_filter()
 		err=1
 		exit
 	fi
+
+	perf lock con -b -L mmap_lock -q -- perf bench mem mmap -t 2 -l 10 > /dev/null 2> ${result}
+
+	# find out the type of mmap_lock
+	test_lock_filter_type=$(head -1 "${result}" | awk '{ print $8 }' | sed -e 's/:.*//')
+
+	if [ "$(grep -c -v "${test_lock_filter_type}" "${result}")" != "0" ]; then
+		echo "[Fail] BPF result should not have non-${test_lock_filter_type} locks:" "$(cat "${result}")"
+		err=1
+		exit
+	fi
 }
 
 test_stack_filter()
diff --git a/tools/perf/util/bpf_lock_contention.c b/tools/perf/util/bpf_lock_contention.c
index cbd7435579feaf8e..cd7ee5d1d1dd654e 100644
--- a/tools/perf/util/bpf_lock_contention.c
+++ b/tools/perf/util/bpf_lock_contention.c
@@ -186,6 +186,7 @@ int lock_contention_prepare(struct lock_contention *con)
 	int ncpus = 1, ntasks = 1, ntypes = 1, naddrs = 1, ncgrps = 1, nslabs = 1;
 	struct evlist *evlist = con->evlist;
 	struct target *target = con->target;
+	bool has_mmap_lock = false;
 
 	/* make sure it loads the kernel map before lookup */
 	map__load(machine__kernel_map(con->machine));
@@ -244,6 +245,11 @@ int lock_contention_prepare(struct lock_contention *con)
 		unsigned long *addrs;
 
 		for (i = 0; i < con->filters->nr_syms; i++) {
+			if (!strcmp(con->filters->syms[i], "mmap_lock")) {
+				has_mmap_lock = true;
+				continue;
+			}
+
 			sym = machine__find_kernel_symbol_by_name(con->machine,
 								  con->filters->syms[i],
 								  &kmap);
@@ -264,7 +270,10 @@ int lock_contention_prepare(struct lock_contention *con)
 			con->filters->addrs = addrs;
 		}
 		naddrs = con->filters->nr_addrs;
-		skel->rodata->has_addr = 1;
+		if (naddrs > 0)
+			skel->rodata->has_addr = 1;
+		else
+			naddrs = 1;
 	}
 
 	/* resolve lock name in delays */
@@ -298,6 +307,7 @@ int lock_contention_prepare(struct lock_contention *con)
 	skel->rodata->aggr_mode = con->aggr_mode;
 	skel->rodata->needs_callstack = con->save_callstack;
 	skel->rodata->lock_owner = con->owner;
+	skel->rodata->has_mmap_lock = has_mmap_lock;
 
 	if (con->aggr_mode == LOCK_AGGR_CGROUP || con->filters->nr_cgrps) {
 		if (cgroup_is_v2("perf_event"))
diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
index 96e7d853b9edf3b5..45ec2fb739842403 100644
--- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
+++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
@@ -184,6 +184,7 @@ const volatile int has_type;
 const volatile int has_addr;
 const volatile int has_cgroup;
 const volatile int has_slab;
+const volatile int has_mmap_lock;
 const volatile int needs_callstack;
 const volatile int stack_skip;
 const volatile int lock_owner;
@@ -214,6 +215,8 @@ int data_map_full;
 struct task_struct *bpf_task_from_pid(s32 pid) __ksym __weak;
 void bpf_task_release(struct task_struct *p) __ksym __weak;
 
+static inline __u32 check_lock_type(__u64 lock, __u32 flags);
+
 static inline __u64 get_current_cgroup_id(void)
 {
 	struct task_struct *task;
@@ -295,6 +298,14 @@ static inline int can_record(u64 *ctx)
 			return 0;
 	}
 
+	if (has_mmap_lock) {
+		__u64 lock = ctx[0];
+		__u32 flag = ctx[1];
+
+		if (check_lock_type(lock, flag) != LCD_F_MMAP_LOCK)
+			return 0;
+	}
+
 	return 1;
 }
 
-- 
2.53.0

Re: [PATCH] perf lock contention: Allow 'mmap_lock' in -L/--lock-filter

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Combining `mmap_lock` with other lock names in the `-L/--lock-filter` option drops all events.
- [High] Specifying only an unknown lock symbol in `-L/--lock-filter` silently traces all locks in the system instead of aborting.
--

commit ba21190eae512a9060154fb29ca23ce62c636cc2
Author: Namhyung Kim <namhyung@kernel.org>
Subject: perf lock contention: Allow 'mmap_lock' in -L/--lock-filter

This commit adds support for filtering lock contention events specifically for
the 'mmap_lock' using the -L/--lock-filter option. It introduces a special
flag to bypass the usual address-based filtering for mmap_lock.

> diff --git a/tools/perf/util/bpf_lock_contention.c b/tools/perf/util/bpf_lock_contention.c
> index cbd7435579fe..cd7ee5d1d1dd 100644
> --- a/tools/perf/util/bpf_lock_contention.c
> +++ b/tools/perf/util/bpf_lock_contention.c

[ ... ]

> @@ -264,7 +270,10 @@ int lock_contention_prepare(struct lock_contention *con)
>  			con->filters->addrs = addrs;
>  		}
>  		naddrs = con->filters->nr_addrs;
> -		skel->rodata->has_addr = 1;
> +		if (naddrs > 0)
> +			skel->rodata->has_addr = 1;
> +		else
> +			naddrs = 1;
>  	}

Prior to this patch, providing an unknown symbol (e.g., -L non_existent)
resulted in naddrs = 0 and has_addr = 1, which correctly caused libbpf to
fail creating the map with 0 entries, gracefully aborting the process.

With this change, if naddrs is 0, has_addr remains 0 but naddrs is
artificially set to 1. The BPF map creation succeeds, but since has_addr
and has_mmap_lock are both 0, the BPF program skips the address filter
entirely.

Does this unintentionally trace all locks in the system instead of gracefully
aborting when an unknown lock name is provided?

> diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> index 96e7d853b9ed..45ec2fb73984 100644
> --- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
> +++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c

[ ... ]

> @@ -295,6 +298,14 @@ static inline int can_record(u64 *ctx)
>  			return 0;
>  	}
>  
> +	if (has_mmap_lock) {
> +		__u64 lock = ctx[0];
> +		__u32 flag = ctx[1];
> +
> +		if (check_lock_type(lock, flag) != LCD_F_MMAP_LOCK)
> +			return 0;
> +	}
> +
>  	return 1;
>  }

The -L option allows filtering by multiple lock names, but can_record()
evaluates active filters sequentially. If a user specifies
-L mmap_lock,other_lock, both has_mmap_lock = 1 and has_addr = 1 will be
set in the BPF rodata.

In this scenario:

1. An other_lock event passes the has_addr check but then reaches the
has_mmap_lock check. Since its type is not LCD_F_MMAP_LOCK, the function
returns 0.

2. An mmap_lock event is evaluated against the has_addr check first.
Because its dynamic address isn't in addr_filter, the lookup fails, and the
function returns 0 before even reaching the has_mmap_lock check.

Will this cause all valid events to be silently dropped when combining
mmap_lock with other lock names?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260504060859.84987-1-namhyung@kernel.org?part=1

Re: [PATCH] perf lock contention: Allow 'mmap_lock' in -L/--lock-filter

[Namhyung Kim]

On Mon, May 04, 2026 at 06:27:43AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Combining `mmap_lock` with other lock names in the `-L/--lock-filter` option drops all events.
> - [High] Specifying only an unknown lock symbol in `-L/--lock-filter` silently traces all locks in the system instead of aborting.
> --
> 
> commit ba21190eae512a9060154fb29ca23ce62c636cc2
> Author: Namhyung Kim <namhyung@kernel.org>
> Subject: perf lock contention: Allow 'mmap_lock' in -L/--lock-filter
> 
> This commit adds support for filtering lock contention events specifically for
> the 'mmap_lock' using the -L/--lock-filter option. It introduces a special
> flag to bypass the usual address-based filtering for mmap_lock.
> 
> > diff --git a/tools/perf/util/bpf_lock_contention.c b/tools/perf/util/bpf_lock_contention.c
> > index cbd7435579fe..cd7ee5d1d1dd 100644
> > --- a/tools/perf/util/bpf_lock_contention.c
> > +++ b/tools/perf/util/bpf_lock_contention.c
> 
> [ ... ]
> 
> > @@ -264,7 +270,10 @@ int lock_contention_prepare(struct lock_contention *con)
> >  			con->filters->addrs = addrs;
> >  		}
> >  		naddrs = con->filters->nr_addrs;
> > -		skel->rodata->has_addr = 1;
> > +		if (naddrs > 0)
> > +			skel->rodata->has_addr = 1;
> > +		else
> > +			naddrs = 1;
> >  	}
> 
> Prior to this patch, providing an unknown symbol (e.g., -L non_existent)
> resulted in naddrs = 0 and has_addr = 1, which correctly caused libbpf to
> fail creating the map with 0 entries, gracefully aborting the process.
> 
> With this change, if naddrs is 0, has_addr remains 0 but naddrs is
> artificially set to 1. The BPF map creation succeeds, but since has_addr
> and has_mmap_lock are both 0, the BPF program skips the address filter
> entirely.
> 
> Does this unintentionally trace all locks in the system instead of gracefully
> aborting when an unknown lock name is provided?

Ok, I think this is a valid concern.  Will update in v2.

> 
> > diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> > index 96e7d853b9ed..45ec2fb73984 100644
> > --- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
> > +++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> 
> [ ... ]
> 
> > @@ -295,6 +298,14 @@ static inline int can_record(u64 *ctx)
> >  			return 0;
> >  	}
> >  
> > +	if (has_mmap_lock) {
> > +		__u64 lock = ctx[0];
> > +		__u32 flag = ctx[1];
> > +
> > +		if (check_lock_type(lock, flag) != LCD_F_MMAP_LOCK)
> > +			return 0;
> > +	}
> > +
> >  	return 1;
> >  }
> 
> The -L option allows filtering by multiple lock names, but can_record()
> evaluates active filters sequentially. If a user specifies
> -L mmap_lock,other_lock, both has_mmap_lock = 1 and has_addr = 1 will be
> set in the BPF rodata.
> 
> In this scenario:
> 
> 1. An other_lock event passes the has_addr check but then reaches the
> has_mmap_lock check. Since its type is not LCD_F_MMAP_LOCK, the function
> returns 0.
> 
> 2. An mmap_lock event is evaluated against the has_addr check first.
> Because its dynamic address isn't in addr_filter, the lookup fails, and the
> function returns 0 before even reaching the has_mmap_lock check.
> 
> Will this cause all valid events to be silently dropped when combining
> mmap_lock with other lock names?

Good point, I'll handle the case mmap_lock and another lock are used
together.

Thanks,
Namhyung