Re: [PATCH RFC 1/9] [SCSI] Detect overflow of sense data buffer

[Hannes Reinecke <hare@suse.de>]

On 01/18/2013 05:46 PM, 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.
>
> 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.
>
And indeed I would rather prefer to have it the other way round;
we're using a fixed sense_buffer within the SCSI stack, which might
not be large enough to hold all sense data.
So I would prefer to have an indicator on whether _the internal_ 
sense buffer overflowed; this would even give us some valid use-case 
now.
Plus we can add the sense buffer overflow bit to that if required.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html