[patch, 2.6.9-rc3-mm2] preemption debugging

Ingo Molnar
2004-10-04T21:38:22Z

* Ingo Molnar <mingo@elte.hu> wrote:

> * Andrew Morton <akpm@osdl.org> wrote:
>
> > I'm suspecting that something is causing preempt_count() to overflow
> > into the softirq counter. An imbalanced preempt_disable(), for
> > example.
>
> yes, that was it. Must not put side-effects into a macro that is NOP
> on !SMP.

btw., this bug was particularly nasty costing me a couple of hours,
since it triggered in neigh_periodic_timer() and due to the softirq
count the mismatch didnt show only until much later in irq_exit() when
there was no trace of the timer context anymore. So i obviously first
suspected the IRQ changes ... To prevent similar bugs:

The patch below does quite extensive preempt-count debugging and catches
this bug (and others) right when they happen. It also consolidates
DEBUG_SMP_PROCESSOR_ID into DEBUG_PREEMPT. We might want to carry this
in -mm a bit?

Ingo

Signed-off-by: Ingo Molnar <mingo@elte.hu>

--- linux/kernel/sched.c.orig
+++ linux/kernel/sched.c
@@ -2963,8 +2963,37 @@ static inline int dependent_idle(const r
}
#endif

-#if defined(CONFIG_PREEMPT) && defined(__smp_processor_id) && \
- defined(CONFIG_DEBUG_SMP_PROCESSOR_ID)
+#if defined(CONFIG_PREEMPT) && defined(CONFIG_DEBUG_PREEMPT)
+
+void fastcall add_preempt_count(int val)
+{
+ /*
+ * Underflow?
+ */
+ BUG_ON(((int)preempt_count() < 0));
+ preempt_count() += val;
+ /*
+ * Spinlock count overflowing soon?
+ */
+ BUG_ON((preempt_count() & PREEMPT_MASK) >= PREEMPT_MASK-10);
+}
+EXPORT_SYMBOL(add_preempt_count);
+
+void fastcall sub_preempt_count(int val)
+{
+ /*
+ * Underflow?
+ */
+ BUG_ON(val > preempt_count());
+ /*
+ * Is the spinlock portion underflowing?
+ */
+ BUG_ON((val < PREEMPT_MASK) && !(preempt_count() & PREEMPT_MASK));
+ preempt_count() -= val;
+}
+EXPORT_SYMBOL(sub_preempt_count);
+
+#ifdef __smp_processor_id
/*
* Debugging check.
*/
@@ -3015,7 +3044,9 @@ out:

EXPORT_SYMBOL(smp_processor_id);

-#endif
+#endif /* __smp_processor_id */
+
+#endif /* PREEMPT && DEBUG_PREEMPT */

#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)

@@ -3330,7 +3361,7 @@ asmlinkage void __sched preempt_schedule
return;

need_resched:
- preempt_count() += PREEMPT_ACTIVE;
+ add_preempt_count(PREEMPT_ACTIVE);
/*
* We keep the big kernel semaphore locked, but we
* clear ->lock_depth so that schedule() doesnt
@@ -3344,7 +3375,7 @@ need_resched:
#ifdef CONFIG_PREEMPT_BKL
task->lock_depth = saved_lock_depth;
#endif
- preempt_count() -= PREEMPT_ACTIVE;
+ sub_preempt_count(PREEMPT_ACTIVE);

/* we could miss a preemption opportunity between schedule and now */
barrier();
@@ -4319,12 +4350,14 @@ asmlinkage long sys_sched_yield(void)

static inline void __cond_resched(void)
{
+
+
if (preempt_count() & PREEMPT_ACTIVE)
return;
do {
- preempt_count() += PREEMPT_ACTIVE;
+ add_preempt_count(PREEMPT_ACTIVE);
schedule();
- preempt_count() -= PREEMPT_ACTIVE;
+ sub_preempt_count(PREEMPT_ACTIVE);
} while (need_resched());
}

--- linux/kernel/irq/handle.c.orig
+++ linux/kernel/irq/handle.c
@@ -77,7 +77,7 @@ irqreturn_t no_action(int cpl, void *dev
*/
void irq_exit(void)
{
- preempt_count() -= IRQ_EXIT_OFFSET;
+ sub_preempt_count(IRQ_EXIT_OFFSET);
if (!in_interrupt() && local_softirq_pending())
do_softirq();
preempt_enable_no_resched();
--- linux/kernel/timer.c.orig
+++ linux/kernel/timer.c
@@ -465,7 +465,14 @@ repeat:
smp_wmb();
timer->base = NULL;
spin_unlock_irq(&base->lock);
- fn(data);
+ {
+ u32 preempt_count = preempt_count();
+ fn(data);
+ if (preempt_count != preempt_count()) {
+ printk("huh, entered %p with %08x, exited with %08x?\n", fn, preempt_count, preempt_count());
+ BUG();
+ }
+ }
spin_lock_irq(&base->lock);
goto repeat;
}
--- linux/kernel/softirq.c.orig
+++ linux/kernel/softirq.c
@@ -142,7 +142,7 @@ void local_bh_enable(void)
* Keep preemption disabled until we are done with
* softirq processing:
*/
- preempt_count() -= SOFTIRQ_OFFSET - 1;
+ sub_preempt_count(SOFTIRQ_OFFSET - 1);

if (unlikely(!in_interrupt() && local_softirq_pending()))
invoke_softirq();
--- linux/include/linux/interrupt.h.orig
+++ linux/include/linux/interrupt.h
@@ -70,9 +70,9 @@ extern void enable_irq(unsigned int irq)

/* SoftIRQ primitives. */
#define local_bh_disable() \
- do { preempt_count() += SOFTIRQ_OFFSET; barrier(); } while (0)
+ do { add_preempt_count(SOFTIRQ_OFFSET); barrier(); } while (0)
#define __local_bh_enable() \
- do { barrier(); preempt_count() -= SOFTIRQ_OFFSET; } while (0)
+ do { barrier(); sub_preempt_count(SOFTIRQ_OFFSET); } while (0)

extern void local_bh_enable(void);

--- linux/include/linux/hardirq.h.orig
+++ linux/include/linux/hardirq.h
@@ -82,10 +82,10 @@ extern void synchronize_irq(unsigned int
#endif

#ifdef CONFIG_GENERIC_HARDIRQS
-#define nmi_enter() (preempt_count() += HARDIRQ_OFFSET)
-#define nmi_exit() (preempt_count() -= HARDIRQ_OFFSET)

-#define irq_enter() (preempt_count() += HARDIRQ_OFFSET)
+#define irq_enter() add_preempt_count(HARDIRQ_OFFSET)
+#define nmi_enter() irq_enter()
+#define nmi_exit() sub_preempt_count(HARDIRQ_OFFSET)
extern void irq_exit(void);
#endif

--- linux/include/linux/smp.h.orig
+++ linux/include/linux/smp.h
@@ -110,7 +110,7 @@ static inline void smp_send_reschedule(i
#endif /* !SMP */

#ifdef __smp_processor_id
-# if defined(CONFIG_PREEMPT) && defined(CONFIG_DEBUG_SMP_PROCESSOR_ID)
+# if defined(CONFIG_PREEMPT) && defined(CONFIG_DEBUG_PREEMPT)
/*
* temporary debugging check detecting places that use
* smp_processor_id() in a potentially unsafe way:
--- linux/include/linux/preempt.h.orig
+++ linux/include/linux/preempt.h
@@ -9,17 +9,18 @@
#include <linux/config.h>
#include <linux/linkage.h>

-#define preempt_count() (current_thread_info()->preempt_count)
+#ifdef CONFIG_DEBUG_PREEMPT
+ extern void fastcall add_preempt_count(int val);
+ extern void fastcall sub_preempt_count(int val);
+#else
+# define add_preempt_count(val) do { preempt_count() += (val); } while (0)
+# define sub_preempt_count(val) do { preempt_count() -= (val); } while (0)
+#endif

-#define inc_preempt_count() \
-do { \
- preempt_count()++; \
-} while (0)
+#define inc_preempt_count() add_preempt_count(1)
+#define dec_preempt_count() sub_preempt_count(1)

-#define dec_preempt_count() \
-do { \
- preempt_count()--; \
-} while (0)
+#define preempt_count() (current_thread_info()->preempt_count)

#ifdef CONFIG_PREEMPT

@@ -51,9 +52,15 @@ do { \

#else

+#ifdef CONFIG_PREEMPT_TIMING
+#define preempt_disable() inc_preempt_count()
+#define preempt_enable_no_resched() dec_preempt_count()
+#define preempt_enable() dec_preempt_count()
+#else
#define preempt_disable() do { } while (0)
#define preempt_enable_no_resched() do { } while (0)
#define preempt_enable() do { } while (0)
+#endif
#define preempt_check_resched() do { } while (0)

#endif
--- linux/lib/Kconfig.debug.orig
+++ linux/lib/Kconfig.debug
@@ -52,14 +52,15 @@ config DEBUG_SPINLOCK_SLEEP
If you say Y here, various routines which may sleep will become very
noisy if they are called with a spinlock held.

-config DEBUG_SMP_PROCESSOR_ID
- bool "Preempt-unsafe smp_processor_id() checking"
+config DEBUG_PREEMPT
+ bool "Debug preemptible kernel"
depends on PREEMPT && X86
default y
help
If you say Y here then the kernel will use a debug variant of the
commonly used smp_processor_id() function and will print warnings
- if kernel code uses it in a preemption-unsafe way.
+ if kernel code uses it in a preemption-unsafe way. Also, the kernel
+ will detect preemption count underflows.

config DEBUG_HIGHMEM
bool "Highmem debugging"