All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Suchit Karunakaran" <suchitkarunakaran@gmail.com>
Cc: bpf@vger.kernel.org, linux-perf-users@vger.kernel.org
Subject: Re: [PATCH RESEND] perf/lock: Fix non-atomic max/time and min_time updates in contention_data
Date: Mon, 04 May 2026 17:23:37 +0000	[thread overview]
Message-ID: <20260504172338.6FA50C2BCB8@smtp.kernel.org> (raw)
In-Reply-To: <20260504135138.17050-1-suchitkarunakaran@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The use of `__sync_val_compare_and_swap` introduces a compiler dependency on BPF ISA v3, breaking the `perf` tool build on older toolchains.
--

commit 30820839a2cfcfa987fbc2e0f79fa0e2f2cf0dc3
Author: Suchit Karunakaran <suchitkarunakaran@gmail.com>

perf/lock: Fix non-atomic max/time and min_time updates in contention_data

This commit replaces bare conditional assignments with a bpf_loop()-based CAS
retry loop to fix non-atomic updates of max_time and min_time.

> diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> index 96e7d853b9edf..5c8431be674ac 100644
> --- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
> +++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
[ ... ]
> @@ -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);

Does this cause build failures on older toolchains?

The built-in __sync_val_compare_and_swap lowers to the BPF_CMPXCHG
instruction, which requires BPF CPU v3 (-mcpu=v3).

The perf build system compiles BPF skeletons using the host compiler's default
target features, without explicitly passing an -mcpu flag. On toolchains
where the default BPF target is v1 or v2 (such as Clang < 18), the compiler
may fail to generate the instruction and throw a fatal error (e.g.,
'hardware compare and swap not supported' or an unresolved libcall to
__sync_val_compare_and_swap_8).

> +			if (r == old_max)
> +				ctx->max_done = 1;
> +		}
> +	}
[ ... ]

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

  reply	other threads:[~2026-05-04 17:23 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-04 13:51 [PATCH RESEND] perf/lock: Fix non-atomic max/time and min_time updates in contention_data Suchit Karunakaran
2026-05-04 17:23 ` sashiko-bot [this message]
2026-05-05 22:35   ` Namhyung Kim
2026-05-06  7:04     ` Suchit Karunakaran

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260504172338.6FA50C2BCB8@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sashiko@lists.linux.dev \
    --cc=suchitkarunakaran@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.