Re: Questions about kprobes implementation on SMP and preemptible kernels

["Abhishek Sagar" <sagar dot abhishek at gmail dot com>]

Hi Quentin,

> 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()?

Disabling interrupts is just one way to make sure that our process
doesn't get preempted. The other ways of getting preempted are
mentioned towards the end in:
http://www.samspublishing.com/articles/article.asp?p=101760&seqNum=3&rl=1

Since interrupts are disabled in the undef exception context, we can
be pretty sure that we won't get scheduled out (and thus preempted).
This however doesn't mean that we cannot encounter code (in the user
pre-handler, for example) which may mark the current process for
rescheduling. If that happens, then the current process will be
preempted at the next preemption checkpoint. If that check point
happens to fall in the kprobe code path with preemption and interrupts
enabled, then we're in trouble. I'm not sure if this is totally
relevant to your larger point of questioning the presence of
preempt_disable() in kprobe_handler with interrupts disabled, but it
adds as a check in case you plan to run things in kprobe_handler with
interrupts enabled (and possibly skipping over preempt_disable).

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

If you're planning to run some part of the kprobe code-path with
interrupts enabled, then that code must be sandwitched between
preempt_disable/enable.

E.g, if you plan to single-step with interrupts enabled:
Consider the following user handler for a jprobe (jp->entry = foo):

1. int foo(/* args */) {
2. 	woo(); /* marks current process's NEED_RESCHED bit */
3. 	spin_lock(lock); /* preempt_count = 1*/
4.	/* critical section */
5.	spin_unlock(lock); /* preempt_count = 0; rechedule! */
6.	jprobe_return();
7. }

Assuming that such a foo() can exists (hope I haven't got it
wrong!)...(5) would lead to rescheduling and the current process might
end up on another processor's run-queue. In that case when foo returns
to kprobe context in (6), it'll end up using an unrelated per-cpu
kprobe control block. A simple preempt_disable() in kprobe_handler
would prevent all this. Just wanted to add this scenario to your
check-list :D

Regards
Abhishek Sagar