Re: [Acx100-devel] Another USB patch

[Denis V. <vd...@il...>]

On Saturday 24 September 2005 17:03, Carlos Martin wrote:
> > start it, and plug/unplug device randomly. May produce
> > interesting fireworks in the logs. 8)
> 
> rmmod'ing it produces quite interesting results already.
> unregister_netdev() happens to call _close() if the iface is up. This
> creates the nice output attached. -32 is the errno for a broken pipe,
> which I think we get because the device is sleeping.

22366569 ==> acxpci_e_cleanup_module
22366587 <== acxpci_e_cleanup_module
usbcore: deregistering driver acx_usb
22366589 ==> acx100usb_i_complete_rx

let the fireworks begin

RETURN RX (2) status=-108 size=0

-ESHUTDOWN

acx: rx error (urb status=-108)
22366589   ==> acx100usb_l_poll_rx
SUBMIT RX (2) inpipe=0xC0008980 size=2358 errcode=-90
22366589   <== acx100usb_l_poll_rx
22366589 <== acx100usb_i_complete_rx

... and here we do the wrong thing calling poll_rx.
I think the fix is:

        switch (urb->status) {
        case 0: /* No error */
                break;
        case -EOVERFLOW:
                printk(KERN_ERR "acx: error in rx, data overrun -> emergency stop\n");
                /* LOCKING BUG! acx100usb_e_close(priv->netdev); */
                goto end_unlock;
        case -ECONNRESET:
                goto do_poll_rx;
+	case -ESHUTDOWN: /* rmmod was called */
+		goto end_unlock;
        default:
                priv->stats.rx_errors++;
                printk("acx: rx error (urb status=%d)\n", urb->status);
                goto do_poll_rx;
        }

BTW, "case -ECONNRESET: goto do_poll_rx" looks wrong also.
ldd3 ch13 says that CONNRESET means that urb was unlinked.
We unlink urbs only on disconnect or close. We shouldn't
then try to re-submit rx urbs via poll_rx if we see CONNRESET.

I guess this is the result:

tx: stop queue on USB disconnect
removing /proc entry driver/acx_wlan0
removing /proc entry driver/acx_wlan0_diag
removing /proc entry driver/acx_wlan0_eeprom
removing /proc entry driver/acx_wlan0_phy
acx_usb: unlink timeout!
acx_usb: unlink timeout!
acx_usb: unlink timeout!
acx_usb: unlink timeout!
acx_usb: unlink timeout!
acx_usb: unlink timeout!
acx_usb: unlink timeout!
acx_usb: unlink timeout!
max lock hold time 100533550 CPU ticks from usb.c:678 to usb.c:695

Then

22366673   ==> acx100usb_e_close
usb.c:1281: sem_down 1 -> 0
22366673     ==> acxusb_s_issue_cmd_timeo_debug
issue_cmd(cmd:ACX1xx_CMD_DISABLE_RX,buflen:0,type:0xFFFFFFFF)
ctrl inpipe=0x80000980 outpipe=0x80000900
sending USB control msg (out) (blocklen=4)
05 00 00 00
wrote 4 bytes
sending USB control msg (in) (acklen=68)
wlan0: issue_cmd(): USB read error -32

This is maybe because disconnect() already sent device to sleep.
We do not need to do DISABLE_RX/TX,SLEEP again in close()!

Thanks!
--
vda