Date: Sat, 29 Nov 2008 11:36:30 GMT From: Weongyo Jeong <weongyo@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 153753 for review Message-ID: <200811291136.mATBaUpx018469@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=153753 Change 153753 by weongyo@weongyo_ws on 2008/11/29 11:35:57 In most cases of calling USB xfer's callback these're called from `ehci0' (or ohci0..) thread but a callback for aborting USB xfer is called `usbtask-hc' thread. Normally it looks it doesn't make problems unless the network load is high. It seems that accessing a IRP structure from multi-threads can make abnormal behavior so with this patch unify code flows to a NDIS Worker thread. But it looks NDIS USB support still has unknown TCP RX/TX problem because sometimes I saw the driver's watchdog timeout using TCP_STREAM of netperf(1) but it's ok to RX/TX with UDP_STREAM. Weird. Affected files ... .. //depot/projects/ndisusb/sys/compat/ndis/subr_usbd.c#14 edit .. //depot/projects/ndisusb/sys/dev/if_ndis/if_ndis.c#9 edit .. //depot/projects/ndisusb/sys/dev/if_ndis/if_ndisvar.h#7 edit Differences ... ==== //depot/projects/ndisusb/sys/compat/ndis/subr_usbd.c#14 (text+ko) ==== @@ -97,6 +97,9 @@ static int32_t usbd_urb2nt(int32_t); static void usbd_xfereof(usbd_xfer_handle, usbd_private_handle, usbd_status); +static void usbd_xferadd(usbd_xfer_handle, usbd_private_handle, + usbd_status, uint8_t); +static void usbd_xfertask(device_object *, void *); static void dummy(void); static union usbd_urb *USBD_CreateConfigurationRequestEx( @@ -120,6 +123,7 @@ static funcptr usbd_ioinvalid_wrap; static funcptr usbd_irpcancel_wrap; static funcptr usbd_irpcancel_iin_wrap; +static funcptr usbd_xfertask_wrap; int usbd_libinit(void) @@ -143,6 +147,8 @@ (funcptr *)&usbd_irpcancel_wrap, 2, WINDRV_WRAP_STDCALL); windrv_wrap((funcptr)usbd_irpcancel_iin, (funcptr *)&usbd_irpcancel_iin_wrap, 2, WINDRV_WRAP_STDCALL); + windrv_wrap((funcptr)usbd_xfertask, + (funcptr *)&usbd_xfertask_wrap, 2, WINDRV_WRAP_STDCALL); /* Create a fake USB driver instance. */ @@ -174,6 +180,7 @@ windrv_unwrap(usbd_iodispatch_wrap); windrv_unwrap(usbd_irpcancel_wrap); windrv_unwrap(usbd_irpcancel_iin_wrap); + windrv_unwrap(usbd_xfertask_wrap); free(usbd_driver.dro_drivername.us_buf, M_DEVBUF); @@ -885,63 +892,44 @@ } static void -usbd_intr(xfer, priv, status) +usbd_xferadd(xfer, priv, status, freexfer) usbd_xfer_handle xfer; usbd_private_handle priv; usbd_status status; + uint8_t freexfer; { irp *ip = priv; device_t dev = IRP_NDIS_DEV(ip); - struct usbd_urb_bulk_or_intr_transfer *ubi; - union usbd_urb *urb; + struct ndis_softc *sc = device_get_softc(dev); + struct ndisusb_xfer *nx; + uint8_t irql; - if (status != USBD_NORMAL_COMPLETION) { - if (status == USBD_NOT_STARTED) - return; - if (status == USBD_STALLED) - usbd_clear_endpoint_stall_async(xfer->pipe); - if (status != USBD_CANCELLED) { - device_printf(dev, "can't process status (%s)\n", - usbd_errstr(status)); - return; - } + nx = malloc(sizeof(struct ndisusb_xfer), M_USBDEV, M_NOWAIT | M_ZERO); + if (nx == NULL) { + device_printf(dev, "out of memory"); + return; } - - urb = usbd_geturb(ip); + nx->nx_xfer = xfer; + nx->nx_priv = priv; + nx->nx_status = status; + nx->nx_freexfer = freexfer; - KASSERT(urb->uu_hdr.uuh_func == URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER, - ("function(%d) isn't for bulk or interrupt", urb->uu_hdr.uuh_func)); + KeAcquireSpinLock(&sc->ndisusb_xferlock, &irql); + InsertTailList((&sc->ndisusb_xferlist), (&nx->nx_xferlist)); + KeReleaseSpinLock(&sc->ndisusb_xferlock, irql); - IoAcquireCancelSpinLock(&ip->irp_cancelirql); + IoQueueWorkItem(sc->ndisusb_xferitem, + (io_workitem_func)usbd_xfertask_wrap, WORKQUEUE_CRITICAL, sc); +} - ip->irp_cancelfunc = NULL; - IRP_NDISUSB_XFER(ip) = NULL; +static void +usbd_intr(xfer, priv, status) + usbd_xfer_handle xfer; + usbd_private_handle priv; + usbd_status status; +{ - switch (status) { - case USBD_NORMAL_COMPLETION: - ubi = &urb->uu_bulkintr; - ubi->ubi_trans_buflen = xfer->actlen; - if (ubi->ubi_trans_flags & USBD_TRANSFER_DIRECTION_IN) - memcpy(ubi->ubi_trans_buf, xfer->buffer, xfer->actlen); - - ip->irp_iostat.isb_info = xfer->actlen; - ip->irp_iostat.isb_status = STATUS_SUCCESS; - USBD_URB_STATUS(urb) = USBD_STATUS_SUCCESS; - break; - case USBD_CANCELLED: - ip->irp_iostat.isb_info = 0; - ip->irp_iostat.isb_status = STATUS_CANCELLED; - USBD_URB_STATUS(urb) = USBD_STATUS_SUCCESS; - break; - default: - ip->irp_iostat.isb_info = 0; - USBD_URB_STATUS(urb) = usbd_usb2urb(status); - ip->irp_iostat.isb_status = usbd_urb2nt(USBD_URB_STATUS(urb)); - break; - } - - IoReleaseCancelSpinLock(ip->irp_cancelirql); - IoCompleteRequest(ip, IO_NO_INCREMENT); + usbd_xferadd(xfer, priv, status, 0); } static void @@ -950,8 +938,108 @@ usbd_private_handle priv; usbd_status status; { - usbd_intr(xfer, priv, status); - usbd_free_xfer(xfer); + + usbd_xferadd(xfer, priv, status, 1); +} + +static void +usbd_xfertask(dobj, arg) + device_object *dobj; + void *arg; +{ + int error; + irp *ip; + device_t dev; + list_entry *l; + struct ndis_softc *sc = arg; + struct ndisusb_xfer *nx; + struct usbd_urb_bulk_or_intr_transfer *ubi; + uint8_t irql; + union usbd_urb *urb; + usbd_private_handle priv; + usbd_status status; + usbd_xfer_handle xfer; + + dev = sc->ndis_dev; + + if (IsListEmpty(&sc->ndisusb_xferlist)) + return; + + KeAcquireSpinLock(&sc->ndisusb_xferlock, &irql); + l = sc->ndisusb_xferlist.nle_flink; + while (l != &sc->ndisusb_xferlist) { + nx = CONTAINING_RECORD(l, struct ndisusb_xfer, nx_xferlist); + xfer = nx->nx_xfer; + priv = nx->nx_priv; + status = nx->nx_status; + error = 0; + ip = priv; + + if (status != USBD_NORMAL_COMPLETION) { + if (status == USBD_NOT_STARTED) { + error = 1; + goto next; + } + if (status == USBD_STALLED) + usbd_clear_endpoint_stall_async(xfer->pipe); + if (status != USBD_CANCELLED) { + device_printf(dev, "can't process status (%s)\n", + usbd_errstr(status)); + error = 1; + goto next; + } + } + + urb = usbd_geturb(ip); + + KASSERT(urb->uu_hdr.uuh_func == + URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER, + ("function(%d) isn't for bulk or interrupt", + urb->uu_hdr.uuh_func)); + + IoAcquireCancelSpinLock(&ip->irp_cancelirql); + + ip->irp_cancelfunc = NULL; + IRP_NDISUSB_XFER(ip) = NULL; + + switch (status) { + case USBD_NORMAL_COMPLETION: + ubi = &urb->uu_bulkintr; + ubi->ubi_trans_buflen = xfer->actlen; + if (ubi->ubi_trans_flags & USBD_TRANSFER_DIRECTION_IN) + memcpy(ubi->ubi_trans_buf, xfer->buffer, + xfer->actlen); + + ip->irp_iostat.isb_info = xfer->actlen; + ip->irp_iostat.isb_status = STATUS_SUCCESS; + USBD_URB_STATUS(urb) = USBD_STATUS_SUCCESS; + break; + case USBD_CANCELLED: + ip->irp_iostat.isb_info = 0; + ip->irp_iostat.isb_status = STATUS_CANCELLED; + USBD_URB_STATUS(urb) = USBD_STATUS_SUCCESS; + break; + default: + ip->irp_iostat.isb_info = 0; + USBD_URB_STATUS(urb) = usbd_usb2urb(status); + ip->irp_iostat.isb_status = + usbd_urb2nt(USBD_URB_STATUS(urb)); + break; + } + + IoReleaseCancelSpinLock(ip->irp_cancelirql); +next: + l = l->nle_flink; + RemoveEntryList(&nx->nx_xferlist); + if (nx->nx_freexfer) + usbd_free_xfer(nx->nx_xfer); + free(nx, M_USBDEV); + if (error) + continue; + /* NB: call after cleaning */ + IoCompleteRequest(ip, IO_NO_INCREMENT); + } + KeReleaseSpinLock(&sc->ndisusb_xferlock, irql); } static int32_t ==== //depot/projects/ndisusb/sys/dev/if_ndis/if_ndis.c#9 (text+ko) ==== @@ -543,7 +543,9 @@ mtx_init(&sc->ndis_mtx, device_get_nameunit(dev), MTX_NETWORK_LOCK, MTX_DEF); KeInitializeSpinLock(&sc->ndis_rxlock); + KeInitializeSpinLock(&sc->ndisusb_xferlock); InitializeListHead(&sc->ndis_shlist); + InitializeListHead(&sc->ndisusb_xferlist); callout_init(&sc->ndis_stat_callout, CALLOUT_MPSAFE); if (sc->ndis_iftype == PCMCIABus) { @@ -607,6 +609,7 @@ sc->ndis_startitem = IoAllocateWorkItem(sc->ndis_block->nmb_deviceobj); sc->ndis_resetitem = IoAllocateWorkItem(sc->ndis_block->nmb_deviceobj); sc->ndis_inputitem = IoAllocateWorkItem(sc->ndis_block->nmb_deviceobj); + sc->ndisusb_xferitem = IoAllocateWorkItem(sc->ndis_block->nmb_deviceobj); KeInitializeDpc(&sc->ndis_rxdpc, ndis_rxeof_xfr_wrap, sc->ndis_block); /* Call driver's init routine. */ @@ -1043,6 +1046,8 @@ IoFreeWorkItem(sc->ndis_resetitem); if (sc->ndis_inputitem != NULL) IoFreeWorkItem(sc->ndis_inputitem); + if (sc->ndisusb_xferitem != NULL) + IoFreeWorkItem(sc->ndisusb_xferitem); bus_generic_detach(dev); ndis_unload_driver(sc); ==== //depot/projects/ndisusb/sys/dev/if_ndis/if_ndisvar.h#7 (text+ko) ==== @@ -117,6 +117,13 @@ #define NDISUSB_IFACE_INDEX 0 #define NDISUSB_INTR_TIMEOUT 1000 #define NDISUSB_TX_TIMEOUT 10000 +struct ndisusb_xfer { + usbd_xfer_handle nx_xfer; + usbd_private_handle nx_priv; + usbd_status nx_status; + uint8_t nx_freexfer; + list_entry nx_xferlist; +}; struct ndis_softc { struct ifnet *ifp; @@ -195,6 +202,9 @@ int ndis_tx_timer; int ndis_hang_timer; + io_workitem *ndisusb_xferitem; + list_entry ndisusb_xferlist; + kspin_lock ndisusb_xferlock; #define NDISUSB_ENDPT_BOUT 0 #define NDISUSB_ENDPT_BIN 1 #define NDISUSB_ENDPT_IIN 2
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200811291136.mATBaUpx018469>