As pointed out by David Higgs, uhidev report functions do a lot of
memory allocations and copies between buffers. This is not uncommon
in USB land due to way DMA buffers are handled.
One way to reduce the number of copies is to submit a transfer with
the USBD_NO_COPY flag. Without it the kernel does two to three copies:
(stack/driver array <-->) malloc'ed array <--> DMA reacheable array
This is not limited to uhidev functions. That's also done for every
transfer sent through an interrupt pipe or for every control request
with data.
I'd like to slowly change the stack such that USBD_NO_COPY becomes the
default behavior. This would first reduces the number of buffer copies
and then allow umass(4) to directly use the DMA reachable buffers
provided by the SCSI stack, which should *at least* reduce the CPU usage
when using USB drives.
Here's a diff about uhidev(4) to illustrate the changes I'm talking
about.
Comments, ok?
Index: uhidev.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
retrieving revision 1.68
diff -u -p -r1.68 uhidev.c
--- uhidev.c 9 Jan 2015 12:09:51 -0000 1.68
+++ uhidev.c 9 Jan 2015 13:31:36 -0000
@@ -671,18 +671,28 @@ int
uhidev_set_report_async(struct uhidev_softc *sc, int type, int id, void *data,
int len)
{
+ struct usbd_xfer *xfer;
usb_device_request_t req;
- char *buf = data;
int actlen = len;
+ char *buf;
+
+ xfer = usbd_alloc_xfer(sc->sc_udev);
+ if (xfer == NULL)
+ return (-1);
+
+ if (id > 0)
+ len++;
+
+ buf = usbd_alloc_buffer(xfer, len);
+ if (buf == NULL)
+ return (-1);
/* Prepend the reportID. */
if (id > 0) {
- len++;
- buf = malloc(len, M_TEMP, M_NOWAIT);
- if (buf == NULL)
- return (-1);
buf[0] = id;
memcpy(buf + 1, data, len - 1);
+ } else {
+ memcpy(buf, data, len);
}
req.bmRequestType = UT_WRITE_CLASS_INTERFACE;
@@ -691,17 +701,9 @@ uhidev_set_report_async(struct uhidev_so
USETW(req.wIndex, sc->sc_ifaceno);
USETW(req.wLength, len);
- if (usbd_do_request_async(sc->sc_udev, &req, buf, NULL, NULL))
+ if (usbd_request_async(xfer, &req, NULL, NULL))
actlen = -1;
- /*
- * Since report requests are write-only it is safe to free
- * the buffer right after submitting the transfer because
- * it won't be used afterward.
- */
- if (id > 0)
- free(buf, M_TEMP, len);
-
return (actlen);
}
@@ -750,11 +752,11 @@ uhidev_get_report_async_cb(struct usbd_x
if (info->id > 0) {
len--;
memcpy(info->data, xfer->buffer + 1, len);
+ } else {
+ memcpy(info->data, xfer->buffer, len);
}
}
info->callback(info->priv, info->id, info->data, len);
- if (info->id > 0)
- free(xfer->buffer, M_TEMP, xfer->length);
free(info, M_TEMP, sizeof(*info));
usbd_free_xfer(xfer);
}
@@ -763,42 +765,48 @@ int
uhidev_get_report_async(struct uhidev_softc *sc, int type, int id, void *data,
int len, void *priv, void (*callback)(void *, int, void *, int))
{
+ struct usbd_xfer *xfer;
usb_device_request_t req;
struct uhidev_async_info *info;
- char *buf = data;
int actlen = len;
+ void *buf;
+
+ xfer = usbd_alloc_xfer(sc->sc_udev);
+ if (xfer == NULL)
+ return (-1);
+
+ if (id > 0)
+ len++;
+
+ buf = usbd_alloc_buffer(xfer, len);
+ if (buf == NULL) {
+ usbd_free_xfer(xfer);
+ return (-1);
+ }
info = malloc(sizeof(*info), M_TEMP, M_NOWAIT);
- if (info == NULL)
+ if (info == NULL) {
+ usbd_free_xfer(xfer);
return (-1);
+ }
info->callback = callback;
info->priv = priv;
info->data = data;
info->id = id;
- if (id > 0) {
- len++;
- buf = malloc(len, M_TEMP, M_NOWAIT|M_ZERO);
- if (buf == NULL) {
- free(info, M_TEMP, sizeof(*info));
- return (-1);
- }
- }
-
req.bmRequestType = UT_READ_CLASS_INTERFACE;
req.bRequest = UR_GET_REPORT;
USETW2(req.wValue, type, id);
USETW(req.wIndex, sc->sc_ifaceno);
USETW(req.wLength, len);
- if (usbd_do_request_async(sc->sc_udev, &req, buf, priv,
- uhidev_get_report_async_cb)) {
+ if (usbd_request_async(xfer, &req, priv, uhidev_get_report_async_cb)) {
free(info, M_TEMP, sizeof(*info));
- if (id > 0)
- free(buf, M_TEMP, len);
+ usbd_free_xfer(xfer);
actlen = -1;
}
+
return (actlen);
}
Index: usbdi.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.77
diff -u -p -r1.77 usbdi.c
--- usbdi.c 9 Jan 2015 12:15:48 -0000 1.77
+++ usbdi.c 9 Jan 2015 13:22:02 -0000
@@ -55,8 +55,7 @@ extern int usbdebug;
#define DPRINTFN(n,x)
#endif
-void usbd_do_request_async_cb(struct usbd_xfer *, void *,
- usbd_status);
+void usbd_request_async_cb(struct usbd_xfer *, void *, usbd_status);
void usbd_start_next(struct usbd_pipe *pipe);
usbd_status usbd_open_pipe_ival(struct usbd_interface *, u_int8_t, u_int8_t,
struct usbd_pipe **, int);
@@ -586,6 +585,7 @@ usbd_status
usbd_clear_endpoint_stall_async(struct usbd_pipe *pipe)
{
struct usbd_device *dev = pipe->device;
+ struct usbd_xfer *xfer;
usb_device_request_t req;
usbd_status err;
@@ -596,7 +596,12 @@ usbd_clear_endpoint_stall_async(struct u
USETW(req.wValue, UF_ENDPOINT_HALT);
USETW(req.wIndex, pipe->endpoint->edesc->bEndpointAddress);
USETW(req.wLength, 0);
- err = usbd_do_request_async(dev, &req, 0, 0, 0);
+
+ xfer = usbd_alloc_xfer(dev);
+ if (xfer == NULL)
+ return (USBD_NOMEM);
+
+ err = usbd_request_async(xfer, &req, NULL, NULL);
return (err);
}
@@ -949,8 +954,7 @@ usbd_do_request_flags(struct usbd_device
}
void
-usbd_do_request_async_cb(struct usbd_xfer *xfer, void *priv,
- usbd_status status)
+usbd_request_async_cb(struct usbd_xfer *xfer, void *priv, usbd_status status)
{
usbd_free_xfer(xfer);
}
@@ -960,19 +964,17 @@ usbd_do_request_async_cb(struct usbd_xfe
* Can be used from interrupt context.
*/
usbd_status
-usbd_do_request_async(struct usbd_device *dev, usb_device_request_t *req,
- void *data, void *priv, usbd_callback callback)
+usbd_request_async(struct usbd_xfer *xfer, usb_device_request_t *req,
+ void *priv, usbd_callback callback)
{
- struct usbd_xfer *xfer;
usbd_status err;
- xfer = usbd_alloc_xfer(dev);
- if (xfer == NULL)
- return (USBD_NOMEM);
if (callback == NULL)
- callback = usbd_do_request_async_cb;
- usbd_setup_default_xfer(xfer, dev, priv, USBD_DEFAULT_TIMEOUT, req,
- data, UGETW(req->wLength), 0, callback);
+ callback = usbd_request_async_cb;
+
+ usbd_setup_default_xfer(xfer, xfer->device, priv,
+ USBD_DEFAULT_TIMEOUT, req, NULL, UGETW(req->wLength),
+ USBD_NO_COPY, callback);
err = usbd_transfer(xfer);
if (err != USBD_IN_PROGRESS) {
usbd_free_xfer(xfer);
Index: usbdi.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/usbdi.h,v
retrieving revision 1.64
diff -u -p -r1.64 usbdi.h
--- usbdi.h 9 Jan 2015 12:07:50 -0000 1.64
+++ usbdi.h 9 Jan 2015 13:14:25 -0000
@@ -121,8 +121,8 @@ usbd_status usbd_open_pipe_intr(struct u
void *buffer, u_int32_t length, usbd_callback, int);
usbd_status usbd_do_request(struct usbd_device *pipe, usb_device_request_t
*req,
void *data);
-usbd_status usbd_do_request_async(struct usbd_device *pipe,
- usb_device_request_t *req, void *data, void *priv, usbd_callback callback);
+usbd_status usbd_request_async(struct usbd_xfer *xfer,
+ usb_device_request_t *req, void *priv, usbd_callback callback);
usbd_status usbd_do_request_flags(struct usbd_device *pipe,
usb_device_request_t *req, void *data, u_int16_t flags, int*, u_int32_t);
usb_interface_descriptor_t *usbd_get_interface_descriptor(
