Re: [PATCH 03/20] scsi_dh_alua: Make stpg synchronous

2015-12-30 Thread Christoph Hellwig
On Tue, Dec 08, 2015 at 08:37:23AM +0100, Hannes Reinecke wrote:
> The 'activate_complete' function needs to be executed after
> stpg has finished, so we can as well execute stpg synchronously
> and call the function directly.

We could also execute that from the end_io handler.  I think that
was Bart's main issue.  I have to admit that I'd prefer synchronous
execution if possible, and with a work item only per port group
and not every SCSI device I think we are fine.

Otherwise looks fine to me:

Reviewed-by: Christoph Hellwig 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 03/20] scsi_dh_alua: Make stpg synchronous

2015-12-07 Thread Hannes Reinecke
The 'activate_complete' function needs to be executed after
stpg has finished, so we can as well execute stpg synchronously
and call the function directly.

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 156 ++---
 1 file changed, 55 insertions(+), 101 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c 
b/drivers/scsi/device_handler/scsi_dh_alua.c
index 7c66e4a..609691f 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -169,81 +169,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)) {
-   if (sense_hdr.sense_key == NOT_READY &&
-   sense_hdr.asc == 0x04 && sense_hdr.ascq == 0x0a) {
-   /* ALUA state transition already in progress */
-   err = SCSI_DH_OK;
-   goto done;
-   }
-   if (sense_hdr.sense_key == UNIT_ATTENTION) {
-   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;
@@ -251,13 +198,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 = 0;
-   rq->end_io_data = h;
 
-   blk_execute_rq_nowait(rq->q, NULL, rq, 1, stpg_endio);
-   return SCSI_DH_OK;
+   blk_execute_rq(rq->q, NULL, rq, 1);
+   if (rq->errors)
+   err = rq->errors;
+
+   blk_put_request(rq);
+
+   return err;
 }
 
 /*
@@ -582,59 +533,63 @@ 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 if the target port group
- * state is found to be in 'transitioning'.
- * If SCSI_DH_OK is returned the passed-in 'fn' function
- * this function will take care of executing 'fn'.
- * Otherwise 'fn' should be executed by the caller with the
- * returned error code.
+ * response. Returns SCSI_DH_RETRY per default to trigger
+ * a re-evaluation of th