Re: [PATCH v2] ARM, ARM64, LONGARCH: Delay HW BP notification to task_work()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Deadlock/hang in blocking syscalls due to missing TWA_SIGNAL when delaying hardware breakpoint notification
--

--- Patch [1]: [PATCH v2] ARM, ARM64, LONGARCH: Delay HW BP notification to task_work() ---
commit 5153bc880ca6886ee1fe14a21ba3ae8477562c21
Author: Sebastian Andrzej Siewior <[email protected]>

ARM, ARM64, LONGARCH: Delay HW BP notification to task_work()
    
This commit delays hardware breakpoint event callbacks to task_work to
prevent "sleeping while atomic" warnings on PREEMPT_RT. The architecture-
specific callbacks now use perf_arch_hwbp_notify() to queue the SIGTRAP
delivery until the return to userland.

Closes: https://lore.kernel.org/all/[email protected]/

> 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)))
[Severity: High]
Will using TWA_RESUME here cause a hang if a hardware watchpoint is triggered
during a blocking syscall like futex() or read()?

When a blocking syscall accesses watched memory (e.g., via get_user()), the
watchpoint synchronously triggers and queues the SIGTRAP task work via
task_work_add(). Since TWA_RESUME only sets TIF_NOTIFY_RESUME, and
signal_pending() ignores TIF_NOTIFY_RESUME, the syscall wait loop will not
abort. 

Does this need to use TWA_SIGNAL instead so that TIF_NOTIFY_SIGNAL is set and
the interruptible sleep correctly aborts to deliver the pending task work?

> +		put_event(bp);
> +}
> +#endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1