sd: Check for unaligned partial completion

Commit Message

Damien Le Moal Feb. 14, 2017, 8:10 a.m. UTC

Commit "mpt3sas: Force request partial completion alignment" was not
considering the case of REQ_TYPE_FS commands not operating on sector
size units (e.g. REQ_OP_ZONE_REPORT and its 64B aligned partial
replies). This could result is incorrectly retrying (forever) those
commands.

Move the partial completion alignement check of mpt3sas to sd_done so
that the check comes after good_bytes & resid corrections done in that
function depending on the request command.

Signed-off-by: Damien Le Moal <[e-mail]>
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 15 ---------------
 drivers/scsi/sd.c                    | 20 ++++++++++++++++++++
 2 files changed, 20 insertions(+), 15 deletions(-)

Comments

Christoph Hellwig Feb. 14, 2017, 8:23 a.m. UTC | #1

> +	unsigned int sector_sz = SCpnt->device->sector_size;

Can you spell out size?

> +	/*
> +	 * In case of bogus fw or device, we could end up having
> +	 * unaligned partial completion. Check this here.
> +	 */
> +	resid = scsi_get_resid(SCpnt);
> +	if (SCpnt->request->cmd_type == REQ_TYPE_FS &&
> +	    resid & (sector_sz - 1)) {

->done is only called for fs requests, so you can remove this check.

Patch

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 0b5b423..1961535 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -4658,7 +4658,6 @@  _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 msix_index, u32 reply)
 	struct MPT3SAS_DEVICE *sas_device_priv_data;
 	u32 response_code = 0;
 	unsigned long flags;
-	unsigned int sector_sz;
 
 	mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
 	scmd = _scsih_scsi_lookup_get_clear(ioc, smid);
@@ -4717,20 +4716,6 @@  _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 msix_index, u32 reply)
 	}
 
 	xfer_cnt = le32_to_cpu(mpi_reply->TransferCount);
-
-	/* In case of bogus fw or device, we could end up having
-	 * unaligned partial completion. We can force alignment here,
-	 * then scsi-ml does not need to handle this misbehavior.
-	 */
-	sector_sz = scmd->device->sector_size;
-	if (unlikely(scmd->request->cmd_type == REQ_TYPE_FS && sector_sz &&
-		     xfer_cnt % sector_sz)) {
-		sdev_printk(KERN_INFO, scmd->device,
-		    "unaligned partial completion avoided (xfer_cnt=%u, sector_sz=%u)\n",
-			    xfer_cnt, sector_sz);
-		xfer_cnt = round_down(xfer_cnt, sector_sz);
-	}
-
 	scsi_set_resid(scmd, scsi_bufflen(scmd) - xfer_cnt);
 	if (ioc_status & MPI2_IOCSTATUS_FLAG_LOG_INFO_AVAILABLE)
 		log_info =  le32_to_cpu(mpi_reply->IOCLogInfo);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 1f5d92a..a3d8bc4 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1790,6 +1790,8 @@  static int sd_done(struct scsi_cmnd *SCpnt)
 {
 	int result = SCpnt->result;
 	unsigned int good_bytes = result ? 0 : scsi_bufflen(SCpnt);
+	unsigned int sector_sz = SCpnt->device->sector_size;
+	unsigned int resid;
 	struct scsi_sense_hdr sshdr;
 	struct scsi_disk *sdkp = scsi_disk(SCpnt->request->rq_disk);
 	struct request *req = SCpnt->request;
@@ -1829,6 +1831,24 @@  static int sd_done(struct scsi_cmnd *SCpnt)
 	}
 	sdkp->medium_access_timed_out = 0;
 
+	/*
+	 * In case of bogus fw or device, we could end up having
+	 * unaligned partial completion. Check this here.
+	 */
+	resid = scsi_get_resid(SCpnt);
+	if (SCpnt->request->cmd_type == REQ_TYPE_FS &&
+	    resid & (sector_sz - 1)) {
+		SCSI_LOG_HLCOMPLETE(1, scmd_printk(KERN_INFO, SCpnt,
+			"Unaligned partial completion (resid=%u, sector_sz=%u)\n",
+			resid, sector_sz));
+		resid = round_up(resid, sector_sz);
+		if (resid < good_bytes)
+			good_bytes -= resid;
+		else
+			good_bytes = 0;
+		scsi_set_resid(SCpnt, resid);
+	}
+
 	if (driver_byte(result) != DRIVER_SENSE &&
 	    (!sense_valid || sense_deferred))
 		goto out;