diff mbox

scsi: sd: Do not override max_sectors_kb sysfs setting

Message ID 20170928013859.1214-1-martin.petersen@oracle.com (mailing list archive)
State Accepted
Headers show

Commit Message

Martin K. Petersen Sept. 28, 2017, 1:38 a.m. UTC
A user may lower the max_sectors_kb setting in sysfs to accommodate
certain workloads. Previously we would always set the max I/O size to
either the block layer default or the optional preferred I/O size
reported by the device.

Keep the current heuristics for the initial setting of max_sectors_kb.
For subsequent invocations, only update the current queue limit if it
exceeds the capabilities of the hardware.

Reported-by: Don Brace <don.brace@microsemi.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/sd.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

Comments

Martin Wilck Sept. 29, 2017, 12:26 p.m. UTC | #1
On Wed, 2017-09-27 at 21:38 -0400, Martin K. Petersen wrote:
> A user may lower the max_sectors_kb setting in sysfs to accommodate
> certain workloads. Previously we would always set the max I/O size to
> either the block layer default or the optional preferred I/O size
> reported by the device.
> 
> Keep the current heuristics for the initial setting of
> max_sectors_kb.
> For subsequent invocations, only update the current queue limit if it
> exceeds the capabilities of the hardware.
> 
> Reported-by: Don Brace <don.brace@microsemi.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
 
This looks good to me. I agree that it's superior to the original
suggestion, because it sets the soft limit to the hard limit when the
device is scanned for the first time.

Regards
Martin
Don Brace Sept. 29, 2017, 1:46 p.m. UTC | #2
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Martin K. Petersen
> Sent: Wednesday, September 27, 2017 8:39 PM
> To: linux-scsi@vger.kernel.org
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Subject: [PATCH] scsi: sd: Do not override max_sectors_kb sysfs setting
> 
> EXTERNAL EMAIL
> 
> 
> A user may lower the max_sectors_kb setting in sysfs to accommodate
> certain workloads. Previously we would always set the max I/O size to
> either the block layer default or the optional preferred I/O size
> reported by the device.
> 
> Keep the current heuristics for the initial setting of max_sectors_kb.
> For subsequent invocations, only update the current queue limit if it
> exceeds the capabilities of the hardware.
> 
> Reported-by: Don Brace <don.brace@microsemi.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

Tested-by: Don Brace <don.brace@microsemi.com>

Really appreciate your attention to this matter.

Thanks,
Don Brace
ESC - Smart Storage
Microsemi Corporation
diff mbox

Patch

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 6549e5ce09ca..b18ba3235900 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3098,8 +3098,6 @@  static int sd_revalidate_disk(struct gendisk *disk)
 		sd_read_security(sdkp, buffer);
 	}
 
-	sdkp->first_scan = 0;
-
 	/*
 	 * We now have all cache related info, determine how we deal
 	 * with flush requests.
@@ -3114,7 +3112,7 @@  static int sd_revalidate_disk(struct gendisk *disk)
 	q->limits.max_dev_sectors = logical_to_sectors(sdp, dev_max);
 
 	/*
-	 * Use the device's preferred I/O size for reads and writes
+	 * Determine the device's preferred I/O size for reads and writes
 	 * unless the reported value is unreasonably small, large, or
 	 * garbage.
 	 */
@@ -3128,8 +3126,19 @@  static int sd_revalidate_disk(struct gendisk *disk)
 		rw_max = min_not_zero(logical_to_sectors(sdp, dev_max),
 				      (sector_t)BLK_DEF_MAX_SECTORS);
 
-	/* Combine with controller limits */
-	q->limits.max_sectors = min(rw_max, queue_max_hw_sectors(q));
+	/* Do not exceed controller limit */
+	rw_max = min(rw_max, queue_max_hw_sectors(q));
+
+	/*
+	 * Only update max_sectors if previously unset or if the current value
+	 * exceeds the capabilities of the hardware.
+	 */
+	if (sdkp->first_scan ||
+	    q->limits.max_sectors > q->limits.max_dev_sectors ||
+	    q->limits.max_sectors > q->limits.max_hw_sectors)
+		q->limits.max_sectors = rw_max;
+
+	sdkp->first_scan = 0;
 
 	set_capacity(disk, logical_to_sectors(sdp, sdkp->capacity));
 	sd_config_write_same(sdkp);