Re: [PATCH] scsi: retry MODE SENSE on unit attention

[Hannes Reinecke <hare@suse.de>]

On 06/11/2015 05:07 PM, Ewan Milne wrote:
> On Thu, 2015-06-11 at 13:01 +0200, Hannes Reinecke wrote:
>> The 'sd' driver is calling scsi_mode_sense() to figure out
>> internal details. But scsi_mode_sense() never checks for
>> any pending unit attentions, so we're getting annoying error
>> messages like:
>>
>> MODE SENSE: unimplemented page/subpage: 0x00/0x00
>>
>> and a possible wrong decision for device cache handling.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>  drivers/scsi/scsi_lib.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>> index 2428d96..d7915c8 100644
>> --- a/drivers/scsi/scsi_lib.c
>> +++ b/drivers/scsi/scsi_lib.c
>> @@ -2423,7 +2423,7 @@ scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage,
>>  	unsigned char cmd[12];
>>  	int use_10_for_ms;
>>  	int header_length;
>> -	int result;
>> +	int result, retry_count = retries;
>>  	struct scsi_sense_hdr my_sshdr;
>>  
>>  	memset(data, 0, sizeof(*data));
>> @@ -2502,6 +2502,11 @@ scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage,
>>  			data->block_descriptor_length = buffer[3];
>>  		}
>>  		data->header_length = header_length;
>> +	} else if ((status_byte(result) == CHECK_CONDITION) &&
>> +		   scsi_sense_valid(sshdr) &&
>> +		   sshdr->sense_key == UNIT_ATTENTION && retry_count) {
>> +		retry_count--;
>> +		goto retry;
>>  	}
>>  
>>  	return result;
> 
> Great, but shouldn't we be doing this more generally?  What about
> scsi_mode_select()?
> 
I haven't seen any issues with scsi_mode_select() as of now, so I
didn't do anything about this :-)

> (And, with the number of status changes that can get reported by
>  UAs, we might want to think about increasing the retry count on
>  these commands up from 3 at some point.)
> 
Hmm. _Actually_, we're not getting _more_ UAs (neither the number
nor the situation at which UAs are being send has changed).
It's just that we're trying to _use_ UAs so these things pop up.
But yeah, raising the number or retries to eg 5 is probably a good idea.

> Reviewed-by: Ewan D. Milne <emilne@redhat.com>
> 
Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (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