Re: [PATCH v3 03/13] arm64: debug: call software break handlers statically

[Mark Rutland <mark.rutland@arm.com>]

On Mon, Jun 16, 2025 at 02:29:37PM +0100, Ada Couprie Diaz wrote:
> On 13/06/2025 07:11, Anshuman Khandual wrote:
> 
> > A small nit - s/break handlers/break point handlers/
> You are right, checking again I was using too small a character limit for
> the summary line (55), which is not relevant. I will write `breakpoint` in
> full, I wasn't happy about leaving it out !

FWIW, saying "software breakpoint" sounds good to me.

[...]

> > On 09/06/25 11:04 PM, Ada Couprie Diaz wrote:
> > > Unify the naming of the software breakpoint handlers to XXX_brk_handler(),
> > > making it clear they are related and to differentiate from the
> > > hardware breakpoints.
> > Unless absolutely necessary - could we please move these renames into a
> > separate patch in itself instead ? That will reduce the churn and help
> > the reviewers see the functional changes more clearly.
> Fair enough, I can move the renames to a later patch to avoid renaming in
> all the places
> that get removed in this patch.
> Would it make sense to combine it with the single step handler renames in
> this case, or would it be better to have two independent commits ?

TBH I think this is fine as-is and doesn't need to be split out. The
renames aid clarity, and the churn to early_brk64() is small.

[...]

> > >   void __init trap_init(void)
> > >   {
> > > -	register_kernel_break_hook(&bug_break_hook);
> > > -#ifdef CONFIG_CFI_CLANG
> > > -	register_kernel_break_hook(&cfi_break_hook);
> > > -#endif
> > > -	register_kernel_break_hook(&fault_break_hook);
> > > -#ifdef CONFIG_KASAN_SW_TAGS
> > > -	register_kernel_break_hook(&kasan_break_hook);
> > > -#endif
> > > -#ifdef CONFIG_UBSAN_TRAP
> > > -	register_kernel_break_hook(&ubsan_break_hook);
> > > -#endif
> > >   	debug_traps_init();
> > >   }
> > debug_traps_init() can be renamed as trap_init() and just drop this
> > redundant indirection. All applicable comments can also be changed
> > as required there after.
> I understand what you mean, but I would be tempted to not change it,
> with the following reasons :
>  - `debug_traps_init()` gets removed entirely in the last commit,
>  - having `trap_init()` in `traps.c` makes more sense than
> `debug-monitors.c` to me
>  - `trap_init()` ends up empty at the end of the series, it could make sense
> to simply
>    remove it entirely, given there is an empty weak definition in
> `init/main.c` already.
> 
> What do you think ?

I agree with your reasoning, please leave it as-is!

Mark.