From: sashiko-bot@kernel.org
To: "Arnaldo Carvalho de Melo" <acme@kernel.org>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH v7 4/9] perf mutex: Add DO_ONCE() for one-time initialization
Date: Fri, 02 Oct 2026 09:13:11 +0000 [thread overview]
Message-ID: <20261002091311.6F81E1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20261001185400.2754753-5-acme@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf: data race and missing memory barriers in DO_ONCE()
--
commit 29753096ef417d4625af9a405f85df097ba45c30
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
perf mutex: Add DO_ONCE() for one-time initialization
This commit introduces a DO_ONCE() macro for lazy initialization across
multiple threads in perf user-space. It mirrors the kernel's DO_ONCE()
behavior using a statically initialized mutex and a boolean flag.
> diff --git a/tools/perf/util/mutex.h b/tools/perf/util/mutex.h
> index b85a80ad60386..87937970615ff 100644
> --- a/tools/perf/util/mutex.h
> +++ b/tools/perf/util/mutex.h
[ ... ]
> +/*
> + * Runs fn() exactly once however many threads race to it, the rest wait
> + * for it to finish; per call site state, so multiple sites wanting the
> + * same once use a common helper, as in the kernel's DO_ONCE().
> + */
> +#define DO_ONCE(fn, ...) \
> + ({ \
> + static bool ___done; \
> + static DEFINE_MUTEX(___once_lock); \
> + bool ___ret = false; \
> + \
> + if (!___done) { \
[Severity: Medium]
Is an acquire memory barrier needed here? On weakly-ordered architectures,
a racing thread evaluating this lockless fast-path without an acquire
barrier (such as smp_load_acquire) could observe ___done as true while
reading stale data from fn()'s side effects. This plain read also
constitutes a data race that could trigger ThreadSanitizer warnings.
> + mutex_lock(&___once_lock); \
> + if (!___done) { \
> + fn(__VA_ARGS__); \
> + ___done = true; \
[Severity: Medium]
Could the CPU or compiler reorder this plain store before the initialization
side effects of fn() are globally visible? A store-release barrier
(such as smp_store_release) might be necessary here so that threads on the
lockless fast-path do not access uninitialized data on architectures like
ARM64 or PowerPC.
> + ___ret = true; \
> + } \
> + mutex_unlock(&___once_lock); \
> + } \
> + ___ret; \
> + })
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261001185400.2754753-1-acme@kernel.org?part=4
next prev parent reply other threads:[~2026-10-02 9:13 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-01 18:53 [PATCH v7 0/9] perf tools: Add progress diagnostics and a false-sharing workload Arnaldo Carvalho de Melo
2026-10-01 18:53 ` [PATCH v7 1/9] perf config: Move perf_config__set_variable() to util/config.c Arnaldo Carvalho de Melo
2026-10-02 9:13 ` sashiko-bot
2026-10-01 18:53 ` [PATCH v7 2/9] perf config: Make perf_etc_perfconfig() never return NULL Arnaldo Carvalho de Melo
2026-10-02 9:13 ` sashiko-bot
2026-10-01 18:53 ` [PATCH v7 3/9] perf mutex: Add DEFINE_MUTEX() static initializer Arnaldo Carvalho de Melo
2026-10-02 9:13 ` sashiko-bot
2026-10-01 18:53 ` [PATCH v7 4/9] perf mutex: Add DO_ONCE() for one-time initialization Arnaldo Carvalho de Melo
2026-10-02 9:13 ` sashiko-bot [this message]
2026-10-01 18:53 ` [PATCH v7 5/9] perf config: Serialize config file access with a mutex Arnaldo Carvalho de Melo
2026-10-02 9:13 ` sashiko-bot
2026-10-01 18:53 ` [PATCH v7 6/9] perf report: Add --progress option Arnaldo Carvalho de Melo
2026-10-02 9:13 ` sashiko-bot
2026-10-01 18:53 ` [PATCH v7 7/9] perf report: Add --no-progress option Arnaldo Carvalho de Melo
2026-10-02 9:13 ` sashiko-bot
2026-10-01 18:53 ` [PATCH v7 8/9] perf scripts: Add perf-stuck, to tell where a running perf is stuck Arnaldo Carvalho de Melo
2026-10-02 9:13 ` sashiko-bot
2026-10-01 18:54 ` [PATCH v7 9/9] perf test: Add false_sharing workload exhibiting cross-CPU false sharing Arnaldo Carvalho de Melo
2026-10-02 9:13 ` sashiko-bot
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=20261002091311.6F81E1F000FF@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=acme@kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/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.