* [PATCH] scsi: retry MODE SENSE on unit attention
@ 2015-06-11 11:01 Hannes Reinecke
2015-06-11 15:07 ` Ewan Milne
0 siblings, 1 reply; 7+ messages in thread
From: Hannes Reinecke @ 2015-06-11 11:01 UTC (permalink / raw)
To: James Bottomley
Cc: linux-scsi, Christoph Hellwig, Nic Bellinger, Hannes Reinecke
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;
--
1.8.5.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: retry MODE SENSE on unit attention
2015-06-11 11:01 Hannes Reinecke
@ 2015-06-11 15:07 ` Ewan Milne
2015-06-12 6:27 ` Hannes Reinecke
0 siblings, 1 reply; 7+ messages in thread
From: Ewan Milne @ 2015-06-11 15:07 UTC (permalink / raw)
To: Hannes Reinecke
Cc: James Bottomley, linux-scsi, Christoph Hellwig, Nic Bellinger
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()?
(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.)
Reviewed-by: Ewan D. Milne <emilne@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: retry MODE SENSE on unit attention
2015-06-11 15:07 ` Ewan Milne
@ 2015-06-12 6:27 ` Hannes Reinecke
2015-06-12 14:08 ` Ewan Milne
0 siblings, 1 reply; 7+ messages in thread
From: Hannes Reinecke @ 2015-06-12 6:27 UTC (permalink / raw)
To: emilne; +Cc: James Bottomley, linux-scsi, Christoph Hellwig, Nic Bellinger
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: retry MODE SENSE on unit attention
2015-06-12 6:27 ` Hannes Reinecke
@ 2015-06-12 14:08 ` Ewan Milne
0 siblings, 0 replies; 7+ messages in thread
From: Ewan Milne @ 2015-06-12 14:08 UTC (permalink / raw)
To: Hannes Reinecke
Cc: James Bottomley, linux-scsi, Christoph Hellwig, Nic Bellinger
On Fri, 2015-06-12 at 08:27 +0200, Hannes Reinecke wrote:
> 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.
Yeah, or maybe add a time limit to we don't drag out the boot time.
We can do this later, though, I'm fine with your patch as it is.
>
> > Reviewed-by: Ewan D. Milne <emilne@redhat.com>
> >
> Cheers,
>
> Hannes
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] scsi: retry MODE SENSE on unit attention
@ 2015-06-12 14:12 Hannes Reinecke
0 siblings, 0 replies; 7+ messages in thread
From: Hannes Reinecke @ 2015-06-12 14:12 UTC (permalink / raw)
To: James Bottomley
Cc: linux-scsi, Christoph Hellwig, Nic Bellinger, Ewan Milne,
Hannes Reinecke
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.
Reviewed-by: Ewan Milne <emilne@redhat.com>
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 f04be65..b66652d 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;
--
1.8.5.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] scsi: retry MODE SENSE on unit attention
@ 2015-07-06 11:16 Hannes Reinecke
2015-07-06 15:19 ` Bart Van Assche
0 siblings, 1 reply; 7+ messages in thread
From: Hannes Reinecke @ 2015-07-06 11:16 UTC (permalink / raw)
To: James Bottomley
Cc: Christoph Hellwig, linux-scsi, Ewan Milne, Hannes Reinecke
The 'sd' driver is calling scsi_mode_sense() to figure out
internal details. But scsi_mode_sense() never checks for
any pending unit attentions, leading to incorrect decisions
during probing.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Ewan Milne <emilne@redhat.com>
---
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 b1a2631..4f3c120 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;
--
1.8.5.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: retry MODE SENSE on unit attention
2015-07-06 11:16 [PATCH] scsi: retry MODE SENSE on unit attention Hannes Reinecke
@ 2015-07-06 15:19 ` Bart Van Assche
0 siblings, 0 replies; 7+ messages in thread
From: Bart Van Assche @ 2015-07-06 15:19 UTC (permalink / raw)
To: Hannes Reinecke, James Bottomley
Cc: Christoph Hellwig, linux-scsi, Ewan Milne
On 07/06/2015 04:16 AM, 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, leading to incorrect decisions
> during probing.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Ewan Milne <emilne@redhat.com>
> ---
> 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 b1a2631..4f3c120 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;
Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-07-06 15:19 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-06 11:16 [PATCH] scsi: retry MODE SENSE on unit attention Hannes Reinecke
2015-07-06 15:19 ` Bart Van Assche
-- strict thread matches above, loose matches on Subject: below --
2015-06-12 14:12 Hannes Reinecke
2015-06-11 11:01 Hannes Reinecke
2015-06-11 15:07 ` Ewan Milne
2015-06-12 6:27 ` Hannes Reinecke
2015-06-12 14:08 ` Ewan Milne
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).