From 467306a60cba4f60e5fbf5411e3849d88b5ed2bf Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Thu, 8 Jul 2004 03:53:54 +0000 Subject: [PATCH] Julian Elischer posted an interesting proof-of-concept to freebsd-current regarding UGEN's use of a 1K stack buffer for bulk IO issues. The small block size resulted in unnecessarily slow performance with certain devices. Implement a fix along the lines described. Create a simple ugen buffer allocator abstraction and a one-entry cache to avoid unnecessary malloc/free sequences. Allow the block size to be set with a sysctl and default it to 16K. Not much uses UGEN. Camera software, mainly. The change appears to slightly improve s10sh transfer performance from my Canon 10D. --- sys/conf/files | 3 +- sys/dev/usbmisc/ugen/Makefile | 4 +-- sys/dev/usbmisc/ugen/ugen.c | 60 +++++++++++++++++++++++--------- sys/dev/usbmisc/ugen/ugenbuf.c | 63 ++++++++++++++++++++++++++++++++++ sys/dev/usbmisc/ugen/ugenbuf.h | 7 ++++ 5 files changed, 117 insertions(+), 20 deletions(-) create mode 100644 sys/dev/usbmisc/ugen/ugenbuf.c create mode 100644 sys/dev/usbmisc/ugen/ugenbuf.h diff --git a/sys/conf/files b/sys/conf/files index 4e34359868..6c3fadf87f 100644 --- a/sys/conf/files +++ b/sys/conf/files @@ -1,5 +1,5 @@ # $FreeBSD: src/sys/conf/files,v 1.340.2.137 2003/06/04 17:10:30 sam Exp $ -# $DragonFly: src/sys/conf/files,v 1.64 2004/06/24 08:15:11 dillon Exp $ +# $DragonFly: src/sys/conf/files,v 1.65 2004/07/08 03:53:52 dillon Exp $ # # The long compile-with and dependency lines are required because of # limitations in config: backslash-newline doesn't work in strings, and @@ -1211,6 +1211,7 @@ dev/usbmisc/ufm/ufm.c optional ufm dev/usbmisc/ubsa/ubsa.c optional ubsa ucom dev/usbmisc/uftdi/uftdi.c optional uftdi ucom dev/usbmisc/ugen/ugen.c optional ugen +dev/usbmisc/ugen/ugenbuf.c optional ugenbuf dev/usbmisc/uhid/uhid.c optional uhid dev/usbmisc/ums/ums.c optional ums dev/usbmisc/uplcom/uplcom.c optional uplcom ucom diff --git a/sys/dev/usbmisc/ugen/Makefile b/sys/dev/usbmisc/ugen/Makefile index 579f284f4a..d5551f1ced 100644 --- a/sys/dev/usbmisc/ugen/Makefile +++ b/sys/dev/usbmisc/ugen/Makefile @@ -1,9 +1,9 @@ # $FreeBSD: src/sys/modules/ugen/Makefile,v 1.7 1999/11/28 18:53:28 bde Exp $ -# $DragonFly: src/sys/dev/usbmisc/ugen/Makefile,v 1.3 2004/01/31 06:56:27 dillon Exp $ +# $DragonFly: src/sys/dev/usbmisc/ugen/Makefile,v 1.4 2004/07/08 03:53:54 dillon Exp $ .PATH: ${.CURDIR}/../../dev/usb KMOD = ugen -SRCS = bus_if.h device_if.h vnode_if.h opt_usb.h ugen.c +SRCS = bus_if.h device_if.h vnode_if.h opt_usb.h ugen.c ugenbuf.c NOMAN = .include diff --git a/sys/dev/usbmisc/ugen/ugen.c b/sys/dev/usbmisc/ugen/ugen.c index cf977062c0..acc536ced1 100644 --- a/sys/dev/usbmisc/ugen/ugen.c +++ b/sys/dev/usbmisc/ugen/ugen.c @@ -2,7 +2,7 @@ * $NetBSD: ugen.c,v 1.27 1999/10/28 12:08:38 augustss Exp $ * $NetBSD: ugen.c,v 1.59 2002/07/11 21:14:28 augustss Exp $ * $FreeBSD: src/sys/dev/usb/ugen.c,v 1.81 2003/11/09 09:17:22 tanimura Exp $ - * $DragonFly: src/sys/dev/usbmisc/ugen/ugen.c,v 1.14 2004/07/04 05:19:53 dillon Exp $ + * $DragonFly: src/sys/dev/usbmisc/ugen/ugen.c,v 1.15 2004/07/08 03:53:54 dillon Exp $ */ /* @@ -80,11 +80,14 @@ #include #include +#include "ugenbuf.h" + +SYSCTL_NODE(_hw_usb, OID_AUTO, ugen, CTLFLAG_RW, 0, "USB ugen"); + #ifdef USB_DEBUG #define DPRINTF(x) if (ugendebug) logprintf x #define DPRINTFN(n,x) if (ugendebug>(n)) logprintf x int ugendebug = 0; -SYSCTL_NODE(_hw_usb, OID_AUTO, ugen, CTLFLAG_RW, 0, "USB ugen"); SYSCTL_INT(_hw_usb_ugen, OID_AUTO, debug, CTLFLAG_RW, &ugendebug, 0, "ugen debug level"); #else @@ -92,9 +95,12 @@ SYSCTL_INT(_hw_usb_ugen, OID_AUTO, debug, CTLFLAG_RW, #define DPRINTFN(n,x) #endif +static int ugen_bufsize = 16384; +SYSCTL_INT(_hw_usb_ugen, OID_AUTO, bufsize, CTLFLAG_RW, + &ugen_bufsize, 0, "ugen temporary buffer size"); + #define UGEN_CHUNK 128 /* chunk size for read */ #define UGEN_IBSIZE 1020 /* buffer size */ -#define UGEN_BBSIZE 1024 #define UGEN_NISOFRAMES 500 /* 0.5 seconds worth */ #define UGEN_NISOREQS 6 /* number of outstanding xfer requests */ @@ -608,11 +614,12 @@ ugen_do_read(struct ugen_softc *sc, int endpt, struct uio *uio, int flag) { struct ugen_endpoint *sce = &sc->sc_endpoints[endpt][IN]; u_int32_t n, tn; - char buf[UGEN_BBSIZE]; + char *buf; usbd_xfer_handle xfer; usbd_status err; int s; int error = 0; + int ugen_bbsize; u_char buffer[UGEN_CHUNK]; DPRINTFN(5, ("%s: ugenread: %d\n", USBDEVNAME(sc->sc_dev), endpt)); @@ -635,6 +642,8 @@ ugen_do_read(struct ugen_softc *sc, int endpt, struct uio *uio, int flag) return (EIO); } + buf = getugenbuf(ugen_bufsize, &ugen_bbsize); + switch (sce->edesc->bmAttributes & UE_XFERTYPE) { case UE_INTERRUPT: /* Block until activity occurred. */ @@ -642,7 +651,8 @@ ugen_do_read(struct ugen_softc *sc, int endpt, struct uio *uio, int flag) while (sce->q.c_cc == 0) { if (flag & IO_NDELAY) { splx(s); - return (EWOULDBLOCK); + error = EWOULDBLOCK; + goto done; } sce->state |= UGEN_ASLP; DPRINTFN(5, ("ugenread: sleep on %p\n", sce)); @@ -675,9 +685,11 @@ ugen_do_read(struct ugen_softc *sc, int endpt, struct uio *uio, int flag) break; case UE_BULK: xfer = usbd_alloc_xfer(sc->sc_udev); - if (xfer == 0) - return (ENOMEM); - while ((n = min(UGEN_BBSIZE, uio->uio_resid)) != 0) { + if (xfer == 0) { + error = ENOMEM; + goto done; + } + while ((n = min(ugen_bbsize, uio->uio_resid)) != 0) { DPRINTFN(1, ("ugenread: start transfer %d bytes\n",n)); tn = n; err = usbd_bulk_transfer( @@ -706,7 +718,8 @@ ugen_do_read(struct ugen_softc *sc, int endpt, struct uio *uio, int flag) while (sce->cur == sce->fill) { if (flag & IO_NDELAY) { splx(s); - return (EWOULDBLOCK); + error = EWOULDBLOCK; + goto done; } sce->state |= UGEN_ASLP; DPRINTFN(5, ("ugenread: sleep on %p\n", sce)); @@ -741,8 +754,11 @@ ugen_do_read(struct ugen_softc *sc, int endpt, struct uio *uio, int flag) default: - return (ENXIO); + error = ENXIO; + break; } +done: + relugenbuf(buf, ugen_bbsize); return (error); } @@ -768,7 +784,8 @@ ugen_do_write(struct ugen_softc *sc, int endpt, struct uio *uio, int flag) struct ugen_endpoint *sce = &sc->sc_endpoints[endpt][OUT]; u_int32_t n; int error = 0; - char buf[UGEN_BBSIZE]; + int ugen_bbsize; + char *buf; usbd_xfer_handle xfer; usbd_status err; @@ -792,12 +809,16 @@ ugen_do_write(struct ugen_softc *sc, int endpt, struct uio *uio, int flag) return (EIO); } + buf = getugenbuf(ugen_bufsize, &ugen_bbsize); + switch (sce->edesc->bmAttributes & UE_XFERTYPE) { case UE_BULK: xfer = usbd_alloc_xfer(sc->sc_udev); - if (xfer == 0) - return (EIO); - while ((n = min(UGEN_BBSIZE, uio->uio_resid)) != 0) { + if (xfer == 0) { + error = EIO; + goto done; + } + while ((n = min(ugen_bbsize, uio->uio_resid)) != 0) { error = uiomove(buf, n, uio); if (error) break; @@ -818,8 +839,10 @@ ugen_do_write(struct ugen_softc *sc, int endpt, struct uio *uio, int flag) break; case UE_INTERRUPT: xfer = usbd_alloc_xfer(sc->sc_udev); - if (xfer == 0) - return (EIO); + if (xfer == 0) { + error = EIO; + goto done; + } while ((n = min(UGETW(sce->edesc->wMaxPacketSize), uio->uio_resid)) != 0) { error = uiomove(buf, n, uio); @@ -841,8 +864,11 @@ ugen_do_write(struct ugen_softc *sc, int endpt, struct uio *uio, int flag) usbd_free_xfer(xfer); break; default: - return (ENXIO); + error = ENXIO; + break; } +done: + relugenbuf(buf, ugen_bbsize); return (error); } diff --git a/sys/dev/usbmisc/ugen/ugenbuf.c b/sys/dev/usbmisc/ugen/ugenbuf.c new file mode 100644 index 0000000000..b465c1ee0a --- /dev/null +++ b/sys/dev/usbmisc/ugen/ugenbuf.c @@ -0,0 +1,63 @@ +/* + * $DragonFly: src/sys/dev/usbmisc/ugen/ugenbuf.c,v 1.1 2004/07/08 03:53:54 dillon Exp $ + */ + +#include +#include +#include +#include +#include +#include "ugenbuf.h" + +static MALLOC_DEFINE(M_UGENBUF, "ugenbufs", "Temporary buffer space"); +static void *ugencache_buf; +static int ugencache_size; + +/* + * getugenbuf() + * + * Allocate a temporary buffer for UGEN. This routine is called from + * mainline code only and the BGL is held. + */ +void * +getugenbuf(int reqsize, int *bsize) +{ + void *buf; + + if (reqsize < 256) + reqsize = 256; + if (reqsize > 262144) + reqsize = 262144; + *bsize = reqsize; + + buf = ugencache_buf; + if (buf == NULL) { + buf = malloc(reqsize, M_UGENBUF, M_WAITOK); + } else if (ugencache_size != reqsize) { + ugencache_buf = NULL; + free(buf, M_UGENBUF); + buf = malloc(reqsize, M_UGENBUF, M_WAITOK); + } else { + buf = ugencache_buf; + ugencache_buf = NULL; + } + return(buf); +} + +/* + * relugenbuf() + * + * Release a temporary buffer for UGEN. This routine is called from + * mainline code only and the BGL is held. + */ +void +relugenbuf(void *buf, int bsize) +{ + if (ugencache_buf == NULL) { + ugencache_buf = buf; + ugencache_size = bsize; + } else { + free(buf, M_UGENBUF); + } +} + diff --git a/sys/dev/usbmisc/ugen/ugenbuf.h b/sys/dev/usbmisc/ugen/ugenbuf.h new file mode 100644 index 0000000000..9fdb178047 --- /dev/null +++ b/sys/dev/usbmisc/ugen/ugenbuf.h @@ -0,0 +1,7 @@ +/* + * $DragonFly: src/sys/dev/usbmisc/ugen/ugenbuf.h,v 1.1 2004/07/08 03:53:54 dillon Exp $ + * + */ + +extern void *getugenbuf(int reqsize, int *bsize); +extern void relugenbuf(void *buf, int bsize); -- 2.41.0