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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Combining a slab cache filter with the 'mmap_lock' filter silently drops all valid slab lock events.
--

commit b47198e2987a248476bc916b47bb7722966773ea
Author: Namhyung Kim <namhyung@kernel.org>

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

This commit allows filtering for the per-process mmap_lock using the
-L/--lock-filter option in perf lock contention by adding a special case
to check the lock type via BPF.

> diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> index 96e7d853b9edf..8b1aa64deb6e4 100644
> --- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
> +++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
[ ... ]
> @@ -291,7 +302,15 @@ static inline int can_record(u64 *ctx)
>  
>  		kmem_cache_addr = (long)bpf_get_kmem_cache(addr);
>  		ok = bpf_map_lookup_elem(&slab_filter, &kmem_cache_addr);
> -		if (!ok)
> +		if (!ok && !has_mmap_lock)
> +			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;
>  	}

If a user specifies both a slab cache filter and the mmap_lock filter,
will valid slab lock events be silently dropped here?

When a lock event successfully matches the slab_filter, ok is non-null.
However, there is no early return like there is for is_addr_ok above.

Execution falls through to the has_mmap_lock block. Since a slab lock
is not an mmap_lock, check_lock_type() will not equal LCD_F_MMAP_LOCK,
and the function returns 0, filtering out the valid slab event.

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