Re: [parisc-linux] 2.4.18-pa35 SMP process hangs on a J200

[Michael S.Zick <mszick@goquest.com>]

[-- Attachment #1: Type: text/plain, Size: 753 bytes --]

On Tuesday 11 June 2002 01:39 pm, Michael S. Zick wrote:
> On Tuesday 11 June 2002 09:58 am, Randolph Chung wrote:
> > think we are looking at multiple bugs here.....
>
> Very likely...
Sirs...
I now have a definate lead for the x86-SMP case, which might 
aid the troubleshooters in the pa-risc branch (diff's attached).

Evidently, some code path is causing these portions of IRQ 
service to be entered with the IRQ state not as designed.

These patches preserve callers interrupt state, eliminating
the erratic behavior, which is only an artifact of the true cause.

True cause still unknown.

Additional note: On the HP models with the worst of the problems;
is the physical_cpu_id and the logical_cpu_id a 1:1 mapping as
it is on x86 systems?

Mike

[-- Attachment #2: x86 branch, don't presume IRQ state --]
[-- Type: text/x-c, Size: 3677 bytes --]

--- linux-2.4.18/arch/i386/kernel/irq.c.org	Thu Oct 25 15:53:46 2001
+++ linux-2.4.18/arch/i386/kernel/irq.c	Tue Jun 11 19:09:14 2002
@@ -387,7 +387,7 @@
 	int retval;
 	int local_enabled;
 	unsigned long flags;
-	int cpu = smp_processor_id();
+	int cpu ;  /* don't initialize cpu here, it changes the flags - MSZ */
 
 	__save_flags(flags);
 	local_enabled = (flags >> EFLAGS_IF_SHIFT) & 1;
@@ -395,6 +395,7 @@
 	retval = 2 + local_enabled;
 
 	/* check for global flags if we're not in an interrupt */
+	cpu = smp_processor_id();
 	if (!local_irq_count(cpu)) {
 		if (local_enabled)
 			retval = 1;
@@ -507,9 +508,12 @@
  
 void disable_irq(unsigned int irq)
 {
+	int cpu_id ;
+
 	disable_irq_nosync(irq);
+	cpu_id = smp_processor_id() ;
 
-	if (!local_irq_count(smp_processor_id())) {
+	if (!local_irq_count(cpu_id)) {
 		do {
 			barrier();
 			cpu_relax();
@@ -806,6 +810,7 @@
 	irq_desc_t *desc;
 	unsigned long val;
 	unsigned long delay;
+	unsigned long flags;  /* Don't presume interrupt state - MSZ */
 
 	down(&probe_sem);
 	/* 
@@ -815,10 +820,12 @@
 	for (i = NR_IRQS-1; i > 0; i--)  {
 		desc = irq_desc + i;
 
-		spin_lock_irq(&desc->lock);
+		spin_lock_irqsave(&desc->lock,flags);
+/* MSZ		spin_lock_irq(&desc->lock); */
 		if (!irq_desc[i].action) 
 			irq_desc[i].handler->startup(i);
-		spin_unlock_irq(&desc->lock);
+		spin_unlock_irqrestore(&desc->lock,flags);
+/* MSZ		spin_unlock_irq(&desc->lock); */
 	}
 
 	/* Wait for longstanding interrupts to trigger. */
@@ -833,13 +840,15 @@
 	for (i = NR_IRQS-1; i > 0; i--) {
 		desc = irq_desc + i;
 
-		spin_lock_irq(&desc->lock);
+		spin_lock_irqsave(&desc->lock,flags);
+/* MSZ		spin_lock_irq(&desc->lock); */
 		if (!desc->action) {
 			desc->status |= IRQ_AUTODETECT | IRQ_WAITING;
 			if (desc->handler->startup(i))
 				desc->status |= IRQ_PENDING;
 		}
-		spin_unlock_irq(&desc->lock);
+		spin_unlock_irqrestore(&desc->lock,flags);
+/* MSZ		spin_unlock_irq(&desc->lock); */
 	}
 
 	/*
@@ -856,7 +865,8 @@
 		irq_desc_t *desc = irq_desc + i;
 		unsigned int status;
 
-		spin_lock_irq(&desc->lock);
+		spin_lock_irqsave(&desc->lock,flags);
+/* MSZ		spin_lock_irq(&desc->lock); */
 		status = desc->status;
 
 		if (status & IRQ_AUTODETECT) {
@@ -868,7 +878,8 @@
 				if (i < 32)
 					val |= 1 << i;
 		}
-		spin_unlock_irq(&desc->lock);
+		spin_unlock_irqrestore(&desc->lock,flags);
+/* MSZ		spin_unlock_irq(&desc->lock); */
 	}
 
 	return val;
@@ -895,13 +906,15 @@
 {
 	int i;
 	unsigned int mask;
+	unsigned int flags;  /* don't presume interrupt state - MSZ */
 
 	mask = 0;
 	for (i = 0; i < NR_IRQS; i++) {
 		irq_desc_t *desc = irq_desc + i;
 		unsigned int status;
 
-		spin_lock_irq(&desc->lock);
+		spin_lock_irqsave(&desc->lock,flags);
+/* MSZ		spin_lock_irq(&desc->lock); */
 		status = desc->status;
 
 		if (status & IRQ_AUTODETECT) {
@@ -911,7 +924,8 @@
 			desc->status = status & ~IRQ_AUTODETECT;
 			desc->handler->shutdown(i);
 		}
-		spin_unlock_irq(&desc->lock);
+		spin_unlock_irqrestore(&desc->lock,flags);
+/* MSZ		spin_unlock_irq(&desc->lock); */
 	}
 	up(&probe_sem);
 
@@ -950,8 +964,10 @@
 	for (i = 0; i < NR_IRQS; i++) {
 		irq_desc_t *desc = irq_desc + i;
 		unsigned int status;
+		unsigned long flags;  /* Don't presume interrupt state -  MSZ */
 
-		spin_lock_irq(&desc->lock);
+		spin_lock_irqsave(&desc->lock,flags);
+/* MSZ		spin_lock_irq(&desc->lock); */
 		status = desc->status;
 
 		if (status & IRQ_AUTODETECT) {
@@ -963,7 +979,8 @@
 			desc->status = status & ~IRQ_AUTODETECT;
 			desc->handler->shutdown(i);
 		}
-		spin_unlock_irq(&desc->lock);
+		spin_unlock_irqrestore(&desc->lock,flags);
+/* MSZ		spin_unlock_irq(&desc->lock); */
 	}
 	up(&probe_sem);
 


[-- Attachment #3: generic, don't presume IRQ state --]
[-- Type: text/x-c, Size: 1756 bytes --]

--- linux-2.4.18/kernel/softirq.c.rml	Tue Jun 11 20:24:24 2002
+++ linux-2.4.18/kernel/softirq.c	Tue Jun 11 20:50:38 2002
@@ -177,11 +177,14 @@
 {
 	int cpu = smp_processor_id();
 	struct tasklet_struct *list;
+	unsigned long flags;  /* Don't presume interrupt state */
 
-	local_irq_disable();
+	local_irq_save(flags);  /* MSZ */
+/* MSZ	local_irq_disable(); */
 	list = tasklet_vec[cpu].list;
 	tasklet_vec[cpu].list = NULL;
-	local_irq_enable();
+	local_irq_restore(flags); /* MSZ */
+/* MSZ	local_irq_enable();  */
 
 	while (list) {
 		struct tasklet_struct *t = list;
@@ -199,11 +202,13 @@
 			tasklet_unlock(t);
 		}
 
-		local_irq_disable();
+		local_irq_save(flags);  /* MSZ */
+/* MSZ		local_irq_disable(); */
 		t->next = tasklet_vec[cpu].list;
 		tasklet_vec[cpu].list = t;
 		__cpu_raise_softirq(cpu, TASKLET_SOFTIRQ);
-		local_irq_enable();
+		local_irq_restore(flags); /* MSZ */
+/* MSZ		local_irq_enable();  */
 	}
 }
 
@@ -211,11 +216,14 @@
 {
 	int cpu = smp_processor_id();
 	struct tasklet_struct *list;
+	unsigned long flags;  /* Don't presume interrupt state */
 
-	local_irq_disable();
+	local_irq_save(flags);  /* MSZ */
+/* MSZ	local_irq_disable(); */
 	list = tasklet_hi_vec[cpu].list;
 	tasklet_hi_vec[cpu].list = NULL;
-	local_irq_enable();
+	local_irq_restore(flags); /* MSZ */
+/* MSZ	local_irq_enable(); */
 
 	while (list) {
 		struct tasklet_struct *t = list;
@@ -233,11 +241,13 @@
 			tasklet_unlock(t);
 		}
 
-		local_irq_disable();
+		local_irq_save(flags);  /* MSZ */
+/* MSZ		local_irq_disable(); */
 		t->next = tasklet_hi_vec[cpu].list;
 		tasklet_hi_vec[cpu].list = t;
 		__cpu_raise_softirq(cpu, HI_SOFTIRQ);
-		local_irq_enable();
+		local_irq_restore(flags); /* MSZ */
+/* MSZ		local_irq_enable(); */
 	}
 }