Re: [PATCH v3 1/6] perf lock contention: Share common BPF and lock type definitions in lock_data.h

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] perf lock: missing CO-RE flavor struct for mutex in get_lock_owner()
--

commit 93be88d7858bcce932211b385210b6872f154e7a
Author: Ian Rogers <irogers@google.com>

perf lock contention: Share common BPF and lock type definitions in lock_data.h

This commit moves lock_type_table, LCB_F_* flags, common BPF lock helpers,
and shared CO-RE structs into tools/perf/util/bpf_skel/lock_data.h so they
can be reused by BPF sample filtering. It also extends get_lock_owner() to
support rt_mutex and check the mutex handoff flag.

> diff --git a/tools/perf/util/bpf_skel/lock_data.h b/tools/perf/util/bpf_skel/lock_data.h
> index 652e114e6b877..8cf24f07204c5 100644
> --- a/tools/perf/util/bpf_skel/lock_data.h
> +++ b/tools/perf/util/bpf_skel/lock_data.h

[ ... ]

> +static __always_inline struct task_struct *get_lock_owner(__u64 lock, __u32 flags)
> +{
> +	struct task_struct *task;
> +	__u64 owner = 0;
> +
> +	if (flags & LCB_F_MUTEX) {
> +		struct mutex *mutex = (void *)lock;
> +
> +		if (bpf_core_field_exists(mutex->owner.counter)) {
> +			owner = BPF_CORE_READ(mutex, owner.counter);
> +			if (owner & 0x4UL)
> +				return NULL;
> +		} else if (bpf_core_type_exists(struct rt_mutex_base)) {
> +			struct rt_mutex_base *rtm = (void *)lock;
> +
> +			owner = (__u64)BPF_CORE_READ(rtm, owner);
> +			return (void *)(owner & ~1UL);
> +		}

[Severity: Low]
When building perf with GEN_VMLINUX_H=1 against a CONFIG_PREEMPT_RT=y
kernel, vmlinux.h is generated from the kernel BTF where struct mutex is
defined as:

include/linux/mutex_types.h:
context_lock_struct(mutex) {
	struct rt_mutex_base	rtmutex;
#ifdef CONFIG_DEBUG_LOCK_ALLOC
	struct lockdep_map	dep_map;
#endif
};

Because struct mutex has no owner member in that configuration, and
bpf_core_field_exists(mutex->owner.counter) and BPF_CORE_READ(mutex,
owner.counter) expand to C member accesses on struct mutex, will this fail
to compile?

Should this use a CO-RE flavor struct (similar to struct rw_semaphore___new
and struct mm_struct___new above) to access owner.counter?

[ ... ]

>
>
>
>

[]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018053710.2572636-1-irogers@google.com?part=1