On 01/22/2013 04:08 PM, Ewan Milne wrote:
On Fri, 2013-01-18 at 16:46 +0000, James Bottomley wrote:
On Fri, 2013-01-18 at 11:27 -0500, Ewan D. Milne wrote:
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -241,6 +241,9 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
if (! scsi_command_normalize_sense(scmd, &sshdr))
return FAILED; /* no valid sense data */
+ if (sshdr.overflow)
+ scmd_printk(KERN_WARNING, scmd, "Sense data overflow");
+
if (scsi_sense_is_deferred(&sshdr))
return NEEDS_RETRY;
@@ -2059,14 +2062,18 @@ int scsi_normalize_sense(const u8 *sense_buffer, int sb_len,
sshdr->asc = sense_buffer[2];
if (sb_len > 3)
sshdr->ascq = sense_buffer[3];
+ if (sb_len > 4)
+ sshdr->overflow = ((sense_buffer[4] & 0x80) != 0);
if (sb_len > 7)
sshdr->additional_length = sense_buffer[7];
} else {
/*
* fixed format
*/
- if (sb_len > 2)
+ if (sb_len > 2) {
+ sshdr->overflow = ((sense_buffer[2] & 0x10) != 0);
sshdr->sense_key = (sense_buffer[2] & 0xf);
+ }
if (sb_len > 7) {
sb_len = (sb_len < (sense_buffer[7] + 8)) ?
sb_len : (sense_buffer[7] + 8);
This isn't the right way to do it: The overflow bit is a recent
introduction in SPC-4. The correct way to tell if we have an overflow
or not is to look at the additional sense length and compare it to the
allocation length; this will work for everything.
Unfortunately, I am not sure that the allocation length that was sent
to the device is always available. I will look into this more closely
but it appeared to me that e.g. FC drivers like qla2xxx get the sense
data automatically from the HBA firmware. In the case of that driver
the host sense buffer size looks like it is hard-coded to 32 bytes,
for all I know the firmware might only asking for 18 bytes.
Hmm.
Maybe we should be adding a 'max_sense_len' field to the SCSI host;
that way we could check against this.
Nevertheless, that information would be lost to us in either case.
I wonder how these vendors propose to handle long sense code data;
silently ignoring it doesn't seem to be the correct way.
Of course, for a normal REQUEST SENSE command where the allocation
length is in the CDB, it would indeed be easy to add a check against
the additional sense length.
I'm not even convinced that overflow is important: for a lot of the
sense probes, we deliberately induce overflows by giving the request
sense command a short buffer. Printing a warning in scsi_check_sense
will get very noisy very fast.
That would indeed be a problem. I didn't see this behavior when testing
the changes but I'll need to investigate this further.
The purpose of detecting the sense data overflow was to provide some
visibility that a device is returning a large amount of sense data that
is currently being silently ignored. In the case of descriptor format
sense data, it is possible that a descriptor we want to examine is
located after one or more other descriptors, and we might not get it
at all if the buffer isn't large enough.
Agreed. but the newest standard won't be adopted to existing
devices, so we should figure out a way of dealing with those, too.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
