Re: [PATCH v1 4/6] LoongArch: Only send SIGTRAP signal if necessary in do_watch()

[Huacai Chen]

Hi, Tiezhu,

On Wed, Aug 26, 2026 at 7:03 PM Tiezhu Yang <yangtiezhu@xxxxxxxxxxx> wrote:
>
> In do_watch(), the kernel unconditionally forces a SIGTRAP signal at the
> end of the handler via force_sig(SIGTRAP). This is essential for ptrace
> operations, but it severely disrupts standard perf_event usage.
>
> Under normal perf usage, it is not necessary to force a SIGTRAP signal
> on every hit. Forcing it unnecessarily aborts the target process if no
> user-space signal handler is registered, which violates performance
> monitoring behaviors and injects context switch overheads.
>
> However, a conditional check using only 'current->ptrace' would block
> legitimate user-requested signals when a perf_event is configured with
> 'attr.sigtrap = 1' for asynchronous user-space tracking.
>
> Fix this by refactoring breakpoint_handler() and watchpoint_handler()
> to return a boolean status indicating whether any triggered breakpoint
> explicitly requires a signal notification. Update do_watch() to enforce
> the SIGTRAP signal only when the process is actively being debugged via
> ptrace, or when the underlying perf_event infrastructure specifically
> demands it via 'attr.sigtrap'.
>
> Fixes: 424421a7f34c ("LoongArch: ptrace: Add hardware single step support")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Tiezhu Yang <yangtiezhu@xxxxxxxxxxx>
> ---
>  arch/loongarch/include/asm/hw_breakpoint.h |  4 ++--
>  arch/loongarch/kernel/hw_breakpoint.c      | 18 ++++++++++++++++--
>  arch/loongarch/kernel/traps.c              |  8 +++++---
>  3 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/hw_breakpoint.h b/arch/loongarch/include/asm/hw_breakpoint.h
> index 5faa97a87a9e..d202052df8a1 100644
> --- a/arch/loongarch/include/asm/hw_breakpoint.h
> +++ b/arch/loongarch/include/asm/hw_breakpoint.h
> @@ -116,8 +116,8 @@ extern void arch_uninstall_hw_breakpoint(struct perf_event *bp);
>  extern int hw_breakpoint_slots(int type);
>  extern void hw_breakpoint_pmu_read(struct perf_event *bp);
>
> -void breakpoint_handler(struct pt_regs *regs);
> -void watchpoint_handler(struct pt_regs *regs);
> +bool breakpoint_handler(struct pt_regs *regs);
> +bool watchpoint_handler(struct pt_regs *regs);
>
>  #ifdef CONFIG_HAVE_HW_BREAKPOINT
>  extern void ptrace_hw_copy_thread(struct task_struct *task);
> diff --git a/arch/loongarch/kernel/hw_breakpoint.c b/arch/loongarch/kernel/hw_breakpoint.c
> index 7f69cf361a5e..3683a52b2368 100644
> --- a/arch/loongarch/kernel/hw_breakpoint.c
> +++ b/arch/loongarch/kernel/hw_breakpoint.c
> @@ -482,10 +482,11 @@ NOKPROBE_SYMBOL(update_bp_registers);
>  /*
>   * Debug exception handlers.
>   */
> -void breakpoint_handler(struct pt_regs *regs)
> +bool breakpoint_handler(struct pt_regs *regs)
>  {
>         int i;
>         struct perf_event *bp, **slots;
> +       bool need_sigtrap = false;
>
>         slots = this_cpu_ptr(bp_on_reg);
>
> @@ -494,18 +495,25 @@ void breakpoint_handler(struct pt_regs *regs)
>                         bp = slots[i];
>                         if (bp == NULL)
>                                 continue;
> +
>                         perf_bp_event(bp, regs);
> +                       if (bp->attr.sigtrap)
> +                               need_sigtrap = true;
> +
>                         csr_write32(0x1 << i, LOONGARCH_CSR_FWPS);
>                         update_bp_registers(regs, 0, 0);
>                 }
>         }
> +
> +       return need_sigtrap;
>  }
>  NOKPROBE_SYMBOL(breakpoint_handler);
>
> -void watchpoint_handler(struct pt_regs *regs)
> +bool watchpoint_handler(struct pt_regs *regs)
>  {
>         int i;
>         struct perf_event *wp, **slots;
> +       bool need_sigtrap = false;
>
>         slots = this_cpu_ptr(wp_on_reg);
>
> @@ -514,11 +522,17 @@ void watchpoint_handler(struct pt_regs *regs)
>                         wp = slots[i];
>                         if (wp == NULL)
>                                 continue;
> +
>                         perf_bp_event(wp, regs);
> +                       if (wp->attr.sigtrap)
> +                               need_sigtrap = true;
> +
>                         csr_write32(0x1 << i, LOONGARCH_CSR_MWPS);
>                         update_bp_registers(regs, 0, 1);
>                 }
>         }
> +
> +       return need_sigtrap;
>  }
>  NOKPROBE_SYMBOL(watchpoint_handler);
>
> diff --git a/arch/loongarch/kernel/traps.c b/arch/loongarch/kernel/traps.c
> index c4d7e55fb3ea..de0c9b03d551 100644
> --- a/arch/loongarch/kernel/traps.c
> +++ b/arch/loongarch/kernel/traps.c
> @@ -811,6 +811,7 @@ asmlinkage void noinstr do_bp(struct pt_regs *regs)
>  asmlinkage void noinstr do_watch(struct pt_regs *regs)
>  {
>         irqentry_state_t state = irqentry_enter(regs);
> +       bool perf_sigtrap = false;
This is not for perf only, so need_sigtrap is better.

>
>  #ifndef CONFIG_HAVE_HW_BREAKPOINT
>         pr_warn("Hardware watch point handler not implemented!\n");
> @@ -851,11 +852,12 @@ asmlinkage void noinstr do_watch(struct pt_regs *regs)
>                         }
>                 }
>         } else {
> -               breakpoint_handler(regs);
> -               watchpoint_handler(regs);
> +               perf_sigtrap |= breakpoint_handler(regs);
> +               perf_sigtrap |= watchpoint_handler(regs);
Let breakpoint_handler() and watchpoint_handler() return "int", then
see below...

>         }
>
> -       force_sig(SIGTRAP);
> +       if (current->ptrace || perf_sigtrap)
Define need_sigtrap as "int" and initialize it as current->ptrace,
then we can only use "if (need_sigtrap)" here.

Huacai

> +               force_sig(SIGTRAP);
>  out:
>  #endif
>         irqentry_exit(regs, state);
> --
> 2.42.0
>