Re: [PATCH 2/2] sd: Try READ CAPACITY 16 first for SBC-2 devices

[Matthew Wilcox <matthew@wil.cx>]

On Sat, Mar 14, 2009 at 03:41:31PM -0500, James Bottomley wrote:
> We're going to have to do something about the scary error messages on
> SBC-2 supporting drives, this is what mine say (and this is after mkp's
> chat reduction):
> 
> sd 1:0:1:0: [sdc] READ CAPACITY(16) failed
> sd 1:0:1:0: [sdc] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
> sd 1:0:1:0: [sdc] Sense Key : Illegal Request [current] 
> sd 1:0:1:0: [sdc] Add. Sense: Invalid command operation code
> sd 1:0:1:0: [sdc] 71096640 512-byte hardware sectors: (36.4 GB/33.9 GiB)

OK, that's relatively easy to fix.  Simply return early if the drive
claims not to understand the command, and it'll try rc10 without printing
the scary messages.  Like this, perhaps (note cunning factoring of code):

(compile tested only, and I'll do you a nice changelog and sign-off for
it if it fixes the problem and you approve of this approach).

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index f8260c0..60b31ea 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1139,6 +1139,14 @@ static int media_not_present(struct scsi_disk *sdkp,
 	return 1;
 }
 
+static int invalid_field_in_cdb(struct scsi_sense_hdr *sshdr)
+{
+	if (!scsi_sense_valid(sshdr))
+		return 0;
+	return sshdr->sense_key == ILLEGAL_REQUEST &&
+			sshdr->asc == 0x24 && sshdr->ascq == 0x0;
+}
+
 /*
  * spinup disk - called only in sd_revalidate_disk()
  */
@@ -1362,6 +1370,8 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
 
 		if (media_not_present(sdkp, &sshdr))
 			return -ENODEV;
+		if (invalid_field_in_cdb(&sshdr))
+			return -EINVAL;
 
 		if (the_result)
 			sense_valid = scsi_sense_valid(&sshdr);
@@ -1739,10 +1749,7 @@ sd_read_cache_type(struct scsi_disk *sdkp, unsigned char *buffer)
 	}
 
 bad_sense:
-	if (scsi_sense_valid(&sshdr) &&
-	    sshdr.sense_key == ILLEGAL_REQUEST &&
-	    sshdr.asc == 0x24 && sshdr.ascq == 0x0)
-		/* Invalid field in CDB */
+	if (invalid_field_in_cdb(&sshdr))
 		sd_printk(KERN_NOTICE, sdkp, "Cache data unavailable\n");
 	else
 		sd_printk(KERN_ERR, sdkp, "Asking for cache data failed\n");

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."