Date: Fri, 12 Nov 2004 14:48:45 -0700 From: Scott Long <scottl@freebsd.org> To: "M. Warner Losh" <imp@bsdimp.com> Cc: current@freebsd.org Subject: Re: usb with fast interrupts Message-ID: <41952FBD.40602@freebsd.org> In-Reply-To: <20041112.143439.33211003.imp@bsdimp.com> References: <20041112.143439.33211003.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
M. Warner Losh wrote:
> Our usb system supports soft interrupts, but we currently don't make
> productive use of them. The following makes interrupts fast
> interrupts and uses taskqueues to queue data to a SWI.
>
> Lemme know if it works for you.
>
> Warner
>
Taskqueues aren't good for timing-sensitive operations. Even though USB
may not be terribly sensitive, I bet you'll actually see performance
drops with things like umass with this. Could you instead just put the
real handler into a kthread and wake it up, or use a swi?
Scott
>
> ------------------------------------------------------------------------
>
> diff --exclude CVS -I\$Revision -I\$Id -I\$Header -I \$FreeBSD -ur /shadow/imp/p4/src/sys/dev/usb/ehci_pci.c /shadow/imp/p4/newcard/src/sys/dev/usb/ehci_pci.c
> --- /shadow/imp/p4/src/sys/dev/usb/ehci_pci.c Sat Nov 6 12:30:30 2004
> +++ /shadow/imp/p4/newcard/src/sys/dev/usb/ehci_pci.c Sat Nov 6 12:18:29 2004
> @@ -282,7 +282,7 @@
> sprintf(sc->sc_vendor, "(0x%04x)", pci_get_vendor(self));
> }
>
> - err = bus_setup_intr(self, sc->irq_res, INTR_TYPE_BIO,
> + err = bus_setup_intr(self, sc->irq_res, INTR_TYPE_BIO | INTR_MPSAFE,
> (driver_intr_t *) ehci_intr, sc, &sc->ih);
> if (err) {
> device_printf(self, "Could not setup irq, %d\n", err);
> diff --exclude CVS -I\$Revision -I\$Id -I\$Header -I \$FreeBSD -ur /shadow/imp/p4/src/sys/dev/usb/ehcivar.h /shadow/imp/p4/newcard/src/sys/dev/usb/ehcivar.h
> --- /shadow/imp/p4/src/sys/dev/usb/ehcivar.h Sat Nov 6 12:30:30 2004
> +++ /shadow/imp/p4/newcard/src/sys/dev/usb/ehcivar.h Sat Nov 6 12:18:30 2004
> @@ -145,8 +145,9 @@
>
> usb_callout_t sc_tmo_pcd;
>
> +#if defined(__NetBSD__) || defined(__OpenBSD__)
> device_ptr_t sc_child; /* /dev/usb# device */
> -
> +#endif
> char sc_dying;
> } ehci_softc_t;
>
> diff --exclude CVS -I\$Revision -I\$Id -I\$Header -I \$FreeBSD -ur /shadow/imp/p4/src/sys/dev/usb/ohci_pci.c /shadow/imp/p4/newcard/src/sys/dev/usb/ohci_pci.c
> --- /shadow/imp/p4/src/sys/dev/usb/ohci_pci.c Sat Nov 6 12:30:30 2004
> +++ /shadow/imp/p4/newcard/src/sys/dev/usb/ohci_pci.c Sat Nov 6 12:18:30 2004
> @@ -281,7 +281,7 @@
> sprintf(sc->sc_vendor, "(0x%04x)", pci_get_vendor(self));
> }
>
> - err = bus_setup_intr(self, sc->irq_res, INTR_TYPE_BIO,
> + err = bus_setup_intr(self, sc->irq_res, INTR_TYPE_BIO | INTR_MPSAFE,
> (driver_intr_t *) ohci_intr, sc, &sc->ih);
> if (err) {
> device_printf(self, "Could not setup irq, %d\n", err);
> diff --exclude CVS -I\$Revision -I\$Id -I\$Header -I \$FreeBSD -ur /shadow/imp/p4/src/sys/dev/usb/ohcivar.h /shadow/imp/p4/newcard/src/sys/dev/usb/ohcivar.h
> --- /shadow/imp/p4/src/sys/dev/usb/ohcivar.h Sat Nov 6 12:30:30 2004
> +++ /shadow/imp/p4/newcard/src/sys/dev/usb/ohcivar.h Sat Nov 6 12:18:30 2004
> @@ -147,7 +147,9 @@
>
> usb_callout_t sc_tmo_rhsc;
>
> +#if defined(__NetBSD__) || defined(__OpenBSD__)
> device_ptr_t sc_child;
> +#endif
> char sc_dying;
> } ohci_softc_t;
>
> diff --exclude CVS -I\$Revision -I\$Id -I\$Header -I \$FreeBSD -ur /shadow/imp/p4/src/sys/dev/usb/uhci_pci.c /shadow/imp/p4/newcard/src/sys/dev/usb/uhci_pci.c
> --- /shadow/imp/p4/src/sys/dev/usb/uhci_pci.c Sat Nov 6 12:30:31 2004
> +++ /shadow/imp/p4/newcard/src/sys/dev/usb/uhci_pci.c Sat Nov 6 12:18:30 2004
> @@ -327,7 +327,7 @@
> break;
> }
>
> - err = bus_setup_intr(self, sc->irq_res, INTR_TYPE_BIO,
> + err = bus_setup_intr(self, sc->irq_res, INTR_TYPE_BIO | INTR_MPSAFE,
> (driver_intr_t *) uhci_intr, sc, &sc->ih);
> if (err) {
> device_printf(self, "Could not setup irq, %d\n", err);
> diff --exclude CVS -I\$Revision -I\$Id -I\$Header -I \$FreeBSD -ur /shadow/imp/p4/src/sys/dev/usb/uhcivar.h /shadow/imp/p4/newcard/src/sys/dev/usb/uhcivar.h
> --- /shadow/imp/p4/src/sys/dev/usb/uhcivar.h Sat Nov 6 12:30:31 2004
> +++ /shadow/imp/p4/newcard/src/sys/dev/usb/uhcivar.h Sat Nov 6 12:18:30 2004
> @@ -193,7 +193,9 @@
> void *sc_shutdownhook; /* cookie from shutdown hook */
> #endif
>
> +#if defined(__NetBSD__) || defined(__OpenBSD__)
> device_ptr_t sc_child; /* /dev/usb# device */
> +#endif
> } uhci_softc_t;
>
> usbd_status uhci_init(uhci_softc_t *);
> diff --exclude CVS -I\$Revision -I\$Id -I\$Header -I \$FreeBSD -ur /shadow/imp/p4/src/sys/dev/usb/usb.c /shadow/imp/p4/newcard/src/sys/dev/usb/usb.c
> --- /shadow/imp/p4/src/sys/dev/usb/usb.c Sat Nov 6 12:30:32 2004
> +++ /shadow/imp/p4/newcard/src/sys/dev/usb/usb.c Sat Nov 6 12:18:31 2004
> @@ -213,6 +213,17 @@
> return (UMATCH_GENERIC);
> }
>
> +#if defined(USB_USE_SOFTINTR) && defined(__HAVE_TASKQUEUE)
> +static void
> +usb_taskqueue_fn(void *argp, int pending)
> +{
> + mtx_lock(&Giant);
> + usbd_bus_handle bus = argp;
> + bus->methods->soft_intr(bus);
> + mtx_unlock(&Giant);
> +}
> +#endif
> +
> USB_ATTACH(usb)
> {
> #if defined(__NetBSD__) || defined(__OpenBSD__)
> @@ -264,6 +275,9 @@
> usb_add_event(USB_EVENT_CTRLR_ATTACH, &ue);
>
> #ifdef USB_USE_SOFTINTR
> +#ifdef __HAVE_TASKQUEUE
> + TASK_INIT(&sc->sc_bus->task, 0, usb_taskqueue_fn, sc->sc_bus);
> +#else
> #ifdef __HAVE_GENERIC_SOFT_INTERRUPTS
> /* XXX we should have our own level */
> sc->sc_bus->soft = softintr_establish(IPL_SOFTNET,
> @@ -277,6 +291,7 @@
> usb_callout_init(sc->sc_bus->softi);
> #endif
> #endif
> +#endif
>
> err = usbd_new_device(USBDEV(sc->sc_dev), sc->sc_bus, 0, speed, 0,
> &sc->sc_port);
> @@ -745,7 +760,7 @@
> void
> usb_needs_explore(usbd_device_handle dev)
> {
> - DPRINTFN(2,("usb_needs_explore\n"));
> + printf("usb_needs_explore\n");
> dev->bus->needs_explore = 1;
> wakeup(&dev->bus->needs_explore);
> }
> @@ -847,6 +862,9 @@
> if (bus->use_polling) {
> bus->methods->soft_intr(bus);
> } else {
> +#ifdef __HAVE_TASKQUEUE
> + taskqueue_enqueue(taskqueue_thread, &bus->task);
> +#else
> #ifdef __HAVE_GENERIC_SOFT_INTERRUPTS
> softintr_schedule(bus->soft);
> #else
> @@ -854,6 +872,7 @@
> callout_reset(&bus->softi, 0, bus->methods->soft_intr,
> bus);
> #endif /* __HAVE_GENERIC_SOFT_INTERRUPTS */
> +#endif
> }
> #else
> bus->methods->soft_intr(bus);
> @@ -920,6 +939,9 @@
> usbd_finish();
>
> #ifdef USB_USE_SOFTINTR
> +#ifdef __HAVE_TASKQUEUE
> + /* XXX should disestablish this task item */
> +#else
> #ifdef __HAVE_GENERIC_SOFT_INTERRUPTS
> if (sc->sc_bus->soft != NULL) {
> softintr_disestablish(sc->sc_bus->soft);
> @@ -927,6 +949,7 @@
> }
> #else
> callout_stop(&sc->sc_bus->softi);
> +#endif
> #endif
> #endif
>
> diff --exclude CVS -I\$Revision -I\$Id -I\$Header -I \$FreeBSD -ur /shadow/imp/p4/src/sys/dev/usb/usb_port.h /shadow/imp/p4/newcard/src/sys/dev/usb/usb_port.h
> --- /shadow/imp/p4/src/sys/dev/usb/usb_port.h Tue Nov 9 17:09:39 2004
> +++ /shadow/imp/p4/newcard/src/sys/dev/usb/usb_port.h Tue Nov 9 17:09:31 2004
> @@ -346,10 +346,9 @@
>
> #define USBVERBOSE
>
> -/* We don't use the soft interrupt code in FreeBSD. */
> -#if 0
> #define USB_USE_SOFTINTR
> -#endif
> +#define __HAVE_TASKQUEUE
> +#include <sys/taskqueue.h>
>
> #define Static static
>
> @@ -525,7 +524,6 @@
> SYSCTL_DECL(_hw_usb);
> #endif
>
> -#endif /* __FreeBSD__ */
> +#endif /* FreeBSD */
>
> #endif /* _USB_PORT_H */
> -
> diff --exclude CVS -I\$Revision -I\$Id -I\$Header -I \$FreeBSD -ur /shadow/imp/p4/src/sys/dev/usb/usbdivar.h /shadow/imp/p4/newcard/src/sys/dev/usb/usbdivar.h
> --- /shadow/imp/p4/src/sys/dev/usb/usbdivar.h Sat Nov 6 12:30:32 2004
> +++ /shadow/imp/p4/newcard/src/sys/dev/usb/usbdivar.h Sat Nov 6 12:18:31 2004
> @@ -122,10 +122,14 @@
> #define USBREV_STR { "unknown", "pre 1.0", "1.0", "1.1", "2.0" }
>
> #ifdef USB_USE_SOFTINTR
> +#ifdef __HAVE_TASKQUEUE
> + struct task task;
> +#else
> #ifdef __HAVE_GENERIC_SOFT_INTERRUPTS
> void *soft; /* soft interrupt cookie */
> #else
> struct callout softi;
> +#endif
> #endif
> #endif
>
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> freebsd-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?41952FBD.40602>