On Mon, 2015-07-06 at 08:13 -0700, Bart Van Assche wrote:
> On 07/06/2015 04:12 AM, Hannes Reinecke wrote:
> > During ALUA state transitions the device might return
> > a sense code 02/04/0a (Logical unit not accessible, asymmetric
> > access state transition). As this is a transient error
> > we should just retry the READ CAPACITY call after 1 second
> > until the state transition finishes and the correct
> > capacity can be returned.
> > At the same time we should break out of the loop after
> > 2 minutes to avoid unbounded retries.
> >
> > Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
> > ---
> > drivers/scsi/sd.c | 21 +++++++++++++++++++++
> > 1 file changed, 21 insertions(+)
> >
> > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> > index 3b2fcb4..f45b8fe 100644
> > --- a/drivers/scsi/sd.c
> > +++ b/drivers/scsi/sd.c
> > @@ -1934,6 +1934,7 @@ static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device *sdp,
> > #endif
> >
> > #define READ_CAPACITY_RETRIES_ON_RESET 10
> > +#define READ_CAPACITY_RETRIES_ON_TRANSITION 120
> >
> > static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
> > unsigned char *buffer)
> > @@ -1943,6 +1944,7 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
> > int sense_valid = 0;
> > int the_result;
> > int retries = 3, reset_retries = READ_CAPACITY_RETRIES_ON_RESET;
> > + int transition_retries = READ_CAPACITY_RETRIES_ON_TRANSITION;
> > unsigned int alignment;
> > unsigned long long lba;
> > unsigned sector_size;
> > @@ -1981,6 +1983,15 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
> > * give it one more chance */
> > if (--reset_retries > 0)
> > continue;
> > + if (sense_valid &&
> > + sshdr.sense_key == NOT_READY &&
> > + sshdr.asc == 0x04 && sshdr.ascq == 0x0A) {
> > + /* ALUA state transition; retry after delay */
> > + if (--transition_retries > 0) {
> > + msleep(1000);
> > + continue;
> > + }
> > + }
> > }
> > retries--;
> >
> > @@ -2039,6 +2050,7 @@ static int read_capacity_10(struct scsi_disk *sdkp, struct scsi_device *sdp,
> > int sense_valid = 0;
> > int the_result;
> > int retries = 3, reset_retries = READ_CAPACITY_RETRIES_ON_RESET;
> > + int transition_retries = READ_CAPACITY_RETRIES_ON_TRANSITION;
> > sector_t lba;
> > unsigned sector_size;
> >
> > @@ -2063,6 +2075,15 @@ static int read_capacity_10(struct scsi_disk *sdkp, struct scsi_device *sdp,
> > * give it one more chance */
> > if (--reset_retries > 0)
> > continue;
> > + if (sense_valid &&
> > + sshdr.sense_key == NOT_READY &&
> > + sshdr.asc == 0x04 && sshdr.ascq == 0x0A) {
> > + /* ALUA state transition; retry after delay */
> > + if (--transition_retries > 0) {
> > + msleep(1000);
> > + continue;
> > + }
> > + }
> > }
> > retries--;
>
> Hello Hannes,
>
> Although I agree that multipathd should handle arrays correctly that
> fail READ CAPACITY commands while transitioning, seeing that a new
> hard-coded timeout is added in the SCSI initiator code does not make me
> enthusiast. The READ_CAPACITY_RETRIES_ON_TRANSITION timeout added
> through this patch is independent of the IMPLICIT TRANSITION TIME in the
> REPORT TARGET PORT GROUPS response. Has the following already been
> considered ?
> - If the capacity is not known, let the scsi_dh_alua handler submit a
> READ CAPACITY command asynchronously after certain target port group
> state transitions. Also let the scsi_dh_alua handler submit a
> notification to user space after the capacity changes from "unknown" to
> "known".
> - Let multipathd ignore paths for which READ CAPACITY failed until the
> capacity becomes known.
Yes, I'm with something like this. The patch you propose will be a busy
wait for userspace which they may not want.
James
--
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
