linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Manjong Lee <mj0123.lee@samsung.com>
To: jejb@linux.ibm.com, martin.petersen@oracle.com,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: seunghwan.hyun@samsung.com, sookwan7.kim@samsung.com,
	nanich.lee@samsung.com, woosung2.lee@samsung.com,
	yt0928.kim@samsung.com, junho89.kim@samsung.com,
	jisoo2146.oh@samsung.com, Manjong Lee <mj0123.lee@samsung.com>
Subject: [PATCH 1/1] scsi: sd: use max_xfer_blocks for set rw_max if max_xfer_blocks is available
Date: Thu, 14 Jan 2021 00:50:08 +0900	[thread overview]
Message-ID: <20210113155009.9592-1-mj0123.lee@samsung.com> (raw)
In-Reply-To: CGME20210113064521epcas1p32f0e65bc54d559b55db65bc5556103e8@epcas1p3.samsung.com

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@samsung.com>
---
 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


WARNING: multiple messages have this Message-ID (diff)
From: Manjong Lee <mj0123.lee@samsung.com>
To: mj0123.lee@samsung.com, hch@lst.de, michael.christie@oracle.com,
	damien.lemoal@wdc.com, oneukum@suse.com, arnd@arndb.de,
	martin.petersen@oracle.com
Cc: jejb@linux.ibm.com, jisoo2146.oh@samsung.com,
	junho89.kim@samsung.com, linux-kernel@vger.kernel.org,
	linux-scsi@vger.kernel.org, nanich.lee@samsung.com,
	seunghwan.hyun@samsung.com, sookwan7.kim@samsung.com,
	woosung2.lee@samsung.com, yt0928.kim@samsung.com
Subject: RE: [PATCH 1/1] scsi: sd: use max_xfer_blocks for set rw_max if max_xfer_blocks is available
Date: Thu, 21 Jan 2021 00:49:46 +0900	[thread overview]
Message-ID: <20210113155009.9592-1-mj0123.lee@samsung.com> (raw) (raw)
Message-ID: <20210120154946.tAcIo8gckdtqWf8MO10e70Nbe_V29P6GR1I5ncd5CLg@z> (raw)
In-Reply-To: <CGME20210113064521epcas1p32f0e65bc54d559b55db65bc5556103e8@epcas1p3.samsung.com>

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@samsung.com>
>---
>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
>
>

       reply	other threads:[~2021-01-13  6:46 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20210113064521epcas1p32f0e65bc54d559b55db65bc5556103e8@epcas1p3.samsung.com>
2021-01-13 15:50 ` Manjong Lee [this message]
     [not found]   ` <CGME20210120064450epcas1p1b00b7a040e0951a2da44abce916e1698@epcas1p1.samsung.com>
2021-01-20  8:00     ` [PATCH 1/1] scsi: sd: use max_xfer_blocks for set rw_max if max_xfer_blocks is available Damien Le Moal
     [not found]       ` <CGME20210122072413epcas1p2d7bd97c9eae97b9b77d13e2c4a2f02f2@epcas1p2.samsung.com>
2021-01-22  7:08         ` Changheun Lee
2021-01-22  7:44           ` Damien Le Moal
2021-01-23  3:38             ` Martin K. Petersen
     [not found]               ` <CGME20210126041455epcas1p2c38ddc3bfe20bcf10217956b47096a33@epcas1p2.samsung.com>
2021-01-26  3:59                 ` Changheun Lee
2021-01-27  3:50                   ` Martin K. Petersen
     [not found]                     ` <CGME20210127070438epcas1p417a8c9288df420b0af1ed9d185c87a22@epcas1p4.samsung.com>
2021-01-27  6:49                       ` Changheun Lee
2021-01-20 15:49 ` Manjong Lee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210113155009.9592-1-mj0123.lee@samsung.com \
    --to=mj0123.lee@samsung.com \
    --cc=jejb@linux.ibm.com \
    --cc=jisoo2146.oh@samsung.com \
    --cc=junho89.kim@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=nanich.lee@samsung.com \
    --cc=seunghwan.hyun@samsung.com \
    --cc=sookwan7.kim@samsung.com \
    --cc=woosung2.lee@samsung.com \
    --cc=yt0928.kim@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).