[5/5] usb: gadget: mass_storage: allow for deeper queue lengths

Message ID 1442339542-17090-6-git-send-email-balbi@ti.com
State Superseded
Headers show

Commit Message

Felipe Balbi Sept. 15, 2015, 5:52 p.m.
Instead of allowing a range of 2 to 4 requests,
let's allow the user choose up to 32 requests
as that will give us a better chance of keeping
controller busy.

We still maintain default of 2 so users shouldn't
be affected.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/gadget/Kconfig                   | 2 +-
 drivers/usb/gadget/function/f_mass_storage.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Alan Stern Sept. 15, 2015, 7:04 p.m. | #1
On Tue, 15 Sep 2015, Felipe Balbi wrote:

> Instead of allowing a range of 2 to 4 requests,
> let's allow the user choose up to 32 requests
> as that will give us a better chance of keeping
> controller busy.
> 
> We still maintain default of 2 so users shouldn't
> be affected.

Was this change inspired by testing?  What were the test results?

Alan Stern

--
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
Felipe Balbi Sept. 15, 2015, 7:14 p.m. | #2
On Tue, Sep 15, 2015 at 03:04:58PM -0400, Alan Stern wrote:
> On Tue, 15 Sep 2015, Felipe Balbi wrote:
> 
> > Instead of allowing a range of 2 to 4 requests,
> > let's allow the user choose up to 32 requests
> > as that will give us a better chance of keeping
> > controller busy.
> > 
> > We still maintain default of 2 so users shouldn't
> > be affected.
> 
> Was this change inspired by testing?  What were the test results?

yeah, this one yeah. With the changes to dwc3 above, I have a much
better chance of keeping the controller's queue busy with a deeper
usb_request queue length. That means that we will be avoid XferComplete
and a subsequent XferNotReady which translates directly to less NAK/NYET
handshakes in the bus. Also less POLLs.

I can send you sniffer traces if you want, but you'd have to make an
account with Total phase to download the Data Center SW to be able to
read it.
Felipe Balbi Sept. 15, 2015, 7:23 p.m. | #3
On Tue, Sep 15, 2015 at 02:14:14PM -0500, Felipe Balbi wrote:
> On Tue, Sep 15, 2015 at 03:04:58PM -0400, Alan Stern wrote:
> > On Tue, 15 Sep 2015, Felipe Balbi wrote:
> > 
> > > Instead of allowing a range of 2 to 4 requests,
> > > let's allow the user choose up to 32 requests
> > > as that will give us a better chance of keeping
> > > controller busy.
> > > 
> > > We still maintain default of 2 so users shouldn't
> > > be affected.
> > 
> > Was this change inspired by testing?  What were the test results?
> 
> yeah, this one yeah. With the changes to dwc3 above, I have a much
> better chance of keeping the controller's queue busy with a deeper
> usb_request queue length. That means that we will be avoid XferComplete
> and a subsequent XferNotReady which translates directly to less NAK/NYET
> handshakes in the bus. Also less POLLs.

oh yeah, and this is probably more important for USB3 controllers which
can add more transfers to the controller's queue while the controller is
processing something currently.

Patch

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index bcf83c0a6e62..33834aa09ed4 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -113,7 +113,7 @@  config USB_GADGET_VBUS_DRAW
 
 config USB_GADGET_STORAGE_NUM_BUFFERS
 	int "Number of storage pipeline buffers"
-	range 2 4
+	range 2 32
 	default 2
 	help
 	   Usually 2 buffers are enough to establish a good buffering
diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index a6eb537d7768..2b9c539c6d60 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -2662,7 +2662,7 @@  EXPORT_SYMBOL_GPL(fsg_common_put);
 /* check if fsg_num_buffers is within a valid range */
 static inline int fsg_num_buffers_validate(unsigned int fsg_num_buffers)
 {
-	if (fsg_num_buffers >= 2 && fsg_num_buffers <= 4)
+	if (fsg_num_buffers >= 2 && fsg_num_buffers <= 32)
 		return 0;
 	pr_err("fsg_num_buffers %u is out of range (%d to %d)\n",
 	       fsg_num_buffers, 2, 4);