| Message ID | 20100609102559.GF20332@in.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Paul Mackerras |
| Headers | show |
On Wed, Jun 09, 2010 at 03:55:59PM +0530, K.Prasad wrote: > + if (!((bp->attr.bp_addr <= dar) && > + (dar <= (bp->attr.bp_addr + bp->attr.bp_len)))) { > + /* > + * This exception is triggered not because of a memory access > + * on the monitored variable but in the double-word address > + * range in which it is contained. We will consume this > + * exception, considering it as 'noise'. > + */ > + info->extraneous_interrupt = true; > + } Ummm, don't you need to add "else info->extraneous_interrupt = false;" here? I don't see anywhere that you ever clear it otherwise. Also, I think you need to do the "if (!info->extraneous_interrupt)" check around the call to perf_bp_event() later on in hw_breakpoint_handler() as well as around the call in single_step_dabr_instruction(). Paul.
On Thu, Jun 10, 2010 at 10:40:24PM +1000, Paul Mackerras wrote: > On Wed, Jun 09, 2010 at 03:55:59PM +0530, K.Prasad wrote: > > > + if (!((bp->attr.bp_addr <= dar) && > > + (dar <= (bp->attr.bp_addr + bp->attr.bp_len)))) { > > + /* > > + * This exception is triggered not because of a memory access > > + * on the monitored variable but in the double-word address > > + * range in which it is contained. We will consume this > > + * exception, considering it as 'noise'. > > + */ > > + info->extraneous_interrupt = true; > > + } > > Ummm, don't you need to add "else info->extraneous_interrupt = false;" > here? I don't see anywhere that you ever clear it otherwise. > > Also, I think you need to do the "if (!info->extraneous_interrupt)" > check around the call to perf_bp_event() later on in > hw_breakpoint_handler() as well as around the call in > single_step_dabr_instruction(). > > Paul. True, I've added the check before perf_bp_event() (wherever it was missing before) in the patchset ver XXIV. Thanks, K.Prasad
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c =================================================================== --- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/hw_breakpoint.c +++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c @@ -202,6 +202,7 @@ int __kprobes hw_breakpoint_handler(stru struct pt_regs *regs = args->regs; int stepped = 1; struct arch_hw_breakpoint *info; + unsigned long dar = regs->dar; /* Disable breakpoints during exception handling */ set_dabr(0); @@ -232,6 +233,21 @@ int __kprobes hw_breakpoint_handler(stru goto out; } + /* + * Verify if dar lies within the address range occupied by the symbol + * being watched to filter extraneous exceptions. + */ + if (!((bp->attr.bp_addr <= dar) && + (dar <= (bp->attr.bp_addr + bp->attr.bp_len)))) { + /* + * This exception is triggered not because of a memory access + * on the monitored variable but in the double-word address + * range in which it is contained. We will consume this + * exception, considering it as 'noise'. + */ + info->extraneous_interrupt = true; + } + /* Do not emulate user-space instructions, instead single-step them */ if (user_mode(regs)) { bp->ctx->task->thread.last_hit_ubp = bp; @@ -286,7 +302,8 @@ int __kprobes single_step_dabr_instructi * We shall invoke the user-defined callback function in the single * stepping handler to confirm to 'trigger-after-execute' semantics */ - perf_bp_event(bp, regs); + if (!bp_info->extraneous_interrupt) + perf_bp_event(bp, regs); /* * Do not disable MSR_SE if the process was already in Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h =================================================================== --- linux-2.6.ppc64_test.orig/arch/powerpc/include/asm/hw_breakpoint.h +++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h @@ -27,6 +27,7 @@ #ifdef CONFIG_HAVE_HW_BREAKPOINT struct arch_hw_breakpoint { + bool extraneous_interrupt; u8 len; /* length of the target data symbol */ int type; unsigned long address;