[PATCH v2] ARM, ARM64, LONGARCH: Delay HW BP notification to task_work()
Sebastian Andrzej Siewior <[email protected]>
| Newsgroups | dev.linux.lists.linux-rt-devel,dev.linux.lists.loongarch,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-perf-users |
|---|---|
| Message-ID | <[email protected]> |
Waiman, Luis, Ada reported that HW breakpoints on ARM64 trigger "sleeping while atomic" warnings on PREEMPT_RT. The hardware event is delivered with disabled interrupts and perf intrastrucure expects disabled interrupts while the overflow callback is invoked. The callback then sends a SIGTRAP signal for which it acquires sighand_struct::siglock, a spinlock_t which becomes a sleeping lock and must not be acquired in atomic context. Delay the event callback until the return to userland. Add perf_arch_hwbp_notify(), a generic perf callback which delayes the actual callback invocation to task_work_add() callback. This callback invokes the architecture defines callback arch_hwbp_send_sig(). This requires struct callback_head and the functions require ARCH_NEED_PERF_HW_NOTIF to be defined. This was reported against ARM64. ARM and LongARCH follow the same pattern are also converted. Reported-by: Luis Claudio R. Goncalves <[email protected]> Reported-by: Waiman Long <[email protected]> Closes: https://lore.kernel.org/all/[email protected]/ Signed-off-by: Sebastian Andrzej Siewior <[email protected]> --- v1…v2: https://lore.kernel.org/all/[email protected]/ - sashiko complained that a memory breakpoint might trigger several times before a signal is sent if the syscall touches the memory (via get_user()) more than once before returning back. This would lead to list corruption in task_work_add(). To handle this, there is now a variable which is set via xchg before task_work_add() and cleared after the signal has been sent. arch/arm/include/asm/hw_breakpoint.h | 1 + arch/arm/kernel/ptrace.c | 6 ++--- arch/arm64/include/asm/hw_breakpoint.h | 1 + arch/arm64/kernel/ptrace.c | 6 ++--- arch/loongarch/include/asm/hw_breakpoint.h | 1 + arch/loongarch/kernel/ptrace.c | 6 ++--- include/linux/hw_breakpoint.h | 3 +++ include/linux/perf_event.h | 4 ++++ kernel/events/core.c | 26 ++++++++++++++++++++++ 9 files changed, 42 insertions(+), 12 deletions(-) diff --git a/arch/arm/include/asm/hw_breakpoint.h b/arch/arm/include/asm/hw_breakpoint.h index e7f9961c53b2d..5b3a373348ff5 100644 --- a/arch/arm/include/asm/hw_breakpoint.h +++ b/arch/arm/include/asm/hw_breakpoint.h @@ -7,6 +7,7 @@ struct task_struct; #ifdef CONFIG_HAVE_HW_BREAKPOINT +#define ARCH_NEED_PERF_HW_NOTIF struct arch_hw_breakpoint_ctrl { u32 __reserved : 9, diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c index 7951b2c06fec6..1650f12a04702 100644 --- a/arch/arm/kernel/ptrace.c +++ b/arch/arm/kernel/ptrace.c @@ -347,9 +347,7 @@ static long ptrace_hbp_idx_to_num(int idx) /* * Handle hitting a HW-breakpoint. */ -static void ptrace_hbptriggered(struct perf_event *bp, - struct perf_sample_data *data, - struct pt_regs *regs) +void arch_hwbp_send_sig(struct perf_event *bp) { struct arch_hw_breakpoint *bkpt = counter_arch_bp(bp); long num; @@ -424,7 +422,7 @@ static struct perf_event *ptrace_hbp_create(struct task_struct *tsk, int type) attr.bp_type = type; attr.disabled = 1; - return register_user_hw_breakpoint(&attr, ptrace_hbptriggered, NULL, + return register_user_hw_breakpoint(&attr, perf_arch_hwbp_notify, NULL, tsk); } diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h index bd81cf17744af..58befb79c885c 100644 --- a/arch/arm64/include/asm/hw_breakpoint.h +++ b/arch/arm64/include/asm/hw_breakpoint.h @@ -124,6 +124,7 @@ extern void hw_breakpoint_pmu_read(struct perf_event *bp); extern int hw_breakpoint_slots(int type); #ifdef CONFIG_HAVE_HW_BREAKPOINT +#define ARCH_NEED_PERF_HW_NOTIF extern void hw_breakpoint_thread_switch(struct task_struct *next); extern void ptrace_hw_copy_thread(struct task_struct *task); #else diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 4d08598e2891d..df6122779ada6 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -168,9 +168,7 @@ void ptrace_disable(struct task_struct *child) /* * Handle hitting a HW-breakpoint. */ -static void ptrace_hbptriggered(struct perf_event *bp, - struct perf_sample_data *data, - struct pt_regs *regs) +void arch_hwbp_send_sig(struct perf_event *bp) { struct arch_hw_breakpoint *bkpt = counter_arch_bp(bp); const char *desc = "Hardware breakpoint trap (ptrace)"; @@ -312,7 +310,7 @@ static struct perf_event *ptrace_hbp_create(unsigned int note_type, attr.bp_type = type; attr.disabled = 1; - bp = register_user_hw_breakpoint(&attr, ptrace_hbptriggered, NULL, tsk); + bp = register_user_hw_breakpoint(&attr, perf_arch_hwbp_notify, NULL, tsk); if (IS_ERR(bp)) return bp; diff --git a/arch/loongarch/include/asm/hw_breakpoint.h b/arch/loongarch/include/asm/hw_breakpoint.h index 5faa97a87a9e2..2b51922eb5492 100644 --- a/arch/loongarch/include/asm/hw_breakpoint.h +++ b/arch/loongarch/include/asm/hw_breakpoint.h @@ -120,6 +120,7 @@ void breakpoint_handler(struct pt_regs *regs); void watchpoint_handler(struct pt_regs *regs); #ifdef CONFIG_HAVE_HW_BREAKPOINT +#define ARCH_NEED_PERF_HW_NOTIF extern void ptrace_hw_copy_thread(struct task_struct *task); extern void hw_breakpoint_thread_switch(struct task_struct *next); #else diff --git a/arch/loongarch/kernel/ptrace.c b/arch/loongarch/kernel/ptrace.c index be38430f7e280..1d5e05a70b8d3 100644 --- a/arch/loongarch/kernel/ptrace.c +++ b/arch/loongarch/kernel/ptrace.c @@ -384,9 +384,7 @@ static int lbt_set(struct task_struct *target, /* * Handle hitting a HW-breakpoint. */ -static void ptrace_hbptriggered(struct perf_event *bp, - struct perf_sample_data *data, - struct pt_regs *regs) +void arch_hwbp_send_sig(struct perf_event *bp) { int i; struct arch_hw_breakpoint *bkpt = counter_arch_bp(bp); @@ -479,7 +477,7 @@ static struct perf_event *ptrace_hbp_create(unsigned int note_type, attr.bp_type = type; attr.disabled = 1; - bp = register_user_hw_breakpoint(&attr, ptrace_hbptriggered, NULL, tsk); + bp = register_user_hw_breakpoint(&attr, perf_arch_hwbp_notify, NULL, tsk); if (IS_ERR(bp)) return bp; diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h index db199d653dd1a..bb4c1064a103d 100644 --- a/include/linux/hw_breakpoint.h +++ b/include/linux/hw_breakpoint.h @@ -85,6 +85,9 @@ extern int register_perf_hw_breakpoint(struct perf_event *bp); extern void unregister_hw_breakpoint(struct perf_event *bp); extern void unregister_wide_hw_breakpoint(struct perf_event * __percpu *cpu_events); extern bool hw_breakpoint_is_used(void); +extern void arch_hwbp_send_sig(struct perf_event *bp); +extern void perf_arch_hwbp_notify(struct perf_event *bp, struct perf_sample_data *data, + struct pt_regs *regs); extern int dbg_reserve_bp_slot(struct perf_event *bp); extern int dbg_release_bp_slot(struct perf_event *bp); diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 48d851fbd8ea5..45c8391975007 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -215,6 +215,10 @@ struct hw_perf_event { /* Last sync'ed generation of filters */ unsigned long addr_filters_gen; +#ifdef ARCH_NEED_PERF_HW_NOTIF + struct callback_head arch_hw_notif; + int arch_hw_notif_busy; +#endif /* * hw_perf_event::state flags; used to track the PERF_EF_* state. diff --git a/kernel/events/core.c b/kernel/events/core.c index ba5bd6a78fe7b..1ca91208e1524 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -13338,6 +13338,28 @@ static void account_event(struct perf_event *event) account_pmu_sb_event(event); } +#ifdef ARCH_NEED_PERF_HW_NOTIF +static void perf_arch_hwbp_send_sig(struct callback_head *head) +{ + struct perf_event *bp; + + bp = container_of(head, struct perf_event, hw.arch_hw_notif); + arch_hwbp_send_sig(bp); + xchg_relaxed(&bp->hw.arch_hw_notif_busy, 0); + put_event(bp); +} + +void perf_arch_hwbp_notify(struct perf_event *bp, struct perf_sample_data *data, + struct pt_regs *regs) +{ + if (WARN_ON_ONCE(!atomic_long_inc_not_zero(&bp->refcount))) + return; + if (xchg_relaxed(&bp->hw.arch_hw_notif_busy, 1) || + WARN_ON_ONCE(task_work_add(current, &bp->hw.arch_hw_notif, TWA_RESUME))) + put_event(bp); +} +#endif + /* * Allocate and initialize an event structure */ @@ -13443,6 +13465,10 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu, } if (overflow_handler) { +#ifdef ARCH_NEED_PERF_HW_NOTIF + if (overflow_handler == perf_arch_hwbp_notify) + init_task_work(&event->hw.arch_hw_notif, perf_arch_hwbp_send_sig); +#endif event->overflow_handler = overflow_handler; event->overflow_handler_context = context; } else if (is_write_backward(event)){ -- 2.55.0