[RFC & PATCH 2.6.9-rc1] Add Blacklist for RAID configuration luns

[RFC & PATCH 2.6.9-rc1] Add Blacklist for RAID configuration luns

[Jeremy Higdon]

Engenio RAIDs have special luns that are used for configuring the RAID.
These luns have a device type of 0 (disk), but do not adhere to
standard disk semantics.  To avoid having to parse potentially dozens
of error messages during initialization, it's good to avoid attaching
these devices to sd.  This can be done by changing the device type
to RAID (0xC).

This is somewhat messy, because the device doesn't actually adhere
to the SCSI Controller Commands specification.  An alternative would
be to access the blacklist flags from sd_probe and look for BLIST_ISRAID
there.

One can imagine several other options.  If this one is acceptable, I
think it gives the clearest output to the user (who isn't aware of
the distinction between a RAID controller that implements the SCC
command set), but I'm willing to be swayed by arguments of architectural
purity  :-)

Note that in the part of the patch that adds the blacklist entry, I
would expect several different vendor IDs to be added . . . .

signed-off-by: Jeremy Higdon <jeremy@sgi.com>

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/09/13 23:00:11-07:00 jeremy@tomahawk.engr.sgi.com 
#   Add SCSI blacklist for RAID and LSI/Engenio UTM lun
# 
# include/scsi/scsi_devinfo.h
#   2004/09/13 23:00:01-07:00 jeremy@tomahawk.engr.sgi.com +1 -0
#   Add SCSI blacklist for RAID and LSI UTM lun
# 
# drivers/scsi/scsi_scan.c
#   2004/09/13 23:00:01-07:00 jeremy@tomahawk.engr.sgi.com +9 -0
#   Add SCSI blacklist for RAID and LSI UTM lun
# 
# drivers/scsi/scsi_devinfo.c
#   2004/09/13 23:00:00-07:00 jeremy@tomahawk.engr.sgi.com +1 -0
#   Add SCSI blacklist for RAID and LSI UTM lun
# 
diff -Nru a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
--- a/drivers/scsi/scsi_devinfo.c	2004-09-13 23:01:40 -07:00
+++ b/drivers/scsi/scsi_devinfo.c	2004-09-13 23:01:40 -07:00
@@ -182,6 +182,7 @@
 	{"SGI", "RAID3", "*", BLIST_SPARSELUN},
 	{"SGI", "RAID5", "*", BLIST_SPARSELUN},
 	{"SGI", "TP9100", "*", BLIST_REPORTLUN2},
+	{"SGI", "Universal Xport", "*", BLIST_ISRAID},
 	{"SMSC", "USB 2 HS-CF", NULL, BLIST_SPARSELUN},
 	{"SONY", "CD-ROM CDU-8001", NULL, BLIST_BORKEN},
 	{"SONY", "TSL", NULL, BLIST_FORCELUN},		/* DDS3 & DDS4 autoloaders */
diff -Nru a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
--- a/drivers/scsi/scsi_scan.c	2004-09-13 23:01:41 -07:00
+++ b/drivers/scsi/scsi_scan.c	2004-09-13 23:01:41 -07:00
@@ -532,6 +532,14 @@
 		inq_result[0] = TYPE_ROM;
 		inq_result[1] |= 0x80;	/* removable */
 	}
+	else if (*bflags & BLIST_ISRAID) {
+		/*
+		 * Some devices that identify themselves as disks are really
+		 * special RAID configuration devices, and should not be
+		 * attached to sd.
+		 */
+		inq_result[0] = (inq_result[0] & 0xE0) | TYPE_RAID;
+	}
 
 	switch (sdev->type = (inq_result[0] & 0x1f)) {
 	case TYPE_TAPE:
@@ -543,6 +551,7 @@
 	case TYPE_MEDIUM_CHANGER:
 	case TYPE_ENCLOSURE:
 	case TYPE_COMM:
+	case TYPE_RAID:
 		sdev->writeable = 1;
 		break;
 	case TYPE_WORM:
diff -Nru a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
--- a/include/scsi/scsi_devinfo.h	2004-09-13 23:01:40 -07:00
+++ b/include/scsi/scsi_devinfo.h	2004-09-13 23:01:40 -07:00
@@ -25,4 +25,5 @@
  					   (if HBA supports more than 8 LUNs) */
 #define BLIST_NOREPORTLUN	0x40000	/* don't try REPORT_LUNS scan (SCSI-3 devs) */
 #define BLIST_NOT_LOCKABLE	0x80000	/* don't use PREVENT-ALLOW commands */
+#define BLIST_ISRAID		0x100000 /* device is actually for RAID config */
 #endif

Re: [RFC & PATCH 2.6.9-rc1] Add Blacklist for RAID configuration luns

[Patrick Mansfield]

On Mon, Sep 13, 2004 at 11:32:37PM -0700, Jeremy Higdon wrote:
> Engenio RAIDs have special luns that are used for configuring the RAID.
> These luns have a device type of 0 (disk), but do not adhere to
> standard disk semantics.  To avoid having to parse potentially dozens
> of error messages during initialization, it's good to avoid attaching
> these devices to sd.  This can be done by changing the device type
> to RAID (0xC).

What error messages are output without your patch?

-- Patrick Mansfield

Re: [RFC & PATCH 2.6.9-rc1] Add Blacklist for RAID configuration luns

[Jeremy Higdon]

On Tue, Sep 14, 2004 at 07:40:23AM -0700, Patrick Mansfield wrote:
> On Mon, Sep 13, 2004 at 11:32:37PM -0700, Jeremy Higdon wrote:
> > Engenio RAIDs have special luns that are used for configuring the RAID.
> > These luns have a device type of 0 (disk), but do not adhere to
> > standard disk semantics.  To avoid having to parse potentially dozens
> > of error messages during initialization, it's good to avoid attaching
> > these devices to sd.  This can be done by changing the device type
> > to RAID (0xC).
> 
> What error messages are output without your patch?
> 
> -- Patrick Mansfield

If you try to read block 1 of the device, for example, you get these
errors (cut & pasted from /var/log/messages):

	Sep 13 16:12:51 perflinux29 kernel: Buffer I/O error on device sdo, logical block 0
	Sep 13 16:12:51 perflinux29 kernel: Buffer I/O error on device sdo, logical block 1
	Sep 13 16:12:51 perflinux29 kernel: Buffer I/O error on device sdo, logical block 2
	Sep 13 16:12:51 perflinux29 kernel: Buffer I/O error on device sdo, logical block 3
	Sep 13 16:12:51 perflinux29 kernel: end_request: I/O error, dev sdt, sector 0

On SLES9, for example, you get these messages when hwscan or blkid
is run (they're opening disk devices and presumably scanning for
partition tables).  There are several system utilities that might
do this sort scanning (lvm, for example).

The error occurs, because the lun in question returns errors for
any request beyond block 64, and the new readahead algorithm
generally issues requests for 128 blocks even when only one block
is requested.

If we convince sd not to attach to this device, the problem goes
away.  Thus, the new blacklist type and new entry.

jeremy

[RFC] [PATCH 2.6.9-rc1] add blacklist attribute indicating no ULD attach

[Jeremy Higdon]

After an IRC chat with jejb, I have a new patch to proffer.

This one takes a different tack that is cleaner (thanks James).


# This is a BitKeeper generated diff -Nru style patch.
# 
# ChangeSet
#   2004/09/14 19:48:01-07:00 jeremy@tomahawk.engr.sgi.com 
#   Add new SCSI blacklist attribute to indicate that a lun should not be
#   attached to an upper level driver, and use that attribute for UTM luns
#   for SGI branded LSI/Engenio RAIDs.
# 
# include/scsi/scsi_devinfo.h
#   2004/09/14 19:47:51-07:00 jeremy@tomahawk.engr.sgi.com +1 -1
#   Add BLIST_NO_ULD_ATTACH blacklist attribute.
# 
# include/scsi/scsi_device.h
#   2004/09/14 19:47:51-07:00 jeremy@tomahawk.engr.sgi.com +1 -0
#   Add no_uld_attach flag to struct scsi_device.
# 
# drivers/scsi/scsi_sysfs.c
#   2004/09/14 19:47:51-07:00 jeremy@tomahawk.engr.sgi.com +2 -0
#   If no_uld_attach flag is set, force "mismatch" result to scsi_bus_match().
# 
# drivers/scsi/scsi_scan.c
#   2004/09/14 19:47:51-07:00 jeremy@tomahawk.engr.sgi.com +2 -7
#   Set no_uld_attach flag for lun if indicated.
# 
# drivers/scsi/scsi_devinfo.c
#   2004/09/14 19:47:51-07:00 jeremy@tomahawk.engr.sgi.com +1 -1
#   Add UTM luns for SGI branded LSI/Engenio RAIDs to blacklist for no ULD attach.
# 
diff -Nru a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
--- a/drivers/scsi/scsi_devinfo.c	2004-09-14 19:58:52 -07:00
+++ b/drivers/scsi/scsi_devinfo.c	2004-09-14 19:58:52 -07:00
@@ -182,6 +182,7 @@
 	{"SGI", "RAID3", "*", BLIST_SPARSELUN},
 	{"SGI", "RAID5", "*", BLIST_SPARSELUN},
 	{"SGI", "TP9100", "*", BLIST_REPORTLUN2},
+	{"SGI", "Universal Xport", "*", BLIST_NO_ULD_ATTACH},
 	{"SMSC", "USB 2 HS-CF", NULL, BLIST_SPARSELUN},
 	{"SONY", "CD-ROM CDU-8001", NULL, BLIST_BORKEN},
 	{"SONY", "TSL", NULL, BLIST_FORCELUN},		/* DDS3 & DDS4 autoloaders */
diff -Nru a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
--- a/drivers/scsi/scsi_scan.c	2004-09-14 19:58:52 -07:00
+++ b/drivers/scsi/scsi_scan.c	2004-09-14 19:58:52 -07:00
@@ -532,6 +532,9 @@
 		inq_result[0] = TYPE_ROM;
 		inq_result[1] |= 0x80;	/* removable */
 	}
+	else if (*bflags & BLIST_NO_ULD_ATTACH) {
+		sdev->no_uld_attach = 1;
+	}
 
 	switch (sdev->type = (inq_result[0] & 0x1f)) {
 	case TYPE_TAPE:
diff -Nru a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
--- a/drivers/scsi/scsi_sysfs.c	2004-09-14 19:58:52 -07:00
+++ b/drivers/scsi/scsi_sysfs.c	2004-09-14 19:58:52 -07:00
@@ -183,6 +183,8 @@
 static int scsi_bus_match(struct device *dev, struct device_driver *gendrv)
 {
 	struct scsi_device *sdp = to_scsi_device(dev);
+	if (sdp->no_uld_attach)
+		return 0;
 	return (sdp->inq_periph_qual == SCSI_INQ_PQ_CON)? 1: 0;
 }
 
diff -Nru a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
--- a/include/scsi/scsi_device.h	2004-09-14 19:58:52 -07:00
+++ b/include/scsi/scsi_device.h	2004-09-14 19:58:52 -07:00
@@ -106,6 +106,7 @@
 	unsigned use_192_bytes_for_3f:1; /* ask for 192 bytes from page 0x3f */
 	unsigned no_start_on_add:1;	/* do not issue start on add */
 	unsigned allow_restart:1; /* issue START_UNIT in error handler */
+	unsigned no_uld_attach:1; /* disable connecting to upper level drivers */
 
 	unsigned int device_blocked;	/* Device returned QUEUE_FULL. */
 
diff -Nru a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
--- a/include/scsi/scsi_devinfo.h	2004-09-14 19:58:52 -07:00
+++ b/include/scsi/scsi_devinfo.h	2004-09-14 19:58:52 -07:00
@@ -25,4 +25,5 @@
  					   (if HBA supports more than 8 LUNs) */
 #define BLIST_NOREPORTLUN	0x40000	/* don't try REPORT_LUNS scan (SCSI-3 devs) */
 #define BLIST_NOT_LOCKABLE	0x80000	/* don't use PREVENT-ALLOW commands */
+#define BLIST_NO_ULD_ATTACH	0x100000 /* device is actually for RAID config */
 #endif

Re: [RFC] [PATCH 2.6.9-rc1] add blacklist attribute indicating no ULD attach

[Jens Axboe]

On Tue, Sep 14 2004, Jeremy Higdon wrote:
> After an IRC chat with jejb, I have a new patch to proffer.
> 
> This one takes a different tack that is cleaner (thanks James).

I like this one better as well, the other one did feel like a bit of a
hack.

-- 
Jens Axboe

Re: [RFC] [PATCH 2.6.9-rc1] add blacklist attribute indicating no ULD attach

[Christoph Hellwig]

On Tue, Sep 14, 2004 at 08:18:34PM -0700, Jeremy Higdon wrote:
>  	}
> +	else if (*bflags & BLIST_NO_ULD_ATTACH) {

Please put the else if on the same line as the closing brace

> +		sdev->no_uld_attach = 1;
> +	}

don't we already carry around the blacklist flags in the scsi dev so
they could be checked laer?  If not maybe we should instead of adding
more and more flags?

Re: [RFC] [PATCH 2.6.9-rc1] add blacklist attribute indicating no ULD attach

[Jeremy Higdon]

On Wed, Sep 15, 2004 at 07:47:44AM +0100, Christoph Hellwig wrote:
> On Tue, Sep 14, 2004 at 08:18:34PM -0700, Jeremy Higdon wrote:
> >  	}
> > +	else if (*bflags & BLIST_NO_ULD_ATTACH) {
> 
> Please put the else if on the same line as the closing brace

I don't like that style, but it does seem to be specified (I
hadn't noticed before) . . .

> > +		sdev->no_uld_attach = 1;
> > +	}
> 
> don't we already carry around the blacklist flags in the scsi dev so
> they could be checked laer?  If not maybe we should instead of adding
> more and more flags?

Outside of the scope of this patch, I think.  They are in the scsi
dev, but there is a little comment about them only being used "only
to pass settings from slave_alloc to scsi core."


Here is a patch to the previous patch.  If desired, once everyone
has commented, I can send the entire thing again.

Signed-off-by: Jeremy Higdon <jeremy@sgi.com>

===== drivers/scsi/scsi_scan.c 1.130 vs edited =====
--- 1.130/drivers/scsi/scsi_scan.c	2004-09-14 19:57:44 -07:00
+++ edited/drivers/scsi/scsi_scan.c	2004-09-15 00:31:52 -07:00
@@ -531,8 +531,7 @@
 		 */
 		inq_result[0] = TYPE_ROM;
 		inq_result[1] |= 0x80;	/* removable */
-	}
-	else if (*bflags & BLIST_NO_ULD_ATTACH) {
+	} else if (*bflags & BLIST_NO_ULD_ATTACH) {
 		sdev->no_uld_attach = 1;
 	}
 

Re: [RFC] [PATCH 2.6.9-rc1] add blacklist attribute indicating no ULD attach

[Jens Axboe]

On Wed, Sep 15 2004, Jeremy Higdon wrote:
> On Wed, Sep 15, 2004 at 07:47:44AM +0100, Christoph Hellwig wrote:
> > On Tue, Sep 14, 2004 at 08:18:34PM -0700, Jeremy Higdon wrote:
> > >  	}
> > > +	else if (*bflags & BLIST_NO_ULD_ATTACH) {
> > 
> > Please put the else if on the same line as the closing brace
> 
> I don't like that style, but it does seem to be specified (I
> hadn't noticed before) . . .

Well if we are going to be picky, you should not be using braces for
this single-line case at all :-)

-- 
Jens Axboe

Re: [RFC] [PATCH 2.6.9-rc1] add blacklist attribute indicating no ULD attach

[Jeremy Higdon]

On Wed, Sep 15, 2004 at 09:40:32AM +0200, Jens Axboe wrote:
> 
> Well if we are going to be picky, you should not be using braces for
> this single-line case at all :-)

I always figured that if one part of the "if" statement had braces,
then the whole thing should.  On a quick scan, I can't see anything
in CodingStyle that mandates minimal brace usage.

Am I missing something, or is this an unwritten advisory?  :-)

thanks

jeremy

Re: [RFC] [PATCH 2.6.9-rc1] add blacklist attribute indicating no ULD attach

[Jens Axboe]

On Wed, Sep 15 2004, Jeremy Higdon wrote:
> On Wed, Sep 15, 2004 at 09:40:32AM +0200, Jens Axboe wrote:
> > 
> > Well if we are going to be picky, you should not be using braces for
> > this single-line case at all :-)
> 
> I always figured that if one part of the "if" statement had braces,
> then the whole thing should.  On a quick scan, I can't see anything
> in CodingStyle that mandates minimal brace usage.
> 
> Am I missing something, or is this an unwritten advisory?  :-)

Maybe unwritten in CodingStyle, but it's definitely written that way in
most of the core kernel :-)

-- 
Jens Axboe

Re: [RFC] [PATCH 2.6.9-rc1] add blacklist attribute indicating no ULD attach

[James Bottomley]

On Wed, 2004-09-15 at 02:47, Christoph Hellwig wrote:
> don't we already carry around the blacklist flags in the scsi dev so
> they could be checked laer?  If not maybe we should instead of adding
> more and more flags?

Not in this case ... I'm planning a wider use of this flag for a lazy
attachment idea I'm hatching.

James

[PATCH 2.6.9-rc1] add blacklist attribute indicating no ULD attach

[Jeremy Higdon]

Add blacklist attribute to disable ULD attachment to a lun, and use
that attribute for SGI-branded Engenio UTM luns.

This incorporates input on braces :-)

Signed-off-by: Jeremy Higdon  <jeremy@sgi.com>

diff -Nru a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
--- a/drivers/scsi/scsi_devinfo.c	2004-09-15 14:21:22 -07:00
+++ b/drivers/scsi/scsi_devinfo.c	2004-09-15 14:21:22 -07:00
@@ -182,6 +182,7 @@
 	{"SGI", "RAID3", "*", BLIST_SPARSELUN},
 	{"SGI", "RAID5", "*", BLIST_SPARSELUN},
 	{"SGI", "TP9100", "*", BLIST_REPORTLUN2},
+	{"SGI", "Universal Xport", "*", BLIST_NO_ULD_ATTACH},
 	{"SMSC", "USB 2 HS-CF", NULL, BLIST_SPARSELUN},
 	{"SONY", "CD-ROM CDU-8001", NULL, BLIST_BORKEN},
 	{"SONY", "TSL", NULL, BLIST_FORCELUN},		/* DDS3 & DDS4 autoloaders */
diff -Nru a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
--- a/drivers/scsi/scsi_scan.c	2004-09-15 14:21:22 -07:00
+++ b/drivers/scsi/scsi_scan.c	2004-09-15 14:21:22 -07:00
@@ -531,7 +531,8 @@
 		 */
 		inq_result[0] = TYPE_ROM;
 		inq_result[1] |= 0x80;	/* removable */
-	}
+	} else if (*bflags & BLIST_NO_ULD_ATTACH)
+		sdev->no_uld_attach = 1;
 
 	switch (sdev->type = (inq_result[0] & 0x1f)) {
 	case TYPE_TAPE:
diff -Nru a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
--- a/drivers/scsi/scsi_sysfs.c	2004-09-15 14:21:22 -07:00
+++ b/drivers/scsi/scsi_sysfs.c	2004-09-15 14:21:22 -07:00
@@ -183,6 +183,8 @@
 static int scsi_bus_match(struct device *dev, struct device_driver *gendrv)
 {
 	struct scsi_device *sdp = to_scsi_device(dev);
+	if (sdp->no_uld_attach)
+		return 0;
 	return (sdp->inq_periph_qual == SCSI_INQ_PQ_CON)? 1: 0;
 }
 
diff -Nru a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
--- a/include/scsi/scsi_device.h	2004-09-15 14:21:22 -07:00
+++ b/include/scsi/scsi_device.h	2004-09-15 14:21:22 -07:00
@@ -106,6 +106,7 @@
 	unsigned use_192_bytes_for_3f:1; /* ask for 192 bytes from page 0x3f */
 	unsigned no_start_on_add:1;	/* do not issue start on add */
 	unsigned allow_restart:1; /* issue START_UNIT in error handler */
+	unsigned no_uld_attach:1; /* disable connecting to upper level drivers */
 
 	unsigned int device_blocked;	/* Device returned QUEUE_FULL. */
 
diff -Nru a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
--- a/include/scsi/scsi_devinfo.h	2004-09-15 14:21:22 -07:00
+++ b/include/scsi/scsi_devinfo.h	2004-09-15 14:21:22 -07:00
@@ -25,4 +25,5 @@
  					   (if HBA supports more than 8 LUNs) */
 #define BLIST_NOREPORTLUN	0x40000	/* don't try REPORT_LUNS scan (SCSI-3 devs) */
 #define BLIST_NOT_LOCKABLE	0x80000	/* don't use PREVENT-ALLOW commands */
+#define BLIST_NO_ULD_ATTACH	0x100000 /* device is actually for RAID config */
 #endif

Re: [PATCH 2.6.9-rc1] add blacklist attribute indicating no ULD attach

[Jens Axboe]

On Wed, Sep 15 2004, Jeremy Higdon wrote:
> Add blacklist attribute to disable ULD attachment to a lun, and use
> that attribute for SGI-branded Engenio UTM luns.
> 
> This incorporates input on braces :-)

Your style is now exemplary, the patch should be hung on the refigerator
as a good example for the entire family :-)

I think the patch should be included.

-- 
Jens Axboe