My USB scanner works again in 6.6 \o/
Thanks to whoever fixed it. I have no idea when and how it happened but
since a few releases back it has always been failed with some I/O error.
But now it just works again as it once did (apart from a weird Xsane
bug where it scans only part of the page which I will ignore for now;
the 'scanimage' command scans as expected).

So I scanned an image and tried to print it.
The default image format produced by scanimage is .pnm, and when I queued
several megabytes of pnm data with lpr my printer started printing page
after page of garbage.
So I turned the printer off and was surprised to see that doing so
made the whole machine hang hard. Nothing even on serial console.

I can reliably reproduce this. With an XHCI_DEBUG kernel I see the
following messages scoll by very fast on the serial console:

xhci_abort_xfer: already done
usbd_abort_pipe: pipe=0xffff800000e84000 xfer=0xfffffd811f8da1e0 
(methods=0xffffffff81efb9f8)
xhci_abort_xfer: xfer=0xfffffd811f8da1e0 status=NORMAL_COMPLETION err=CANCELLED 
actlen=23 len=64 idx=-1
xhci_abort_xfer: already done
usbd_abort_pipe: pipe=0xffff800000e84000 xfer=0xfffffd811f8da1e0 
(methods=0xffffffff81efb9f8)
xhci_abort_xfer: xfer=0xfffffd811f8da1e0 status=NORMAL_COMPLETION err=CANCELLED 
actlen=23 len=64 idx=-1
xhci_abort_xfer: already done
usbd_abort_pipe: pipe=0xffff800000e84000 xfer=0xfffffd811f8da1e0 
(methods=0xffffffff81efb9f8)
xhci_abort_xfer: xfer=0xfffffd811f8da1e0 status=NORMAL_COMPLETION err=CANCELLED 
actlen=23 len=64 idx=-1
xhci_abort_xfer: already done
usbd_abort_pipe: pipe=0xffff800000e84000 xfer=0xfffffd811f8da1e0 
(methods=0xffffffff81efb9f8)
xhci_abort_xfer: xfer=0xfffffd811f8da1e0 status=NORMAL_COMPLETION err=CANCELLED 
actlen=23 len=64 idx=-1
xhci_abort_xfer: already done

Here is the loop we are in at that point, in usbdi.c's usbd_abort_pipe:

        while ((xfer = SIMPLEQ_FIRST(&pipe->queue)) != NULL) {
                DPRINTFN(2,("%s: pipe=%p xfer=%p (methods=%p)\n", __func__,
                    pipe, xfer, pipe->methods));
                /* Make the HC abort it (and invoke the callback). */
                pipe->methods->abort(xfer);
                /* XXX only for non-0 usbd_clear_endpoint_stall(pipe); */
        }


Note how this will keep looping forever if the xfer is never taken off
the queue. I don't understand internal USB stack interface contracts
(does anyone, really?) but assuming that the abort method is always
expected to dequeue the transfer by calling the usb_transfer_complete()
function, then the following patch allows this loop to exit when I turn
off my printer while it is printing.

Debug output looks like this with the patch and the machine keeps
running fine:

xhci0: txerr? code 4
xhci0: txerr? code 4
xhci0: txerr? code 4
usb0: usb_needs_explore
usb_add_task: task=0xffff800000093e80 state=0 type=1
usb_explore: usb0
usbd_abort_pipe: pipe=0xffff800000e80000
usbd_abort_pipe: pipe=0xffff800000ddf000
usbd_abort_pipe: pipe=0xffff800000ddf000 xfer=0xfffffd811f8da0f0 
(methods=0xffffffff81f15fc8)
xhci_abort_xfer: xfer=0xfffffd811f8da0f0 status=NORMAL_COMPLETION err=CANCELLED 
actlen=23 len=64 idx=-1
xhci0: xhci_cmd_stop_ep dev 10 dci 3
xhci0: event error code=19, result=33
trb=0xffff8000220ed7a8 (0x00000000096bd0c0 0x13000000 0xa008400)
xhci0: error stopping endpoint
xhci0: xhci_cmd_configure_ep dev 10
usbd_abort_pipe: pipe=0xffff800000ddf000
xhci0: xhci_cmd_configure_ep dev 10
ulpt0 detached
usbd_abort_pipe: pipe=0xffff800000e81000
xhci0: xhci_cmd_configure_ep dev 10
xhci0: xhci_cmd_slot_control


Is this OK or do we need a diferent fix elsewhere?

diff b8f72638b63831677f92fc0c98e5d2d00058e226 /usr/src (staged changes)
blob - f9d7eb3a4018dd5b628f0d866630f5732cf53593
blob + 39351eccfcb192c909c428b04da93843e4659b8a
--- sys/dev/usb/xhci.c
+++ sys/dev/usb/xhci.c
@@ -2174,7 +2174,8 @@ xhci_abort_xfer(struct usbd_xfer *xfer, usbd_status st
        }
 
        /* Transfer is already done. */
-       if (xfer->status != USBD_IN_PROGRESS) {
+       if (xfer->status != USBD_IN_PROGRESS && !(xfer->status ==
+           USBD_NORMAL_COMPLETION && status == USBD_CANCELLED)) {
                DPRINTF(("%s: already done \n", __func__));
                return;
        }

Reply via email to