From: Jaidev Shastri <redacted>
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 <redacted>
---
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 [off-list ref]
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 [off-list ref]
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().
quoted hunk
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().
quoted hunk
+ if (!smp_load_acquire(&constraints_initialized))
return -ENOMEM;
/* Basic checks */
[ ... ]
quoted hunk
@@ -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?
quoted hunk
+ if (!smp_load_acquire(&constraints_initialized))
return false;
for_each_possible_cpu(cpu) {
[ ... ]
quoted hunk
@@ -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