Re: Questions about kprobes implementation on SMP and preemptible kernels

[Ananth N Mavinakayanahalli <ananth at in dot ibm dot com>]

On Sat, Jan 27, 2007 at 05:59:48PM -0600, Quentin Barnes wrote:
> On Tue, Jan 16, 2007 at 11:09:55AM +0530, Ananth N Mavinakayanahalli wrote:
> >On Mon, Jan 15, 2007 at 09:48:26PM -0600, Quentin Barnes wrote:
> [...]
> >>Q #3:
> >>The ARM kprobes model uses an undefined instruction for its
> >>kprobe.  This is necessary since ARM's breakpoint instruction (BKPT)
> >>triggers entry into the same CPU service mode as the kernel runs
> >>in.  On ARM, this is bad to do.  No problem though since undefined
> >>instructions have their own mode.  Due to this, kprobe_handler() is
> >>called from ARM's undefined instruction handler, do_undefinstr().
> >>Currently interrupts remain disabled all the way through the
> >>kprobe's undef instruction exception handling into kprobe_handler()
> >>and up through returning.  I would like to eventually change the
> >>code so that interrupts can be re-enabled at some point to reduce
> >>interrupt latency.
> 
> I determined this was a bad idea with the current kprobe design
> and should never be done for now.  See below.
> 
> >>When handling an undefined instruction the
> >>kernel is running in an exception context.  Looking around at the
> >>kernel code, just running in Linux exception handler context is no
> >>guarantee of disabling preemption.  Is this true?  If so, I'd have
> >>to call preempt_disable() to prevent preemption before re-enabling
> >>interrupts.
> >
> >On architectures which run kprobes with interrupts enabled, its not
> >uncommon to have interrupts occur while we are processing a kprobe. In
> >fact, the kprobe reentry handling code was a byproduct of such a
> >scenario happening.
> 
> I'm not sure I follow this point clearly enough.
> 
> As far as I follow, recursion for kprobes is only handled to no more
> than one level when re-entered and only from a kprobe encountered
> in a pre- or post-handler.  In that case, kprobe_status is set to
> KPROBE_REENTER and the pre- and post-handlers aren't run for that
> one recursion level so we don't recurse yet again.
> 
> If we're processing a kprobe with interrupts enabled and an
> interrupt fires and its handler has a kprobe, we would re-enter.  At
> this point, this would "work" in the sense that kprobe's pre- and
> post-handler would not run even though they may not contain any
> kprobes themselves.  Is this considered an acceptable behavior?

Yes. At this point, you just increment struct kprobe->nmissed to
indicate that you missed running the handlers and move on.

> If the same scenario happened above (kprobe in interrupt handler
> and its interrupt fired) but this time we're already nested from
> handling a kprobe and another kprobe in its pre- or post-handler,
> save_previous_kprobe() would overwrite an already active previous
> kprobe state.  This would seem to be a serious bug.

Though such a scenario is eminently possible, we haven't encountered it
in the real world (yet :). AFAIR, Anil had a patch to handle this
specific case. Not sure what the status of the patch is at the moment.
The other question here is, how many levels of nesting would you allow?
What'd you do beyond that nesting level?

> Which architectures allow interrupts while handling a kprobe?

Powerpc and AFAIR x86_64 and IA64 run with interrupts enabled
throughout.

Ananth

> 
> Because of the above problem, I decided that kprobe_handler()
> as currently designed should never re-enable interrupts during
> handling of a kprobe.
> 
> >Ananth
> 
> Quentin