| Message ID | 1440679281-13234-16-git-send-email-hare@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2015-08-27 at 14:41 +0200, Hannes Reinecke wrote: > Most sense code is already handled in the generic > code, so we shouldn't be adding special cases here. > However, when doing so we need to check for > unit attention whenever we're sending an internal > command. > > Reviewed-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Hannes Reinecke <hare@suse.de> > --- > drivers/scsi/device_handler/scsi_dh_alua.c | 50 +++++++----------------------- > 1 file changed, 11 insertions(+), 39 deletions(-) > > diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c > index 4157fe2..dbe9ff2 100644 > --- a/drivers/scsi/device_handler/scsi_dh_alua.c > +++ b/drivers/scsi/device_handler/scsi_dh_alua.c > @@ -91,7 +91,6 @@ struct alua_dh_data { > #define ALUA_POLICY_SWITCH_ALL 1 > > static char print_alua_state(int); > -static int alua_check_sense(struct scsi_device *, struct scsi_sense_hdr *); > > static void release_port_group(struct kref *kref) > { > @@ -323,28 +322,6 @@ static int alua_check_sense(struct scsi_device *sdev, > * LUN Not Accessible - ALUA state transition > */ > return ADD_TO_MLQUEUE; > - if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0b) > - /* > - * LUN Not Accessible -- Target port in standby state > - */ > - return SUCCESS; > - if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0c) > - /* > - * LUN Not Accessible -- Target port in unavailable state > - */ > - return SUCCESS; > - if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x12) > - /* > - * LUN Not Ready -- Offline > - */ > - return SUCCESS; > - if (sdev->allow_restart && > - sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x02) > - /* > - * if the device is not started, we need to wake > - * the error handler to start the motor > - */ > - return FAILED; > break; > case UNIT_ATTENTION: > if (sense_hdr->asc == 0x29 && sense_hdr->ascq == 0x00) > @@ -359,7 +336,7 @@ static int alua_check_sense(struct scsi_device *sdev, > return ADD_TO_MLQUEUE; > if (sense_hdr->asc == 0x2a && sense_hdr->ascq == 0x01) > /* > - * Mode Parameters Changed > + * Mode Parameter Changed See below. > */ > return ADD_TO_MLQUEUE; > if (sense_hdr->asc == 0x2a && sense_hdr->ascq == 0x06) > @@ -372,18 +349,6 @@ static int alua_check_sense(struct scsi_device *sdev, > * Implicit ALUA state transition failed > */ > return ADD_TO_MLQUEUE; > - if (sense_hdr->asc == 0x3f && sense_hdr->ascq == 0x03) > - /* > - * Inquiry data has changed > - */ > - return ADD_TO_MLQUEUE; ??? See below. > - if (sense_hdr->asc == 0x3f && sense_hdr->ascq == 0x0e) > - /* > - * REPORTED_LUNS_DATA_HAS_CHANGED is reported > - * when switching controllers on targets like > - * Intel Multi-Flex. We can just retry. > - */ > - return ADD_TO_MLQUEUE; ??? See below. > break; > } > > @@ -448,9 +413,16 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg, int w > pg->flags |= ALUA_RTPG_EXT_HDR_UNSUPP; > goto retry; > } > - > - err = alua_check_sense(sdev, &sense_hdr); > - if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) { > + /* > + * Retry on ALUA state transition or if any > + * UNIT ATTENTION occurred. > + */ > + if (sense_hdr.sense_key == NOT_READY && > + sense_hdr.asc == 0x04 && sense_hdr.ascq == 0x0a) > + err = SCSI_DH_RETRY; > + else if (sense_hdr.sense_key == UNIT_ATTENTION) > + err = SCSI_DH_RETRY; > + if (err == SCSI_DH_RETRY && time_before(jiffies, expiry)) { > sdev_printk(KERN_ERR, sdev, "%s: rtpg retry\n", > ALUA_DH_NAME); > scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr); "Mode Parameters Changed" comment should not have been changed to "Mode Parameter Changed". The actual T10 text is "Mode Parameters Changed", so leave it the way it is. The sense code handling in the UA case for ASC/ASCQ 3F 03 is changed by this patch to return SUCCESS from scsi_check_sense() instead of ADD_TO_MLQUEUE from alua_check_sense(), and the sense code handling in the UA case for ASC/ASCQ 3F 0E is changed by this patch to return NEEDS_RETRY from scsi_check_sense() instead of ADD_TO_MLQUEUE from alua_check_sense(). So we will not reissue the command on INQUIRY DATA HAS CHANGED and will have different flow of control on REPORTED LUNS DATA HAS CHANGED. What is the reason for this? 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 09/22/2015 09:10 PM, Ewan Milne wrote: > On Thu, 2015-08-27 at 14:41 +0200, Hannes Reinecke wrote: >> Most sense code is already handled in the generic >> code, so we shouldn't be adding special cases here. >> However, when doing so we need to check for >> unit attention whenever we're sending an internal >> command. >> >> Reviewed-by: Christoph Hellwig <hch@lst.de> >> Signed-off-by: Hannes Reinecke <hare@suse.de> >> --- >> drivers/scsi/device_handler/scsi_dh_alua.c | 50 +++++++----------------------- >> 1 file changed, 11 insertions(+), 39 deletions(-) >> >> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c >> index 4157fe2..dbe9ff2 100644 >> --- a/drivers/scsi/device_handler/scsi_dh_alua.c >> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c >> @@ -91,7 +91,6 @@ struct alua_dh_data { >> #define ALUA_POLICY_SWITCH_ALL 1 >> >> static char print_alua_state(int); >> -static int alua_check_sense(struct scsi_device *, struct scsi_sense_hdr *); >> >> static void release_port_group(struct kref *kref) >> { >> @@ -323,28 +322,6 @@ static int alua_check_sense(struct scsi_device *sdev, >> * LUN Not Accessible - ALUA state transition >> */ >> return ADD_TO_MLQUEUE; >> - if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0b) >> - /* >> - * LUN Not Accessible -- Target port in standby state >> - */ >> - return SUCCESS; >> - if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0c) >> - /* >> - * LUN Not Accessible -- Target port in unavailable state >> - */ >> - return SUCCESS; >> - if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x12) >> - /* >> - * LUN Not Ready -- Offline >> - */ >> - return SUCCESS; >> - if (sdev->allow_restart && >> - sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x02) >> - /* >> - * if the device is not started, we need to wake >> - * the error handler to start the motor >> - */ >> - return FAILED; >> break; >> case UNIT_ATTENTION: >> if (sense_hdr->asc == 0x29 && sense_hdr->ascq == 0x00) >> @@ -359,7 +336,7 @@ static int alua_check_sense(struct scsi_device *sdev, >> return ADD_TO_MLQUEUE; >> if (sense_hdr->asc == 0x2a && sense_hdr->ascq == 0x01) >> /* >> - * Mode Parameters Changed >> + * Mode Parameter Changed > > See below. > >> */ >> return ADD_TO_MLQUEUE; >> if (sense_hdr->asc == 0x2a && sense_hdr->ascq == 0x06) >> @@ -372,18 +349,6 @@ static int alua_check_sense(struct scsi_device *sdev, >> * Implicit ALUA state transition failed >> */ >> return ADD_TO_MLQUEUE; >> - if (sense_hdr->asc == 0x3f && sense_hdr->ascq == 0x03) >> - /* >> - * Inquiry data has changed >> - */ >> - return ADD_TO_MLQUEUE; > > ??? See below. > >> - if (sense_hdr->asc == 0x3f && sense_hdr->ascq == 0x0e) >> - /* >> - * REPORTED_LUNS_DATA_HAS_CHANGED is reported >> - * when switching controllers on targets like >> - * Intel Multi-Flex. We can just retry. >> - */ >> - return ADD_TO_MLQUEUE; > > ??? See below. > >> break; >> } >> >> @@ -448,9 +413,16 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg, int w >> pg->flags |= ALUA_RTPG_EXT_HDR_UNSUPP; >> goto retry; >> } >> - >> - err = alua_check_sense(sdev, &sense_hdr); >> - if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) { >> + /* >> + * Retry on ALUA state transition or if any >> + * UNIT ATTENTION occurred. >> + */ >> + if (sense_hdr.sense_key == NOT_READY && >> + sense_hdr.asc == 0x04 && sense_hdr.ascq == 0x0a) >> + err = SCSI_DH_RETRY; >> + else if (sense_hdr.sense_key == UNIT_ATTENTION) >> + err = SCSI_DH_RETRY; >> + if (err == SCSI_DH_RETRY && time_before(jiffies, expiry)) { >> sdev_printk(KERN_ERR, sdev, "%s: rtpg retry\n", >> ALUA_DH_NAME); >> scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr); > > "Mode Parameters Changed" comment should not have been changed to "Mode Parameter Changed". > The actual T10 text is "Mode Parameters Changed", so leave it the way it is. > Okay. > The sense code handling in the UA case for ASC/ASCQ 3F 03 is changed by this patch to > return SUCCESS from scsi_check_sense() instead of ADD_TO_MLQUEUE from alua_check_sense(), > and the sense code handling in the UA case for ASC/ASCQ 3F 0E is changed by this patch to > return NEEDS_RETRY from scsi_check_sense() instead of ADD_TO_MLQUEUE from alua_check_sense(). > So we will not reissue the command on INQUIRY DATA HAS CHANGED and will have different > flow of control on REPORTED LUNS DATA HAS CHANGED. What is the reason for this? > 'INQUIRY DATA HAS CHANGED' will be handled with a later patch, so I had it removed from here. But indeed, these hunks shouldn't be removed. I'll be reverting that. Cheers, Hannes
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c index 4157fe2..dbe9ff2 100644 --- a/drivers/scsi/device_handler/scsi_dh_alua.c +++ b/drivers/scsi/device_handler/scsi_dh_alua.c @@ -91,7 +91,6 @@ struct alua_dh_data { #define ALUA_POLICY_SWITCH_ALL 1 static char print_alua_state(int); -static int alua_check_sense(struct scsi_device *, struct scsi_sense_hdr *); static void release_port_group(struct kref *kref) { @@ -323,28 +322,6 @@ static int alua_check_sense(struct scsi_device *sdev, * LUN Not Accessible - ALUA state transition */ return ADD_TO_MLQUEUE; - if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0b) - /* - * LUN Not Accessible -- Target port in standby state - */ - return SUCCESS; - if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0c) - /* - * LUN Not Accessible -- Target port in unavailable state - */ - return SUCCESS; - if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x12) - /* - * LUN Not Ready -- Offline - */ - return SUCCESS; - if (sdev->allow_restart && - sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x02) - /* - * if the device is not started, we need to wake - * the error handler to start the motor - */ - return FAILED; break; case UNIT_ATTENTION: if (sense_hdr->asc == 0x29 && sense_hdr->ascq == 0x00) @@ -359,7 +336,7 @@ static int alua_check_sense(struct scsi_device *sdev, return ADD_TO_MLQUEUE; if (sense_hdr->asc == 0x2a && sense_hdr->ascq == 0x01) /* - * Mode Parameters Changed + * Mode Parameter Changed */ return ADD_TO_MLQUEUE; if (sense_hdr->asc == 0x2a && sense_hdr->ascq == 0x06) @@ -372,18 +349,6 @@ static int alua_check_sense(struct scsi_device *sdev, * Implicit ALUA state transition failed */ return ADD_TO_MLQUEUE; - if (sense_hdr->asc == 0x3f && sense_hdr->ascq == 0x03) - /* - * Inquiry data has changed - */ - return ADD_TO_MLQUEUE; - if (sense_hdr->asc == 0x3f && sense_hdr->ascq == 0x0e) - /* - * REPORTED_LUNS_DATA_HAS_CHANGED is reported - * when switching controllers on targets like - * Intel Multi-Flex. We can just retry. - */ - return ADD_TO_MLQUEUE; break; } @@ -448,9 +413,16 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg, int w pg->flags |= ALUA_RTPG_EXT_HDR_UNSUPP; goto retry; } - - err = alua_check_sense(sdev, &sense_hdr); - if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) { + /* + * Retry on ALUA state transition or if any + * UNIT ATTENTION occurred. + */ + if (sense_hdr.sense_key == NOT_READY && + sense_hdr.asc == 0x04 && sense_hdr.ascq == 0x0a) + err = SCSI_DH_RETRY; + else if (sense_hdr.sense_key == UNIT_ATTENTION) + err = SCSI_DH_RETRY; + if (err == SCSI_DH_RETRY && time_before(jiffies, expiry)) { sdev_printk(KERN_ERR, sdev, "%s: rtpg retry\n", ALUA_DH_NAME); scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr);