This is the mail archive of the mailing list for the systemtap project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Questions about kprobes implementation on SMP and preemptible kernels

On Mon, Jan 15, 2007 at 09:48:26PM -0600, Quentin Barnes wrote:
> There's a few dangling issues in my mind that have come up in trying
> to wrap up the remaining kprobes implementation for ARM.  Maybe
> these are more general Linux kernel programming exception handling
> questions on SMP and preemptible configurations, but since they're
> related to existing kprobes implementations, I'll ask them here.
> Q #1:
> In all the arch implementations of kprobe_exceptions_notify() (except
> avr32), they all do:
> =====
>        /* kprobe_running() needs smp_processor_id() */
>        preempt_disable();
>        if (kprobe_running() &&
>            kprobe_fault_handler(args->regs, args->trapnr))
>                ret = NOTIFY_STOP;
>        preempt_enable();
>        break;
> =====
> The preempt_disable()/preempt_enable() block here just looks wrong
> to me.
> One of two things must be true.  Either: 1) There is a
> guaranteed unbroken chain of execution on the processor from
> the point of its exception occurring all the way through to
> kprobe_exceptions_notify() and onward, or 2) There is no
> guarantee of an unbroken chain -- execution is allowed to
> swap to another processor between the exception and executing
> kprobe_exceptions_notify().
> If #1 is true, preempt_disable()/preempt_enable() is superfluous and
> calling kprobe_running() is safe without the protection.  If #2 is
> true, then calling preempt_disable()/preempt_enable() is pointless
> since there is no guarantee we're on the same processor as the
> exception so calling kprobe_running() is an unreliable way to
> determine what processor the probe that triggered the exception was
> on.  A way to solve that would be the ID of the processor that
> triggered the exception must be saved as part of the exception
> context and that state must be accessed to determine the processor
> that kprobe triggered on instead of just calling kprobe_running().
> Can someone explain which is true?  (Or is neither true and my
> understanding of the Linux kernel is just wrong?)

1 is true. The notify_page_fault() call in do_page_fault() happens early
enough and there isn't anything in the page_fault handling code up to
this point, to cause a broken chain of execution. The reason we have a
preempt_(en/dis)able in there is 'cos the Linux kernel has multiple
interfaces to get smp processor id and there are caveats for usage of
each variant. kprobe_running() calls __get_cpu_var() which requires
preemption to be disabled and on some archs, ends up calling
smp_processor_id() which in turn, is not implicitly preemption safe.

> Q #2:
> A similar question applies to preempt_disable() in kprobe_handler().
> On architectures where interrupts remain disabled from the point of
> the kprobes exception all the way into kprobe_handler() is there a
> need to also call preempt_disable()?  In other words, in the Linux
> kernel model, whenever interrupts are disabled, does that also mean
> as a side effect that preemption is disabled, correct?  (That seems
> true due to the preemptible() macro definition in hardirq.h.)

Correct, AFAIK. _spin_lock_irqsave() seems to confirm that.

> I'm wondering if preempt_disable() is also superfluous in
> kprobe_handler() too for architectures where interrupts are disabled
> throughout the exception and into resuming from the exception when
> the previous context had been modified to resume with interrupts
> disabled.  Now this model may not be used on all architectures,
> but on the ones that it is, is this true that since interrupts are
> disabled that preempt_disable() is superfluous in kprobe_handler()?

Based on the above reasoning, I think so, yes.

> 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.  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.

And yes, a preempt_disable() before re-enabling interrupts would work
and you'll need to be careful - preempt_(en/dis)able are nested calls.
You'll need to take care of balancing the enable/disable calls on all

AFAIK, s390 also uses such a trick (using an undefined instruction). Not
sure how this situation is handled there. You may want to check with
Mike Grundy/Dave Wilder (on cc) for hints.


> I poked around the 'net some.  I found some docs on hard irqs, soft
> irqs, tasklets, timers, and such, but I had trouble finding current
> detailed docs that explains the kernel's exception processing
> programming model.  I would expect exceptions to be in the same
> class as hard irqs since they are a synchronous version of the same
> thing, but I don't see the low level exception and interrupt code
> doing equivalent kernel status bookkeeping.
> Quentin

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]