Patch to fix lockup on ppa insert

Patch to fix lockup on ppa insert

[John Cavan]

Similar to the imm patch, it's working for me.

John

diff -ru linux.clean/drivers/scsi/ppa.h linux.current/drivers/scsi/ppa.h
--- linux.clean/drivers/scsi/ppa.h      Thu Sep 14 20:27:05 2000
+++ linux.current/drivers/scsi/ppa.h    Thu Nov 16 07:26:38 2000
@@ -170,7 +170,7 @@
                eh_device_reset_handler:        NULL,                  
\
                eh_bus_reset_handler:           ppa_reset,             
\
                eh_host_reset_handler:          ppa_reset,             
\
-               use_new_eh_code:                1,                     
\
+               use_new_eh_code:                0,                     
\
                bios_param:                     ppa_biosparam,         
\
                this_id:                        -1,                    
\
                sg_tablesize:                   SG_ALL,                
\
diff -ru linux.clean/drivers/scsi/ppa.c linux.current/drivers/scsi/ppa.c
--- linux.clean/drivers/scsi/ppa.c      Thu Nov 16 07:25:29 2000
+++ linux.current/drivers/scsi/ppa.c    Thu Nov 16 07:28:10 2000
@@ -215,8 +215,10 @@
        }
        try_again = 1;
        goto retry_entry;
-    } else
+    } else {
+       host->use_new_eh_code = 1;
        return 1;               /* return number of hosts detected */
+    }
 }
 
 /* This is to give the ppa driver a way to modify the timings (and
other
-
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/

[PATCH] Re: Patch to fix lockup on ppa insert

[John Cavan]

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

John Cavan wrote:
> 
> Similar to the imm patch, it's working for me.
> 
> John

Again... not all screwed up...

[-- Attachment #2: ppa.diff --]
[-- Type: text/plain, Size: 929 bytes --]

patch -ur linux.clean/drivers/scsi/ppa.h linux.current/drivers/scsi/ppa.h
--- linux.clean/drivers/scsi/ppa.h	Thu Sep 14 20:27:05 2000
+++ linux.current/drivers/scsi/ppa.h	Thu Nov 16 07:26:38 2000
@@ -170,7 +170,7 @@
 		eh_device_reset_handler:	NULL,			\
 		eh_bus_reset_handler:		ppa_reset,		\
 		eh_host_reset_handler:		ppa_reset,		\
-		use_new_eh_code:		1,			\
+		use_new_eh_code:		0,			\
 		bios_param:			ppa_biosparam,		\
 		this_id:			-1,			\
 		sg_tablesize:			SG_ALL,			\
patch -ur linux.clean/drivers/scsi/ppa.c linux.current/drivers/scsi/ppa.c
--- linux.clean/drivers/scsi/ppa.c	Thu Nov 16 07:25:29 2000
+++ linux.current/drivers/scsi/ppa.c	Thu Nov 16 07:28:10 2000
@@ -215,8 +215,10 @@
 	}
 	try_again = 1;
 	goto retry_entry;
-    } else
+    } else {
+	host->use_new_eh_code = 1;
 	return 1;		/* return number of hosts detected */
+    }
 }
 
 /* This is to give the ppa driver a way to modify the timings (and other

Re: [PATCH] Re: Patch to fix lockup on ppa insert

[Jens Axboe]

On Thu, Nov 16 2000, John Cavan wrote:
> > Similar to the imm patch, it's working for me.
> > 
> > John
> 
> Again... not all screwed up...
> patch -ur linux.clean/drivers/scsi/ppa.h linux.current/drivers/scsi/ppa.h
> --- linux.clean/drivers/scsi/ppa.h	Thu Sep 14 20:27:05 2000
> +++ linux.current/drivers/scsi/ppa.h	Thu Nov 16 07:26:38 2000
> @@ -170,7 +170,7 @@
>  		eh_device_reset_handler:	NULL,			\
>  		eh_bus_reset_handler:		ppa_reset,		\
>  		eh_host_reset_handler:		ppa_reset,		\
> -		use_new_eh_code:		1,			\
> +		use_new_eh_code:		0,			\

Wouldn't it be more interesting to fix the reason the new error
handling code dies with imm and ppa?

-- 
* Jens Axboe <axboe@suse.de>
* SuSE Labs
-
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/

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

[John Cavan]

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

Jens Axboe wrote:
> Wouldn't it be more interesting to fix the reason the new error
> handling code dies with imm and ppa?

Yes it would... :o) I think I've got it here.

The new error handling code spinlocks the IRQ which cause the lowlevel
parport driver to choke. 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.

John

[-- Attachment #2: zip.patch --]
[-- Type: text/plain, Size: 1316 bytes --]

diff -urN -X /usr/src/dontdiff linux.clean/drivers/scsi/imm.c linux.current/drivers/scsi/imm.c
--- linux.clean/drivers/scsi/imm.c	Thu Nov 16 07:25:29 2000
+++ linux.current/drivers/scsi/imm.c	Thu Nov 16 21:39:10 2000
@@ -122,7 +122,15 @@
     struct Scsi_Host *hreg;
     int ports;
     int i, nhosts, try_again;
-    struct parport *pb = parport_enumerate();
+    struct parport *pb;
+
+    /*
+     * unlock to allow the lowlevel parport driver to probe
+     * the irqs
+     */
+    spin_unlock_irq(&io_request_lock);
+    pb = parport_enumerate();
+    spin_lock_irq(&io_request_lock);
 
     printk("imm: Version %s\n", IMM_VERSION);
     nhosts = 0;
diff -urN -X /usr/src/dontdiff linux.clean/drivers/scsi/ppa.c linux.current/drivers/scsi/ppa.c
--- linux.clean/drivers/scsi/ppa.c	Thu Nov 16 07:25:29 2000
+++ linux.current/drivers/scsi/ppa.c	Thu Nov 16 21:37:33 2000
@@ -111,7 +111,15 @@
     struct Scsi_Host *hreg;
     int ports;
     int i, nhosts, try_again;
-    struct parport *pb = parport_enumerate();
+    struct parport *pb;
+
+    /*
+     * unlock to allow the lowlevel parport driver to probe
+     * the irqs
+     */
+    spin_unlock_irq(&io_request_lock);
+    pb = parport_enumerate();
+    spin_lock_irq(&io_request_lock);
 
     printk("ppa: Version %s\n", PPA_VERSION);
     nhosts = 0;

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

[Tim Waugh]

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

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?

Also, what bit of the parport code is tripping over the lock?
Request_module or something?

A nicer fix would probably be to use parport_register_driver, but
that's likely to be too big a change right now.

Tim.
*/

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

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

[John Cavan]

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?

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.

> A nicer fix would probably be to use parport_register_driver, but
> that's likely to be too big a change right now.

I agree and it's recommended in the parport code. Now if I can get
enough time, I will take a stab at it, but nobody should be relying on
me for it. :o)

John
-
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/

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

[Tim Waugh]

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

On Sat, Nov 18, 2000 at 05:10:24PM -0500, John Cavan wrote:

> it is. The new scsi error stuff does mention that drivers must
> spinunlock/spinlock if it enables interrupts.

Okay, I guess it is safe then.

Tim.
*/

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]