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

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 !

On 09/06/25 11:04 PM, Ada Couprie Diaz wrote:

quoted

[...]

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 ?

quoted

[...]
   
  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 ?

Thanks,
Ada