From: Namhyung Kim <namhyung@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: acme@kernel.org, adrian.hunter@intel.com, bvanassche@acm.org,
elver@google.com, james.clark@linaro.org, jolsa@kernel.org,
justinstitt@google.com, linux-kernel@vger.kernel.org,
linux-perf-users@vger.kernel.org, llvm@lists.linux.dev,
mingo@redhat.com, morbo@google.com, nathan@kernel.org,
ndesaulniers@google.com, peterz@infradead.org
Subject: Re: [PATCH v12 17/20] perf config, intel-tpebs, env: Add __guarded_by and __must_hold annotations
Date: Wed, 11 Nov 2026 23:06:56 -0800 [thread overview]
Message-ID: <avVlkHSqveDsxDXl@google.com> (raw)
In-Reply-To: <20261111174213.3526604-18-irogers@google.com>
On Wed, Nov 11, 2026 at 09:42:09AM -0800, Ian Rogers wrote:
> Add __guarded_by() and __must_hold() annotations to static parser and
> config state in util/config.c, TPEBS state in util/intel-tpebs.c, and
> bpf_progs and cgroups trees in struct perf_env.
Looks like an unrelated set of changes. Probably better to split. I
think TPEBS changes can go to the previous commit.
Thanks,
Namhyung
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/bpf-event.h | 5 ++--
> tools/perf/util/config.c | 47 +++++++++++++++++++++--------------
> tools/perf/util/env.c | 1 +
> tools/perf/util/env.h | 19 ++++++++------
> tools/perf/util/intel-tpebs.c | 18 ++++++++------
> 5 files changed, 54 insertions(+), 36 deletions(-)
>
> diff --git a/tools/perf/util/bpf-event.h b/tools/perf/util/bpf-event.h
> index da4eeb4a1a73..3f3323f98f49 100644
> --- a/tools/perf/util/bpf-event.h
> +++ b/tools/perf/util/bpf-event.h
> @@ -6,11 +6,11 @@
> #include <linux/rbtree.h>
> #include <api/fd/array.h>
> #include <stdio.h>
> +#include "env.h"
>
> struct bpf_prog_info;
> struct machine;
> union perf_event;
> -struct perf_env;
> struct perf_sample;
> struct perf_session;
> struct record_opts;
> @@ -42,7 +42,8 @@ int machine__process_bpf(struct machine *machine, union perf_event *event,
> int evlist__add_bpf_sb_event(struct evlist *evlist, struct perf_env *env);
> void __bpf_event__print_bpf_prog_info(struct perf_bpil *info_linear,
> struct perf_env *env,
> - FILE *fp);
> + FILE *fp)
> + __must_hold_shared(&env->bpf_progs.lock);
> void bpf_metadata_free(struct bpf_metadata *metadata);
> #else
> static inline int machine__process_bpf(struct machine *machine __maybe_unused,
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 287401f159e6..0e6b77a0a5e7 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -64,15 +64,29 @@ struct perf_stat_config stat_config = {
>
> char buildid_dir[MAXPATHLEN]; /* root dir for buildid, binary cache */
>
> -static FILE *config_file;
> -static const char *config_file_name;
> -static int config_linenr;
> -static int config_file_eof;
> -static struct perf_config_set *config_set;
> +/* Parsing and rewriting share the static parser state. */
> +static DEFINE_MUTEX(config_mutex);
> +
> +/* Serializes whole perf_config__set_variable() updates. */
> +static DEFINE_MUTEX(config_update_mutex);
> +
> +/*
> + * Protects the global config_set. Separate from config_mutex because
> + * initializing config_set parses the config files, which acquires
> + * config_mutex.
> + */
> +static DEFINE_MUTEX(config_set_mutex);
> +
> +static FILE *config_file __guarded_by(&config_mutex);
> +static const char *config_file_name __guarded_by(&config_mutex);
> +static int config_linenr __guarded_by(&config_mutex);
> +static int config_file_eof __guarded_by(&config_mutex);
> +static struct perf_config_set *config_set __guarded_by(&config_set_mutex);
>
> const char *config_exclusive_filename;
>
> static int get_next_char(void)
> + __must_hold(&config_mutex)
> {
> int c;
> FILE *f;
> @@ -99,8 +113,9 @@ static int get_next_char(void)
> }
>
> static char *parse_value(void)
> + __must_hold(&config_mutex)
> {
> - static char value[1024];
> + static char value[1024] __guarded_by(&config_mutex);
> int quote = 0, comment = 0, space = 0;
> size_t len = 0;
>
> @@ -170,6 +185,7 @@ static inline int iskeychar(int c)
> }
>
> static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)
> + __must_hold(&config_mutex)
> {
> int c;
> char *value;
> @@ -201,6 +217,7 @@ static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)
> }
>
> static int get_extended_base_var(char *name, int baselen, int c)
> + __must_hold(&config_mutex)
> {
> do {
> if (c == '\n')
> @@ -237,6 +254,7 @@ static int get_extended_base_var(char *name, int baselen, int c)
> }
>
> static int get_base_var(char *name)
> + __must_hold(&config_mutex)
> {
> int baselen = 0;
>
> @@ -257,10 +275,11 @@ static int get_base_var(char *name)
> }
>
> static int perf_parse_file(config_fn_t fn, void *data)
> + __must_hold(&config_mutex)
> {
> int comment = 0;
> int baselen = 0;
> - static char var[MAXNAME];
> + static char var[MAXNAME] __guarded_by(&config_mutex);
>
> /* U+FEFF Byte Order Mark in UTF8 */
> static const unsigned char *utf8_bom = (unsigned char *) "\xef\xbb\xbf";
> @@ -549,12 +568,6 @@ int perf_default_config(const char *var, const char *value,
> return 0;
> }
>
> -/* Parsing and rewriting share the static parser state. */
> -static DEFINE_MUTEX(config_mutex);
> -
> -/* Serializes whole perf_config__set_variable() updates. */
> -static DEFINE_MUTEX(config_update_mutex);
> -
> static int perf_config_from_file(config_fn_t fn, const char *filename, void *data)
> {
> int ret;
> @@ -737,6 +750,7 @@ static int set_value(struct perf_config_item *item, const char *value)
>
> static int collect_config(const char *var, const char *value,
> void *perf_config_set)
> + __must_hold(&config_mutex)
> {
> int ret = -1;
> char *ptr, *key;
> @@ -853,11 +867,8 @@ struct perf_config_set *perf_config_set__load_file(const char *file)
> return set;
> }
>
> -/* Not config_mutex: building the set parses the config files, which takes it. */
> -static DEFINE_MUTEX(config_set_mutex);
> -
> -/* Called with config_set_mutex held. */
> static int perf_config__init(void)
> + __must_hold(&config_set_mutex)
> {
> if (config_set == NULL)
> config_set = perf_config_set__new();
> @@ -975,7 +986,7 @@ int perf_config__set_variable(const char *var, const char *value)
>
> /* Static: the parser publishes it as config_file_name. */
> {
> - static char path[PATH_MAX];
> + static char path[PATH_MAX] __guarded_by(&config_update_mutex);
> char *user_config = mkpath(path, sizeof(path), "%s/.perfconfig", getenv("HOME"));
>
> config_filename = config_exclusive_filename ?: user_config;
> diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
> index 50993b0dedc5..9e8e5d1d822a 100644
> --- a/tools/perf/util/env.c
> +++ b/tools/perf/util/env.c
> @@ -302,6 +302,7 @@ void perf_env__exit(struct perf_env *env)
> }
>
> void perf_env__init(struct perf_env *env)
> + __context_unsafe(/* initialization */)
> {
> memset(env, 0, sizeof(*env));
> #ifdef HAVE_LIBBPF_SUPPORT
> diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
> index 23d35425efe3..6b2083e3640c 100644
> --- a/tools/perf/util/env.h
> +++ b/tools/perf/util/env.h
> @@ -139,16 +139,16 @@ struct perf_env {
> */
> struct {
> struct rw_semaphore lock;
> - struct rb_root infos;
> - u32 infos_cnt;
> - struct rb_root btfs;
> - u32 btfs_cnt;
> + struct rb_root infos __guarded_by(&lock);
> + u32 infos_cnt __guarded_by(&lock);
> + struct rb_root btfs __guarded_by(&lock);
> + u32 btfs_cnt __guarded_by(&lock);
> } bpf_progs;
> #endif // HAVE_LIBBPF_SUPPORT
> /* same reason as above (for perf-top) */
> struct {
> struct rw_semaphore lock;
> - struct rb_root tree;
> + struct rb_root tree __guarded_by(&lock);
> } cgroups;
>
> /* For fast cpu to numa node lookup via perf_env__numa_node */
> @@ -223,7 +223,8 @@ int perf_env__nr_cpus_avail(struct perf_env *env);
> void perf_env__init(struct perf_env *env);
> #ifdef HAVE_LIBBPF_SUPPORT
> bool __perf_env__insert_bpf_prog_info(struct perf_env *env,
> - struct bpf_prog_info_node *info_node);
> + struct bpf_prog_info_node *info_node)
> + __must_hold(&env->bpf_progs.lock);
> bool perf_env__insert_bpf_prog_info(struct perf_env *env,
> struct bpf_prog_info_node *info_node);
> struct bpf_prog_info_node *perf_env__find_bpf_prog_info(struct perf_env *env,
> @@ -233,9 +234,11 @@ void perf_env__iterate_bpf_prog_info(struct perf_env *env,
> void *data),
> void *data);
> bool perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node);
> -bool __perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node);
> +bool __perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node)
> + __must_hold(&env->bpf_progs.lock);
> struct btf_node *perf_env__find_btf(struct perf_env *env, __u32 btf_id);
> -struct btf_node *__perf_env__find_btf(struct perf_env *env, __u32 btf_id);
> +struct btf_node *__perf_env__find_btf(struct perf_env *env, __u32 btf_id)
> + __must_hold_shared(&env->bpf_progs.lock);
> #endif // HAVE_LIBBPF_SUPPORT
>
> int perf_env__numa_node(struct perf_env *env, struct perf_cpu cpu);
> diff --git a/tools/perf/util/intel-tpebs.c b/tools/perf/util/intel-tpebs.c
> index 2edd8297fa1b..d79d42a36a53 100644
> --- a/tools/perf/util/intel-tpebs.c
> +++ b/tools/perf/util/intel-tpebs.c
> @@ -39,15 +39,16 @@
> bool tpebs_recording;
> enum tpebs_mode tpebs_mode;
> static DEFINE_MUTEX(tpebs_mtx);
> -static LIST_HEAD(tpebs_results);
> -static pthread_t tpebs_reader_thread;
> -static struct child_process tpebs_cmd;
> +static struct list_head tpebs_results __guarded_by(&tpebs_mtx) = LIST_HEAD_INIT(tpebs_results);
> +static pthread_t tpebs_reader_thread __guarded_by(&tpebs_mtx);
> +static struct child_process tpebs_cmd __guarded_by(&tpebs_mtx);
> static struct cond tpebs_cmd_cond = { .cond = PTHREAD_COND_INITIALIZER };
> -static int control_fd[2] = { -1, -1 }, ack_fd[2] = { -1, -1 };
> -static bool tpebs_stopping;
> -static bool tpebs_enabled;
> -static bool tpebs_cmd_in_flight;
> -static uint64_t tpebs_gen;
> +static int control_fd[2] __guarded_by(&tpebs_mtx) = { -1, -1 };
> +static int ack_fd[2] __guarded_by(&tpebs_mtx) = { -1, -1 };
> +static bool tpebs_stopping __guarded_by(&tpebs_mtx);
> +static bool tpebs_enabled __guarded_by(&tpebs_mtx);
> +static bool tpebs_cmd_in_flight __guarded_by(&tpebs_mtx);
> +static uint64_t tpebs_gen __guarded_by(&tpebs_mtx);
>
> struct tpebs_retire_lat {
> struct list_head nd;
> @@ -67,6 +68,7 @@ static struct tpebs_retire_lat *tpebs_retire_lat__find(struct evsel *evsel)
> __must_hold(&tpebs_mtx);
>
> static int evsel__tpebs_start_perf_record(struct evsel *evsel)
> + __must_hold(&tpebs_mtx)
> {
> const char **record_argv;
> int tpebs_event_size = 0, i = 0, ret;
> --
> 2.56.0.542.g3ef89b7020-goog
>
next prev parent reply other threads:[~2026-11-12 7:06 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-11 17:41 [PATCH v12 00/20] tools/perf: Adopt kernel compiler-context-analysis.h annotations Ian Rogers
2026-11-11 17:41 ` [PATCH v12 01/20] compiler-context-analysis: Avoid unused parameter warning in _context_unsafe_alias() Ian Rogers
2026-11-11 17:41 ` [PATCH v12 02/20] tools headers compiler: Synchronize compiler_types.h with the kernel sources Ian Rogers
2026-11-12 6:31 ` Namhyung Kim
2026-11-11 17:41 ` [PATCH v12 03/20] tools headers: Synchronize linux/compiler-context-analysis.h " Ian Rogers
2026-11-11 17:41 ` [PATCH v12 04/20] perf mutex: Use kernel style compiler-context-analysis.h annotations Ian Rogers
2026-11-11 17:41 ` [PATCH v12 05/20] perf build: Enable additional Clang thread-safety flags Ian Rogers
2026-11-11 17:41 ` [PATCH v12 06/20] perf comm: Add __guarded_by annotations to struct comm_strs Ian Rogers
2026-11-11 17:41 ` [PATCH v12 07/20] perf vdso: Fix unlocked __dsos__add() and race in __machine__addnew_vdso() Ian Rogers
2026-11-11 17:42 ` [PATCH v12 08/20] perf dsos: Skip BPF and OOL DSOs in dsos__find_kernel_dso() Ian Rogers
2026-11-12 6:44 ` Namhyung Kim
2026-11-11 17:42 ` [PATCH v12 09/20] perf dsos: Ensure DSOs in auxtrace, machine, and symbol are in dsos Ian Rogers
2026-11-11 17:42 ` [PATCH v12 10/20] perf dso: Pass dso_data to close_data_fd() and dso__list_del() Ian Rogers
2026-11-11 17:42 ` [PATCH v12 11/20] perf dso: Use DEFINE_MUTEX for dso__data_open_lock Ian Rogers
2026-11-11 17:42 ` [PATCH v12 12/20] perf dsos: Use read lock fast path in dsos__findnew_id() Ian Rogers
2026-11-12 6:53 ` Namhyung Kim
2026-11-11 17:42 ` [PATCH v12 13/20] perf dso: Expand context analysis annotations Ian Rogers
2026-11-11 17:42 ` [PATCH v12 14/20] perf thread: Expand context analysis annotations to threads and namespaces_lock Ian Rogers
2026-11-11 17:42 ` [PATCH v12 15/20] perf intel-tpebs: Fix race in tpebs_stop() and tpebs_cmd.out fd handling Ian Rogers
2026-11-12 7:19 ` Namhyung Kim
2026-11-11 17:42 ` [PATCH v12 16/20] perf intel-tpebs: Use DEFINE_MUTEX for tpebs_mtx Ian Rogers
2026-11-12 7:08 ` Namhyung Kim
2026-11-11 17:42 ` [PATCH v12 17/20] perf config, intel-tpebs, env: Add __guarded_by and __must_hold annotations Ian Rogers
2026-11-12 7:06 ` Namhyung Kim [this message]
2026-11-11 17:42 ` [PATCH v12 18/20] perf maps: Fix lock inversion in maps__load_first() Ian Rogers
2026-11-11 17:42 ` [PATCH v12 19/20] perf maps: Fix map index lookup fallback and removal Ian Rogers
2026-11-11 17:42 ` [PATCH v12 20/20] perf maps: Add context analysis annotations Ian Rogers
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=avVlkHSqveDsxDXl@google.com \
--to=namhyung@kernel.org \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=bvanassche@acm.org \
--cc=elver@google.com \
--cc=irogers@google.com \
--cc=james.clark@linaro.org \
--cc=jolsa@kernel.org \
--cc=justinstitt@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=mingo@redhat.com \
--cc=morbo@google.com \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=peterz@infradead.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox