From owner-freebsd-current@FreeBSD.ORG Fri Nov 12 21:47:56 2004 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 1EDF016A4CE for ; Fri, 12 Nov 2004 21:47:56 +0000 (GMT) Received: from pooker.samsco.org (pooker.samsco.org [168.103.85.57]) by mx1.FreeBSD.org (Postfix) with ESMTP id A0DF043D2D for ; Fri, 12 Nov 2004 21:47:55 +0000 (GMT) (envelope-from scottl@freebsd.org) Received: from [192.168.254.11] (junior-wifi.samsco.home [192.168.254.11]) (authenticated bits=0) by pooker.samsco.org (8.12.11/8.12.10) with ESMTP id iACLndtq006245; Fri, 12 Nov 2004 14:49:39 -0700 (MST) (envelope-from scottl@freebsd.org) Message-ID: <41952FBD.40602@freebsd.org> Date: Fri, 12 Nov 2004 14:48:45 -0700 From: Scott Long User-Agent: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.7.2) Gecko/20040929 X-Accept-Language: en-us, en MIME-Version: 1.0 To: "M. Warner Losh" References: <20041112.143439.33211003.imp@bsdimp.com> In-Reply-To: <20041112.143439.33211003.imp@bsdimp.com> X-Enigmail-Version: 0.86.1.0 X-Enigmail-Supports: pgp-inline, pgp-mime Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, hits=0.0 required=3.8 tests=none autolearn=no version=2.63 X-Spam-Checker-Version: SpamAssassin 2.63 (2004-01-11) on pooker.samsco.org cc: current@freebsd.org Subject: Re: usb with fast interrupts X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Nov 2004 21:47:56 -0000 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 > > #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"