[PATCH] sd: Fix rw_max for devices that report an optimal xfer size

[PATCH] sd: Fix rw_max for devices that report an optimal xfer size

[Martin K. Petersen]

For historic reasons, io_opt is in bytes and max_sectors in block layer
sectors. This interface inconsistency is error prone and should be
fixed. But for 4.4--4.7 let's make the unit difference explicit via a
wrapper function.

Fixes: d0eb20a863ba ("sd: Optimal I/O size is in bytes, not sectors")
Cc: stable@vger.kernel.org # 4.4+
Reported-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/sd.c | 8 ++++----
 drivers/scsi/sd.h | 5 +++++
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index f459dff30512..60bff78e9ead 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2867,10 +2867,10 @@ static int sd_revalidate_disk(struct gendisk *disk)
 	if (sdkp->opt_xfer_blocks &&
 	    sdkp->opt_xfer_blocks <= dev_max &&
 	    sdkp->opt_xfer_blocks <= SD_DEF_XFER_BLOCKS &&
-	    sdkp->opt_xfer_blocks * sdp->sector_size >= PAGE_SIZE)
-		rw_max = q->limits.io_opt =
-			sdkp->opt_xfer_blocks * sdp->sector_size;
-	else
+	    logical_to_bytes(sdp, sdkp->opt_xfer_blocks) >= PAGE_SIZE) {
+		q->limits.io_opt = logical_to_bytes(sdp, sdkp->opt_xfer_blocks);
+		rw_max = logical_to_sectors(sdp, sdkp->opt_xfer_blocks);
+	} else
 		rw_max = BLK_DEF_MAX_SECTORS;
 
 	/* Combine with controller limits */
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 654630bb7d0e..765a6f1ac1b7 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -151,6 +151,11 @@ static inline sector_t logical_to_sectors(struct scsi_device *sdev, sector_t blo
 	return blocks << (ilog2(sdev->sector_size) - 9);
 }
 
+static inline unsigned int logical_to_bytes(struct scsi_device *sdev, sector_t blocks)
+{
+	return blocks * sdev->sector_size;
+}
+
 /*
  * A DIF-capable target device can be formatted with different
  * protection schemes.  Currently 0 through 3 are defined:
-- 
2.7.4

Re: [PATCH] sd: Fix rw_max for devices that report an optimal xfer size

[Bart Van Assche]

On 05/19/2016 08:16 PM, Martin K. Petersen wrote:
> For historic reasons, io_opt is in bytes and max_sectors in block layer
> sectors. This interface inconsistency is error prone and should be
> fixed. But for 4.4--4.7 let's make the unit difference explicit via a
> wrapper function.

Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>

Re: [PATCH] sd: Fix rw_max for devices that report an optimal xfer size

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH] sd: Fix rw_max for devices that report an optimal xfer size

[Andrew Patterson]

On 06/01/2016 01:52 AM, Christoph Hellwig wrote:
> Looks fine,
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> --
> 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
> 

I tested this patch on a 3Par array (32K optimal transfer length) that has
issues without this patch. After applying this patch on a linux-stable
kernel, optimal_io_size is correctly reported as 32K.

Tested-by: Andrew Patterson <andrew.patterson@hpe.com>

-- 
Andrew Patterson
Hewlett-Packard Enterprise

RE: [PATCH] sd: Fix rw_max for devices that report an optimal xfer size

[Long Li]

It tested well with hyper-v virtual disk.

Tested-by: Long Li <longli@microsoft.com>

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Andrew Patterson
> Sent: Wednesday, June 1, 2016 2:57 PM
> To: Christoph Hellwig <hch@infradead.org>; Martin K. Petersen
> <martin.petersen@oracle.com>
> Cc: linux-scsi@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH] sd: Fix rw_max for devices that report an optimal xfer
> size
> 
> On 06/01/2016 01:52 AM, Christoph Hellwig wrote:
> > Looks fine,
> >
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > --
> > 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
> > https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fvger.k
> > ernel.org%2fmajordomo-
> info.html&data=01%7c01%7clongli%40microsoft.com%
> >
> 7c424f9a9645da456fa38d08d38a68db13%7c72f988bf86f141af91ab2d7cd011db
> 47%
> > 7c1&sdata=cojfwxQ8%2bZLpkiT5mFlsclG6DDLBWPk8qNAg3crPtIc%3d
> >
> 
> I tested this patch on a 3Par array (32K optimal transfer length) that has
> issues without this patch. After applying this patch on a linux-stable kernel,
> optimal_io_size is correctly reported as 32K.
> 
> Tested-by: Andrew Patterson <andrew.patterson@hpe.com>
> 
> --
> Andrew Patterson
> Hewlett-Packard Enterprise
> --
> 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
> https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fvger.ker
> nel.org%2fmajordomo-
> info.html&data=01%7c01%7clongli%40microsoft.com%7c424f9a9645da456fa3
> 8d08d38a68db13%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cojfwx
> Q8%2bZLpkiT5mFlsclG6DDLBWPk8qNAg3crPtIc%3d