From: Frederic Weisbecker on
On Fri, Jul 23, 2010 at 10:16:01AM +0800, Dongdong Deng wrote:
> The hw_breakpoint subsystem consumes all the hardware
> breakpoint exceptions since it hooks the notify_die
> handlers first, this means that kgdb doesn't get the
> opportunity to handle hw breakpoint exceptions generated
> by kgdb itself.
>
> This patch adds an extend flag to perf_event_attr for
> hw_breakpoint_handler() to decide to pass or stop the
> DIE_DEBUG notification.
>
> As KGDB set that flag, hw_breakpoint_handler() will pass
> the DIE_DEBUG notification, thus kgdb have the chance
> to take DIE_DEBUG notification.
>
> Signed-off-by: Dongdong Deng <dongdong.deng(a)windriver.com>
> Reviewed-by: Bruce Ashfield <bruce.ashfield(a)windriver.com>
> ---
> arch/x86/kernel/hw_breakpoint.c | 14 ++++++++++++++
> arch/x86/kernel/kgdb.c | 2 ++
> include/linux/perf_event.h | 9 +++++++++
> 3 files changed, 25 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
> index a8f1b80..b38f786 100644
> --- a/arch/x86/kernel/hw_breakpoint.c
> +++ b/arch/x86/kernel/hw_breakpoint.c
> @@ -406,6 +406,8 @@ EXPORT_SYMBOL_GPL(hw_breakpoint_restore);
> * 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().
> + * iii) The source of hw breakpoint event want to handle the event
> + * by itself, currently just KGDB have this notion.
> *
> * NOTIFY_STOP returned for all other cases
> *
> @@ -464,6 +466,18 @@ static int __kprobes hw_breakpoint_handler(struct die_args *args)
> break;
> }
>
> + if (bp->attr.flag == SKIP_HWBP_EVENT_PERF_FLAG) {
> + /*
> + * when attr.flag is set to SKIP_HWBP_EVENT_PERF_FLAG
> + * it indicates currently hw breakpoint event
> + * source want to handle this event by itself.
> + * thus return NOTIFY_DONE here.
> + */
> + rc = NOTIFY_DONE;
> + rcu_read_unlock();
> + break;
> + }
> +



No. We really shouldn't make a user ABI change (adding attr.flag) just
to solve an in-kernel-only problem.

And moreover we probably don't need flags at all. Why not just turning kgdb handler
into a higher priority?

I don't even remember why kgdb has its own handler instead of using the
struct perf_event:overflow_handler. May be that's because of the early breakpoints.

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