[PATCH v3 03/13] arm64: debug: call software break handlers statically
Mark Rutland
mark.rutland at arm.com
Wed Jun 18 03:28:45 PDT 2025
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.
More information about the linux-arm-kernel
mailing list