diff mbox

[09/17] scsi_dh_alua: switch to scsi_execute()

Message ID 1430743343-47174-10-git-send-email-hare@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

Hannes Reinecke May 4, 2015, 12:42 p.m. UTC
All commands are issued synchronously, so no need to open-code
scsi_execute anymore.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 119 +++++++----------------------
 1 file changed, 26 insertions(+), 93 deletions(-)

Comments

Christoph Hellwig May 6, 2015, 9:26 a.m. UTC | #1
On Mon, May 04, 2015 at 02:42:15PM +0200, Hannes Reinecke wrote:
> All commands are issued synchronously, so no need to open-code
> scsi_execute anymore.

Currently the alua codes doesn't set REQ_PREEMPT, and uses a GFP_NOIO
allocation.  Please document why changing these is safe/and or desirable.

Also it would be good to a change like this to the other device handlers,
as the code is copy and pasted everywhere, and only hp_sw has an asynchronous
case that can be handled separately.
--
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
Hannes Reinecke May 6, 2015, 9:58 a.m. UTC | #2
On 05/06/2015 11:26 AM, Christoph Hellwig wrote:
> On Mon, May 04, 2015 at 02:42:15PM +0200, Hannes Reinecke wrote:
>> All commands are issued synchronously, so no need to open-code
>> scsi_execute anymore.
> 
> Currently the alua codes doesn't set REQ_PREEMPT, and uses a GFP_NOIO
> allocation.  Please document why changing these is safe/and or desirable.
> 
> Also it would be good to a change like this to the other device handlers,
> as the code is copy and pasted everywhere, and only hp_sw has an asynchronous
> case that can be handled separately.
> 
Well, yes, of course.
However, for ALUA this will only work once the previous alua patches
are in. So I'd prefer to have another patchset on top of this,
converting the other device handlers.

As for the REQ_PREEMPT / GFP_NOIO I'll have a look and document
or fix is as appropriate.

Cheers,

Hannes
diff mbox

Patch

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 0b8e111..0cc18e3 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -103,80 +103,29 @@  static int realloc_buffer(struct alua_dh_data *h, unsigned len)
 	return 0;
 }
 
-static struct request *get_alua_req(struct scsi_device *sdev,
-				    void *buffer, unsigned buflen, int rw)
-{
-	struct request *rq;
-	struct request_queue *q = sdev->request_queue;
-
-	rq = blk_get_request(q, rw, GFP_NOIO);
-
-	if (IS_ERR(rq)) {
-		sdev_printk(KERN_INFO, sdev,
-			    "%s: blk_get_request failed\n", __func__);
-		return NULL;
-	}
-	blk_rq_set_block_pc(rq);
-
-	if (buflen && blk_rq_map_kern(q, rq, buffer, buflen, GFP_NOIO)) {
-		blk_put_request(rq);
-		sdev_printk(KERN_INFO, sdev,
-			    "%s: blk_rq_map_kern failed\n", __func__);
-		return NULL;
-	}
-
-	rq->cmd_flags |= REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT |
-			 REQ_FAILFAST_DRIVER;
-	rq->retries = ALUA_FAILOVER_RETRIES;
-	rq->timeout = ALUA_FAILOVER_TIMEOUT * HZ;
-
-	return rq;
-}
-
 /*
  * submit_rtpg - Issue a REPORT TARGET GROUP STATES command
  * @sdev: sdev the command should be sent to
  */
-static unsigned submit_rtpg(struct scsi_device *sdev, unsigned char *buff,
+static int submit_rtpg(struct scsi_device *sdev, unsigned char *buff,
 			    int bufflen, unsigned char *sense, int flags)
 {
-	struct request *rq;
-	int err;
-
-	rq = get_alua_req(sdev, buff, bufflen, READ);
-	if (!rq) {
-		err = DRIVER_BUSY << 24;
-		goto done;
-	}
+	u8 cdb[16];
+	int req_flags = REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT |
+		REQ_FAILFAST_DRIVER;
 
 	/* Prepare the command. */
-	rq->cmd[0] = MAINTENANCE_IN;
+	memset(cdb, 0x0, 16);
+	cdb[0] = MAINTENANCE_IN;
 	if (!(flags & ALUA_RTPG_EXT_HDR_UNSUPP))
-		rq->cmd[1] = MI_REPORT_TARGET_PGS | MI_EXT_HDR_PARAM_FMT;
+		cdb[1] = MI_REPORT_TARGET_PGS | MI_EXT_HDR_PARAM_FMT;
 	else
-		rq->cmd[1] = MI_REPORT_TARGET_PGS;
-	rq->cmd[6] = (bufflen >> 24) & 0xff;
-	rq->cmd[7] = (bufflen >> 16) & 0xff;
-	rq->cmd[8] = (bufflen >>  8) & 0xff;
-	rq->cmd[9] = bufflen & 0xff;
-	rq->cmd_len = COMMAND_SIZE(MAINTENANCE_IN);
-
-	rq->sense = sense;
-	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
-	rq->sense_len = 0;
-
-	err = blk_execute_rq(rq->q, NULL, rq, 1);
-	if (err < 0) {
-		if (!rq->errors)
-			err = DID_ERROR << 16;
-		else
-			err = rq->errors;
-		if (rq->sense_len)
-			err |= (DRIVER_SENSE << 24);
-	}
-	blk_put_request(rq);
-done:
-	return err;
+		cdb[1] = MI_REPORT_TARGET_PGS;
+	put_unaligned_be32(bufflen, &cdb[6]);
+
+	return scsi_execute(sdev, cdb, DMA_FROM_DEVICE, buff, bufflen,
+			    sense, ALUA_FAILOVER_TIMEOUT * HZ,
+			    ALUA_FAILOVER_RETRIES, req_flags, NULL);
 }
 
 /*
@@ -186,45 +135,29 @@  done:
  * to 'active/optimized' and let the array firmware figure out
  * the states of the remaining groups.
  */
-static unsigned submit_stpg(struct scsi_device *sdev, int group_id,
-			    unsigned char *sense)
+static int submit_stpg(struct scsi_device *sdev, int group_id,
+		       unsigned char *sense)
 {
-	struct request *rq;
+	u8 cdb[COMMAND_SIZE(MAINTENANCE_OUT)];
 	unsigned char stpg_data[8];
 	int stpg_len = 8;
-	int err;
+	int req_flags = REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT |
+		REQ_FAILFAST_DRIVER;
 
 	/* Prepare the data buffer */
 	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, stpg_data, stpg_len, WRITE);
-	if (!rq)
-		return SCSI_DH_RES_TEMP_UNAVAIL;
-
 	/* Prepare the command. */
-	rq->cmd[0] = MAINTENANCE_OUT;
-	rq->cmd[1] = MO_SET_TARGET_PGS;
-	put_unaligned_be32(stpg_len, &rq->cmd[6]);
-	rq->cmd_len = COMMAND_SIZE(MAINTENANCE_OUT);
-
-	rq->sense = sense;
-	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
-	rq->sense_len = 0;
-
-	err = blk_execute_rq(rq->q, NULL, rq, 1);
-	if (err < 0) {
-		if (!rq->errors)
-			err = DID_ERROR << 16;
-		else
-			err = rq->errors;
-		if (rq->sense_len)
-			err |= (DRIVER_SENSE << 24);
-	}
-	blk_put_request(rq);
-
-	return err;
+	memset(cdb, 0x0, COMMAND_SIZE(MAINTENANCE_OUT));
+	cdb[0] = MAINTENANCE_OUT;
+	cdb[1] = MO_SET_TARGET_PGS;
+	put_unaligned_be32(stpg_len, &cdb[6]);
+
+	return scsi_execute(sdev, cdb, DMA_TO_DEVICE, stpg_data, stpg_len,
+			    sense, ALUA_FAILOVER_TIMEOUT * HZ,
+			    ALUA_FAILOVER_RETRIES, req_flags, NULL);
 }
 
 /*