Hi,

turns out much interrupt work (like ping -f localhost) was a pretty
reliable trigger for the problem I've seen (uhci failes do IOC after
some queued bulk processing).

Hence all kinds of races and interrupt pileup was what I was looking for.
One candidate is the HC which updates various fields in the TD and QH's
during scheduling. Especially qh->element is changed whenever a TD from
this QH is transfered. Having pci-consistent mappings does not protect us
from situtations where the HC changes things concurrently. Ok, "don't
touch active TD's" is a rule to follow but what could protect us when we
assume some state of the system would be not have changed?
I mean all kinds of read-modify-write logic.

Well, admittedly, I don't have complete understood how the HCD works in
this detail. So what I've tried was somewhat a blind man's approach.
However, tentatively not using uhci_append_queued_urb() is a big success
for me - in combination with depth-first linking!.

The problem disappeared, nothing seems to be broken, queued bulk with uhci
is now working for me. I've just transfered several GB in a row, all in
2112 byte queued transfers. Rock solid - withstands even floodpings from
localhost and remote (which saturates eth0 on this box - "too much work in
interrupt"). And oh, btw: for an idle testbox the sustained throughput
averaged over 1 hour was >1.1MB/sec - i.e. same as usb-ohci and about 10%
gain over usb-uhci!

Ok, I'm not really sure, what the reason was and if I would have broken
something else - but I have no such indication so far. So I'd like to ask
everyone with problem using queued bulk on uhci to test the patch and
report, how it behaves.
Besides few cosmetic changes (/proc inconsistencies removed, GFP_DMA not
needed) it doesn't touch anything except the queued-bulk submit-path.

patch vs. 2.4.4 below. should be ok for -ac too.

Comments? Ideas?

Martin

------------------

--- linux-2.4.4/drivers/usb.orig/uhci.c Wed May  2 19:41:41 2001
+++ linux-2.4.4/drivers/usb/uhci.c      Wed May  2 19:53:49 2001
@@ -181,7 +181,9 @@
        dma_addr_t dma_handle;
        struct uhci_td *td;
 
-       td = pci_pool_alloc(uhci->td_pool, GFP_DMA | GFP_ATOMIC, &dma_handle);
+       /* pci_pool uses pci_alloc_consistent() which adds GFP_DMA if required */
+
+       td = pci_pool_alloc(uhci->td_pool, GFP_ATOMIC, &dma_handle);
        if (!td)
                return NULL;
 
@@ -358,7 +360,9 @@
        dma_addr_t dma_handle;
        struct uhci_qh *qh;
 
-       qh = pci_pool_alloc(uhci->qh_pool, GFP_DMA | GFP_ATOMIC, &dma_handle);
+       /* pci_pool uses pci_alloc_consistent() which adds GFP_DMA if required */
+
+       qh = pci_pool_alloc(uhci->qh_pool, GFP_ATOMIC, &dma_handle);
        if (!qh)
                return NULL;
 
@@ -1275,13 +1279,22 @@
        urbp->qh = qh;
        qh->urbp = urbp;
 
-       /* Always assume breadth first */
-       uhci_insert_tds_in_qh(qh, urb, 1);
+       /* queued bulk: depth first - otherwise: assume breadth first */
+
+       if (urb->transfer_flags & USB_QUEUE_BULK)
+               uhci_insert_tds_in_qh(qh, urb, 0);
+       else
+               uhci_insert_tds_in_qh(qh, urb, 1);
+
+#if 0
+       /* doesn't work - but normal insert works for queued bulk too! */
 
        if (urb->transfer_flags & USB_QUEUE_BULK && eurb)
                uhci_append_queued_urb(uhci, eurb, urb);
        else
-               uhci_insert_qh(uhci, uhci->skel_bulk_qh, qh);
+#endif
+
+       uhci_insert_qh(uhci, uhci->skel_bulk_qh, qh);
 
        uhci_inc_fsbr(uhci, urb);
 
@@ -2819,7 +2832,7 @@
        uhci->proc_entry = ent;
 #endif
 
-       request_region(uhci->io_addr, io_size, "usb-uhci");
+       request_region(uhci->io_addr, io_size, "uhci");
 
        reset_hc(uhci);
 
@@ -2827,7 +2840,7 @@
        start_hc(uhci);
 
        retval = -EBUSY;
-       if (request_irq(irq, uhci_interrupt, SA_SHIRQ, "usb-uhci", uhci) == 0) {
+       if (request_irq(irq, uhci_interrupt, SA_SHIRQ, "uhci", uhci) == 0) {
                uhci->irq = irq;
 
                pci_write_config_word(dev, USBLEGSUP, USBLEGSUP_DEFAULT);
@@ -2986,7 +2999,7 @@
 up_failed:
 
 #ifdef CONFIG_PROC_FS
-       remove_proc_entry("uhci", 0);
+       remove_proc_entry("driver/uhci", 0);
 
 proc_failed:
 #endif
@@ -3006,7 +3019,7 @@
                printk(KERN_INFO "uhci: not all urb_priv's were freed\n");
 
 #ifdef CONFIG_PROC_FS
-       remove_proc_entry("uhci", 0);
+       remove_proc_entry("driver/uhci", 0);
 #endif
 
        if (errbuf)



_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
http://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to