RE: [PATCH 1/1] scsi: sd: use max_xfer_blocks for set rw_max if max_xfer_blocks is available

[Manjong Lee]

Add recipients for more reviews.

>SCSI device has max_xfer_size and opt_xfer_size,
>but current kernel uses only opt_xfer_size.
>
>It causes the limitation on setting IO chunk size,
>although it can support larger one.
>
>So, I propose this patch to use max_xfer_size in case it has valid value.
>It can support to use the larger chunk IO on SCSI device.
>
>For example,
>This patch is effective in case of some SCSI device like UFS
>with opt_xfer_size 512KB, queue depth 32 and max_xfer_size over 512KB.
>
>I expect both the performance improvement
>and the efficiency use of smaller command queue depth.
>
>Signed-off-by: Manjong Lee <mj0123.lee@xxxxxxxxxxx>
>---
>drivers/scsi/sd.c | 56 +++++++++++++++++++++++++++++++++++++++++++----
>1 file changed, 52 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
>index 679c2c025047..de59f01c1304 100644
>--- a/drivers/scsi/sd.c
>+++ b/drivers/scsi/sd.c
>@@ -3108,6 +3108,53 @@ static void sd_read_security(struct scsi_disk *sdkp, unsigned char *buffer)
>sdkp->security = 1;
>}
>
>+static bool sd_validate_max_xfer_size(struct scsi_disk *sdkp,
>+				      unsigned int dev_max)
>+{
>+	struct scsi_device *sdp = sdkp->device;
>+	unsigned int max_xfer_bytes =
>+		logical_to_bytes(sdp, sdkp->max_xfer_blocks);
>+
>+	if (sdkp->max_xfer_blocks == 0)
>+		return false;
>+
>+	if (sdkp->max_xfer_blocks > SD_MAX_XFER_BLOCKS) {
>+		sd_first_printk(KERN_WARNING, sdkp,
>+				"Maximal transfer size %u logical blocks " \
>+				"> sd driver limit (%u logical blocks)\n",
>+				sdkp->max_xfer_blocks, SD_DEF_XFER_BLOCKS);
>+		return false;
>+	}
>+
>+	if (sdkp->max_xfer_blocks > dev_max) {
>+		sd_first_printk(KERN_WARNING, sdkp,
>+				"Maximal transfer size %u logical blocks "
>+				"> dev_max (%u logical blocks)\n",
>+				sdkp->max_xfer_blocks, dev_max);
>+		return false;
>+	}
>+
>+	if (max_xfer_bytes < PAGE_SIZE) {
>+		sd_first_printk(KERN_WARNING, sdkp,
>+				"Maximal transfer size %u bytes < " \
>+				"PAGE_SIZE (%u bytes)\n",
>+				max_xfer_bytes, (unsigned int)PAGE_SIZE);
>+		return false;
>+	}
>+
>+	if (max_xfer_bytes & (sdkp->physical_block_size - 1)) {
>+		sd_first_printk(KERN_WARNING, sdkp,
>+				"Maximal transfer size %u bytes not a " \
>+				"multiple of physical block size (%u bytes)\n",
>+				max_xfer_bytes, sdkp->physical_block_size);
>+		return false;
>+	}
>+
>+	sd_first_printk(KERN_INFO, sdkp, "Maximal transfer size %u bytes\n",
>+			max_xfer_bytes);
>+	return true;
>+}
>+
>/*
>* Determine the device's preferred I/O size for reads and writes
>* unless the reported value is unreasonably small, large, not a
>@@ -3233,12 +3280,13 @@ static int sd_revalidate_disk(struct gendisk *disk)
>
>/* Initial block count limit based on CDB TRANSFER LENGTH field size. */
>dev_max = sdp->use_16_for_rw ? SD_MAX_XFER_BLOCKS : SD_DEF_XFER_BLOCKS;
>-
>-	/* Some devices report a maximum block count for READ/WRITE requests. */
>-	dev_max = min_not_zero(dev_max, sdkp->max_xfer_blocks);
>q->limits.max_dev_sectors = logical_to_sectors(sdp, dev_max);
>
>-	if (sd_validate_opt_xfer_size(sdkp, dev_max)) {
>+	if (sd_validate_max_xfer_size(sdkp, dev_max)) {
>+		q->limits.io_opt = 0;
>+		rw_max = logical_to_sectors(sdp, sdkp->max_xfer_blocks);
>+		q->limits.max_dev_sectors = rw_max;
>+	} else if (sd_validate_opt_xfer_size(sdkp, dev_max)) {
>q->limits.io_opt = logical_to_bytes(sdp, sdkp->opt_xfer_blocks);
>rw_max = logical_to_sectors(sdp, sdkp->opt_xfer_blocks);
>} else {
>-- 
>2.29.0
>
>