| Message ID | 1434020506-41897-1-git-send-email-hare@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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> -- 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
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
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 -- 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
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;
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(-)