[PATCH RFC v4 1/6] arm64: support single-step and breakpoint handler hooks

Will Deacon will.deacon at arm.com
Tue Dec 3 14:44:12 EST 2013


On Tue, Dec 03, 2013 at 02:33:17PM +0000, Sandeepa Prabhu wrote:
> Hi Will,
> 
> Sorry for responding to this after long-time, I missed this review
> during Linaro connect travels.

No problem.

> >> @@ -215,7 +257,10 @@ static int single_step_handler(unsigned long addr, unsigned int esr,
> >>                */
> >>               user_rewind_single_step(current);
> >>       } else {
> >> -             /* TODO: route to KGDB */
> >> +             /* call registered single step handlers */
> >
> > Don't bother with this comment (it's crystal clear from the code).
> OK, I will remove this unnecessary print.

Thanks.

> >> +static LIST_HEAD(break_hook);
> >> +DEFINE_RWLOCK(break_hook_lock);
> >
> > This guy can be a plain old spinlock. That way, the readers have less
> > overhead but things still work because we only call a single hook function.
> well, kprobes need to support recursive breakpoints (i.e. breakpoint
> handler executing BRK once again)
> so I converted this lock to rw_lock.  I should put this info in commit
> description to be more clearer.

Actually, this is one place where a comment in the code *would* be useful!

> Let me know if you find any issue with re-cursing in breakpoint exception?

Sounds ok to me. With those changes:

  Acked-by: Will Deacon <will.deacon at arm.com>

Cheers,

Will



More information about the linux-arm-kernel mailing list