This is the mail archive of the systemtap@sourceware.org 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


It seems to me that the preempt_disable()/preempt_enable() calls in
kprobe_exceptions_notify() are either at best inert, or possibly
hiding an existing bug.

The decision to sandwich kprobe_exceptions_notify between preempt_disable()/preempt_enable() is to 'explicitly' disable preemption. Since interrupts are already disabled, and there is no way of being preempted, you're spot on in pointing out that they're not really needed. But it seems more like a matter of convention to use explicit preemption locks around preemption sensitive regions. So yes...their use here is kind of inert.

Quoting from Documentation/preempt-locking.txt:

"...But keep in mind that 'irqs disabled' is a fundamentally unsafe way of
disabling preemption - any spin_unlock() decreasing the preemption count
to 0 might trigger a reschedule. A simple printk() might trigger a reschedule.
So use this implicit preemption-disabling property only if you know that the
affected codepath does not do any of this. Best policy is to use this only for
small, atomic code that you wrote and which calls no complex functions."

It goes on to add:

"It is possible to prevent a preemption event using local_irq_disable and
local_irq_save.  Note, when doing so, you must be very careful to not cause
an event that would set need_resched and result in a preemption check.  When
in doubt, rely on locking or explicit preemption disabling."

Seems like even though there are sections in the kernel which now seem
like non-preemptible (because they run with interrupts disabled), they
can be turned into preemptible sections in the future. The sections
which explicitly want to run in a preemption safe environment must
take proper preemption locks to safeguard themselves permanently.
kprobe_exceptions_notify is such a function because of its use of
per-cpu variables. So if in the future, the ARM undef exception
handling code becomes preemptibe, the per-cpu variable dependent
code-path in it (like kprobe_handler) would remain safe even then.

The confusion which now arises, is that the quote suggests that even
with local_irq_disable'd the current process can be rescheduled. But
having a look at preempt_schedule(), it shows the following check
right at the beginning:

if (likely(ti->preempt_count || irqs_disabled()))        /* --->
notice the 'likely' optimization */
               return;

It doesn't seem like preemption can be triggered in scenario quoted
from preempt-locking.txt ...can somebody explain this?

I'm wondering if I found a bug on the ARM implementation of prefetch
and data abort exception handlers for SMP platforms with kernel
preemption enabled.  Immediately after switching to SVC mode in
__pabt_svc and __dabt_svc, the handler re-enables IRQs (interrupts)
if they were enabled prior to the exception.  If it re-enables
interrupts at this point, it seems to me that a preemptive kernel
(CONFIG_PREEMPT defined) could switch execution to another processor
breaking the chain of execution before it has a chance to note which
processor triggered the exception.  Any ARM Linux kernel experts on
this list to comment, or do I need to bounce this to another list?

In the __irq_svc path (IRQ interrupt handling), the handler
increments the preempt count before reenabling IRQs (and later
restores its previous value after servicing the interrupt) when
built for a preemptive kernel.

The __irq_svc_ path explicitly disables preemption because rescheduling is not an option in hard irq context. The abort mode handling code-path however seems to have been opened up for preemption (upon entry into SVC mode). In case of prefetch abort, the processor sensitive information has already been gathered in registers r0 and r1 before which interrupts are enabled. So preempting them seems safe. Similarly, in __dabt_svc, the processor specific data gathering routines are called before interrupts are enabled. So preemption seems safe there as well. I've just skimmed through that part of entry-armv.S, so kindly correct me if I've missed something apparent.

Regards
Abhishek Sagar


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