So I'd like to remove the following hack from uhci(4) which is the 
only piece of code that justifies another layer of abstraction around
the memory allocation logic for the various USB controllers.

I couldn't find any historical explanation for this code, not even from
the NetBSD source repository. But what this hack does is to allocate
then free some TDs, if the requested size is bigger than some magic
value, to put them in the free list to avoid allocating them in
interrupt context. 

But as the comment says there's no guarantee that these TDs will still
be on the free list when you'll need them.  There's also no guarantee
that the number of TDs that you're pre-allocating will be big enough for
your needs.  Anyway, after some time, the free list should already have
enough TDs so that you won't need to allocate new ones.

I'm running this without problem on amd64 with an "Intel 82801GB USB"
and I'm quite confident this hack can goes away as neither ohci(4) nor
ehci(4) use this kind of TDs pre-allocation when allocating memory for
a transfer.


While here, put some splusb() around the code that modify the pointer to
the first element of the list of free TDs, because it can also be
modified from interrupt context.


ok?

Index: uhci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uhci.c,v
retrieving revision 1.93
diff -u -p -r1.93 uhci.c
--- uhci.c      28 Mar 2013 03:58:03 -0000      1.93
+++ uhci.c      12 Apr 2013 15:11:55 -0000
@@ -628,31 +628,6 @@ usbd_status
 uhci_allocm(struct usbd_bus *bus, usb_dma_t *dma, u_int32_t size)
 {
        struct uhci_softc *sc = (struct uhci_softc *)bus;
-       u_int32_t n;
-
-       /*
-        * XXX
-        * Since we are allocating a buffer we can assume that we will
-        * need TDs for it.  Since we don't want to allocate those from
-        * an interrupt context, we allocate them here and free them again.
-        * This is no guarantee that we'll get the TDs next time...
-        */
-       n = size / 8;
-       if (n > 16) {
-               u_int32_t i;
-               uhci_soft_td_t **stds;
-               DPRINTF(("uhci_allocm: get %d TDs\n", n));
-               stds = malloc(sizeof(uhci_soft_td_t *) * n, M_TEMP,
-                             M_NOWAIT | M_ZERO);
-               if (stds == NULL)
-                       panic("uhci_allocm");
-               for(i=0; i < n; i++)
-                       stds[i] = uhci_alloc_std(sc);
-               for(i=0; i < n; i++)
-                       if (stds[i] != NULL)
-                               uhci_free_std(sc, stds[i]);
-               free(stds, M_TEMP);
-       }
 
        return (usb_allocmem(&sc->sc_bus, size, 0, dma));
 }
@@ -1584,6 +1559,7 @@ uhci_alloc_std(uhci_softc_t *sc)
        usbd_status err;
        int i, offs;
        usb_dma_t dma;
+       int s;
 
        if (sc->sc_freetds == NULL) {
                DPRINTFN(2,("uhci_alloc_std: allocating chunk\n"));
@@ -1591,6 +1567,7 @@ uhci_alloc_std(uhci_softc_t *sc)
                          UHCI_TD_ALIGN, &dma);
                if (err)
                        return (0);
+               s = splusb();
                for(i = 0; i < UHCI_STD_CHUNK; i++) {
                        offs = i * UHCI_STD_SIZE;
                        std = KERNADDR(&dma, offs);
@@ -1598,16 +1575,23 @@ uhci_alloc_std(uhci_softc_t *sc)
                        std->link.std = sc->sc_freetds;
                        sc->sc_freetds = std;
                }
+               splx(s);
        }
+
+       s = splusb();
        std = sc->sc_freetds;
        sc->sc_freetds = std->link.std;
        memset(&std->td, 0, sizeof(uhci_td_t));
-       return std;
+       splx(s);
+
+       return (std);
 }
 
 void
 uhci_free_std(uhci_softc_t *sc, uhci_soft_td_t *std)
 {
+       int s;
+
 #ifdef DIAGNOSTIC
 #define TD_IS_FREE 0x12345678
        if (letoh32(std->td.td_token) == TD_IS_FREE) {
@@ -1616,8 +1600,11 @@ uhci_free_std(uhci_softc_t *sc, uhci_sof
        }
        std->td.td_token = htole32(TD_IS_FREE);
 #endif
+
+       s = splusb();
        std->link.std = sc->sc_freetds;
        sc->sc_freetds = std;
+       splx(s);
 }
 
 uhci_soft_qh_t *

Reply via email to