Re: [PATCH] (new for ppa and imm) Re: [PATCH] Re: Patch to fix lockup on

[Douglas Gilbert <dougg@torque.net>]

 	
Content-Type: multipart/mixed;
 boundary="------------4B08707380476D0EA895C09B"

This is a multi-part message in MIME format.
--------------4B08707380476D0EA895C09B
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

John Cavan wrote:
> Tim Waugh wrote:
> > 
> > On Thu, Nov 16, 2000 at 09:50:40PM -0500, John Cavan wrote:
> > 
> > > [...] This patch unlocks, allows the lowlevel driver to do it's
> > > probes, and then relocks. It could probably be more granular in the
> > > parport_pc code, but my own home tests show it to be working fine.
> > 
> > Is that safe?

Safer than an oops/lockup :-)

> I'm not sure. I know why it causes the NMI lockup, but I'm not enough of
> an expert to sort it out. I've got a pretty good feel for the Zip
> driver, but not the parport or scsi code yet, so I don't know how safe
> it is. The new scsi error stuff does mention that drivers must
> spinunlock/spinlock if it enables interrupts.
>
> > Also, what bit of the parport code is tripping over the lock?
> > Request_module or something?
> 
> During the init phase of the parport_pc module it probes and enables the
> IRQ(s) of the parallel port, but the scsi layer has them locked.

John and Tim,
At least using imm on my SMP machine (BP6 dual celery)
I found that I had to go a bit further than John's patch.
Basically I have unlocked the whole of imm_detect().
It was necessary to unblock parport_enumerate()
but not sufficient.

Please see attachment. I don't have a ppa device to test.
Eric Y. makes some comments just before the (*detect())
call in scsi.c relating to low level driver detect routines.

Doug Gilbert
--------------4B08707380476D0EA895C09B
Content-Type: text/plain; charset=us-ascii;
 name="imm.cx2_diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="imm.cx2_diff"

--- linux/drivers/scsi/imm.c	Mon Nov 20 16:36:19 2000
+++ linux/drivers/scsi/imm.cx2	Mon Nov 20 22:46:10 2000
@@ -122,14 +122,17 @@
     struct Scsi_Host *hreg;
     int ports;
     int i, nhosts, try_again;
-    struct parport *pb = parport_enumerate();
+    struct parport *pb;
 
     printk("imm: Version %s\n", IMM_VERSION);
+    spin_unlock_irq(&io_request_lock);
+    pb = parport_enumerate();
     nhosts = 0;
     try_again = 0;
 
     if (!pb) {
 	printk("imm: parport reports no devices.\n");
+        spin_lock_irq(&io_request_lock);
 	return 0;
     }
   retry_entry:
@@ -154,6 +157,7 @@
 		    printk(KERN_ERR "imm%d: failed to claim parport because a "
 		      "pardevice is owning the port for too longtime!\n",
 			   i);
+                    spin_lock_irq(&io_request_lock);
 		    return 0;
 		}
 	    }
@@ -208,12 +212,16 @@
 	nhosts++;
     }
     if (nhosts == 0) {
-	if (try_again == 1)
+	if (try_again == 1) {
+            spin_lock_irq(&io_request_lock);
 	    return 0;
+	}
 	try_again = 1;
 	goto retry_entry;
-    } else
+    } else {
+        spin_lock_irq(&io_request_lock);
 	return 1;		/* return number of hosts detected */
+    }
 }
 
 /* This is to give the imm driver a way to modify the timings (and other

--------------4B08707380476D0EA895C09B--


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/