Date: Fri, 14 Aug 2015 12:57:53 +0000 (UTC) From: Hans Petter Selasky <hselasky@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r286773 - in head/sys: boot/kshim dev/usb dev/usb/controller Message-ID: <201508141257.t7ECvrc5037656@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: hselasky Date: Fri Aug 14 12:57:53 2015 New Revision: 286773 URL: https://svnweb.freebsd.org/changeset/base/286773 Log: Improve the realtime properties of USB transfers for embedded systems like RPI-B and RPI-2. Description of problem: USB transfers can process data in their callbacks sometimes causing unacceptable latency for other USB transfers. Separate BULK completion callbacks from CONTROL, INTERRUPT and ISOCHRONOUS callbacks, and give BULK completion callbacks lesser execution priority than the others. This way USB audio won't be interfered by heavy USB ethernet usage for example. Further serve USB transfer completion in a round robin fashion, instead of only serving the most CPU hungry. This has been done by adding a third flag to USB transfer queue structure which keeps track of looping callbacks. The "command" callback function then decides what to do when looping. MFC after: 2 weeks Modified: head/sys/boot/kshim/bsd_kernel.h head/sys/dev/usb/controller/usb_controller.c head/sys/dev/usb/usb_bus.h head/sys/dev/usb/usb_device.c head/sys/dev/usb/usb_hub.c head/sys/dev/usb/usb_process.h head/sys/dev/usb/usb_transfer.c head/sys/dev/usb/usbdi.h Modified: head/sys/boot/kshim/bsd_kernel.h ============================================================================== --- head/sys/boot/kshim/bsd_kernel.h Fri Aug 14 12:23:20 2015 (r286772) +++ head/sys/boot/kshim/bsd_kernel.h Fri Aug 14 12:57:53 2015 (r286773) @@ -43,7 +43,8 @@ #define M_USBDEV 0 #define USB_PROC_MAX 3 #define USB_BUS_GIANT_PROC(bus) (usb_process + 2) -#define USB_BUS_NON_GIANT_PROC(bus) (usb_process + 2) +#define USB_BUS_NON_GIANT_BULK_PROC(bus) (usb_process + 2) +#define USB_BUS_NON_GIANT_ISOC_PROC(bus) (usb_process + 2) #define USB_BUS_EXPLORE_PROC(bus) (usb_process + 0) #define USB_BUS_CONTROL_XFER_PROC(bus) (usb_process + 1) #define SYSCTL_DECL(...) Modified: head/sys/dev/usb/controller/usb_controller.c ============================================================================== --- head/sys/dev/usb/controller/usb_controller.c Fri Aug 14 12:23:20 2015 (r286772) +++ head/sys/dev/usb/controller/usb_controller.c Fri Aug 14 12:57:53 2015 (r286773) @@ -231,7 +231,8 @@ usb_detach(device_t dev) /* Get rid of USB callback processes */ usb_proc_free(USB_BUS_GIANT_PROC(bus)); - usb_proc_free(USB_BUS_NON_GIANT_PROC(bus)); + usb_proc_free(USB_BUS_NON_GIANT_ISOC_PROC(bus)); + usb_proc_free(USB_BUS_NON_GIANT_BULK_PROC(bus)); /* Get rid of USB explore process */ @@ -395,7 +396,8 @@ usb_bus_explore(struct usb_proc_msg *pm) */ usb_proc_rewakeup(USB_BUS_CONTROL_XFER_PROC(bus)); usb_proc_rewakeup(USB_BUS_GIANT_PROC(bus)); - usb_proc_rewakeup(USB_BUS_NON_GIANT_PROC(bus)); + usb_proc_rewakeup(USB_BUS_NON_GIANT_ISOC_PROC(bus)); + usb_proc_rewakeup(USB_BUS_NON_GIANT_BULK_PROC(bus)); #endif USB_BUS_UNLOCK(bus); @@ -860,9 +862,13 @@ usb_attach_sub(device_t dev, struct usb_ &bus->bus_mtx, device_get_nameunit(dev), USB_PRI_MED)) { device_printf(dev, "WARNING: Creation of USB Giant " "callback process failed.\n"); - } else if (usb_proc_create(USB_BUS_NON_GIANT_PROC(bus), + } else if (usb_proc_create(USB_BUS_NON_GIANT_ISOC_PROC(bus), + &bus->bus_mtx, device_get_nameunit(dev), USB_PRI_HIGHEST)) { + device_printf(dev, "WARNING: Creation of USB non-Giant ISOC " + "callback process failed.\n"); + } else if (usb_proc_create(USB_BUS_NON_GIANT_BULK_PROC(bus), &bus->bus_mtx, device_get_nameunit(dev), USB_PRI_HIGH)) { - device_printf(dev, "WARNING: Creation of USB non-Giant " + device_printf(dev, "WARNING: Creation of USB non-Giant BULK " "callback process failed.\n"); } else if (usb_proc_create(USB_BUS_EXPLORE_PROC(bus), &bus->bus_mtx, device_get_nameunit(dev), USB_PRI_MED)) { Modified: head/sys/dev/usb/usb_bus.h ============================================================================== --- head/sys/dev/usb/usb_bus.h Fri Aug 14 12:23:20 2015 (r286772) +++ head/sys/dev/usb/usb_bus.h Fri Aug 14 12:57:53 2015 (r286773) @@ -57,19 +57,26 @@ struct usb_bus { struct root_hold_token *bus_roothold; #endif +/* convenience macros */ +#define USB_BUS_TT_PROC(bus) USB_BUS_NON_GIANT_ISOC_PROC(bus) +#define USB_BUS_CS_PROC(bus) USB_BUS_NON_GIANT_ISOC_PROC(bus) + #if USB_HAVE_PER_BUS_PROCESS #define USB_BUS_GIANT_PROC(bus) (&(bus)->giant_callback_proc) -#define USB_BUS_NON_GIANT_PROC(bus) (&(bus)->non_giant_callback_proc) +#define USB_BUS_NON_GIANT_ISOC_PROC(bus) (&(bus)->non_giant_isoc_callback_proc) +#define USB_BUS_NON_GIANT_BULK_PROC(bus) (&(bus)->non_giant_bulk_callback_proc) #define USB_BUS_EXPLORE_PROC(bus) (&(bus)->explore_proc) #define USB_BUS_CONTROL_XFER_PROC(bus) (&(bus)->control_xfer_proc) - /* - * There are two callback processes. One for Giant locked - * callbacks. One for non-Giant locked callbacks. This should - * avoid congestion and reduce response time in most cases. + * There are three callback processes. One for Giant locked + * callbacks. One for non-Giant locked non-periodic callbacks + * and one for non-Giant locked periodic callbacks. This + * should avoid congestion and reduce response time in most + * cases. */ struct usb_process giant_callback_proc; - struct usb_process non_giant_callback_proc; + struct usb_process non_giant_isoc_callback_proc; + struct usb_process non_giant_bulk_callback_proc; /* Explore process */ struct usb_process explore_proc; Modified: head/sys/dev/usb/usb_device.c ============================================================================== --- head/sys/dev/usb/usb_device.c Fri Aug 14 12:23:20 2015 (r286772) +++ head/sys/dev/usb/usb_device.c Fri Aug 14 12:57:53 2015 (r286773) @@ -2181,7 +2181,7 @@ usb_free_device(struct usb_device *udev, * anywhere: */ USB_BUS_LOCK(udev->bus); - usb_proc_mwait(USB_BUS_NON_GIANT_PROC(udev->bus), + usb_proc_mwait(USB_BUS_CS_PROC(udev->bus), &udev->cs_msg[0], &udev->cs_msg[1]); USB_BUS_UNLOCK(udev->bus); Modified: head/sys/dev/usb/usb_hub.c ============================================================================== --- head/sys/dev/usb/usb_hub.c Fri Aug 14 12:23:20 2015 (r286772) +++ head/sys/dev/usb/usb_hub.c Fri Aug 14 12:57:53 2015 (r286773) @@ -346,7 +346,7 @@ uhub_tt_buffer_reset_async_locked(struct } up->req_reset_tt = req; /* get reset transfer started */ - usb_proc_msignal(USB_BUS_NON_GIANT_PROC(udev->bus), + usb_proc_msignal(USB_BUS_TT_PROC(udev->bus), &hub->tt_msg[0], &hub->tt_msg[1]); } #endif @@ -1579,7 +1579,7 @@ uhub_detach(device_t dev) #if USB_HAVE_TT_SUPPORT /* Make sure our TT messages are not queued anywhere */ USB_BUS_LOCK(bus); - usb_proc_mwait(USB_BUS_NON_GIANT_PROC(bus), + usb_proc_mwait(USB_BUS_TT_PROC(bus), &hub->tt_msg[0], &hub->tt_msg[1]); USB_BUS_UNLOCK(bus); #endif Modified: head/sys/dev/usb/usb_process.h ============================================================================== --- head/sys/dev/usb/usb_process.h Fri Aug 14 12:23:20 2015 (r286772) +++ head/sys/dev/usb/usb_process.h Fri Aug 14 12:57:53 2015 (r286773) @@ -34,6 +34,7 @@ #endif /* defines */ +#define USB_PRI_HIGHEST PI_SWI(SWI_TTY) #define USB_PRI_HIGH PI_SWI(SWI_NET) #define USB_PRI_MED PI_SWI(SWI_CAMBIO) Modified: head/sys/dev/usb/usb_transfer.c ============================================================================== --- head/sys/dev/usb/usb_transfer.c Fri Aug 14 12:23:20 2015 (r286772) +++ head/sys/dev/usb/usb_transfer.c Fri Aug 14 12:57:53 2015 (r286773) @@ -872,6 +872,19 @@ done: } } +static uint8_t +usbd_transfer_setup_has_bulk(const struct usb_config *setup_start, + uint16_t n_setup) +{ + while (n_setup--) { + uint8_t type = setup_start[n_setup].type; + if (type == UE_BULK || type == UE_BULK_INTR || + type == UE_TYPE_ANY) + return (1); + } + return (0); +} + /*------------------------------------------------------------------------* * usbd_transfer_setup - setup an array of USB transfers * @@ -1013,9 +1026,12 @@ usbd_transfer_setup(struct usb_device *u else if (xfer_mtx == &Giant) info->done_p = USB_BUS_GIANT_PROC(udev->bus); + else if (usbd_transfer_setup_has_bulk(setup_start, n_setup)) + info->done_p = + USB_BUS_NON_GIANT_BULK_PROC(udev->bus); else info->done_p = - USB_BUS_NON_GIANT_PROC(udev->bus); + USB_BUS_NON_GIANT_ISOC_PROC(udev->bus); } /* reset sizes */ @@ -2280,10 +2296,8 @@ usbd_callback_ss_done_defer(struct usb_x * will have a Lock Order Reversal, LOR, if we try to * proceed ! */ - if (usb_proc_msignal(info->done_p, - &info->done_m[0], &info->done_m[1])) { - /* ignore */ - } + (void) usb_proc_msignal(info->done_p, + &info->done_m[0], &info->done_m[1]); } else { /* clear second recurse flag */ pq->recurse_2 = 0; @@ -2307,23 +2321,26 @@ usbd_callback_wrapper(struct usb_xfer_qu struct usb_xfer_root *info = xfer->xroot; USB_BUS_LOCK_ASSERT(info->bus, MA_OWNED); - if (!mtx_owned(info->xfer_mtx) && !SCHEDULER_STOPPED()) { + if ((pq->recurse_3 != 0 || mtx_owned(info->xfer_mtx) == 0) && + SCHEDULER_STOPPED() == 0) { /* * Cases that end up here: * * 5) HW interrupt done callback or other source. + * 6) HW completed transfer during callback */ - DPRINTFN(3, "case 5\n"); + DPRINTFN(3, "case 5 and 6\n"); /* * We have to postpone the callback due to the fact we * will have a Lock Order Reversal, LOR, if we try to - * proceed ! + * proceed! + * + * Postponing the callback also ensures that other USB + * transfer queues get a chance. */ - if (usb_proc_msignal(info->done_p, - &info->done_m[0], &info->done_m[1])) { - /* ignore */ - } + (void) usb_proc_msignal(info->done_p, + &info->done_m[0], &info->done_m[1]); return; } /* @@ -2694,7 +2711,7 @@ usbd_pipe_start(struct usb_xfer_queue *p } else if (udev->ctrl_xfer[1]) { info = udev->ctrl_xfer[1]->xroot; usb_proc_msignal( - USB_BUS_NON_GIANT_PROC(info->bus), + USB_BUS_CS_PROC(info->bus), &udev->cs_msg[0], &udev->cs_msg[1]); } else { /* should not happen */ @@ -3019,9 +3036,11 @@ usb_command_wrapper(struct usb_xfer_queu if (!pq->recurse_1) { - do { + /* clear third recurse flag */ + pq->recurse_3 = 0; - /* set both recurse flags */ + do { + /* set two first recurse flags */ pq->recurse_1 = 1; pq->recurse_2 = 1; @@ -3040,6 +3059,12 @@ usb_command_wrapper(struct usb_xfer_queu (pq->command) (pq); DPRINTFN(6, "cb %p (leave)\n", pq->curr); + /* + * Set third recurse flag to indicate + * recursion happened: + */ + pq->recurse_3 = 1; + } while (!pq->recurse_2); /* clear first recurse flag */ @@ -3315,7 +3340,8 @@ usbd_transfer_poll(struct usb_xfer **ppx USB_BUS_CONTROL_XFER_PROC(udev->bus)->up_msleep = 0; USB_BUS_EXPLORE_PROC(udev->bus)->up_msleep = 0; USB_BUS_GIANT_PROC(udev->bus)->up_msleep = 0; - USB_BUS_NON_GIANT_PROC(udev->bus)->up_msleep = 0; + USB_BUS_NON_GIANT_ISOC_PROC(udev->bus)->up_msleep = 0; + USB_BUS_NON_GIANT_BULK_PROC(udev->bus)->up_msleep = 0; /* poll USB hardware */ (udev->bus->methods->xfer_poll) (udev->bus); Modified: head/sys/dev/usb/usbdi.h ============================================================================== --- head/sys/dev/usb/usbdi.h Fri Aug 14 12:23:20 2015 (r286772) +++ head/sys/dev/usb/usbdi.h Fri Aug 14 12:57:53 2015 (r286773) @@ -128,6 +128,8 @@ struct usb_xfer_queue { void (*command) (struct usb_xfer_queue *pq); uint8_t recurse_1:1; uint8_t recurse_2:1; + uint8_t recurse_3:1; + uint8_t reserved:5; }; /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201508141257.t7ECvrc5037656>