Re: [PATCH] perf/hw_breakpoint: publish constraints_initialized with release semantics

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf: incorrect function name in memory barrier comment
--

commit 663e001a0e49d6fb96e95a7c75fb6d384e54fc61
Author: Jaidev Shastri <jaidevshastri@vt.edu>

perf/hw_breakpoint: publish constraints_initialized with release semantics

This commit updates the constraints_initialized flag to be written with
smp_store_release() and read with smp_load_acquire(). This ensures proper
memory ordering when __reserve_bp_slot() and hw_breakpoint_is_used() access
the breakpoint slot tables.

>     init_breakpoint_slots() fills __nr_bp_slots[], cpu_pinned and
>     tsk_pinned_all and then sets constraints_initialized with a plain store.

[Severity: Medium]
Does the commit message reference the wrong function here? The diff shows the
store to constraints_initialized is actually happening in init_hw_breakpoint()
rather than init_breakpoint_slots().

> diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
> index 789add0c185a0..8532c8a35b839 100644
> --- a/kernel/events/hw_breakpoint.c
> +++ b/kernel/events/hw_breakpoint.c
> @@ -577,7 +577,8 @@ static int __reserve_bp_slot(struct perf_event *bp, u64 bp_type)
>  	int weight;
>  
>  	/* We couldn't initialize breakpoint constraints on boot */
> -	if (!constraints_initialized)
> +	/* Pairs with the smp_store_release() in init_breakpoint_slots(). */

[Severity: Medium]
Does this comment misidentify the function containing the pairing store? As
seen below in the diff, the smp_store_release() is located in
init_hw_breakpoint().

> +	if (!smp_load_acquire(&constraints_initialized))
>  		return -ENOMEM;
>  
>  	/* Basic checks */

[ ... ]

> @@ -897,7 +898,8 @@ bool hw_breakpoint_is_used(void)
>  {
>  	int cpu;
>  
> -	if (!constraints_initialized)
> +	/* Pairs with the smp_store_release() in init_breakpoint_slots(). */

[Severity: Medium]
Similarly, does this comment also misidentify the pairing function?

> +	if (!smp_load_acquire(&constraints_initialized))
>  		return false;
>  
>  	for_each_possible_cpu(cpu) {

[ ... ]

> @@ -1017,7 +1019,8 @@ int __init init_hw_breakpoint(void)
>  	if (ret)
>  		return ret;
>  
> -	constraints_initialized = true;
> +	/* Pairs with the smp_load_acquire() in the slot reservation paths. */
> +	smp_store_release(&constraints_initialized, true);
>  
>  	perf_pmu_register(&perf_breakpoint, "breakpoint", PERF_TYPE_BREAKPOINT);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-mb-hw-breakpoint-v1-1-59aa06e1acf9@vt.edu?part=1