Re: [PATCH v5 4/8] perf lock contention: Share common BPF and lock type definitions in lock_data.h

[Namhyung Kim <namhyung@kernel.org>]

On Tue, Oct 20, 2026 at 01:58:54PM -0700, Ian Rogers wrote:
> Move lock_type_table, LCB_F_* flags, and the common BPF lock helpers
> (get_lock_owner, get_current_mmap_lock, get_current_siglock,
> get_lock_addr, check_lock_type, do_collect_zone_lock,
> do_collect_lock_syms, do_slab_cache_iter) and shared CO-RE structs into
> tools/perf/util/bpf_skel/lock_data.h so they can be reused by BPF
> sample filtering.

This is fine.

> 
> Also add rw_semaphore.count and struct rt_mutex_base to the minimal
> vmlinux.h and extend get_lock_owner() to support rt_mutex and check
> the mutex handoff flag.

Please do this in a separate commit.  I'd like to see this kind of
commit only moves the existing code.


> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/builtin-lock.c                     |  27 --
>  .../perf/util/bpf_skel/lock_contention.bpf.c  | 265 +-------------
>  tools/perf/util/bpf_skel/lock_data.h          | 338 ++++++++++++++++++
>  tools/perf/util/bpf_skel/vmlinux/vmlinux.h    |   5 +
>  tools/perf/util/lock-contention.c             |  19 +
>  tools/perf/util/lock-contention.h             |  13 +
>  6 files changed, 393 insertions(+), 274 deletions(-)
> 
> diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> index 778923c92f42..a031709a2cc2 100644
> --- a/tools/perf/builtin-lock.c
> +++ b/tools/perf/builtin-lock.c
> @@ -1482,33 +1482,6 @@ static void sort_result(void)
>  	}
>  }
>  
> -static const struct {
> -	unsigned int flags;
> -	/*
> -	 * Name of the lock flags (access), with delimeter ':'.
> -	 * For example, rwsem:R of rwsem:W.
> -	 */
> -	const char *flags_name;
> -	/* Name of the lock (type), for example, rwlock or rwsem. */
> -	const char *lock_name;
> -} lock_type_table[] = {
> -	{ 0,				"semaphore",	"semaphore" },
> -	{ LCB_F_SPIN,			"spinlock",	"spinlock" },
> -	{ LCB_F_SPIN | LCB_F_READ,	"rwlock:R",	"rwlock" },
> -	{ LCB_F_SPIN | LCB_F_WRITE,	"rwlock:W",	"rwlock" },
> -	{ LCB_F_READ,			"rwsem:R",	"rwsem" },
> -	{ LCB_F_WRITE,			"rwsem:W",	"rwsem" },
> -	{ LCB_F_RT,			"rt-mutex",	"rt-mutex" },
> -	{ LCB_F_RT | LCB_F_READ,	"rwlock-rt:R",	"rwlock-rt" },
> -	{ LCB_F_RT | LCB_F_WRITE,	"rwlock-rt:W",	"rwlock-rt" },
> -	{ LCB_F_PERCPU | LCB_F_READ,	"pcpu-sem:R",	"percpu-rwsem" },
> -	{ LCB_F_PERCPU | LCB_F_WRITE,	"pcpu-sem:W",	"percpu-rwsem" },
> -	{ LCB_F_MUTEX,			"mutex",	"mutex" },
> -	{ LCB_F_MUTEX | LCB_F_SPIN,	"mutex",	"mutex" },
> -	/* alias for optimistic spinning only */
> -	{ LCB_F_MUTEX | LCB_F_SPIN,	"mutex:spin",	"mutex-spin" },
> -};
> -
>  static const char *get_type_flags_name(unsigned int flags)
>  {
>  	flags &= LCB_F_TYPE_MASK;
> diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> index 0d9c6f55050e..0f2118006005 100644
> --- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
> +++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> @@ -8,23 +8,9 @@
>  
>  #include "lock_data.h"
>  
> -/* for collect_lock_syms().  4096 was rejected by the verifier */
> -#define MAX_CPUS  1024
> -
> -/* for collect_zone_lock().  It should be more than the actual zones. */
> -#define MAX_ZONES  10
> -
>  /* for do_lock_delay().  Arbitrarily set to 1 million. */
>  #define MAX_LOOP  (1U << 20)
>  
> -/* lock contention flags from include/trace/events/lock.h */
> -#define LCB_F_SPIN	(1U << 0)
> -#define LCB_F_READ	(1U << 1)
> -#define LCB_F_WRITE	(1U << 2)
> -#define LCB_F_RT	(1U << 3)
> -#define LCB_F_PERCPU	(1U << 4)
> -#define LCB_F_MUTEX	(1U << 5)
> -
>  /* callstack storage  */
>  struct {
>  	__uint(type, BPF_MAP_TYPE_STACK_TRACE);
> @@ -96,13 +82,6 @@ struct {
>  	__uint(max_entries, MAX_ENTRIES);
>  } task_data SEC(".maps");
>  
> -struct {
> -	__uint(type, BPF_MAP_TYPE_HASH);
> -	__uint(key_size, sizeof(__u64));
> -	__uint(value_size, sizeof(__u32));
> -	__uint(max_entries, MAX_ENTRIES);
> -} lock_syms SEC(".maps");
> -
>  struct {
>  	__uint(type, BPF_MAP_TYPE_HASH);
>  	__uint(key_size, sizeof(__u32));
> @@ -138,6 +117,13 @@ struct {
>  	__uint(max_entries, 1);
>  } cgroup_filter SEC(".maps");
>  
> +struct {
> +	__uint(type, BPF_MAP_TYPE_HASH);
> +	__uint(key_size, sizeof(__u64));
> +	__uint(value_size, sizeof(__u32));
> +	__uint(max_entries, MAX_ENTRIES);
> +} lock_syms SEC(".maps");
> +

Looks like un uncessasry change.


>  struct {
>  	__uint(type, BPF_MAP_TYPE_HASH);
>  	__uint(key_size, sizeof(long));
> @@ -159,22 +145,6 @@ struct {
>  	__uint(max_entries, 1);
>  } lock_delays SEC(".maps");
>  
> -struct rw_semaphore___old {
> -	struct task_struct *owner;
> -} __attribute__((preserve_access_index));
> -
> -struct rw_semaphore___new {
> -	atomic_long_t owner;
> -} __attribute__((preserve_access_index));
> -
> -struct mm_struct___old {
> -	struct rw_semaphore mmap_sem;
> -} __attribute__((preserve_access_index));
> -
> -struct mm_struct___new {
> -	struct rw_semaphore mmap_lock;
> -} __attribute__((preserve_access_index));
> -
>  struct cas_ctx {
>  	struct contention_data *data;
>  	u64 duration;
> @@ -182,8 +152,6 @@ struct cas_ctx {
[SNIP]
> diff --git a/tools/perf/util/bpf_skel/lock_data.h b/tools/perf/util/bpf_skel/lock_data.h
> index 652e114e6b87..317d8ccae63e 100644
> --- a/tools/perf/util/bpf_skel/lock_data.h
> +++ b/tools/perf/util/bpf_skel/lock_data.h
> @@ -45,6 +45,14 @@ struct contention_task_data {
>  #define LCB_F_SLAB_ID_END	(1U << 26)
>  #define LCB_F_SLAB_ID_MASK	0x03FF0000U
>  
> +/* lock contention flags from include/trace/events/lock.h */
> +#define LCB_F_SPIN	(1U << 0)
> +#define LCB_F_READ	(1U << 1)
> +#define LCB_F_WRITE	(1U << 2)
> +#define LCB_F_RT	(1U << 3)
> +#define LCB_F_PERCPU	(1U << 4)
> +#define LCB_F_MUTEX	(1U << 5)
> +
>  #define LCB_F_TYPE_MAX		(1U << 7)
>  #define LCB_F_TYPE_MASK		0x0000007FU
>  
> @@ -76,4 +84,334 @@ struct slab_cache_data {
>  	char name[SLAB_NAME_MAX];
>  };
>  
> +#ifdef __BPF__

Why is this needed?

Thanks,
Namhyung