Skip to contentSkip to conversation

Add Blacklist for RAID configuration luns

RFC PATCH In review Jeremy Higdon 5 participants 13 messages Sep 14, 2004 – Sep 16, 2004 SCSI

lore
Share & tools
lore

Jeremy Higdon

author
#0 lore raw

RFC PATCH 1/1 Add Blacklist for RAID configuration luns

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 <[email protected]>

# This is a BitKeeper generated diff -Nru style patch. #
# ChangeSet
# 2004/09/13 23:00:11-07:00 [email protected]
# Add SCSI blacklist for RAID and LSI/Engenio UTM lun
#

# include/scsi/scsi_devinfo.h
#   2004/09/13 23:00:01-07:00 [email protected] +1 -0
#   Add SCSI blacklist for RAID and LSI UTM lun
# 
# drivers/scsi/scsi_scan.c
#   2004/09/13 23:00:01-07:00 [email protected] +9 -0
#   Add SCSI blacklist for RAID and LSI UTM lun
# 
# drivers/scsi/scsi_devinfo.c
#   2004/09/13 23:00:00-07:00 [email protected] +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:

include/scsi/scsi_devinfo.h +1 -0
diff -Nru a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
@@ -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
#1 lore raw
Quoted 6 lines · 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).

What error messages are output without your patch?

-- Patrick Mansfield

author
#2 lore raw
Quoted 11 lines · Patrick Mansfield
Quoted 6 lines · 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).

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

author
#3 lore raw

RFC PATCH 1/1 add blacklist attribute indicating no ULD attach

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 [email protected] 
#   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 [email protected] +1 -1
#   Add BLIST_NO_ULD_ATTACH blacklist attribute.
# 
# include/scsi/scsi_device.h
#   2004/09/14 19:47:51-07:00 [email protected] +1 -0
#   Add no_uld_attach flag to struct scsi_device.
# 
# drivers/scsi/scsi_sysfs.c
#   2004/09/14 19:47:51-07:00 [email protected] +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 [email protected] +2 -7
#   Set no_uld_attach flag for lun if indicated.
# 
# drivers/scsi/scsi_devinfo.c
#   2004/09/14 19:47:51-07:00 [email protected] +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:

drivers/scsi/scsi_sysfs.c +2 -0
diff -Nru a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
@@ -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;
 }
include/scsi/scsi_device.h +1 -0
diff -Nru a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
@@ -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. */
include/scsi/scsi_devinfo.h +1 -0
diff -Nru a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
@@ -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
#4 lore raw

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

Quoted 3 lines · 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).

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

Signature · 1 line
Jens Axboe
#5 lore raw

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

In reply to Jeremy Higdon

Review

Quoted 2 lines · Jeremy Higdon
(quoted diff) +1 -0
 	}
+	else if (*bflags & BLIST_NO_ULD_ATTACH) {

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

Review

Quoted 2 lines
(quoted diff) +2 -0
+		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?

author
#6 lore raw

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

Quoted 5 lines · Christoph Hellwig

Review

Quoted 2 lines · Jeremy Higdon
(quoted diff) +1 -0
 	}
+	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) . . .

Quoted 6 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+		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 <[email protected]>

===== drivers/scsi/scsi_scan.c 1.130 vs edited =====

drivers/scsi/scsi_scan.c +1 -2
@@ -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;
 	}
#7 lore raw

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

Quoted 9 lines · Jeremy Higdon
Quoted 5 lines · Christoph Hellwig

Review

Quoted 2 lines · Jeremy Higdon
(quoted diff) +1 -0
 	}
+	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 :-)

Signature · 1 line
Jens Axboe
author
#8 lore raw

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

Quoted 3 lines · Jens Axboe

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

#9 lore raw

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

Quoted 10 lines · Jeremy Higdon
Quoted 3 lines · Jens Axboe

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 :-)

Signature · 1 line
Jens Axboe
#10 lore raw

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

In reply to Christoph Hellwig
Quoted 3 lines · Christoph Hellwig

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

author
#11 lore raw

PATCH 1/1 add blacklist attribute indicating no ULD attach

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 <[email protected]>
drivers/scsi/scsi_devinfo.c +1 -0
diff -Nru a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
@@ -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 */
drivers/scsi/scsi_scan.c +2 -1
diff -Nru a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
@@ -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:
drivers/scsi/scsi_sysfs.c +2 -0
diff -Nru a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
@@ -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;
 }
include/scsi/scsi_device.h +1 -0
diff -Nru a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
@@ -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. */
include/scsi/scsi_devinfo.h +1 -0
diff -Nru a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
@@ -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
#12 lore raw

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

Quoted 4 lines · 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 :-)

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.

Signature · 1 line
Jens Axboe