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.dea...@arm.com>
Cheers,
Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
