PERFORCE change 153753 for review

Weongyo Jeong weongyo at FreeBSD.org
Sat Nov 29 03:36:31 PST 2008


http://perforce.freebsd.org/chv.cgi?CH=153753

Change 153753 by weongyo at 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


More information about the p4-projects mailing list