Re: [PATCH v6 3/3] perf symbol: Lazily compute idle and use the perf_env

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] Data race and undefined behavior due to unsynchronized bitfield updates in `struct symbol`.
--

commit 2ee008011b6949869303359cb5ce16b0c47431af
Author: Ian Rogers <irogers@google.com>
Subject: perf symbol: Lazily compute idle and use the perf_env

This commit defers computing whether a symbol is an idle function to a lazy
evaluation using symbol__is_idle. It modifies struct symbol to cache this
lazily computed value using an idle:2 bitfield, checking architectural and
kernel versions using the perf_env.

Link: https://lore.kernel.org/lkml/20260219113850.354271-1-tmricht@linux.ibm.com/
Link: https://lore.kernel.org/lkml/20260323085255.98173-1-jameshongleiwang@126.com/

> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index bd6eb90c86683..7e0036f801858 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
[ ... ]
> @@ -58,8 +65,8 @@ struct symbol {
>  	u8		type:4;
>  	/** ELF binding type as defined for st_info. E.g. STB_WEAK or STB_GLOBAL. */
>  	u8		binding:4;
> -	/** Set true for kernel symbols of idle routines. */
> -	u8		idle:1;
> +	/** Cache for symbol__is_idle holding enum symbol_idle_kind values. */
> +	u8		idle:2;
>  	/** Resolvable but tools ignore it (e.g. idle routines). */
>  	u8		ignore:1;
>  	/** Symbol for an inlined function. */

Could this lazy update of the idle bitfield cause a data race?

Because idle is a bitfield sharing a byte with ignore, inlined, annotate2,
and ifunc_alias, writing to it performs a non-atomic read-modify-write on
the entire byte.

> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index fd332db561575..482fd47bead24 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
[ ... ]
> +bool symbol__is_idle(struct symbol *sym, const struct dso *dso, struct perf_env *env)
>  {
[ ... ]
> +	if (bsearch(name, idle_symbols, ARRAY_SIZE(idle_symbols),
> +		    sizeof(idle_symbols[0]), sym_name_cmp)) {
> +		sym->idle = SYMBOL_IDLE__IDLE;
> +		return true;
> +	}

In perf top, sample processing runs continuously in a background thread,
calling symbol__is_idle() and writing to sym->idle:

Thread 1 (Sample processing):
    sym->idle = SYMBOL_IDLE__IDLE;

Concurrently, the TUI thread processes user interactions and can set flags
like annotate2:

Thread 2 (TUI / User Interaction):
    sym->annotate2 = 1;

If these threads access the same symbol simultaneously, could one thread's
write overwrite the other's modifications, silently corrupting the symbol
state and causing features like UI annotation to break?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260409230620.4176210-1-irogers@google.com?part=3