Re: [PATCH] usb: gadget: storage_common: make FSG_NUM_BUFFERSvariable size

From: Michal Nazarewicz
Date: Mon Aug 08 2011 - 05:40:22 EST


On Mon, 08 Aug 2011 02:22:14 +0200, Per Forlin <per.forlin@xxxxxxxxxx> wrote:
FSG_NUM_BUFFERS is set to 2 as default.
Usually 2 buffers are enough to establish a good
double buffering pipeline. But when dealing with expensive
request preparation (i.e. dma_map) there may be benefits of
increasing the number of buffers. There is an extra cost for
every first request, the others are prepared in parallell with
an ongoing transfer. Every time all buffers are consumed there is
an additional cost for the next first request. Increasing the number
of buffers decreases the risk of running out of buffers.

[... benchmark snip ...]

There may not be one optimal number for all boards. That is the
reason for adding the number to Kconfig,

It could actually be turned into a run-time configuration, but
that's a bit more intrusive, so adding Kconfig should be enough
for now.

Signed-off-by: Per Forlin <per.forlin@xxxxxxxxxx>

Acked-by: Michal Nazarewicz <mina86@xxxxxxxxxx>


Some not very relevant comments below:

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 029e288..bbd17f3 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -96,6 +96,21 @@ config USB_GADGET_VBUS_DRAW
This value will be used except for system-specific gadget
drivers that have more specific information.
+config USB_GADGET_STORAGE_NUM_BUFFERS
+ int "Number of storage pipline buffers"

âpipelineâ, missing âeâ.

+ range 2 64
+ default 2
+ help
+ Usually 2 buffers are enough to establish a good
+ double buffering pipeline. But when dealing with expensive

s/double//. By definition, âdoubleâ buffering uses two buffers, isn't it?
So if we change number of buffers, it's no longer âdoubleâ.

+ request preparation (i.e. dma_map) there may be benefits of
+ increasing the number of buffers. There is an extra cost for
+ every first request, the others are prepared in parallell with

âparallelâ, too many âlâs.

+ an ongoing transfer. Every time all buffers are consumed there is
+ an additional cost for the next first request. Increasing the number

âNext first requestâ sounds a bit illogical to me.

+ of buffers decreases the risk of running out of buffers.
+ If unsure, say 2.
+
config USB_GADGET_SELECTED
boolean
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index 1fa4f70..512d9cf 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -262,8 +262,8 @@ static struct fsg_lun *fsg_lun_from_dev(struct device *dev)
#define EP0_BUFSIZE 256
#define DELAYED_STATUS (EP0_BUFSIZE + 999) /* An impossibly large value */
-/* Number of buffers we will use. 2 is enough for double-buffering */
-#define FSG_NUM_BUFFERS 2
+/* Number of buffers we will use. 2 is usually enough for double-buffering */

s/double-buffering/good buffering pipeline/?

+#define FSG_NUM_BUFFERS CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS
/* Default size of buffer length. */
#define FSG_BUFLEN ((u32)16384)

--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michal "mina86" Nazarewicz (o o)
ooo +-----<email/xmpp: mnazarewicz@xxxxxxxxxx>-----ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/