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 <[email protected]>
>> Signed-off-by: Hannes Reinecke <[email protected]>
>> ---
>>  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
-- 
Dr. Hannes Reinecke                            zSeries & Storage
[email protected]                                   +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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to