On Fri, Jun 19, 2009 at 03:04:09PM +1000, David Gibson wrote: > On Thu, Jun 18, 2009 at 11:50:45PM +0530, K.Prasad wrote: > > On Wed, Jun 17, 2009 at 02:32:24PM +1000, David Gibson wrote: > > > On Wed, Jun 10, 2009 at 02:38:06PM +0530, K.Prasad wrote: > [snip]
With apologies for the long delay here's my response. Some work that was required to push the generic breakpoint patches into mainline (which is now pushed to 2.6.32 merge window) and the preparation of a paper kept me away from responding. Also to add that I will be on a week long vacation starting next week followed by travel to Linux Symposium and I will not be able to actively respond to mails till the week beginning 20th of this month. But kindly drop in your comments as I would like to have the PPC64 patches ready well before the next merge window opens! > > Ah, right, yes. This seems a really non obvious control flow for a > NULL triggered value to lead to EINVAL. I'd prefer: > > if (!bp->triggered) > return -EINVAL > > /* Then the kernel address test */ > > return arch_store_info(bp, tsk); > Ok. It is a minor change and I will do it. > [snip] > > > > + /* Verify if dar lies within the address range occupied by the > > > > symbol > > > > + * being watched. Since we cannot get the symbol size for > > > > + * user-space requests we skip this check in that case > > > > + */ > > > > + if ((hbp_kernel_pos == 0) && > > > > + !((bp->info.address <= dar) && > > > > + (dar <= (bp->info.address + bp->info.symbolsize)))) > > > > + /* > > > > + * 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'. > > > > + */ > > > > + goto out; > > > > + > > > > + (bp->triggered)(bp, regs); > > > > > > So this confuses me. You go to great efforts to step over the > > > instruction to generate a SIGTRAP after the instruction, for > > > consistency with x86. But that SIGTRAP is *never* used, since the > > > only way to set userspace breakpoints is through ptrace at the moment. > > > At the same time, the triggered function is called here before the > > > instruction is executed, so not consistent with x86 anyway. If the SIGTRAP generation code is in ptrace_triggered() then it would benefit only ptrace (although it is the lone user of HW-breakpoints at the moment). Given that SIGTRAP is the only means for the user-space to detect a breakpoint 'hit' on one of its monitored addresses, we will generate it for all cases by keeping the code in hw_breakpoint.c (and not in ptrace_triggered()). However I believe that your concern here is about the timing of the SIGTRAP signal generation and I agree with it...please find further comments about it below. > > > > > > It just seems strange to me that sending a SIGTRAP is a special case > > > anyway. Why can't sending a SIGTRAP be just a particular triggered > > > function. > > > > The consistency in the interface across architectures that I referred to > > in my previous mail was w.r.t. continuous triggering of breakpoints and > > not to implement a trigger-before or trigger-after behaviour across > > architectures. In fact, this behaviour differs even on the same > > processor depending upon the breakpoint type (trigger-before for > > instructions and trigger-after for data in x86), and introducing > > uniformity might be a) at the cost of loss of some unique & innovative > > uses for each of them b) may not be always possible e.g. trigger-after > > to trigger-before. > > Hrm. Well (a) is why I was suggesting an option to allow trigger > before on powerpc. Plus, I don't see why you think (a) is important > for the triggered function, but not for the timing of a SIGTRAP to > userspace. > True that the timing of signal generation is inconsistent for user-space when requested through ptrace vs requested through the interface. The requirement for having a continuous breakpoint (as opposed to the one-shot behaviour of ptrace) doesn't allow the signal to be delivered in the time-window between the exception and the instruction execution. I'm contemplating a few ways to work-arounds to ensure consistent signal delivery timing. One such work-around would be like this: hbp_handler(1)-->SIGTRAP-->signal_handler-->hbp_handler(2)--> disable_hbp+enable_ss-->single_step_handler(3)-->enable_hbp+disable_ss This method would take three exceptions for every breakpoint-hit (as enumerated above) and will be expensive. Any alternative suggestions with lower run-time overhead would greatly benefit the patch. > As for (b), well there are already a bunch of things which can only be > done on certain archs/processors, so I don't see that's anything > really new. Yes, the timing of SIGTRAP generation will remain dependant on the host processor but the user should be allowed to receive breakpoints continuously irrespective of the architecture. A one-shot behaviour only on PPC64 (and only in user-space) doesn't seem nice. > > How do you handle continuous breakpoints in the trigger-before case > (instruction breakpoints) on x86? Do you need to step over an > instruction is the same manner as you do for powerpc? In this case > where does x86 send the SIGTRAP? > Instructions breakpoints are trigger-before in x86 but the hardware eases the task by providing RF flag (in EFLAGS register) that would temporarily disable the instruction breakpoint while the causative instruction is executed. > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ > _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson Thanks, K.Prasad _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
