Re: [Patch 00/12] Hardware Breakpoint Interfaces

[Alan Stern]

On Tue, 12 May 2009, K.Prasad wrote:

> hmmh...The (un)register_user_<> and __(un)register_user_<> break-up
> looks so modular and am not sure if combining them would be of any
> benefit (and as you might be aware, the compiler can always choose
> to inline though). And doing this might not be helpful if we like to
> virtualise the debug registers (as in the original implementation) in
> the future.

You can keep them separate if you like, but then at least make the "__" 
routines static and remove them from the .h file.

> The __modify_user_hw_breakpoint() had the "__" prefix because it had
> "int pos" as a parameter, although it can be eliminated by adding a loop
> that iterates through the breakpoints of "tsk". If you prefer
> modify_user_hw_breakpoint(), I will introduce the same in
> kernel/hw_breakpoint.c which acts as a wrapper over __modify_user_<> and
> takes the hw_breakpoint_lock.
> 
> Let me know what you think on the above.

Go ahead and introduce modify_user_hw_breakpoint().  Then the spinlock
can become static.


> Ok. Will change the code as suggested. The return values in the above
> implementation will be modified in the second pass here:
> 
> rc = __modify_user_hw_breakpoint(i, tsk, bp);
> 
> I will save the 'rc' value from the first pass in a separate variable
> and use that as the return value. Operations in the second pass (such as
> unregister/modify are not expected to fail).

That's right.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/