Thread (36 messages) read the whole thread 36 messages, 5 authors, 2025-09-10

Re: [PATCH v2 06/18] perf/hw_breakpoint: add arch-independent hw_breakpoint_modify_local()

From: Peter Zijlstra <peterz@infradead.org>
Date: 2025-09-04 06:45:15
Also in: linux-mm, linux-perf-users, lkml

On Thu, Sep 04, 2025 at 08:21:03AM +0800, Jinchao Wang wrote:
quoted hunk ↗ jump to hunk
Introduce hw_breakpoint_modify_local() as a generic helper to modify an
existing hardware breakpoint. The function invokes
hw_breakpoint_arch_parse() and delegates the reinstall step to the
architecture via arch_reinstall_hw_breakpoint().

A weak default implementation of arch_reinstall_hw_breakpoint() is
provided, returning -EOPNOTSUPP on architectures without support.

This makes the interface arch-independent while allowing x86 (and others)
to provide their own implementation.

Signed-off-by: Jinchao Wang <redacted>
---
 include/linux/hw_breakpoint.h |  1 +
 kernel/events/hw_breakpoint.c | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)
diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
index db199d653dd1..9453b5bdb443 100644
--- a/include/linux/hw_breakpoint.h
+++ b/include/linux/hw_breakpoint.h
@@ -67,6 +67,7 @@ extern int
 modify_user_hw_breakpoint_check(struct perf_event *bp, struct perf_event_attr *attr,
 				bool check);
 
+int hw_breakpoint_modify_local(struct perf_event *bp, struct perf_event_attr *attr);
 /*
  * Kernel breakpoints are not associated with any particular thread.
  */
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 8ec2cb688903..ff428739f71e 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -983,6 +983,24 @@ static void hw_breakpoint_del(struct perf_event *bp, int flags)
 	arch_uninstall_hw_breakpoint(bp);
 }
 
+int hw_breakpoint_modify_local(struct perf_event *bp, struct perf_event_attr *attr)
+{
+	int err;
+
+	err = hw_breakpoint_arch_parse(bp, attr, counter_arch_bp(bp));
+	if (err)
+		return err;
+
+	return arch_reinstall_hw_breakpoint(bp);
+}
+EXPORT_SYMBOL(hw_breakpoint_modify_local);
+
+/* weak fallback for arches without support */
+__weak int arch_reinstall_hw_breakpoint(struct perf_event *bp)
+{
+	return -EOPNOTSUPP;
+}
Again, so much fail :/

So we have:

{register,modify,unregister}_user_hw_breakpoint()

and

{register,unregister}_wide_hw_breakpoint()

And you choose to extend this latter with hw_breakpoint_modify_local()
instead of sticking with the naming scheme and say adding:

modify_wide_hw_breakpoint_local().

Also, again, that EXPORT is a fail, these other interfaces are all
EXPORT_SYMBOL_GPL().

Also note that modify_user_hw_breakpoint() doesn't seem to need new arch
hooks. Yet you fail to explain why you think you do.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help