Messages in this thread | | | Date | Fri, 30 Oct 2009 03:04:40 +0530 | From | "K.Prasad" <> | Subject | Re: [RFC Patch 2/4] Allow breakpoints to be enabled/disabled without yielding the breakpoint request through new APIs - <enable><disable>_hw_breakpoint() |
| |
On Wed, Oct 28, 2009 at 04:31:49PM -0400, Steven Rostedt wrote: > On Tue, 2009-10-27 at 02:49 +0530, K.Prasad wrote: > > > Index: linux-2.6-tip.perf_hbkpt/include/asm-generic/hw_breakpoint.h > > =================================================================== > > --- linux-2.6-tip.perf_hbkpt.orig/include/asm-generic/hw_breakpoint.h > > +++ linux-2.6-tip.perf_hbkpt/include/asm-generic/hw_breakpoint.h > > @@ -102,11 +102,22 @@ > > * ---------------------------------------------------------------------- > > */ > > struct hw_breakpoint { > > + /* > > + * Denotes if a breakpoint is currently enabled in physical debug > > + * registers. Not to be set directly by the end-user. Must be > > + * operated through <enable><disable>_hw_breakpoint() APIs only. > > + */ > > + atomic_t enabled; > > Just a nit, but by looking at this structure as a whole, it's not easy > to see that the above comment is talking just about the "enabled" part > of the structure or the entire structure itself. > > Probably want to add ... > > /* > * The enabled item denotes if ... > > > -- Steve >
I will modify the comments to read as under...the role of the 'enabled' flag has undergone changes and the comments will reflect that.
struct hw_breakpoint { + /* + * The 'enabled' flag denotes if a breakpoint hit would in-turn invoke + * the 'triggered' function. Not to be set directly by the end-user. + * Must be operated through <enable><disable>_hw_breakpoint() APIs only. + */ + atomic_t enabled; void (*triggered)(struct hw_breakpoint *, struct pt_regs *); const cpumask_t *cpumask; struct arch_hw_breakpoint info;
Thanks, K.Prasad
| |