Re: [3/4] [PATCH]Diskdump - yet another crash dump function

[Ingo Molnar]

* Nobuhiro Tachino <ntachino@xxxxxxxxxx> wrote:

> Your new dump_run_timers() calls __run_timers() directly. I think
> that's the reason of unstability. __run_timers() calls
> spin_unlock_irq() and enables IRQ, but diskdump expects everything
> runs with IRQ disabled.

indeed!

luckily we can solve this in the upstream kernel without too much fuss,
see the patch below. All callers of __run_timers() run with irqs
enabled.

(NOTE: we unconditionally disable interrupts after having run the timer
fn - this solves the problem of a timer fn keeping irqs disabled.)

does this patch stabilize diskdump?

	Ingo

--- linux/kernel/timer.c.orig
+++ linux/kernel/timer.c
@@ -423,8 +423,9 @@ static int cascade(tvec_base_t *base, tv
 static inline void __run_timers(tvec_base_t *base)
 {
 	struct timer_list *timer;
+	unsigned long flags;
 
-	spin_lock_irq(&base->lock);
+	spin_lock_irqsave(&base->lock, flags);
 	while (time_after_eq(jiffies, base->timer_jiffies)) {
 		struct list_head work_list = LIST_HEAD_INIT(work_list);
 		struct list_head *head = &work_list;
@@ -453,14 +454,14 @@ repeat:
 			set_running_timer(base, timer);
 			smp_wmb();
 			timer->base = NULL;
-			spin_unlock_irq(&base->lock);
+			spin_unlock_irqrestore(&base->lock, flags);
 			fn(data);
 			spin_lock_irq(&base->lock);
 			goto repeat;
 		}
 	}
 	set_running_timer(base, NULL);
-	spin_unlock_irq(&base->lock);
+	spin_unlock_irqrestore(&base->lock, flags);
 }
 
 #ifdef CONFIG_NO_IDLE_HZ
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/