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?
Reviewed-by: Ewan D. Milne <emilne@xxxxxxxxxx>
--
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
