Re: [PATCH 3/3] perf/core: publish perf_event_cache with release semantics

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] perf: unnecessary and inconsistent memory barriers for boot-initialized variable
--

commit 502c74ee5072b4297e79a78cf708d77e31d6b0e5
Author: Jaidev Shastri <jaidevshastri@vt.edu>
Subject: perf/core: publish perf_event_cache with release semantics

This patch replaces plain reads and writes of perf_event_cache with
smp_store_release() and smp_load_acquire() to address a static checker
warning about memory consistency.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 3b5c592630e17..74cfb13fec7c0 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -5313,7 +5313,8 @@ static void free_event_rcu(struct rcu_head *head)
>  		put_pid_ns(event->ns);
>  	perf_event_free_filter(event);
>  	kfree(event->addr_filter_ranges);
> -	kmem_cache_free(perf_event_cache, event);
> +	/* Pairs with the smp_store_release() in perf_event_init(). */
> +	kmem_cache_free(smp_load_acquire(&perf_event_cache), event);

[Severity: Low]
Is this acquire barrier necessary? Since perf_event_cache is initialized
synchronously during early kernel boot, it should already be causally
guaranteed to be fully visible across all CPUs by the time any tasks are
spawned to interact with it.

Additionally, if a memory barrier were required to prevent data races, is
it problematic that the allocation path still uses a plain load?

kernel/events/core.c:perf_event_alloc() {
    ...
    event = kmem_cache_alloc_node(perf_event_cache, GFP_KERNEL | __GFP_ZERO, node);
    ...
}

[ ... ]

> @@ -15404,7 +15405,8 @@ void __init perf_event_init(void)
>  	ret = init_hw_breakpoint();
>  	WARN(ret, "hw_breakpoint initialization failed with: %d", ret);
>  
> -	perf_event_cache = KMEM_CACHE(perf_event, SLAB_PANIC);
> +	/* Pairs with the smp_load_acquire() in free_event_rcu(). */
> +	smp_store_release(&perf_event_cache, KMEM_CACHE(perf_event, SLAB_PANIC));

[Severity: Low]
Since perf_event_init() is an __init function that runs synchronously during
early system boot before tasks could potentially allocate or free perf events,
does this require a release barrier? 

Instead of adding synchronization overhead to silence the static checker,
would it be more appropriate to annotate the declaration of perf_event_cache
with __ro_after_init? This documents the initialization phase ordering and
safely silences static checkers while avoiding unnecessary code complexity.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-mb-perf-core-v1-0-ac56a719c02b@vt.edu?part=3