[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Pine.LNX.4.44L0.0905200951030.2914-100000@iolanthe.rowland.org>
Date: Wed, 20 May 2009 10:04:09 -0400 (EDT)
From: Alan Stern <stern@...land.harvard.edu>
To: "K.Prasad" <prasad@...ux.vnet.ibm.com>
cc: Frederic Weisbecker <fweisbec@...il.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...e.hu>
Subject: Re: [Patch 1/2] Improvements and minor fixes to HW Breakpoint
interface
On Wed, 20 May 2009, K.Prasad wrote:
> --- linux-2.6-tip.hbkpt.orig/arch/x86/kernel/hw_breakpoint.c
> +++ linux-2.6-tip.hbkpt/arch/x86/kernel/hw_breakpoint.c
> @@ -296,6 +296,19 @@ void arch_flush_thread_hw_breakpoint(str
>
> /*
> * Handle debug exception notifications.
> + *
> + * Return value is either NOTIFY_STOP or NOTIFY_DONE as explained below.
> + *
> + * NOTIFY_DONE returned if one of the following conditions is true.
> + * i) When the causative address is from user-space and the exception
> + * is a valid one i.e. not triggered a result of lazy debug register
--------------------^----^ missing ',' characters
---------------------------------------^ missing "as"
> + * switching
> + * ii) When there are more bits than trap<n> set in DR6 register (such
> + * as BD, BS or BT) indicating that more than one debug condition is
> + * met and requires some more action in do_debug().
> + *
> + * NOTIFY_STOP returned for all other cases
> + *
> */
> int __kprobes hw_breakpoint_handler(struct die_args *args)
> {
> @@ -346,8 +359,10 @@ int __kprobes hw_breakpoint_handler(stru
> * or due to the delay between updates of hbp_kernel_pos
> * and this_hbp_kernel.
> */
> - if (!bp)
> + if (!bp) {
> + rc = NOTIFY_STOP;
> continue;
> + }
>
> (bp->triggered)(bp, args->regs);
This is the same mistake as before. If rc had previously been set to
NOTIFY_DONE then you will unconditionally change it back to
NOTIFY_STOP. You should make the code do what the comment says, set rc
to NOTIFY_DONE when a non-NULL user breakpoint occurs:
if (i >= hbp_kernel_pos) {
bp = per_cpu(this_hbp_kernel[i], cpu);
} else {
bp = current->thread.hbp[i];
if (bp)
rc = NOTIFY_DONE;
}
/*
* bp can be NULL due to lazy debug register switching
* or due to the delay between updates of hbp_kernel_pos
* and this_hbp_kernel.
*/
if (!bp)
continue;
(bp->triggered)(bp, args->regs);
The rest of the changes are okay.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists