Re: usb with fast interrupts

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
> _at__at_ -282,7 +282,7 _at__at_
>  		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
> _at__at_ -145,8 +145,9 _at__at_
>  
>  	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
> _at__at_ -281,7 +281,7 _at__at_
>  		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
> _at__at_ -147,7 +147,9 _at__at_
>  
>  	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
> _at__at_ -327,7 +327,7 _at__at_
>  		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
> _at__at_ -193,7 +193,9 _at__at_
>  	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
> _at__at_ -213,6 +213,17 _at__at_
>  	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__)
> _at__at_ -264,6 +275,9 _at__at_
>  	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,
> _at__at_ -277,6 +291,7 _at__at_
>  	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);
> _at__at_ -745,7 +760,7 _at__at_
>  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);
>  }
> _at__at_ -847,6 +862,9 _at__at_
>  	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
> _at__at_ -854,6 +872,7 _at__at_
>  			callout_reset(&bus->softi, 0, bus->methods->soft_intr,
>  			    bus);
>  #endif /* __HAVE_GENERIC_SOFT_INTERRUPTS */
> +#endif
>  	}
>  #else
>         bus->methods->soft_intr(bus);
> _at__at_ -920,6 +939,9 _at__at_
>  	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);
> _at__at_ -927,6 +949,7 _at__at_
>  	}
>  #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
> _at__at_ -346,10 +346,9 _at__at_
>  
>  #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
>  
> _at__at_ -525,7 +524,6 _at__at_
>  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
> _at__at_ -122,10 +122,14 _at__at_
>  #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_at_freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd.org"