Re: [Patch 00/12] Hardware Breakpoint Interfaces

Alan Stern
2009-05-12T20:39:28Z

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