diff mbox series

[7/8] sd: Clean up sd_setup_read_write_cmnd()

Message ID 20190111000858.76261-8-bvanassche@acm.org (mailing list archive)
State Superseded
Headers show
Series sd patches for kernel v5.1 | expand

Commit Message

Bart Van Assche Jan. 11, 2019, 12:08 a.m. UTC
From: "Martin K. Petersen" <martin.petersen@oracle.com>

Rework sd_setup_read_write_cmnd() so it becomes more readable. Put all
the sanity checking at the head of the function and sanitize the logged
error messages. Move the legacy SCSI logging calls to the end of the
functions and reduce conditional nesting.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
[ bvanassche: ported this patch from kernel v4.11 to kernel v5.0 ]
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/sd.c | 80 +++++++++++++++++------------------------------
 1 file changed, 28 insertions(+), 52 deletions(-)

Comments

Hannes Reinecke Jan. 11, 2019, 7:38 a.m. UTC | #1
On 1/11/19 1:08 AM, Bart Van Assche wrote:
> From: "Martin K. Petersen" <martin.petersen@oracle.com>
> 
> Rework sd_setup_read_write_cmnd() so it becomes more readable. Put all
> the sanity checking at the head of the function and sanitize the logged
> error messages. Move the legacy SCSI logging calls to the end of the
> functions and reduce conditional nesting.
> 
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> [ bvanassche: ported this patch from kernel v4.11 to kernel v5.0 ]
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   drivers/scsi/sd.c | 80 +++++++++++++++++------------------------------
>   1 file changed, 28 insertions(+), 52 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
diff mbox series

Patch

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 039b0c7f178e..4e293e4c6823 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1154,12 +1154,11 @@  static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
 {
 	struct request *rq = SCpnt->request;
 	struct scsi_device *sdp = SCpnt->device;
-	struct gendisk *disk = rq->rq_disk;
-	struct scsi_disk *sdkp = scsi_disk(disk);
+	struct scsi_disk *sdkp = scsi_disk(rq->rq_disk);
 	sector_t lba = sectors_to_logical(sdp, blk_rq_pos(rq));
 	sector_t threshold;
 	unsigned int nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
-	unsigned int dif, dix;
+	bool dif, dix;
 	unsigned int mask = logical_to_sectors(sdp, 1) - 1;
 	bool write = rq_data_dir(rq) == WRITE;
 	unsigned char protect, fua;
@@ -1168,29 +1167,21 @@  static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
 	ret = scsi_init_io(SCpnt);
 	if (ret != BLK_STS_OK)
 		return ret;
+
 	WARN_ON_ONCE(SCpnt != rq->special);
 
-	SCSI_LOG_HLQUEUE(1,
-		scmd_printk(KERN_INFO, SCpnt,
-			"%s: block=%llu, count=%d\n",
-			__func__, (unsigned long long)lba, nr_blocks));
-
-	if (!sdp || !scsi_device_online(sdp) ||
-	    lba + blk_rq_sectors(rq) > get_capacity(disk)) {
-		SCSI_LOG_HLQUEUE(2, scmd_printk(KERN_INFO, SCpnt,
-						"Finishing %u sectors\n",
-						blk_rq_sectors(rq)));
-		SCSI_LOG_HLQUEUE(2, scmd_printk(KERN_INFO, SCpnt,
-						"Retry with 0x%p\n", SCpnt));
+	if (!scsi_device_online(sdp) || sdp->changed) {
+		scmd_printk(KERN_ERR, SCpnt, "device offline or changed\n");
 		return BLK_STS_IOERR;
 	}
 
-	if (sdp->changed) {
-		/*
-		 * quietly refuse to do anything to a changed disc until 
-		 * the changed bit has been reset
-		 */
-		/* printk("SCSI disk has been changed or is not present. Prohibiting further I/O.\n"); */
+	if (blk_rq_pos(rq) + blk_rq_sectors(rq) > get_capacity(rq->rq_disk)) {
+		scmd_printk(KERN_ERR, SCpnt, "access beyond end of device\n");
+		return BLK_STS_IOERR;
+	}
+
+	if ((blk_rq_pos(rq) & mask) || (blk_rq_sectors(rq) & mask)) {
+		scmd_printk(KERN_ERR, SCpnt, "request not aligned to the logical block size\n");
 		return BLK_STS_IOERR;
 	}
 
@@ -1210,37 +1201,13 @@  static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
 		}
 	}
 
-	SCSI_LOG_HLQUEUE(2, scmd_printk(KERN_INFO, SCpnt, "block=%llu\n",
-					(unsigned long long)lba));
-
-	if ((blk_rq_pos(rq) & mask) || (blk_rq_sectors(rq) & mask)) {
-		scmd_printk(KERN_ERR, SCpnt, "request not aligned to the logical block size\n");
-		return BLK_STS_IOERR;
-	}
-
-	if (rq_data_dir(rq) == WRITE) {
-		SCpnt->cmnd[0] = WRITE_6;
-
-		if (blk_integrity_rq(rq))
-			t10_pi_prepare(SCpnt->request, sdkp->protection_type);
-
-	} else if (rq_data_dir(rq) == READ) {
-		SCpnt->cmnd[0] = READ_6;
-	} else {
-		scmd_printk(KERN_ERR, SCpnt, "Unknown command %d\n", req_op(rq));
-		return BLK_STS_IOERR;
-	}
-
-	SCSI_LOG_HLQUEUE(2, scmd_printk(KERN_INFO, SCpnt,
-					"%s %d/%u 512 byte blocks.\n",
-					(rq_data_dir(rq) == WRITE) ?
-					"writing" : "reading", nr_blocks,
-					blk_rq_sectors(rq)));
-
 	fua = rq->cmd_flags & REQ_FUA ? 0x8 : 0;
 	dix = scsi_prot_sg_count(SCpnt);
 	dif = scsi_host_dif_capable(SCpnt->device->host, sdkp->protection_type);
 
+	if (write && dix)
+		t10_pi_prepare(SCpnt->request, sdkp->protection_type);
+
 	if (dif || dix)
 		protect = sd_setup_protect_cmnd(SCpnt, dix, dif);
 	else
@@ -1253,8 +1220,7 @@  static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
 		ret = sd_setup_rw16_cmnd(SCpnt, write, lba, nr_blocks,
 					 protect | fua);
 	} else if ((nr_blocks > 0xff) || (lba > 0x1fffff) ||
-		   scsi_device_protection(SCpnt->device) ||
-		   SCpnt->device->use_10_for_rw) {
+		   sdp->use_10_for_rw || protect) {
 		ret = sd_setup_rw10_cmnd(SCpnt, write, lba, nr_blocks,
 					 protect | fua);
 	} else {
@@ -1265,8 +1231,6 @@  static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
 	if (unlikely(ret != BLK_STS_OK))
 		return ret;
 
-	SCpnt->sdb.length = nr_blocks * sdp->sector_size;
-
 	/*
 	 * We shouldn't disconnect in the middle of a sector, so with a dumb
 	 * host adapter, it's safe to assume that we can at least transfer
@@ -1275,6 +1239,18 @@  static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
 	SCpnt->transfersize = sdp->sector_size;
 	SCpnt->underflow = nr_blocks << 9;
 	SCpnt->allowed = SD_MAX_RETRIES;
+	SCpnt->sdb.length = nr_blocks * sdp->sector_size;
+
+	SCSI_LOG_HLQUEUE(1,
+			 scmd_printk(KERN_INFO, SCpnt,
+				     "%s: block=%llu, count=%d\n", __func__,
+				     (unsigned long long)blk_rq_pos(rq),
+				     blk_rq_sectors(rq)));
+	SCSI_LOG_HLQUEUE(2,
+			 scmd_printk(KERN_INFO, SCpnt,
+				     "%s %d/%u 512 byte blocks.\n",
+				     write ? "writing" : "reading", nr_blocks,
+				     blk_rq_sectors(rq)));
 
 	/*
 	 * This indicates that the command is ready from our end to be