sd: workaround invalid OPTIMAL TRANSFER LENGTH

PATCH In review Mike Snitzer 3 participants 4 messages Apr 3, 2013 – Apr 25, 2013 SCSI

Share & tools

Mike Snitzer

author

2013-04-03 20:21 UTC #0 lore raw

PATCH 1/1 sd: workaround invalid OPTIMAL TRANSFER LENGTH

Workaround disk firmware that improperly sets OPTIMAL TRANSFER LENGTH to 0xFFFFFFFF (aka UINT_MAX or 4294967295U) by assuming this _optional_ BLOCK LIMITS VPD field was not specified (0).

It should be noted that a disk firmware fix is being developed but that these disks are already in the wild (and when optimal_io_size is so large it prevents standard Linux distribution installers, e.g. Anaconda, from creating partitions smaller than 4GB via libparted due to parted looking to align the created partitions relative to optimal_io_size).

The disk model in this instance is "SEAGATE ST900MM0006". The BLOCK LIMITS VPD page for this disk is:

# sg_inq --vpd --page=0xb0 /dev/sdb
VPD INQUIRY: Block limits page (SBC)
  Optimal transfer length granularity: 1 blocks
  Maximum transfer length: 0 blocks
  Optimal transfer length: 4294967295 blocks
  Maximum prefetch, xdread, xdwrite transfer length: 0 blocks
  Maximum unmap LBA count: 0
  Maximum unmap block descriptor count: 0
  Optimal unmap granularity: 0
  Unmap granularity alignment valid: 0
  Unmap granularity alignment: 0

Before this fix:

# cat /sys/block/sdb/queue/optimal_io_size
4294966784

After this fix:

# cat /sys/block/sdb/queue/optimal_io_size
0

Signed-off-by Mike Snitzer <[email protected]>

---
 drivers/scsi/sd.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

drivers/scsi/sd.c +5 -1

index 7992635..34638c1 100644
@@ -2516,6 +2516,7 @@ static void sd_read_app_tag_own(struct scsi_disk *sdkp, unsigned char *buffer)
  */
 static void sd_read_block_limits(struct scsi_disk *sdkp)
 {
+	unsigned int optimal_transfer_length;
 	unsigned int sector_sz = sdkp->device->sector_size;
 	const int vpd_len = 64;
 	unsigned char *buffer = kmalloc(vpd_len, GFP_KERNEL);

@@ -2527,8 +2528,11 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
 
 	blk_queue_io_min(sdkp->disk->queue,
 			 get_unaligned_be16(&buffer[6]) * sector_sz);
+	optimal_transfer_length = get_unaligned_be32(&buffer[12]);
+	if (optimal_transfer_length == UINT_MAX)
+		optimal_transfer_length = 0; /* firmware bug, use 0 instead */
 	blk_queue_io_opt(sdkp->disk->queue,
-			 get_unaligned_be32(&buffer[12]) * sector_sz);
+			 optimal_transfer_length * sector_sz);
 
 	if (buffer[3] == 0x3c) {
 		unsigned int lba_count, desc_count;

Ewan Milne

Acked-by on v1 1/1

2013-04-03 20:38 UTC #1 lore raw

Quoted 4 lines · Mike Snitzer

Workaround disk firmware that improperly sets OPTIMAL TRANSFER LENGTH to 0xFFFFFFFF (aka UINT_MAX or 4294967295U) by assuming this _optional_ BLOCK LIMITS VPD field was not specified (0).

Acked-by Ewan D. Milne <[email protected]>

Martin K. Petersen

2013-04-25 01:19 UTC #2 lore raw

In reply to Mike Snitzer

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Mike" == Mike Snitzer <[email protected]> writes:

Mike,

Following up on our discussion at LSF/MM last week...

Mike> Workaround disk firmware that improperly sets OPTIMAL TRANSFER Mike> LENGTH to 0xFFFFFFFF (aka UINT_MAX or 4294967295U) by assuming Mike> this _optional_ BLOCK LIMITS VPD field was not specified (0).

My only real objection is that UINT_MAX appears to be completely coincidental. What if the next drive specifies UINT_MAX-1 or $RANDOM?

Lacking a decent heuristic I'd rather just blacklist the offending drive using the skip_vpd_pages knob we already have in place.

Signature · 14 lines

Martin K. Petersen	Oracle Linux Engineering


[SCSI] Workaround for disks that report bad optimal transfer length

Not all disks fill out the VPD pages correctly. Add a blacklist flag
that allows us ignore the SBC-3 VPD pages for a given device. The
BLIST_SKIP_VPD_PAGES flag triggers our existing skip_vpd_pages
scsi_device parameter to bypass VPD scanning.

Also blacklist the offending Seagate drive model.

Reported-by: Mike Snitzer <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>

drivers/scsi/scsi_devinfo.c +1 -0

index 43fca91..f969aca 100644
@@ -228,6 +228,7 @@ static struct {
 	{"SanDisk", "ImageMate CF-SD1", NULL, BLIST_FORCELUN},
 	{"SEAGATE", "ST34555N", "0930", BLIST_NOTQ},	/* Chokes on tagged INQUIRY */
 	{"SEAGATE", "ST3390N", "9546", BLIST_NOTQ},
+	{"SEAGATE", "ST900MM0006", NULL, BLIST_SKIP_VPD_PAGES},
 	{"SGI", "RAID3", "*", BLIST_SPARSELUN},
 	{"SGI", "RAID5", "*", BLIST_SPARSELUN},
 	{"SGI", "TP9100", "*", BLIST_REPORTLUN2},

drivers/scsi/scsi_scan.c +3 -0

index 3e58b22..b9e39e0 100644
@@ -924,6 +924,9 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 	if (*bflags & BLIST_NO_DIF)
 		sdev->no_dif = 1;
 
+	if (*bflags & BLIST_SKIP_VPD_PAGES)
+		sdev->skip_vpd_pages = 1;
+
 	transport_configure_device(&sdev->sdev_gendev);
 
 	if (sdev->host->hostt->slave_configure) {

include/scsi/scsi_devinfo.h +1 -0

index cc1f3e7..447d2d7 100644
@@ -31,4 +31,5 @@
 #define BLIST_MAX_512		0x800000 /* maximum 512 sector cdb length */
 #define BLIST_ATTACH_PQ3	0x1000000 /* Scan: Attach to PQ3 devices */
 #define BLIST_NO_DIF		0x2000000 /* Disable T10 PI (DIF) */
+#define BLIST_SKIP_VPD_PAGES	0x4000000 /* Ignore SBC-3 VPD pages */
 #endif

Mike Snitzer

author

2013-04-25 01:44 UTC #3 lore raw

Quoted 10 lines · Martin K. Petersen

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Mike" == Mike Snitzer <[email protected]> writes:
Mike,
Following up on our discussion at LSF/MM last week...
Mike> Workaround disk firmware that improperly sets OPTIMAL TRANSFER Mike> LENGTH to 0xFFFFFFFF (aka UINT_MAX or 4294967295U) by assuming Mike> this _optional_ BLOCK LIMITS VPD field was not specified (0).

Quoted 5 lines

My only real objection is that UINT_MAX appears to be completely coincidental. What if the next drive specifies UINT_MAX-1 or $RANDOM?
Lacking a decent heuristic I'd rather just blacklist the offending drive using the skip_vpd_pages knob we already have in place.

OK, sounds fine to me, thanks.