[PATCH 0/3] Resend: Handle PQ3 devs better

[PATCH 0/3] Resend: Handle PQ3 devs better

[Kurt Garloff]

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

Hi,

resending the patches ...
Changes since last posting:
- I reversed the order of patches 2 and 3, as one patch (now 3)
  refers to a blacklist flag introduced in the other.
- Andrew Morton has merged the patches into -mm and fixed up compiler
  warnings that happened if CONFIG_SCSI_LOGGING was disabled.
  I merged that into patch 3.
- The blist flag 0x800000 has been consumed by BLIST_MAX_512, so
  go for 0x1000000 instead.

Description of the patchset:

The trouble is that there are devices that report a LUN with
peripheral qualifier 3 on LUN0.
We currently don't handle this well yet:
- Blacklist flags are not used to do special handling if needed.
- If the device does NOT support the REPORT_LUNS scan, we won't
  see any LUN at all, as we don't even look for LUN 1 then.

Yes, such devices do exist, actually, see PATCH2 ..

The patches do:
- Unconditionally at least look for LUN 1 if LUN 0 was a PQ3
  device (but abort scanning there is SPARSELUN is not set
  and no device can be attached to LUN 1)
- Return the blacklist flags for further usage even if LUN 0
  was a PQ3 device.
- Introduce a blacklist flag BLIST_ATTACH_PQ3 that allows
  a PQ3 device to be registered with the OS and thus sg can
  attach to it. Add two devices to the list.
  (One could argue that this should be default, but who knows
   what virtual scsi driver will break then ...)
- Improve the scsi logging information for the the user about
  the PQ3 issue.

Patches are agains 2.6.17rc3, please consider merging for 2.6.17/18.

Best,
-- 
Kurt Garloff, Head Architect Linux R&D, Novell Inc.

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

Re: [PATCH 1/3] Continue after PQ3 on LUN0

[Kurt Garloff]

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

From: Kurt Garloff <garloff@suse.de>
Subject: Tolerate PQ 3 at LUN 0 for scanning

Some devices report a peripheral qualifier of 3 for LUN 0; with the original
code, we would still try a REPORT_LUNS scan (if SCSI level is >= 3 or if we 
have the BLIST_REPORTLUNS2 passed in), but NOT any sequential scan.
Also, the device at LUN 0 (which is not connected according to the PQ) is not
registered with the OS.

Unfortunately, SANs exist that are SCSI-2 and do NOT support REPORT_LUNS, but
report a unknown device with PQ 3 on LUN 0. We still need to scan them, and
most probably we even need BLIST_SPARSELUN (and BLIST_LARGELUN). See the next
patch for an example.

This is patch 1/3:
If we end up in sequential scan, at least try LUN 1 for devices
that reported a PQ of 3 for LUN 0.
Also return blacklist flags, even for PQ3 devices.

Signed-off-by: Kurt Garloff <garloff@suse.de>

diff -uNrp linux-2.6-hg/drivers/scsi.0/scsi_scan.c linux-2.6-hg/drivers/scsi/scsi_scan.c
--- linux-2.6-hg/drivers/scsi.0/scsi_scan.c	2006-04-19 08:55:03.000000000 +0200
+++ linux-2.6-hg/drivers/scsi/scsi_scan.c	2006-04-24 07:37:39.000000000 +0200
@@ -880,6 +880,8 @@ static int scsi_probe_and_add_lun(struct
 	if (scsi_probe_lun(sdev, result, result_len, &bflags))
 		goto out_free_result;
 
+	if (bflagsp)
+		*bflagsp = bflags;
 	/*
 	 * result contains valid SCSI INQUIRY data.
 	 */
@@ -920,8 +922,6 @@ static int scsi_probe_and_add_lun(struct
 			sdev->lockable = 0;
 			scsi_unlock_floptical(sdev, result);
 		}
-		if (bflagsp)
-			*bflagsp = bflags;
 	}
 
  out_free_result:
@@ -946,7 +946,6 @@ static int scsi_probe_and_add_lun(struct
  * scsi_sequential_lun_scan - sequentially scan a SCSI target
  * @starget:	pointer to target structure to scan
  * @bflags:	black/white list flag for LUN 0
- * @lun0_res:	result of scanning LUN 0
  *
  * Description:
  *     Generally, scan from LUN 1 (LUN 0 is assumed to already have been
@@ -956,8 +955,7 @@ static int scsi_probe_and_add_lun(struct
  *     Modifies sdevscan->lun.
  **/
 static void scsi_sequential_lun_scan(struct scsi_target *starget,
-				     int bflags, int lun0_res, int scsi_level,
-				     int rescan)
+				     int bflags, int scsi_level, int rescan)
 {
 	unsigned int sparse_lun, lun, max_dev_lun;
 	struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
@@ -978,13 +976,6 @@ static void scsi_sequential_lun_scan(str
 		sparse_lun = 0;
 
 	/*
-	 * If not sparse lun and no device attached at LUN 0 do not scan
-	 * any further.
-	 */
-	if (!sparse_lun && (lun0_res != SCSI_SCAN_LUN_PRESENT))
-		return;
-
-	/*
 	 * If less than SCSI_1_CSS, and no special lun scaning, stop
 	 * scanning; this matches 2.4 behaviour, but could just be a bug
 	 * (to continue scanning a SCSI_1_CSS device).
@@ -1395,7 +1386,7 @@ static void __scsi_scan_target(struct de
 			 * do a sequential scan.
 			 */
 			scsi_sequential_lun_scan(starget, bflags,
-				       	res, starget->scsi_level, rescan);
+						 starget->scsi_level, rescan);
 	}
 
  out_reap:
-- 
Kurt Garloff, Head Architect Linux R&D, Novell Inc.

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

Re: [PATCH 2/3] BLIST_ATTACH_PQ3 flag

[Kurt Garloff]

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


From: Kurt Garloff <garloff@suse.de>
Subject: Introduce BLIST_ATTACH_PQ3 flag

Some devices report a peripheral qualifier of 3 for LUN 0; with the original
code, we would still try a REPORT_LUNS scan (if SCSI level is >= 3 or if we 
have the BLIST_REPORTLUNS2 passed in), but NOT any sequential scan.
Also, the device at LUN 0 (which is not connected according to the PQ) is not
registered with the OS.

Unfortunately, SANs exist that are SCSI-2 and do NOT support REPORT_LUNS, but
report a unknown device with PQ 3 on LUN 0. We still need to scan them, and
most probably we even need BLIST_SPARSELUN (and BLIST_LARGELUN). See the
referenced device for an example.

This is patch 2/3:
Implement the blacklist flag BLIST_ATTACH_PQ3 that makes the scsi
 scanning code register PQ3 devices and continues scanning; only sg
 will attach thanks to scsi_bus_match().

Signed-off-by: Kurt Garloff <garloff@suse.de>

diff -uNrp linux-2.6-hg/include/scsi.1/scsi_devinfo.h linux-2.6-hg/include/scsi/scsi_devinfo.h
--- linux-2.6-hg/include/scsi.1/scsi_devinfo.h	2006-04-19 08:55:05.000000000 +0200
+++ linux-2.6-hg/include/scsi/scsi_devinfo.h	2006-04-24 07:40:43.000000000 +0200
@@ -29,4 +29,5 @@
 #define BLIST_SELECT_NO_ATN	0x200000 /* select without ATN */
 #define BLIST_RETRY_HWERROR	0x400000 /* retry HARDWARE_ERROR */
 #define BLIST_MAX_512		0x800000 /* maximum 512 sector cdb length */
+#define BLIST_ATTACH_PQ3	0x1000000 /* Scan: Attach to PQ3 devices */
 #endif
diff -uNrp linux-2.6-hg/drivers/scsi.1/scsi_devinfo.c linux-2.6-hg/drivers/scsi/scsi_devinfo.c
--- linux-2.6-hg/drivers/scsi.1/scsi_devinfo.c	2006-04-19 08:55:03.000000000 +0200
+++ linux-2.6-hg/drivers/scsi/scsi_devinfo.c	2006-04-24 07:40:03.000000000 +0200
@@ -157,6 +157,8 @@ static struct {
 	{"HITACHI", "DF400", "*", BLIST_SPARSELUN},
 	{"HITACHI", "DF500", "*", BLIST_SPARSELUN},
 	{"HITACHI", "DF600", "*", BLIST_SPARSELUN},
+	{"HITACHI", "DISK-SUBSYSTEM", "*", BLIST_ATTACH_PQ3 | BLIST_SPARSELUN | BLIST_LARGELUN},
+	{"HITACHI", "OPEN-E", "*", BLIST_ATTACH_PQ3 | BLIST_SPARSELUN | BLIST_LARGELUN},
 	{"HP", "A6189A", NULL, BLIST_SPARSELUN | BLIST_LARGELUN},	/* HP VA7400 */
 	{"HP", "OPEN-", "*", BLIST_SPARSELUN | BLIST_LARGELUN}, /* HP XP Arrays */
 	{"HP", "NetRAID-4M", NULL, BLIST_FORCELUN},
diff -uNrp linux-2.6-hg/drivers/scsi.1/scsi_scan.c linux-2.6-hg/drivers/scsi/scsi_scan.c
--- linux-2.6-hg/drivers/scsi.1/scsi_scan.c	2006-04-24 07:37:39.000000000 +0200
+++ linux-2.6-hg/drivers/scsi/scsi_scan.c	2006-04-24 07:40:03.000000000 +0200
@@ -885,7 +885,7 @@ static int scsi_probe_and_add_lun(struct
 	/*
 	 * result contains valid SCSI INQUIRY data.
 	 */
-	if ((result[0] >> 5) == 3) {
+	if (((result[0] >> 5) == 3) && !(bflags & BLIST_ATTACH_PQ3)) {
 		/*
 		 * For a Peripheral qualifier 3 (011b), the SCSI
 		 * spec says: The device server is not capable of
-- 
Kurt Garloff, Head Architect Linux R&D, Novell Inc.

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

Re: [PATCH 3/3] Print warning for PQ3 devs

[Kurt Garloff]

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


From: Kurt Garloff <garloff@suse.de>
Subject: Log a message if we stop scanning because dev reports PQ==3

Some devices report a peripheral qualifier of 3 for LUN 0; with the original
code, we would still try a REPORT_LUNS scan (if SCSI level is >= 3 or if we 
have the BLIST_REPORTLUNS2 passed in), but NOT any sequential scan.
Also, the device at LUN 0 (which is not connected according to the PQ) is not
registered with the OS.

Unfortunately, SANs exist that are SCSI-2 and do NOT support REPORT_LUNS, but
report a unknown device with PQ 3 on LUN 0. We still need to scan them, and
most probably we even need BLIST_SPARSELUN (and BLIST_LARGELUN). See the bug
reference for an infamous example.

SCSI_CHECK_LOGGING macro and warning fixes from Andrew Morton.

This patch 3/3:
If a PQ3 device is found, log a message that describes the device
(INQUIRY DATA and C:B:T:U tuple) and make a suggestion for blacklisting
it.

Signed-off-by: Kurt Garloff <garloff@suse.de>

diff -uNrp linux-2.6-hg/drivers/scsi.2/scsi_logging.h linux-2.6-hg/drivers/scsi/scsi_logging.h
--- linux-2.6-hg/drivers/scsi.2/scsi_logging.h	2005-07-19 03:34:43.000000000 +0200
+++ linux-2.6-hg/drivers/scsi/scsi_logging.h	2006-04-27 23:03:29.000000000 +0200
@@ -45,10 +45,12 @@ extern unsigned int scsi_logging_level;
         ((scsi_logging_level >> (SHIFT)) & ((1 << (BITS)) - 1))
 
 #define SCSI_CHECK_LOGGING(SHIFT, BITS, LEVEL, CMD)		\
-{								\
+do {								\
         if (unlikely((SCSI_LOG_LEVEL(SHIFT, BITS)) > (LEVEL)))	\
-		(CMD);						\
-}
+		do {						\
+			CMD;					\
+		} while (0);					\
+} while (0)
 #else
 #define SCSI_CHECK_LOGGING(SHIFT, BITS, LEVEL, CMD)
 #endif /* CONFIG_SCSI_LOGGING */
diff -uNrp linux-2.6-hg/drivers/scsi.2/scsi_scan.c linux-2.6-hg/drivers/scsi/scsi_scan.c
--- linux-2.6-hg/drivers/scsi.2/scsi_scan.c	2006-04-24 07:40:03.000000000 +0200
+++ linux-2.6-hg/drivers/scsi/scsi_scan.c	2006-04-27 23:05:14.000000000 +0200
@@ -816,6 +816,32 @@ static inline void scsi_destroy_sdev(str
 	put_device(&sdev->sdev_gendev);
 }
 
+#ifdef CONFIG_SCSI_LOGGING
+/** 
+ * scsi_inq_str - print INQUIRY data from min to max index,
+ * strip trailing whitespace
+ * @buf:   Output buffer with at least end-first+1 bytes of space
+ * @inq:   Inquiry buffer (input)
+ * @first: Offset of string into inq
+ * @end:   Index after last character in inq
+ */
+static unsigned char *scsi_inq_str(unsigned char *buf, unsigned char *inq,
+				   unsigned first, unsigned end)
+{
+	unsigned term = 0, idx;
+
+	for (idx = 0; idx + first < end && idx + first < inq[4] + 5; idx++) {
+		if (inq[idx+first] > ' ') {
+			buf[idx] = inq[idx+first];
+			term = idx+1;
+		} else {
+			buf[idx] = ' ';
+		}
+	}
+	buf[term] = 0;
+	return buf;
+}
+#endif
 
 /**
  * scsi_probe_and_add_lun - probe a LUN, if a LUN is found add it
@@ -896,9 +922,22 @@ static int scsi_probe_and_add_lun(struct
 		 * logical disk configured at sdev->lun, but there
 		 * is a target id responding.
 		 */
-		SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO
-					"scsi scan: peripheral qualifier of 3,"
-					" no device added\n"));
+		SCSI_LOG_SCAN_BUS(2, sdev_printk(KERN_INFO, sdev, "scsi scan:"
+				   " peripheral qualifier of 3, device not"
+				   " added\n"))
+		if (lun == 0) {
+			SCSI_LOG_SCAN_BUS(1, {
+				unsigned char vend[9];
+				unsigned char mod[17];
+
+				sdev_printk(KERN_INFO, sdev,
+					"scsi scan: consider passing scsi_mod."
+					"dev_flags=%s:%s:0x240 or 0x1000240\n",
+					scsi_inq_str(vend, result, 8, 16),
+					scsi_inq_str(mod, result, 16, 32));
+			});
+		}
+		
 		res = SCSI_SCAN_TARGET_PRESENT;
 		goto out_free_result;
 	}
-- 
Kurt Garloff, Head Architect Linux R&D, Novell Inc.

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

Re: [PATCH 0/3] Resend: Handle PQ3 devs better

[James Bottomley]

On Mon, 2006-05-01 at 02:44 +0200, Kurt Garloff wrote:
> resending the patches ...
> Changes since last posting:
> - I reversed the order of patches 2 and 3, as one patch (now 3)
>   refers to a blacklist flag introduced in the other.
> - Andrew Morton has merged the patches into -mm and fixed up compiler
>   warnings that happened if CONFIG_SCSI_LOGGING was disabled.
>   I merged that into patch 3.
> - The blist flag 0x800000 has been consumed by BLIST_MAX_512, so
>   go for 0x1000000 instead.

OK ... I already did that merger; could you take a look and see what's
in scsi-misc-2.6 and submit patches against that.

Thanks,

James