[RFC,2/2] usb: storage: scsiglue: limit USB3 devices to 2048 sectors

Message ID 1446670112-29088-2-git-send-email-balbi@ti.com
State New
Headers show

Commit Message

Felipe Balbi Nov. 4, 2015, 8:48 p.m.
USB3 devices, because they are much newer, have much
less chance of having issues with larger transfers.

We still keep a limit because anything above 2048
sectors really rendered negligible speed
improvements, so we will simply ignore
that. Transferring 1MiB should already give us
pretty good performance.

Signed-off-by: Felipe Balbi <balbi@ti.com>

---
 drivers/usb/storage/scsiglue.c | 16 +++++++++++-----
 drivers/usb/storage/usb.c      |  4 ++++
 include/linux/usb_usual.h      |  2 ++
 3 files changed, 17 insertions(+), 5 deletions(-)

-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Felipe Balbi Nov. 4, 2015, 9:10 p.m. | #1
Hi,

Greg KH <greg@kroah.com> writes:
> On Wed, Nov 04, 2015 at 02:48:32PM -0600, Felipe Balbi wrote:

>> USB3 devices, because they are much newer, have much

>> less chance of having issues with larger transfers.

>> 

>> We still keep a limit because anything above 2048

>> sectors really rendered negligible speed

>> improvements, so we will simply ignore

>> that. Transferring 1MiB should already give us

>> pretty good performance.

>> 

>> Signed-off-by: Felipe Balbi <balbi@ti.com>

>> ---

>>  drivers/usb/storage/scsiglue.c | 16 +++++++++++-----

>>  drivers/usb/storage/usb.c      |  4 ++++

>>  include/linux/usb_usual.h      |  2 ++

>>  3 files changed, 17 insertions(+), 5 deletions(-)

>> 

>> diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c

>> index 00aadf3a3857..bda2b3c9ae42 100644

>> --- a/drivers/usb/storage/scsiglue.c

>> +++ b/drivers/usb/storage/scsiglue.c

>> @@ -115,13 +115,19 @@ static int slave_configure(struct scsi_device *sdev)

>>  {

>>  	struct us_data *us = host_to_us(sdev->host);

>>  

>> -	/* Many devices have trouble transferring more than 32KB at a time,

>> -	 * while others have trouble with more than 64K. At this time we

>> -	 * are limiting both to 32K (64 sectores).

>> -	 */

>> -	if (us->fflags & (US_FL_MAX_SECTORS_64 | US_FL_MAX_SECTORS_MIN)) {

>> +	if (us->fflags & US_FL_USB3) {

>> +		/* USB3 devices will be limitted at 2048 sectors. This gives us

>

> "limited"?


hehe, that was bad :-p

> Other than that, this looks good to me, did you have a chance to test a

> Windows machine to see what it does?


yeah, it always does 128 sectors no matter if it's usb2 or usb3. Mac OS
X will do 256 sectors for USB2 and 2048 for USB3. Tested both by copying
formatting and copying large files over.

-- 
balbi

Patch

diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index 00aadf3a3857..bda2b3c9ae42 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -115,13 +115,19 @@  static int slave_configure(struct scsi_device *sdev)
 {
 	struct us_data *us = host_to_us(sdev->host);
 
-	/* Many devices have trouble transferring more than 32KB at a time,
-	 * while others have trouble with more than 64K. At this time we
-	 * are limiting both to 32K (64 sectores).
-	 */
-	if (us->fflags & (US_FL_MAX_SECTORS_64 | US_FL_MAX_SECTORS_MIN)) {
+	if (us->fflags & US_FL_USB3) {
+		/* USB3 devices will be limitted at 2048 sectors. This gives us
+		 * close to 25% throughput improvement on some devices.
+		 */
+		blk_queue_max_hw_sectors(sdev->request_queue, 2048);
+	} else if (us->fflags & (US_FL_MAX_SECTORS_64 | US_FL_MAX_SECTORS_MIN)) {
 		unsigned int max_sectors = 64;
 
+		/* Many devices have trouble transferring more than 32KB at a
+		 * time, while others have trouble with more than 64K. At this
+		 * time we are limiting both to 32K (64 sectores).
+		 */
+
 		if (us->fflags & US_FL_MAX_SECTORS_MIN)
 			max_sectors = PAGE_CACHE_SIZE >> 9;
 		if (queue_max_hw_sectors(sdev->request_queue) > max_sectors)
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 43576ed31ccd..b7735c9850f8 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -594,6 +594,10 @@  static int get_device_info(struct us_data *us, const struct usb_device_id *id,
 		return -ENODEV;
 	}
 
+	/* Flag USB3 devices so we can increase max_sectors to 2048 sectors. */
+	if (dev->speed == USB_SPEED_SUPER)
+		us->fflags |= US_FL_USB3;
+
 	/*
 	 * This flag is only needed when we're in high-speed, so let's
 	 * disable it if we're in full-speed
diff --git a/include/linux/usb_usual.h b/include/linux/usb_usual.h
index 7f5f78bd15ad..b76b0faaedc8 100644
--- a/include/linux/usb_usual.h
+++ b/include/linux/usb_usual.h
@@ -79,6 +79,8 @@ 
 		/* Cannot handle MI_REPORT_SUPPORTED_OPERATION_CODES */	\
 	US_FLAG(MAX_SECTORS_240,	0x08000000)		\
 		/* Sets max_sectors to 240 */			\
+	US_FLAG(USB3,		0x10000000)			\
+		/* This is a USB3 Storage Device */		\
 
 #define US_FLAG(name, value)	US_FL_##name = value ,
 enum { US_DO_ALL_FLAGS };