Re: Handling erroneous READ CAPACITY response in sd.c

[Luben Tuikov <luben_tuikov@adaptec.com>]

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

Eero Volotinen wrote:
> Alan Stern wrote:
> 
>> People:
>>
>> We would like to test a method for determining automatically when the 
>> capacity value returned by a drive is too high.  If this works it 
>> means that the unusual_devs.h entries for your devices can be removed, 
>> and we'll never be troubled by similarly broken devices again.
>>
>> Please try applying the patches below.  The first one removes the 
>> existing
>> unusual_devs entries and the second one includes the code we want to 
>> test.  Test results are written to the system log, and you don't need 
>> to turn on
>> the usb-storage verbose debugging.  In your replies, include the dmesg 
>> output showing what happens when you plug in or turn on your drives.
> 
> 
> Does not work on my machine. It detects disk and then it hangs.
> 
> usb 1-4: new high speed USB device using address 3
> Initializing USB Mass Storage driver...
> usb 1-4: control timeout on ep0in
> usb 1-4: string descriptor 0 read error: -71
> usb 1-4: string descriptor 0 read error: -71
> usb 1-4: string descriptor 0 read error: -71
> usb 1-4: string descriptor 0 read error: -71
> scsi0 : SCSI emulation for USB Mass Storage devices
> usbcore: registered new driver usb-storage
> USB Mass Storage support registered.
> usb 1-4: string descriptor 0 read error: -71
> usb-storage: device found at 3
> usb 1-4: string descriptor 0 read error: -71

Alan, is this error message something to be concerned with?

> usb-storage: waiting for device to settle before scanning
> inserting floppy driver for 2.6.10-rc1alan
> floppy0: no floppy controllers found
>   Vendor: HDS72808  Model: 0PLAT20           Rev: PF2O
>   Type:   Direct-Access                      ANSI SCSI revision: 00
> sd_read_true_cap: sda: spec behavior

Ok, the above line tells that the device _does_ support
the PMI bit, and testing with the PMI bit on returned
values as _intended_ by the spec.

> sd_read_format_cap: sda: no media or unformatted media

Alan, let's weed out _only_ Descriptor Type 0 (reserved).
All others seem to be reporting _some_ capacity...
(see attached patch, I've also removed the macro defs)

The attached patch ignore the "No Media Present",
and trusts the "The reported value is for the maximum
capacity of a media that the Logical Unit is capable
of reading." -- MMC4r00, table 422.

> SCSI device sda: 160836481 512-byte hdwr sectors (82348 MB)
> sda: assuming drive cache: write through
>  sda:SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 160836480
> Buffer I/O error on device sda, logical block 160836480
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 160836480
> Buffer I/O error on device sda, logical block 160836480
>  sda1
> Attached scsi disk sda at scsi0, channel 0, id 0, lun 0
> usb-storage: device scan complete
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 0
> Buffer I/O error on device sda, logical block 0
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 1
> Buffer I/O error on device sda, logical block 1
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 2
> Buffer I/O error on device sda, logical block 2
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 3
> Buffer I/O error on device sda, logical block 3
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 4
> Buffer I/O error on device sda, logical block 4
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 5
> Buffer I/O error on device sda, logical block 5
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 6
> Buffer I/O error on device sda, logical block 6
> usb 1-4: USB disconnect, address 3
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 7
> Buffer I/O error on device sda, logical block 7
> SCSI error : <0 0 0 0> return code = 0x70000
> end_request: I/O error, dev sda, sector 8
> Buffer I/O error on device sda, logical block 8

Ok, so reading the last sector and the first 9 sectors
fails.

Either the device got disrupted earlier, or reading
the "last" LBA confused it.

>  target0:0:0: Illegal state transition <NULL>->cancel
> Badness in scsi_device_set_state at drivers/scsi/scsi_lib.c:1713

How did we get here...?

Eero, can you try the attached patch?  I wonder if we
can take the maximum capacity media which could be supported?
(This is "patch 2", you still have to apply "patch 1" which
removes the broken devices from unusual devs.)

Thanks,
	Luben


[-- Attachment #2: cap.patch --]
[-- Type: application/octet-stream, Size: 5425 bytes --]

===== drivers/scsi/sd.c 1.161 vs edited =====
--- 1.161/drivers/scsi/sd.c	2004-10-15 10:46:07 -04:00
+++ edited/drivers/scsi/sd.c	2004-10-25 13:28:23 -04:00
@@ -987,6 +987,152 @@
 	}
 }
 
+/* sd_read_true_cap: Some device servers incorrectly return the
+ * capacity as opposed to the LBA of the last logical block of the
+ * block device.
+ *
+ * We try to fix this as follows: Let x = Returned LBA from the last
+ * READ CAPACITY command issued (result in "buffer").  Reissue the
+ * READ CAPACITY command as follows: set the partial medium indicator
+ * (PMI) bit to one; set the LBA to x - 1. Fire off that READ CAPACITY
+ * command.
+ *
+ * If we get success,
+ *       If Returned LBA > x - 1, then capacity is x+1, spec behavior.
+ *       Else Returned LBA <= x - 1, then capacity is x, broken device server.
+ * Else error, nothing can be assumed, capacity is x+1.
+ */
+static void sd_read_true_cap(struct scsi_disk *sd, char *diskname,
+			     struct scsi_request *SRpnt, unsigned char *buffer,
+			     int longrc)
+{
+	unsigned char cmd[16];
+	unsigned char buf[12];
+
+	/* save the old buffer contents here */
+	memcpy(buf, buffer, 12);
+
+	if (longrc) {
+		u64 *lba = (u64 *) (cmd+2);
+		u64 rlba;
+
+		memset((void *) cmd, 0, 16);
+		cmd[0] = SERVICE_ACTION_IN;
+		cmd[1] = SAI_READ_CAPACITY_16;
+		cmd[13] = 12;
+
+		rlba = be64_to_cpu(*(u64 *)buffer);
+		rlba -= 1;
+		*lba = cpu_to_be64(rlba);
+		/* turn on the PMI bit */
+		cmd[14] |= 1;
+		memset((void *) buffer, 0, 12);
+	} else {
+		u32 *lba = (u32 *) (cmd+2);
+		u32 rlba;
+
+		cmd[0] = READ_CAPACITY;
+		memset((void *) &cmd[1], 0, 9);
+		
+		rlba = be32_to_cpu(*(u32 *)buffer);
+		rlba -= 1;
+		*lba = cpu_to_be32(rlba);
+		/* turn on the PMI bit */
+		cmd[8] |= 1;
+		memset((void *) buffer, 0, 8);
+	}
+
+	SRpnt->sr_cmd_len = 0;
+	SRpnt->sr_sense_buffer[0] = 0;
+	SRpnt->sr_sense_buffer[2] = 0;
+	SRpnt->sr_data_direction = DMA_FROM_DEVICE;
+	
+	scsi_wait_req(SRpnt, (void *) cmd, (void *) buffer,
+		      longrc ? 12 : 8, SD_TIMEOUT, SD_MAX_RETRIES);
+
+	if (SRpnt->sr_result) {
+		/* Nothing can be assumed. */
+		printk(KERN_NOTICE "%s: %s: PMI not supported\n",
+		       __FUNCTION__, diskname);
+		memcpy(buffer, buf, 12);
+		return;
+	}
+
+	if (longrc) {
+		u64 rlba = be64_to_cpu(*(u64 *)buffer);
+		u64 x    = be64_to_cpu(*(u64 *)buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	} else {
+		u32 rlba = be32_to_cpu(*(u32 *)buffer);
+		u32 x    = be32_to_cpu(*(u32 *)buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	}
+	printk(KERN_NOTICE "%s: %s: broken device server\n", __FUNCTION__,
+	       diskname);
+	return;
+
+ out_spec:
+	/* Capacity is x+1, spec behavior. */
+	printk(KERN_NOTICE "%s: %s: spec behavior\n", __FUNCTION__, diskname);
+	memcpy(buffer, buf, 12);
+} /* end sd_read_true_cap() */
+
+
+/* Try to use the READ FORMAT CAPACITIES command to check whether the
+ * READ CAPACITY result is correct.
+ */
+static void sd_read_format_cap(struct scsi_disk *sd, char *diskname,
+		       struct scsi_request *SRpnt, unsigned char *buffer)
+{
+	unsigned char cmd[12];
+	unsigned char buf[12];
+	u32 rlba;
+	u32 x;
+
+	/* save the old buffer contents here */
+	memcpy(buf, buffer, 12);
+
+	rlba = be32_to_cpu(*(u32 *)buffer);
+
+	cmd[0] = 0x23;          /* READ_FORMAT_CAPACITIES */
+	memset(&cmd[1], 0, 11);
+	cmd[8] = 12;
+	memset(buffer, 0, 12);
+
+	SRpnt->sr_cmd_len = 0;
+	SRpnt->sr_sense_buffer[0] = 0;
+	SRpnt->sr_sense_buffer[2] = 0;
+	SRpnt->sr_data_direction = DMA_FROM_DEVICE;
+
+	scsi_wait_req(SRpnt, (void *) cmd, (void *) buffer,
+		      12, SD_TIMEOUT, SD_MAX_RETRIES);
+
+	if (SRpnt->sr_result) {
+		/* Nothing can be assumed. */
+		printk(KERN_NOTICE "%s: %s: READ FORMAT CAPACITIES "
+		       "not supported\n", __FUNCTION__, diskname);
+		goto done;
+	}
+
+	x = be32_to_cpu(*(u32 *)(buffer + 4));
+	printk(KERN_NOTICE "%s: %s: %u vs. %u\n",
+	       __FUNCTION__, diskname, rlba, x);
+	if (0 < x && x <= rlba) {
+		u32 *lba = (u32 *) buf;
+		
+		printk(KERN_NOTICE "%s: %s: using lower capacity\n",
+		       __FUNCTION__, diskname);
+		x -= 1;
+		*lba = cpu_to_be32(x);
+	}
+ done:
+ 	memcpy(buffer, buf, 12);
+}
+
 /*
  * read disk capacity
  */
@@ -1070,7 +1216,14 @@
 		
 		sdkp->capacity = 1 + (sector_t) 0xffffffff;		
 		goto got_data;
-	}	
+	}
+
+	/* Check if the device reported CAPACITY as opposed to
+	 * the maxumum LBA (as per the SBC spec).
+	 */
+	sd_read_true_cap(sdkp, diskname, SRpnt, buffer, longrc);
+ 	if (!longrc)
+ 		sd_read_format_cap(sdkp, diskname, SRpnt, buffer);
 	
 	if (!longrc) {
 		sector_size = (buffer[4] << 24) |
@@ -1078,12 +1231,14 @@
 		if (buffer[0] == 0xff && buffer[1] == 0xff &&
 		    buffer[2] == 0xff && buffer[3] == 0xff) {
 			if(sizeof(sdkp->capacity) > 4) {
-				printk(KERN_NOTICE "%s : very big device. try to use"
-				       " READ CAPACITY(16).\n", diskname);
+				printk(KERN_NOTICE "%s : very big device. "
+				       "try to use READ CAPACITY(16).\n",
+				       diskname);
 				longrc = 1;
 				goto repeat;
 			} else {
-				printk(KERN_ERR "%s: too big for kernel.  Assuming maximum 2Tb\n", diskname);
+				printk(KERN_ERR "%s: too big for kernel. "
+				       "Assuming maximum 2Tb\n", diskname);
 			}
 		}
 		sdkp->capacity = 1 + (((sector_t)buffer[0] << 24) |
@@ -1102,7 +1257,7 @@
 			
 		sector_size = (buffer[8] << 24) |
 			(buffer[9] << 16) | (buffer[10] << 8) | buffer[11];
-	}	
+	}
 
 got_data:
 	if (sector_size == 0) {