[PATCH] 5/6: scsi_inq_timeout

[PATCH] 5/6: scsi_inq_timeout

[Kurt Garloff]

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


Feature.

Introduce a new SCSI scanning boot/module parameter
scsi_inq_timeout=N
which sets the timeout used when the INQUIRY is send in the SCSI 
scanning procedure to N seconds. (Half a second is added, so 0 is safe.)
This is useful, because some devices need an excessive amount of time to 
recover from the bootup SCSI reset.
Also changes the default for PPC64, as some largely used crap chip there
requires it.

Note that the second INQUIRY (with more than 36 bytes) only gets half 
the amount of seconds, the long delay for the first is only due to the 
recovery needed from the initial bus reset.

--- linux-2.6.5.noreplunhost/drivers/scsi/scsi_scan.c.orig	2004-04-15 22:51:08.000000000 +0200
+++ linux-2.6.5.noreplunhost/drivers/scsi/scsi_scan.c	2004-04-16 19:30:43.289826746 +0200
@@ -125,6 +125,22 @@ MODULE_PARM_DESC(scsi_allow_ghost_device
 		 "allow devices marked as being offline to be accessed anyway "
 		 "(0 = off, else allow ghosts on lun 0 through allow_ghost_devices - 1");
 
+/* Some AChip ARC765 based DVD-ROM's take 15 or more seconds
+ * to reset.  A scan will fail if made right after a reset.
+ * It's completely broken device behaviour: SCSI specification
+ * says devices need to be able to respond to INQUIRY always
+ * (after a selection timeout ... of 250ms).
+ */
+#ifdef __powerpc64__
+static unsigned int scsi_inq_timeout = SCSI_TIMEOUT/HZ+25;
+#else
+static unsigned int scsi_inq_timeout = SCSI_TIMEOUT/HZ+3;
+#endif
+
+module_param_named(scsi_inq_timeout, scsi_inq_timeout, int, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(scsi_inq_timeout, 
+		 "Timeout (in seconds) waiting for devices to answer INQUIRY."
+		 " Default is 6. Some non-compliant devices need more.");
 
 /**
  * scsi_unlock_floptical - unlock device via a special MODE SENSE command
@@ -376,7 +392,7 @@ static void scsi_probe_lun(struct scsi_r
 
 	memset(inq_result, 0, 36);
 	scsi_wait_req(sreq, (void *) scsi_cmd, (void *) inq_result, 36,
-		      SCSI_TIMEOUT + 4 * HZ, 3);
+		      HZ/2 + HZ*scsi_inq_timeout, 3);
 
 	SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO "scsi scan: 1st INQUIRY %s with"
 			" code 0x%x\n", sreq->sr_result ?
@@ -425,7 +441,7 @@ static void scsi_probe_lun(struct scsi_r
 		memset(inq_result, 0, possible_inq_resp_len);
 		scsi_wait_req(sreq, (void *) scsi_cmd,
 			      (void *) inq_result,
-			      possible_inq_resp_len, SCSI_TIMEOUT + 4 * HZ, 3);
+			      possible_inq_resp_len, (1+scsi_inq_timeout)*(HZ/2), 3);
 		SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO "scsi scan: 2nd INQUIRY"
 				" %s with code 0x%x\n", sreq->sr_result ?
 				"failed" : "successful", sreq->sr_result));
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

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

Re: [PATCH] 5/6: scsi_inq_timeout

[Christoph Hellwig]

On Sun, Apr 18, 2004 at 08:59:57PM +0200, Kurt Garloff wrote:
> +#ifdef __powerpc64__
> +static unsigned int scsi_inq_timeout = SCSI_TIMEOUT/HZ+25;
> +#else
> +static unsigned int scsi_inq_timeout = SCSI_TIMEOUT/HZ+3;
> +#endif

Huh?  per-arch ifdefs in the scsi code are completely bogus.

Re: [PATCH] 5/6: scsi_inq_timeout

[Kurt Garloff]

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

On Sun, Apr 18, 2004 at 08:21:09PM +0100, Christoph Hellwig wrote:
> On Sun, Apr 18, 2004 at 08:59:57PM +0200, Kurt Garloff wrote:
> > +#ifdef __powerpc64__
> > +static unsigned int scsi_inq_timeout = SCSI_TIMEOUT/HZ+25;
> > +#else
> > +static unsigned int scsi_inq_timeout = SCSI_TIMEOUT/HZ+3;
> > +#endif
> 
> Huh?  per-arch ifdefs in the scsi code are completely bogus.

Well, those broken chips seem to be very common on machines from
a specific vendor that ships PPC64 hardware ...
That hunk could be dropped of course in mainline, I would not mind.

Regards,
-- 
Kurt Garloff                   <kurt@garloff.de>             [Koeln, DE]
Physics:Plasma modeling <garloff@plasimo.phys.tue.nl> [TU Eindhoven, NL]
Linux: SUSE Labs (Head)        <garloff@suse.de>    [SUSE Nuernberg, DE]

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

Re: [PATCH] 5/6: scsi_inq_timeout

[Patrick Mansfield]

On Mon, Apr 19, 2004 at 12:16:10AM +0200, Kurt Garloff wrote:
> On Sun, Apr 18, 2004 at 08:21:09PM +0100, Christoph Hellwig wrote:
> > On Sun, Apr 18, 2004 at 08:59:57PM +0200, Kurt Garloff wrote:
> > > +#ifdef __powerpc64__
> > > +static unsigned int scsi_inq_timeout = SCSI_TIMEOUT/HZ+25;
> > > +#else
> > > +static unsigned int scsi_inq_timeout = SCSI_TIMEOUT/HZ+3;
> > > +#endif
> > 
> > Huh?  per-arch ifdefs in the scsi code are completely bogus.
> 
> Well, those broken chips seem to be very common on machines from
> a specific vendor that ships PPC64 hardware ...
> That hunk could be dropped of course in mainline, I would not mind.

Why not increase it for all cases? And/or make it a config value? 

It won't slow down functioning hardware, just the cases where we get no
response.

-- Patrick Mansfield

Re: [PATCH] 5/6: scsi_inq_timeout

[Kurt Garloff]

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

On Mon, Apr 19, 2004 at 09:41:06AM -0700, Patrick Mansfield wrote:
> Why not increase it for all cases? And/or make it a config value? 

Config value is stpuid. I made it a boot parameter.

> It won't slow down functioning hardware, just the cases where we get no
> response.

True, you should get selection timeouts there.
(Or the equivalent on non SPI transports.)

Still, 5s is already quite long; I would dislike the kernel needing
25s to recognize a broken device, just because somebody had built
screwed up hardware.

I'll drop the arch specific default.

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

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