You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/07/12 06:22:15 UTC
[GitHub] [incubator-nuttx] eenurkka opened a new pull request, #6601: usbdev/usbmsc: introduce USBMSC_WRMULTIPLE for faster writes
eenurkka opened a new pull request, #6601:
URL: https://github.com/apache/incubator-nuttx/pull/6601
This patch introduces a configuration option USBMSC_WRMULTIPLE,
which is used to store multiple blocks into a larger chunk that
then gets written via the mmcsd_writemultiple() (in case mmcsd
is used).
The bottleneck with the current implementation is the poor
performance due to short block writes. USBMSC_DRVR_WRITE()
always writes only one sector (with eMMC that's usually 512 bytes).
eMMC devices usually erase much larger regions with near constant
time (see Jedec JESD84-B51, Extended CSD register byte [225],
SUPER_PAGE_SIZE): 'This register defines one or multiple of
programmable boundary unit that is programmed at the same time.'
If USBMSC_WRMULTIPLE is defined, then USBMSC_NWRREQS is used to
allocate the write buffer size. We don't want this to be the
default behavior yet as this may reveal unseen bugs in usb drivers
due to the faster overall performance.
Sample configurations with measured performance:
- Without USBMSC_WRMULTIPLE: 470 Kb/s
- With USBMSC_WRMULTIPLE, CONFIG_USBMSC_NWRREQS=4: 1.1 Mb/s
(dd with bs=2k)
- With USBMSC_WRMULTIPLE, CONFIG_USBMSC_NWRREQS=16: 5.2 Mb/s
(dd with bs=8k)
No doubt, this feature alone may make the mass storage work 10
times faster than before with eMMC cards.
Signed-off-by: Eero Nurkkala <ee...@offcode.fi>
## Summary
This speeds up the mass storage emmcsd write operations significantly.
## Impact
Due to the high risk, the behavior is behind #ifdefs. Enabling this feature
is likely to reveal USB driver issues, like it did with risc-v/mpfs as well.
## Testing
Sending 1.9 Gb image on Polarfire MPFS kit with varying bs options:
dd if=core-image-minimal-dev-icicle-kit-es-amp-20211208101259.rootfs.wic of=/dev/sdb bs=2k status=progress
dd if=core-image-minimal-dev-icicle-kit-es-amp-20211208101259.rootfs.wic of=/dev/sdb bs=8k status=progress
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6601: usbdev/usbmsc: introduce USBMSC_WRMULTIPLE for faster writes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6601:
URL: https://github.com/apache/incubator-nuttx/pull/6601#discussion_r918732010
##########
drivers/usbdev/usbmsc_scsi.c:
##########
@@ -2454,9 +2470,34 @@ static int usbmsc_cmdwritestate(FAR struct usbmsc_dev_s *priv)
priv->nsectbytes += nbytes;
priv->nreqbytes -= nbytes;
+#ifdef CONFIG_USBMSC_WRMULTIPLE
+ uint32_t nrbufs = MIN(priv->u.xfrlen, CONFIG_USBMSC_NWRREQS);
+
/* Is the I/O buffer full? */
- if (priv->nsectbytes >= lun->sectorsize)
+ if ((priv->nsectbytes >= lun->sectorsize * priv->u.xfrlen) ||
+ (priv->nsectbytes >= lun->sectorsize * CONFIG_USBMSC_NWRREQS))
+ {
+ /* Yes.. Write next sectors */
+
+ nwritten = USBMSC_DRVR_WRITE(lun, priv->iobuffer,
+ priv->sector, nrbufs);
+ if (nwritten < 0)
+ {
+ usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_CMDWRITEWRITEFAIL),
+ -nwritten);
+ lun->sd = SCSI_KCQME_WRITEFAULTAUTOREALLOCFAILED;
+ lun->sdinfo = priv->sector;
+ goto errout;
+ }
+
+ priv->nsectbytes = 0;
+ priv->residue -= lun->sectorsize * nrbufs;
+ priv->u.xfrlen -= nrbufs;
+ priv->sector += nrbufs;
+ }
Review Comment:
no, I mean this:
```
#ifdef ...
if (...)
{
....
}
else
#else
if (...)
{
...
}
#endif
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6601: usbdev/usbmsc: introduce USBMSC_WRMULTIPLE for faster writes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6601:
URL: https://github.com/apache/incubator-nuttx/pull/6601#discussion_r918680277
##########
drivers/usbdev/usbmsc_scsi.c:
##########
@@ -2444,8 +2444,24 @@ static int usbmsc_cmdwritestate(FAR struct usbmsc_dev_s *priv)
src = &req->buf[xfrd - priv->nreqbytes];
dest = &priv->iobuffer[priv->nsectbytes];
- nbytes = MIN(lun->sectorsize - priv->nsectbytes, priv->nreqbytes);
+#ifdef CONFIG_USBMSC_WRMULTIPLE
+ /* nbytes may end up being zero, after which the loop no longer
+ * proceeds but will be stuck forever. Make sure nbytes isn't
+ * zero.
+ */
+ if (lun->sectorsize > priv->nsectbytes)
+ {
+ nbytes = MIN(lun->sectorsize - priv->nsectbytes,
+ priv->nreqbytes);
+ }
+ else
+ {
+ nbytes = priv->nreqbytes;
+ }
+#else
+ nbytes = MIN(lun->sectorsize - priv->nsectbytes, priv->nreqbytes);
Review Comment:
why the old code doesn't suffer this issue?
##########
drivers/usbdev/usbmsc_scsi.c:
##########
@@ -2454,9 +2470,34 @@ static int usbmsc_cmdwritestate(FAR struct usbmsc_dev_s *priv)
priv->nsectbytes += nbytes;
priv->nreqbytes -= nbytes;
+#ifdef CONFIG_USBMSC_WRMULTIPLE
+ uint32_t nrbufs = MIN(priv->u.xfrlen, CONFIG_USBMSC_NWRREQS);
+
/* Is the I/O buffer full? */
- if (priv->nsectbytes >= lun->sectorsize)
+ if ((priv->nsectbytes >= lun->sectorsize * priv->u.xfrlen) ||
+ (priv->nsectbytes >= lun->sectorsize * CONFIG_USBMSC_NWRREQS))
+ {
+ /* Yes.. Write next sectors */
+
+ nwritten = USBMSC_DRVR_WRITE(lun, priv->iobuffer,
+ priv->sector, nrbufs);
+ if (nwritten < 0)
+ {
+ usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_CMDWRITEWRITEFAIL),
+ -nwritten);
+ lun->sd = SCSI_KCQME_WRITEFAULTAUTOREALLOCFAILED;
+ lun->sdinfo = priv->sector;
+ goto errout;
+ }
+
+ priv->nsectbytes = 0;
+ priv->residue -= lun->sectorsize * nrbufs;
+ priv->u.xfrlen -= nrbufs;
+ priv->sector += nrbufs;
+ }
Review Comment:
```suggestion
}
else
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] eenurkka commented on a diff in pull request #6601: usbdev/usbmsc: introduce USBMSC_WRMULTIPLE for faster writes
Posted by GitBox <gi...@apache.org>.
eenurkka commented on code in PR #6601:
URL: https://github.com/apache/incubator-nuttx/pull/6601#discussion_r918759915
##########
drivers/usbdev/usbmsc_scsi.c:
##########
@@ -2454,9 +2470,34 @@ static int usbmsc_cmdwritestate(FAR struct usbmsc_dev_s *priv)
priv->nsectbytes += nbytes;
priv->nreqbytes -= nbytes;
+#ifdef CONFIG_USBMSC_WRMULTIPLE
+ uint32_t nrbufs = MIN(priv->u.xfrlen, CONFIG_USBMSC_NWRREQS);
+
/* Is the I/O buffer full? */
- if (priv->nsectbytes >= lun->sectorsize)
+ if ((priv->nsectbytes >= lun->sectorsize * priv->u.xfrlen) ||
+ (priv->nsectbytes >= lun->sectorsize * CONFIG_USBMSC_NWRREQS))
+ {
+ /* Yes.. Write next sectors */
+
+ nwritten = USBMSC_DRVR_WRITE(lun, priv->iobuffer,
+ priv->sector, nrbufs);
+ if (nwritten < 0)
+ {
+ usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_CMDWRITEWRITEFAIL),
+ -nwritten);
+ lun->sd = SCSI_KCQME_WRITEFAULTAUTOREALLOCFAILED;
+ lun->sdinfo = priv->sector;
+ goto errout;
+ }
+
+ priv->nsectbytes = 0;
+ priv->residue -= lun->sectorsize * nrbufs;
+ priv->u.xfrlen -= nrbufs;
+ priv->sector += nrbufs;
+ }
Review Comment:
If I update, then it won't compile. That 'else' would point to nowhere, eg. :
if (something)
{
do something
}
else
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6601: usbdev/usbmsc: introduce USBMSC_WRMULTIPLE for faster writes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6601:
URL: https://github.com/apache/incubator-nuttx/pull/6601#discussion_r918814915
##########
drivers/usbdev/usbmsc_scsi.c:
##########
@@ -2454,9 +2470,34 @@ static int usbmsc_cmdwritestate(FAR struct usbmsc_dev_s *priv)
priv->nsectbytes += nbytes;
priv->nreqbytes -= nbytes;
+#ifdef CONFIG_USBMSC_WRMULTIPLE
+ uint32_t nrbufs = MIN(priv->u.xfrlen, CONFIG_USBMSC_NWRREQS);
+
/* Is the I/O buffer full? */
- if (priv->nsectbytes >= lun->sectorsize)
+ if ((priv->nsectbytes >= lun->sectorsize * priv->u.xfrlen) ||
+ (priv->nsectbytes >= lun->sectorsize * CONFIG_USBMSC_NWRREQS))
+ {
+ /* Yes.. Write next sectors */
+
+ nwritten = USBMSC_DRVR_WRITE(lun, priv->iobuffer,
+ priv->sector, nrbufs);
+ if (nwritten < 0)
+ {
+ usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_CMDWRITEWRITEFAIL),
+ -nwritten);
+ lun->sd = SCSI_KCQME_WRITEFAULTAUTOREALLOCFAILED;
+ lun->sdinfo = priv->sector;
+ goto errout;
+ }
+
+ priv->nsectbytes = 0;
+ priv->residue -= lun->sectorsize * nrbufs;
+ priv->u.xfrlen -= nrbufs;
+ priv->sector += nrbufs;
+ }
Review Comment:
Sorry, I misunderstand your change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6601: usbdev/usbmsc: introduce USBMSC_WRMULTIPLE for faster writes
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6601:
URL: https://github.com/apache/incubator-nuttx/pull/6601#discussion_r918663893
##########
drivers/usbdev/Kconfig:
##########
@@ -799,6 +799,20 @@ config USBMSC_NRDREQS
---help---
The number of write/read requests that can be in flight
+config USBMSC_WRMULTIPLE
+ bool "Write multiple blocks at once if possible"
+ default n
+ ---help---
+ Store multiple blocks and write them in a single request. This
+ speeds up the writing significantly with eMMC devices, for example,
+ because writing 512 bytes may be as fast as writing the complete
+ SUPER_PAGE_SIZE (see extended CSD [225] bits 0-3), which may be up
+ to 64Kb. Real-life example with different block sizes, using the
+ dd command with argument bs=512, bs=8k (USBMSC_NWRREQS = 4, and 16):
+ 512b: 470 kb/s, 8k: 5.3 Mb/s. This is more than a tenfold increase
Review Comment:
Please change spaces to TABs here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] eenurkka commented on a diff in pull request #6601: usbdev/usbmsc: introduce USBMSC_WRMULTIPLE for faster writes
Posted by GitBox <gi...@apache.org>.
eenurkka commented on code in PR #6601:
URL: https://github.com/apache/incubator-nuttx/pull/6601#discussion_r918743495
##########
drivers/usbdev/usbmsc_scsi.c:
##########
@@ -2454,9 +2470,34 @@ static int usbmsc_cmdwritestate(FAR struct usbmsc_dev_s *priv)
priv->nsectbytes += nbytes;
priv->nreqbytes -= nbytes;
+#ifdef CONFIG_USBMSC_WRMULTIPLE
+ uint32_t nrbufs = MIN(priv->u.xfrlen, CONFIG_USBMSC_NWRREQS);
+
/* Is the I/O buffer full? */
- if (priv->nsectbytes >= lun->sectorsize)
+ if ((priv->nsectbytes >= lun->sectorsize * priv->u.xfrlen) ||
+ (priv->nsectbytes >= lun->sectorsize * CONFIG_USBMSC_NWRREQS))
+ {
+ /* Yes.. Write next sectors */
+
+ nwritten = USBMSC_DRVR_WRITE(lun, priv->iobuffer,
+ priv->sector, nrbufs);
+ if (nwritten < 0)
+ {
+ usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_CMDWRITEWRITEFAIL),
+ -nwritten);
+ lun->sd = SCSI_KCQME_WRITEFAULTAUTOREALLOCFAILED;
+ lun->sdinfo = priv->sector;
+ goto errout;
+ }
+
+ priv->nsectbytes = 0;
+ priv->residue -= lun->sectorsize * nrbufs;
+ priv->u.xfrlen -= nrbufs;
+ priv->sector += nrbufs;
+ }
Review Comment:
```
--- a/drivers/usbdev/usbmsc_scsi.c
+++ b/drivers/usbdev/usbmsc_scsi.c
@@ -2496,6 +2496,7 @@ static int usbmsc_cmdwritestate(FAR struct usbmsc_dev_s *priv)
priv->u.xfrlen -= nrbufs;
priv->sector += nrbufs;
}
+ else
#else
if ((priv->nsectbytes >= lun->sectorsize))
{
```
will result in:
```
make[1]: Entering directory '/home/eenurkka/nuttx-upstream/scsi/drivers'
CC: usbdev/usbmsc_scsi.c
usbdev/usbmsc_scsi.c: In function 'usbmsc_cmdwritestate':
usbdev/usbmsc_scsi.c:2522:9: error: expected expression before '}' token
2522 | }
| ^
make[1]: *** [Makefile:91: usbmsc_scsi.o] Error 1
```
Maybe I'm missing something?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6601: usbdev/usbmsc: introduce USBMSC_WRMULTIPLE for faster writes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6601:
URL: https://github.com/apache/incubator-nuttx/pull/6601
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] eenurkka commented on a diff in pull request #6601: usbdev/usbmsc: introduce USBMSC_WRMULTIPLE for faster writes
Posted by GitBox <gi...@apache.org>.
eenurkka commented on code in PR #6601:
URL: https://github.com/apache/incubator-nuttx/pull/6601#discussion_r918718174
##########
drivers/usbdev/usbmsc_scsi.c:
##########
@@ -2454,9 +2470,34 @@ static int usbmsc_cmdwritestate(FAR struct usbmsc_dev_s *priv)
priv->nsectbytes += nbytes;
priv->nreqbytes -= nbytes;
+#ifdef CONFIG_USBMSC_WRMULTIPLE
+ uint32_t nrbufs = MIN(priv->u.xfrlen, CONFIG_USBMSC_NWRREQS);
+
/* Is the I/O buffer full? */
- if (priv->nsectbytes >= lun->sectorsize)
+ if ((priv->nsectbytes >= lun->sectorsize * priv->u.xfrlen) ||
+ (priv->nsectbytes >= lun->sectorsize * CONFIG_USBMSC_NWRREQS))
+ {
+ /* Yes.. Write next sectors */
+
+ nwritten = USBMSC_DRVR_WRITE(lun, priv->iobuffer,
+ priv->sector, nrbufs);
+ if (nwritten < 0)
+ {
+ usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_CMDWRITEWRITEFAIL),
+ -nwritten);
+ lun->sd = SCSI_KCQME_WRITEFAULTAUTOREALLOCFAILED;
+ lun->sdinfo = priv->sector;
+ goto errout;
+ }
+
+ priv->nsectbytes = 0;
+ priv->residue -= lun->sectorsize * nrbufs;
+ priv->u.xfrlen -= nrbufs;
+ priv->sector += nrbufs;
+ }
Review Comment:
You mean get rid of the #else and put else instead? It will not work then, condition if ((priv->nsectbytes >= lun->sectorsize)) is taken and it will not gather multiple packets?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6601: usbdev/usbmsc: introduce USBMSC_WRMULTIPLE for faster writes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6601:
URL: https://github.com/apache/incubator-nuttx/pull/6601#discussion_r918755693
##########
drivers/usbdev/usbmsc_scsi.c:
##########
@@ -2454,9 +2470,34 @@ static int usbmsc_cmdwritestate(FAR struct usbmsc_dev_s *priv)
priv->nsectbytes += nbytes;
priv->nreqbytes -= nbytes;
+#ifdef CONFIG_USBMSC_WRMULTIPLE
+ uint32_t nrbufs = MIN(priv->u.xfrlen, CONFIG_USBMSC_NWRREQS);
+
/* Is the I/O buffer full? */
- if (priv->nsectbytes >= lun->sectorsize)
+ if ((priv->nsectbytes >= lun->sectorsize * priv->u.xfrlen) ||
+ (priv->nsectbytes >= lun->sectorsize * CONFIG_USBMSC_NWRREQS))
+ {
+ /* Yes.. Write next sectors */
+
+ nwritten = USBMSC_DRVR_WRITE(lun, priv->iobuffer,
+ priv->sector, nrbufs);
+ if (nwritten < 0)
+ {
+ usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_CMDWRITEWRITEFAIL),
+ -nwritten);
+ lun->sd = SCSI_KCQME_WRITEFAULTAUTOREALLOCFAILED;
+ lun->sdinfo = priv->sector;
+ goto errout;
+ }
+
+ priv->nsectbytes = 0;
+ priv->residue -= lun->sectorsize * nrbufs;
+ priv->u.xfrlen -= nrbufs;
+ priv->sector += nrbufs;
+ }
Review Comment:
It's strange, could you update your change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] eenurkka commented on a diff in pull request #6601: usbdev/usbmsc: introduce USBMSC_WRMULTIPLE for faster writes
Posted by GitBox <gi...@apache.org>.
eenurkka commented on code in PR #6601:
URL: https://github.com/apache/incubator-nuttx/pull/6601#discussion_r918790457
##########
drivers/usbdev/usbmsc_scsi.c:
##########
@@ -2454,9 +2470,34 @@ static int usbmsc_cmdwritestate(FAR struct usbmsc_dev_s *priv)
priv->nsectbytes += nbytes;
priv->nreqbytes -= nbytes;
+#ifdef CONFIG_USBMSC_WRMULTIPLE
+ uint32_t nrbufs = MIN(priv->u.xfrlen, CONFIG_USBMSC_NWRREQS);
+
/* Is the I/O buffer full? */
- if (priv->nsectbytes >= lun->sectorsize)
+ if ((priv->nsectbytes >= lun->sectorsize * priv->u.xfrlen) ||
+ (priv->nsectbytes >= lun->sectorsize * CONFIG_USBMSC_NWRREQS))
+ {
+ /* Yes.. Write next sectors */
+
+ nwritten = USBMSC_DRVR_WRITE(lun, priv->iobuffer,
+ priv->sector, nrbufs);
+ if (nwritten < 0)
+ {
+ usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_CMDWRITEWRITEFAIL),
+ -nwritten);
+ lun->sd = SCSI_KCQME_WRITEFAULTAUTOREALLOCFAILED;
+ lun->sdinfo = priv->sector;
+ goto errout;
+ }
+
+ priv->nsectbytes = 0;
+ priv->residue -= lun->sectorsize * nrbufs;
+ priv->u.xfrlen -= nrbufs;
+ priv->sector += nrbufs;
+ }
Review Comment:
Ok, I understand it now. But if there's such an else, then if ((priv->nsectbytes >= lun->sectorsize)) would always be taken in preference to the previous if, and this would always send single blocks? Usually with dualspeed usb we receive 512-byte blocks (usb bulk), and with the else, it would always send single blocks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] eenurkka commented on a diff in pull request #6601: usbdev/usbmsc: introduce USBMSC_WRMULTIPLE for faster writes
Posted by GitBox <gi...@apache.org>.
eenurkka commented on code in PR #6601:
URL: https://github.com/apache/incubator-nuttx/pull/6601#discussion_r918671788
##########
drivers/usbdev/Kconfig:
##########
@@ -799,6 +799,20 @@ config USBMSC_NRDREQS
---help---
The number of write/read requests that can be in flight
+config USBMSC_WRMULTIPLE
+ bool "Write multiple blocks at once if possible"
+ default n
+ ---help---
+ Store multiple blocks and write them in a single request. This
+ speeds up the writing significantly with eMMC devices, for example,
+ because writing 512 bytes may be as fast as writing the complete
+ SUPER_PAGE_SIZE (see extended CSD [225] bits 0-3), which may be up
+ to 64Kb. Real-life example with different block sizes, using the
+ dd command with argument bs=512, bs=8k (USBMSC_NWRREQS = 4, and 16):
+ 512b: 470 kb/s, 8k: 5.3 Mb/s. This is more than a tenfold increase
Review Comment:
Right, originally had, but messed it up. Will fix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] eenurkka commented on a diff in pull request #6601: usbdev/usbmsc: introduce USBMSC_WRMULTIPLE for faster writes
Posted by GitBox <gi...@apache.org>.
eenurkka commented on code in PR #6601:
URL: https://github.com/apache/incubator-nuttx/pull/6601#discussion_r918678899
##########
drivers/usbdev/Kconfig:
##########
@@ -799,6 +799,20 @@ config USBMSC_NRDREQS
---help---
The number of write/read requests that can be in flight
+config USBMSC_WRMULTIPLE
+ bool "Write multiple blocks at once if possible"
+ default n
+ ---help---
+ Store multiple blocks and write them in a single request. This
+ speeds up the writing significantly with eMMC devices, for example,
+ because writing 512 bytes may be as fast as writing the complete
+ SUPER_PAGE_SIZE (see extended CSD [225] bits 0-3), which may be up
+ to 64Kb. Real-life example with different block sizes, using the
+ dd command with argument bs=512, bs=8k (USBMSC_NWRREQS = 4, and 16):
+ 512b: 470 kb/s, 8k: 5.3 Mb/s. This is more than a tenfold increase
Review Comment:
Fixed now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] eenurkka commented on a diff in pull request #6601: usbdev/usbmsc: introduce USBMSC_WRMULTIPLE for faster writes
Posted by GitBox <gi...@apache.org>.
eenurkka commented on code in PR #6601:
URL: https://github.com/apache/incubator-nuttx/pull/6601#discussion_r918709665
##########
drivers/usbdev/usbmsc_scsi.c:
##########
@@ -2444,8 +2444,24 @@ static int usbmsc_cmdwritestate(FAR struct usbmsc_dev_s *priv)
src = &req->buf[xfrd - priv->nreqbytes];
dest = &priv->iobuffer[priv->nsectbytes];
- nbytes = MIN(lun->sectorsize - priv->nsectbytes, priv->nreqbytes);
+#ifdef CONFIG_USBMSC_WRMULTIPLE
+ /* nbytes may end up being zero, after which the loop no longer
+ * proceeds but will be stuck forever. Make sure nbytes isn't
+ * zero.
+ */
+ if (lun->sectorsize > priv->nsectbytes)
+ {
+ nbytes = MIN(lun->sectorsize - priv->nsectbytes,
+ priv->nreqbytes);
+ }
+ else
+ {
+ nbytes = priv->nreqbytes;
+ }
+#else
+ nbytes = MIN(lun->sectorsize - priv->nsectbytes, priv->nreqbytes);
Review Comment:
If sectorsize is nsectbytes, it has already exited the loop. So there it's unnecessary
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6601: usbdev/usbmsc: introduce USBMSC_WRMULTIPLE for faster writes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6601:
URL: https://github.com/apache/incubator-nuttx/pull/6601#discussion_r918779438
##########
drivers/usbdev/usbmsc_scsi.c:
##########
@@ -2454,9 +2470,34 @@ static int usbmsc_cmdwritestate(FAR struct usbmsc_dev_s *priv)
priv->nsectbytes += nbytes;
priv->nreqbytes -= nbytes;
+#ifdef CONFIG_USBMSC_WRMULTIPLE
+ uint32_t nrbufs = MIN(priv->u.xfrlen, CONFIG_USBMSC_NWRREQS);
+
/* Is the I/O buffer full? */
- if (priv->nsectbytes >= lun->sectorsize)
+ if ((priv->nsectbytes >= lun->sectorsize * priv->u.xfrlen) ||
+ (priv->nsectbytes >= lun->sectorsize * CONFIG_USBMSC_NWRREQS))
+ {
+ /* Yes.. Write next sectors */
+
+ nwritten = USBMSC_DRVR_WRITE(lun, priv->iobuffer,
+ priv->sector, nrbufs);
+ if (nwritten < 0)
+ {
+ usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_CMDWRITEWRITEFAIL),
+ -nwritten);
+ lun->sd = SCSI_KCQME_WRITEFAULTAUTOREALLOCFAILED;
+ lun->sdinfo = priv->sector;
+ goto errout;
+ }
+
+ priv->nsectbytes = 0;
+ priv->residue -= lun->sectorsize * nrbufs;
+ priv->u.xfrlen -= nrbufs;
+ priv->sector += nrbufs;
+ }
Review Comment:
else should be part of the next if:
```
else
#else
if ((priv->nsectbytes >= lun->sectorsize))
#endif
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org