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

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

[Jaidev Shastri via B4 Relay]

From: Jaidev Shastri <jaidevshastri@vt.edu>

init_breakpoint_slots() fills __nr_bp_slots[], cpu_pinned and
tsk_pinned_all and then sets constraints_initialized with a plain store.
__reserve_bp_slot() and hw_breakpoint_is_used() test the flag with a
plain load before they use those tables.

Set the flag with smp_store_release() and read it with
smp_load_acquire().

Found with MBCheck, a static herd7-based memory consistency checker.

Signed-off-by: Jaidev Shastri <jaidevshastri@vt.edu>
---
 kernel/events/hw_breakpoint.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 789add0c1..8532c8a35 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(). */
+	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(). */
+	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);
 

---
base-commit: 93f51579e7df248780214094418f205253383cc5
change-id: 20260921-mb-hw-breakpoint-79dbe06527a9

Best regards,
--  
Jaidev Shastri <jaidevshastri@vt.edu>

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

[sashiko-bot]

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