lkml.org 
[lkml]   [2013]   [Dec]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH RFC v4 1/6] arm64: support single-step and breakpoint handler hooks
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@arm.com>
Cheers,

Will


\
 
 \ /
  Last update: 2013-12-03 21:01    [W:2.042 / U:0.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site