On 09/22/2015 08:50 PM, Ewan Milne wrote:
> On Thu, 2015-08-27 at 14:41 +0200, Hannes Reinecke wrote:
>> We should be issuing STPG synchronously as we need to
>> evaluate the return code on failure.
>>
>> Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
>> ---
>> drivers/scsi/device_handler/scsi_dh_alua.c | 179 +++++++++++++----------------
>> 1 file changed, 83 insertions(+), 96 deletions(-)
>>
>> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
>> index 9e2b3af..fd0385e 100644
>> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
>> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
>> @@ -172,76 +172,28 @@ done:
>> }
>>
>> /*
>> - * stpg_endio - Evaluate SET TARGET GROUP STATES
>> - * @sdev: the device to be evaluated
>> - * @state: the new target group state
>> - *
>> - * Evaluate a SET TARGET GROUP STATES command response.
>> - */
>> -static void stpg_endio(struct request *req, int error)
>> -{
>> - struct alua_dh_data *h = req->end_io_data;
>> - struct scsi_sense_hdr sense_hdr;
>> - unsigned err = SCSI_DH_OK;
>> -
>> - if (host_byte(req->errors) != DID_OK ||
>> - msg_byte(req->errors) != COMMAND_COMPLETE) {
>> - err = SCSI_DH_IO;
>> - goto done;
>> - }
>> -
>> - if (scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE,
>> - &sense_hdr)) {
>> - err = alua_check_sense(h->sdev, &sense_hdr);
>> - if (err == ADD_TO_MLQUEUE) {
>> - err = SCSI_DH_RETRY;
>> - goto done;
>> - }
>> - sdev_printk(KERN_INFO, h->sdev, "%s: stpg failed\n",
>> - ALUA_DH_NAME);
>> - scsi_print_sense_hdr(h->sdev, ALUA_DH_NAME, &sense_hdr);
>> - err = SCSI_DH_IO;
>> - } else if (error)
>> - err = SCSI_DH_IO;
>> -
>> - if (err == SCSI_DH_OK) {
>> - h->state = TPGS_STATE_OPTIMIZED;
>> - sdev_printk(KERN_INFO, h->sdev,
>> - "%s: port group %02x switched to state %c\n",
>> - ALUA_DH_NAME, h->group_id,
>> - print_alua_state(h->state));
>> - }
>> -done:
>> - req->end_io_data = NULL;
>> - __blk_put_request(req->q, req);
>> - if (h->callback_fn) {
>> - h->callback_fn(h->callback_data, err);
>> - h->callback_fn = h->callback_data = NULL;
>> - }
>> - return;
>> -}
>> -
>> -/*
>> * submit_stpg - Issue a SET TARGET GROUP STATES command
>> *
>> * Currently we're only setting the current target port group state
>> * to 'active/optimized' and let the array firmware figure out
>> * the states of the remaining groups.
>> */
>> -static unsigned submit_stpg(struct alua_dh_data *h)
>> +static unsigned submit_stpg(struct scsi_device *sdev, int group_id,
>> + unsigned char *sense)
>> {
>> struct request *rq;
>> + unsigned char stpg_data[8];
>> int stpg_len = 8;
>> - struct scsi_device *sdev = h->sdev;
>> + int err = 0;
>>
>> /* Prepare the data buffer */
>> - memset(h->buff, 0, stpg_len);
>> - h->buff[4] = TPGS_STATE_OPTIMIZED & 0x0f;
>> - put_unaligned_be16(h->group_id, &h->buff[6]);
>> + memset(stpg_data, 0, stpg_len);
>> + stpg_data[4] = TPGS_STATE_OPTIMIZED & 0x0f;
>> + put_unaligned_be16(group_id, &stpg_data[6]);
>>
>> - rq = get_alua_req(sdev, h->buff, stpg_len, WRITE);
>> + rq = get_alua_req(sdev, stpg_data, stpg_len, WRITE);
>> if (!rq)
>> - return SCSI_DH_RES_TEMP_UNAVAIL;
>> + return DRIVER_BUSY << 24;
>>
>> /* Prepare the command. */
>> rq->cmd[0] = MAINTENANCE_OUT;
>> @@ -249,13 +201,17 @@ static unsigned submit_stpg(struct alua_dh_data *h)
>> put_unaligned_be32(stpg_len, &rq->cmd[6]);
>> rq->cmd_len = COMMAND_SIZE(MAINTENANCE_OUT);
>>
>> - rq->sense = h->sense;
>> + rq->sense = sense;
>> memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
>> - rq->sense_len = h->senselen = 0;
>> - rq->end_io_data = h;
>> + rq->sense_len = 0;
>> +
>> + blk_execute_rq(rq->q, NULL, rq, 1);
>> + if (rq->errors)
>> + err = rq->errors;
>> +
>> + blk_put_request(rq);
>>
>> - blk_execute_rq_nowait(rq->q, NULL, rq, 1, stpg_endio);
>> - return SCSI_DH_OK;
>> + return err;
>> }
>>
>> /*
>> @@ -619,6 +575,68 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_dh_data *h, int wait_
>> }
>>
>> /*
>> + * alua_stpg - Issue a SET TARGET GROUP STATES command
>> + *
>> + * Issue a SET TARGET GROUP STATES command and evaluate the
>> + * response. Returns SCSI_DH_RETRY per default to trigger
>> + * a re-evaluation of the target group state.
>> + */
>> +static unsigned alua_stpg(struct scsi_device *sdev, struct alua_dh_data *h)
>> +{
>> + int retval;
>> + struct scsi_sense_hdr sense_hdr;
>> +
>> + if (!(h->tpgs & TPGS_MODE_EXPLICIT)) {
>> + /* Only implicit ALUA supported, retry */
>> + return SCSI_DH_RETRY;
>> + }
>> + switch (h->state) {
>> + case TPGS_STATE_OPTIMIZED:
>> + return SCSI_DH_OK;
>> + case TPGS_STATE_NONOPTIMIZED:
>> + if ((h->flags & ALUA_OPTIMIZE_STPG) &&
>> + !h->pref &&
>> + (h->tpgs & TPGS_MODE_IMPLICIT))
>> + return SCSI_DH_OK;
>> + break;
>> + case TPGS_STATE_STANDBY:
>> + case TPGS_STATE_UNAVAILABLE:
>> + break;
>> + case TPGS_STATE_OFFLINE:
>> + return SCSI_DH_IO;
>> + break;
>> + case TPGS_STATE_TRANSITIONING:
>> + break;
>> + default:
>> + sdev_printk(KERN_INFO, sdev,
>> + "%s: stpg failed, unhandled TPGS state %d",
>> + ALUA_DH_NAME, h->state);
>> + return SCSI_DH_NOSYS;
>> + break;
>> + }
>> + /* Set state to transitioning */
>> + h->state = TPGS_STATE_TRANSITIONING;
>> + retval = submit_stpg(sdev, h->group_id, h->sense);
>> +
>> + if (retval) {
>> + if (!scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE,
>> + &sense_hdr)) {
>> + sdev_printk(KERN_INFO, sdev,
>> + "%s: stpg failed, result %d",
>> + ALUA_DH_NAME, retval);
>> + if (driver_byte(retval) == DRIVER_BUSY)
>> + return SCSI_DH_DEV_TEMP_BUSY;
>> + } else {
>> + sdev_printk(KERN_INFO, h->sdev, "%s: stpg failed\n",
>> + ALUA_DH_NAME);
>> + scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr);
>> + }
>> + }
>> + /* Retry RTPG */
>> + return SCSI_DH_RETRY;
>> +}
>> +
>> +/*
>> * alua_initialize - Initialize ALUA state
>> * @sdev: the device to be initialized
>> *
>> @@ -695,7 +713,6 @@ static int alua_activate(struct scsi_device *sdev,
>> {
>> struct alua_dh_data *h = sdev->handler_data;
>> int err = SCSI_DH_OK;
>> - int stpg = 0;
>>
>> err = alua_rtpg(sdev, h, 1);
>> if (err != SCSI_DH_OK)
>> @@ -704,39 +721,9 @@ static int alua_activate(struct scsi_device *sdev,
>> if (optimize_stpg)
>> h->flags |= ALUA_OPTIMIZE_STPG;
>>
>> - if (h->tpgs & TPGS_MODE_EXPLICIT) {
>> - switch (h->state) {
>> - case TPGS_STATE_NONOPTIMIZED:
>> - stpg = 1;
>> - if ((h->flags & ALUA_OPTIMIZE_STPG) &&
>> - (!h->pref) &&
>> - (h->tpgs & TPGS_MODE_IMPLICIT))
>> - stpg = 0;
>> - break;
>> - case TPGS_STATE_STANDBY:
>> - case TPGS_STATE_UNAVAILABLE:
>> - stpg = 1;
>> - break;
>> - case TPGS_STATE_OFFLINE:
>> - err = SCSI_DH_IO;
>> - break;
>> - case TPGS_STATE_TRANSITIONING:
>> - err = SCSI_DH_RETRY;
>> - break;
>> - default:
>> - break;
>> - }
>> - }
>> -
>> - if (stpg) {
>> - h->callback_fn = fn;
>> - h->callback_data = data;
>> - err = submit_stpg(h);
>> - if (err == SCSI_DH_OK)
>> - return 0;
>> - h->callback_fn = h->callback_data = NULL;
>> - }
>> -
>> + err = alua_stpg(sdev, h);
>> + if (err == SCSI_DH_RETRY)
>> + err = alua_rtpg(sdev, h, 1);
>> out:
>> if (fn)
>> fn(data, err);
>
> submit_stpg() now returns DRIVER_BUSY << 24 instead of SCSI_DH_RES_TEMP_UNAVAIL,
> so you are changing the return code semantics here as in patch 5/23. That's
> fine, but probably should be mentioned in the patch description.
>
> The removed stpg_endio() code *did* evaluate the SCSI error code, and the code
> that has replaced it doesn't exactly do the same thing, so what difference in
> behavior are we going to get here?
>
Correct. Is fixed up with the next version of the patchset.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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
