Re: 2.5.40-mm2

[Andrew Morton]

Dave Hansen wrote:
> 
> Andrew Morton wrote:
> >   Ingo said that his 2.4-based per-cpu-pages patch was beneficial to
> >   specweb, but nobody has tested these patches with specweb.  Hint.
> 
> cc'ing Ingo, because I think this might be related to the timer bh
> removal.
> 
> 2.5.40 doesn't last very long under Specweb.  It always dies out with
> one of these oopses after a little while:
> 
> CPU:    3
> EIP:    0060:[<801204a9>]    Not tainted
> EFLAGS: 00010006
> EIP is at run_timer_tasklet+0xcd/0x13c

Well from a quick peek, there is some funny stuff happening in the
timer code.

- del_timer_sync() iterates across all CPUs, but does not do
  actually _do_ anything for each CPU.  (I suspect this may
  be the source of your crash - del_timer_sync() is bust)

- the back-to-back preempt_disable()/preempt_enable() is
  unusual.  What's that for?

- __run_timers() is doing spin_unlock_irq() inside spin_lock_irqsave().
  That's probably not a bug in this context, but it's a wart.

This help?

--- 2.5.40/kernel/timer.c~timer-tricks	Sun Oct  6 14:50:39 2002
+++ 2.5.40-akpm/kernel/timer.c	Sun Oct  6 14:52:34 2002
@@ -265,20 +265,19 @@ repeat:
  */
 int del_timer_sync(timer_t *timer)
 {
-	tvec_base_t *base = tvec_bases;
 	int i, ret;
 
 	ret = del_timer(timer);
 
 	for (i = 0; i < NR_CPUS; i++) {
+		tvec_base_t *base;
+
 		if (!cpu_online(i))
 			continue;
+		base = tvec_bases + i;
 		if (base->running_timer == timer) {
-			while (base->running_timer == timer) {
+			while (base->running_timer == timer)
 				cpu_relax();
-				preempt_disable();
-				preempt_enable();
-			}
 			break;
 		}
 		base++;
@@ -359,9 +358,9 @@ repeat:
 #if CONFIG_SMP
 			base->running_timer = timer;
 #endif
-			spin_unlock_irq(&base->lock);
+			spin_unlock_irqrestore(&base->lock, flags);
 			fn(data);
-			spin_lock_irq(&base->lock);
+			spin_lock_irqsave(&base->lock, flags);
 			goto repeat;
 		}
 		++base->timer_jiffies; 

.