Re: [PATCH v2 RESEND] Hardware breakpoints: Invoke __perf_event_disable()if interrupts are already disabled

From: Naveen N. Rao
Date: Thu Jul 19 2012 - 07:17:09 EST

On 07/18/2012 05:27 PM, Frederic Weisbecker wrote:
On Wed, Jul 18, 2012 at 04:00:46PM +0530, Naveen N. Rao wrote:
Please find v2 of the patch from Prasad, based on Peter Zijlstra's
feedback. This applies on top of v3.5-rc7. This has been tested and
found to work fine by Edjunior.


From: K.Prasad <Prasad.Krishnan@xxxxxxxxx>

While debugging a warning message on PowerPC while using hardware
breakpoints, it was discovered that when perf_event_disable is invoked
through hw_breakpoint_handler function with interrupts disabled, a
subsequent IPI in the code path would trigger a WARN_ON_ONCE message in
smp_call_function_single function.

This patch calls __perf_event_disable() when interrupts are already
disabled, instead of perf_event_disable().

Reported-by: Edjunior Barbosa Machado <emachado@xxxxxxxxxxxxxxxxxx>
Signed-off-by: K.Prasad <Prasad.Krishnan@xxxxxxxxx>
Signed-off-by: Naveen N. Rao <naveen.n.rao@xxxxxxxxxxxxxxxxxx>
include/linux/perf_event.h | 2 ++
kernel/events/core.c | 2 +-
kernel/events/hw_breakpoint.c | 10 +++++++++-
3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 45db49f..c289ba0 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1292,6 +1292,7 @@ extern int perf_swevent_get_recursion_context(void);
extern void perf_swevent_put_recursion_context(int rctx);
extern void perf_event_enable(struct perf_event *event);
extern void perf_event_disable(struct perf_event *event);
+extern int __perf_event_disable(void *info);
extern void perf_event_task_tick(void);
static inline void
@@ -1330,6 +1331,7 @@ static inline int perf_swevent_get_recursion_context(void) { return -1; }
static inline void perf_swevent_put_recursion_context(int rctx) { }
static inline void perf_event_enable(struct perf_event *event) { }
static inline void perf_event_disable(struct perf_event *event) { }
+static inline int __perf_event_disable(void *info) { }
static inline void perf_event_task_tick(void) { }

diff --git a/kernel/events/core.c b/kernel/events/core.c
index d7d71d6..0ad0fc9 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1253,7 +1253,7 @@ retry:
* Cross CPU call to disable a performance event
-static int __perf_event_disable(void *info)
+int __perf_event_disable(void *info)
struct perf_event *event = info;
struct perf_event_context *ctx = event->ctx;
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index bb38c4d..483f14a 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -453,7 +453,15 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att
int old_type = bp->attr.bp_type;
int err = 0;

- perf_event_disable(bp);
+ /*
+ * modify_user_hw_breakpoint can be invoked with IRQs disabled and hence it
+ * will not be possible to raise IPIs that invoke __perf_event_disable.
+ * So call the function directly.
+ */
+ if (irqs_disabled())
+ __perf_event_disable(bp);
+ else
+ perf_event_disable(bp);

This only works if we are sure the bp is on the current CPU. Do we
have that guarantee?

Yes. This is being hit during bp exception processing and is specific to ppc where we disable interrupts: hw_breakpoint_handler->perf_bp_event->ptrace_triggered->modify_user_hw_breakpoint()


bp->attr.bp_addr = attr->bp_addr;
bp->attr.bp_type = attr->bp_type;

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at