BPF List
 help / color / mirror / Atom feed
* [PATCH 1/2] perf/lock: Fix non-atomic max/time and min_time updates in contention_data
@ 2026-05-07 18:42 Suchit Karunakaran
  2026-05-07 18:42 ` [PATCH 2/2] perf build: compile BPF skeletons with -mcpu=v3 Suchit Karunakaran
  0 siblings, 1 reply; 4+ messages in thread
From: Suchit Karunakaran @ 2026-05-07 18:42 UTC (permalink / raw)
  To: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin,
	jolsa, irogers, adrian.hunter, james.clark
  Cc: linux-perf-users, linux-kernel, bpf, Suchit Karunakaran

The update_contention_data() had a FIXME noting that max_time and
min_time updates lacked atomicity. Two CPUs could simultaneously
read a stale value, pass the comparison check and race on the
write-back, with the smaller value potentially overwriting the
larger one and silently corrupting the statistics.

Fix this by replacing the bare conditional assignments with a
bpf_loop()-based CAS retry loop. Each field tracks its own
convergence independently via max_done/min_done flags in cas_ctx,
so a successful CAS on one field is never retried even if the
other field needs more attempts.

Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
---
 .../perf/util/bpf_skel/lock_contention.bpf.c  | 50 +++++++++++++++++--
 1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
index 96e7d853b9ed..5c8431be674a 100644
--- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
+++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
@@ -175,6 +175,13 @@ struct mm_struct___new {
 	struct rw_semaphore mmap_lock;
 } __attribute__((preserve_access_index));
 
+struct cas_ctx {
+	struct contention_data *data;
+	u64 duration;
+	int max_done;
+	int min_done;
+};
+
 extern struct kmem_cache *bpf_get_kmem_cache(u64 addr) __ksym __weak;
 
 /* control flags */
@@ -486,16 +493,49 @@ static inline s32 get_owner_stack_id(u64 *stacktrace)
 	return -1;
 }
 
+static long cas_min_max_cb(u64 idx, void *arg)
+{
+	struct cas_ctx *ctx = arg;
+
+	if (!ctx->max_done) {
+		u64 old_max = ctx->data->max_time;
+		if (old_max >= ctx->duration) {
+			ctx->max_done = 1;
+		} else {
+			u64 r = __sync_val_compare_and_swap(
+				&ctx->data->max_time, old_max, ctx->duration);
+			if (r == old_max)
+				ctx->max_done = 1;
+		}
+	}
+
+	if (!ctx->min_done) {
+		u64 old_min = ctx->data->min_time;
+		if (old_min <= ctx->duration) {
+			ctx->min_done = 1;
+		} else {
+			u64 r = __sync_val_compare_and_swap(
+				&ctx->data->min_time, old_min, ctx->duration);
+			if (r == old_min)
+				ctx->min_done = 1;
+		}
+	}
+
+	return (ctx->max_done && ctx->min_done) ? 1 : 0;
+}
+
 static inline void update_contention_data(struct contention_data *data, u64 duration, u32 count)
 {
 	__sync_fetch_and_add(&data->total_time, duration);
 	__sync_fetch_and_add(&data->count, count);
 
-	/* FIXME: need atomic operations */
-	if (data->max_time < duration)
-		data->max_time = duration;
-	if (data->min_time > duration)
-		data->min_time = duration;
+	struct cas_ctx ctx = {
+		.data     = data,
+		.duration = duration,
+		.max_done = 0,
+		.min_done = 0,
+	};
+	bpf_loop(64, cas_min_max_cb, &ctx, 0);
 }
 
 static inline void update_owner_stat(u32 id, u64 duration, u32 flags)
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] perf build: compile BPF skeletons with -mcpu=v3
  2026-05-07 18:42 [PATCH 1/2] perf/lock: Fix non-atomic max/time and min_time updates in contention_data Suchit Karunakaran
@ 2026-05-07 18:42 ` Suchit Karunakaran
  2026-05-07 23:49   ` sashiko-bot
  0 siblings, 1 reply; 4+ messages in thread
From: Suchit Karunakaran @ 2026-05-07 18:42 UTC (permalink / raw)
  To: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin,
	jolsa, irogers, adrian.hunter, james.clark
  Cc: linux-perf-users, linux-kernel, bpf, Suchit Karunakaran

The lock_contention BPF program uses __sync_val_compare_and_swap()
to atomically update the max_time and min_time fields in
contention_data. This builtin lowers to the BPF_CMPXCHG instruction,
which is only available in BPF ISA v3. Without an explicit -mcpu flag,
Clang targets BPF v1/v2 by default on older toolchains (Clang < 18),
causing build errors when v3 instructions are emitted.

Add -mcpu=v3 to CLANG_OPTIONS, which is used exclusively in the BPF
skeleton compilation rule.

Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
---
 tools/perf/Makefile.perf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index cee19c923c06..a9a8c84b6b00 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -1195,7 +1195,7 @@ ifneq ($(CROSS_COMPILE),)
 CLANG_TARGET_ARCH = --target=$(notdir $(CROSS_COMPILE:%-=%))
 endif
 
-CLANG_OPTIONS = -Wall
+CLANG_OPTIONS = -Wall -mcpu=v3
 CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
 BPF_INCLUDE := -I$(SKEL_TMP_OUT)/.. -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES)
 TOOLS_UAPI_INCLUDE := -I$(srctree)/tools/include/uapi
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] perf build: compile BPF skeletons with -mcpu=v3
  2026-05-07 18:42 ` [PATCH 2/2] perf build: compile BPF skeletons with -mcpu=v3 Suchit Karunakaran
@ 2026-05-07 23:49   ` sashiko-bot
  2026-05-10  6:47     ` Namhyung Kim
  0 siblings, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-05-07 23:49 UTC (permalink / raw)
  To: Suchit Karunakaran; +Cc: linux-perf-users, bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Adding `-mcpu=v3` to `CLANG_OPTIONS` forces all BPF skeletons to use the BPF v3 ISA, breaking backward compatibility for perf tools designed to run on older kernels.
--

commit 2e8791aeccf694e1ae9ed9fda46e3a6f73332838
Author: Suchit Karunakaran <suchitkarunakaran@gmail.com>

perf build: compile BPF skeletons with -mcpu=v3

This commit adds -mcpu=v3 to CLANG_OPTIONS to support the atomic
cmpxchg instruction required by the lock_contention BPF program.
This forces Clang to emit BPF ISA v3 instructions during skeleton
compilation.

> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index cee19c923c060..a9a8c84b6b006 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf

[ ... ]

> @@ -1195,7 +1195,7 @@ ifneq ($(CROSS_COMPILE),)
>  CLANG_TARGET_ARCH = --target=$(notdir $(CROSS_COMPILE:%-=%))
>  endif
>  
> -CLANG_OPTIONS = -Wall
> +CLANG_OPTIONS = -Wall -mcpu=v3

Does this change break backward compatibility for other perf tools running
on older kernels?

Since CLANG_OPTIONS is used in the pattern rule for compiling all BPF
skeletons, this forces every BPF program in perf (such as
augmented_raw_syscalls.bpf.c and off_cpu.bpf.c) to target the BPF ISA v3.

Kernels older than 5.1 do not support v3 instructions and will reject
these programs during the verifier load phase.

Should the -mcpu=v3 flag be restricted exclusively to the target that
needs it, perhaps using a target-specific append for lock_contention.bpf.o?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260507184238.65280-1-suchitkarunakaran@gmail.com?part=2

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] perf build: compile BPF skeletons with -mcpu=v3
  2026-05-07 23:49   ` sashiko-bot
@ 2026-05-10  6:47     ` Namhyung Kim
  0 siblings, 0 replies; 4+ messages in thread
From: Namhyung Kim @ 2026-05-10  6:47 UTC (permalink / raw)
  To: acme; +Cc: Suchit Karunakaran, linux-perf-users, bpf

Hi Arnaldo,

On Thu, May 07, 2026 at 11:49:59PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Adding `-mcpu=v3` to `CLANG_OPTIONS` forces all BPF skeletons to use the BPF v3 ISA, breaking backward compatibility for perf tools designed to run on older kernels.
> --
> 
> commit 2e8791aeccf694e1ae9ed9fda46e3a6f73332838
> Author: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> 
> perf build: compile BPF skeletons with -mcpu=v3
> 
> This commit adds -mcpu=v3 to CLANG_OPTIONS to support the atomic
> cmpxchg instruction required by the lock_contention BPF program.
> This forces Clang to emit BPF ISA v3 instructions during skeleton
> compilation.
> 
> > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > index cee19c923c060..a9a8c84b6b006 100644
> > --- a/tools/perf/Makefile.perf
> > +++ b/tools/perf/Makefile.perf
> 
> [ ... ]
> 
> > @@ -1195,7 +1195,7 @@ ifneq ($(CROSS_COMPILE),)
> >  CLANG_TARGET_ARCH = --target=$(notdir $(CROSS_COMPILE:%-=%))
> >  endif
> >  
> > -CLANG_OPTIONS = -Wall
> > +CLANG_OPTIONS = -Wall -mcpu=v3
> 
> Does this change break backward compatibility for other perf tools running
> on older kernels?
> 
> Since CLANG_OPTIONS is used in the pattern rule for compiling all BPF
> skeletons, this forces every BPF program in perf (such as
> augmented_raw_syscalls.bpf.c and off_cpu.bpf.c) to target the BPF ISA v3.
> 
> Kernels older than 5.1 do not support v3 instructions and will reject
> these programs during the verifier load phase.

I think it's old enough and we can move on to ensure v3 instructions.

Thanks,
Namhyung

> 
> Should the -mcpu=v3 flag be restricted exclusively to the target that
> needs it, perhaps using a target-specific append for lock_contention.bpf.o?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260507184238.65280-1-suchitkarunakaran@gmail.com?part=2

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-10  6:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-07 18:42 [PATCH 1/2] perf/lock: Fix non-atomic max/time and min_time updates in contention_data Suchit Karunakaran
2026-05-07 18:42 ` [PATCH 2/2] perf build: compile BPF skeletons with -mcpu=v3 Suchit Karunakaran
2026-05-07 23:49   ` sashiko-bot
2026-05-10  6:47     ` Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox