All of lore.kernel.org
 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; 5+ 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] 5+ 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; 5+ 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] 5+ 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; 5+ 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] 5+ 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
  2026-05-14 23:52       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 5+ 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] 5+ messages in thread

* Re: [PATCH 2/2] perf build: compile BPF skeletons with -mcpu=v3
  2026-05-10  6:47     ` Namhyung Kim
@ 2026-05-14 23:52       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-05-14 23:52 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Suchit Karunakaran, linux-perf-users, bpf

On Sat, May 09, 2026 at 11:47:45PM -0700, Namhyung Kim wrote:
> 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.

Can I take this as an Reviewed-by or Acked-by?

- Arnaldo
 
> 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] 5+ messages in thread

end of thread, other threads:[~2026-05-14 23:52 UTC | newest]

Thread overview: 5+ 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
2026-05-14 23:52       ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.