Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 26 Jan 2009 20:59:41 +0000 (UTC)
From:      Maksim Yevmenkin <emax@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r187741 - head/sys/dev/usb2/bluetooth
Message-ID:  <200901262059.n0QKxfl8062890@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: emax
Date: Mon Jan 26 20:59:41 2009
New Revision: 187741
URL: http://svn.freebsd.org/changeset/base/187741

Log:
  Clean up ng_ubt2. Get rid of excessive use of NG_NODE_REF/UNREF().
  Make detach() completely synchronous. Properly handle stalled USB
  transfers (use internal mechanism instead of submitting own control
  transfers). Rename/remove a couple of variables and update comments.
  This work was done in close collaboration with HPS.
  
  Reviewed by:	HPS

Modified:
  head/sys/dev/usb2/bluetooth/ng_ubt2.c
  head/sys/dev/usb2/bluetooth/ng_ubt2_var.h

Modified: head/sys/dev/usb2/bluetooth/ng_ubt2.c
==============================================================================
--- head/sys/dev/usb2/bluetooth/ng_ubt2.c	Mon Jan 26 20:58:05 2009	(r187740)
+++ head/sys/dev/usb2/bluetooth/ng_ubt2.c	Mon Jan 26 20:59:41 2009	(r187741)
@@ -33,29 +33,25 @@
 
 /*
  * NOTE: ng_ubt2 driver has a split personality. On one side it is
- * a USB2 device driver and on the other it is a Netgraph node. This
+ * a USB device driver and on the other it is a Netgraph node. This
  * driver will *NOT* create traditional /dev/ enties, only Netgraph 
  * node.
  *
- * NOTE ON LOCKS USED: ng_ubt2 drives uses 3 locks (mutexes)
+ * NOTE ON LOCKS USED: ng_ubt2 drives uses 2 locks (mutexes)
  *
- * 1) sc_if_mtx[0] - lock for device's interface #0. This lock is used
- *    by USB2 for any USB request going over device's interface #0, i.e.
- *    interrupt, control and bulk transfers.
+ * 1) sc_if_mtx - lock for device's interface #0 and #1. This lock is used
+ *    by USB for any USB request going over device's interface #0 and #1,
+ *    i.e. interrupt, control, bulk and isoc. transfers.
  * 
- * 2) sc_if_mtx[1] - lock for device's interface #1. This lock is used
- *    by USB2 for any USB request going over device's interface #1, i.e
- *    isoc. transfers.
- * 
- * 3) sc_mbufq_mtx - lock for mbufq and task flags. This lock is used
- *    to protect device's outgoing mbuf queues and task flags. This lock
- *    *SHOULD NOT* be grabbed for a long time. In fact, think of it as a 
- *    spin lock.
+ * 2) sc_ng_mtx - this lock is used to protect shared (between USB, Netgraph
+ *    and Taskqueue) data, such as outgoing mbuf queues, task flags and hook
+ *    pointer. This lock *SHOULD NOT* be grabbed for a long time. In fact,
+ *    think of it as a spin lock.
  *
  * NOTE ON LOCKING STRATEGY: ng_ubt2 driver operates in 3 different contexts.
  *
  * 1) USB context. This is where all the USB related stuff happens. All
- *    callbacks run in this context. All callbacks are called (by USB2) with
+ *    callbacks run in this context. All callbacks are called (by USB) with
  *    appropriate interface lock held. It is (generally) allowed to grab
  *    any additional locks.
  *
@@ -63,35 +59,37 @@
  *    Since we mark node as WRITER, the Netgraph node will be "locked" (from
  *    Netgraph point of view). Any variable that is only modified from the
  *    Netgraph context does not require any additonal locking. It is generally
- *    *NOT* allowed to grab *ANY* additional lock. Whatever you do, *DO NOT*
- *    grab any long-sleep lock in the Netgraph context. In fact, the only
- *    lock that is allowed in the Netgraph context is the sc_mbufq_mtx lock.
+ *    *NOT* allowed to grab *ANY* additional locks. Whatever you do, *DO NOT*
+ *    grab any lock in the Netgraph context that could cause de-scheduling of
+ *    the Netgraph thread for significant amount of time. In fact, the only
+ *    lock that is allowed in the Netgraph context is the sc_ng_mtx lock.
+ *    Also make sure that any code that is called from the Netgraph context
+ *    follows the rule above.
  *
- * 3) Taskqueue context. This is where ubt_task runs. Since we are NOT allowed
- *    to grab any locks in the Netgraph context, and, USB2 requires us to
- *    grab interface lock before doing things with transfers, we need to
- *    transition from the Netgraph context to the Taskqueue context before
- *    we can call into USB2 subsystem.
+ * 3) Taskqueue context. This is where ubt_task runs. Since we are generally
+ *    NOT allowed to grab any lock that could cause de-scheduling in the
+ *    Netgraph context, and, USB requires us to grab interface lock before
+ *    doing things with transfers, it is safer to transition from the Netgraph
+ *    context to the Taskqueue context before we can call into USB subsystem.
  *
  * So, to put everything together, the rules are as follows.
  *	It is OK to call from the USB context or the Taskqueue context into
  * the Netgraph context (i.e. call NG_SEND_xxx functions). In other words
  * it is allowed to call into the Netgraph context with locks held.
  *	Is it *NOT* OK to call from the Netgraph context into the USB context,
- * because USB2 requires us to grab interface locks and we can not do that.
- * To avoid this, we set task flags to indicate which actions we want to
- * perform and schedule ubt_task which would run in the Taskqueue context.
+ * because USB requires us to grab interface locks, and, it is safer to
+ * avoid it. So, to make things safer we set task flags to indicate which
+ * actions we want to perform and schedule ubt_task which would run in the
+ * Taskqueue context.
  *	Is is OK to call from the Taskqueue context into the USB context,
  * and, ubt_task does just that (i.e. grabs appropriate interface locks
- * before calling into USB2).
- *	Access to the outgoing queues and task flags is controlled by the
- * sc_mbufq_mtx lock. It is an unavoidable evil. Again, sc_mbufq_mtx should
- * really be a spin lock.
- *	All USB callbacks accept Netgraph node pointer as private data. To
- * ensure that Netgraph node pointer remains valid for the duration of the
- * transfer, we grab a referrence to the node. In other words, if transfer is
- * pending, then we should have a referrence on the node. NG_NODE_[NOT_]VALID
- * macro is used to check if node is still present and pointer is valid.
+ * before calling into USB).
+ *	Access to the outgoing queues, task flags and hook pointer is
+ * controlled by the sc_ng_mtx lock. It is an unavoidable evil. Again,
+ * sc_ng_mtx should really be a spin lock (and it is very likely to an
+ * equivalent of spin lock due to adaptive nature of freebsd mutexes).
+ *	All USB callbacks accept softc pointer as a private data. USB ensures
+ * that this pointer is valid.
  */
 
 #include <dev/usb2/include/usb2_devid.h>
@@ -128,9 +126,10 @@ static device_probe_t	ubt_probe;
 static device_attach_t	ubt_attach;
 static device_detach_t	ubt_detach;
 
-static int		ubt_task_schedule(ubt_softc_p, int);
+static void		ubt_task_schedule(ubt_softc_p, int);
 static task_fn_t	ubt_task;
-static void		ubt_xfer_start(ubt_softc_p, int);
+
+#define	ubt_xfer_start(sc, i)	usb2_transfer_start((sc)->sc_xfer[(i)])
 
 /* Netgraph methods */
 static ng_constructor_t	ng_ubt_constructor;
@@ -243,15 +242,13 @@ static struct ng_type	typestruct =
 /* USB methods */
 static usb2_callback_t	ubt_ctrl_write_callback;
 static usb2_callback_t	ubt_intr_read_callback;
-static usb2_callback_t	ubt_intr_read_clear_stall_callback;
 static usb2_callback_t	ubt_bulk_read_callback;
-static usb2_callback_t	ubt_bulk_read_clear_stall_callback;
 static usb2_callback_t	ubt_bulk_write_callback;
-static usb2_callback_t	ubt_bulk_write_clear_stall_callback;
 static usb2_callback_t	ubt_isoc_read_callback;
 static usb2_callback_t	ubt_isoc_write_callback;
 
-static int	ubt_isoc_read_one_frame(struct usb2_xfer *, int);
+static int		ubt_fwd_mbuf_up(ubt_softc_p, struct mbuf **);
+static int		ubt_isoc_read_one_frame(struct usb2_xfer *, int);
 
 /*
  * USB config
@@ -279,8 +276,9 @@ static const struct usb2_config		ubt_con
 		.type =		UE_BULK,
 		.endpoint =	UE_ADDR_ANY,
 		.direction =	UE_DIR_OUT,
+		.if_index = 	0,
 		.mh.bufsize =	UBT_BULK_WRITE_BUFFER_SIZE,
-		.mh.flags =	{ .pipe_bof = 1, },
+		.mh.flags =	{ .pipe_bof = 1, .force_short_xfer = 1, },
 		.mh.callback =	&ubt_bulk_write_callback,
 	},
 	/* Incoming bulk transfer - ACL packets */
@@ -288,6 +286,7 @@ static const struct usb2_config		ubt_con
 		.type =		UE_BULK,
 		.endpoint =	UE_ADDR_ANY,
 		.direction =	UE_DIR_IN,
+		.if_index = 	0,
 		.mh.bufsize =	UBT_BULK_READ_BUFFER_SIZE,
 		.mh.flags =	{ .pipe_bof = 1, .short_xfer_ok = 1, },
 		.mh.callback =	&ubt_bulk_read_callback,
@@ -297,6 +296,7 @@ static const struct usb2_config		ubt_con
 		.type =		UE_INTERRUPT,
 		.endpoint =	UE_ADDR_ANY,
 		.direction =	UE_DIR_IN,
+		.if_index = 	0,
 		.mh.flags =	{ .pipe_bof = 1, .short_xfer_ok = 1, },
 		.mh.bufsize =	UBT_INTR_BUFFER_SIZE,
 		.mh.callback =	&ubt_intr_read_callback,
@@ -306,43 +306,11 @@ static const struct usb2_config		ubt_con
 		.type =		UE_CONTROL,
 		.endpoint =	0x00,	/* control pipe */
 		.direction =	UE_DIR_ANY,
+		.if_index = 	0,
 		.mh.bufsize =	UBT_CTRL_BUFFER_SIZE,
 		.mh.callback =	&ubt_ctrl_write_callback,
 		.mh.timeout =	5000,	/* 5 seconds */
 	},
-	/* Outgoing control transfer to clear stall on outgoing bulk transfer */
-	[UBT_IF_0_BULK_CS_WR] = {
-		.type =		UE_CONTROL,
-		.endpoint =	0x00,	/* control pipe */
-		.direction =	UE_DIR_ANY,
-		.mh.bufsize =	sizeof(struct usb2_device_request),
-		.mh.callback =	&ubt_bulk_write_clear_stall_callback,
-		.mh.timeout =	1000,	/* 1 second */
-		.mh.interval =	50,	/* 50ms */
-	},
-	/* Outgoing control transfer to clear stall on incoming bulk transfer */
-	[UBT_IF_0_BULK_CS_RD] = {
-		.type =		UE_CONTROL,
-		.endpoint =	0x00,	/* control pipe */
-		.direction =	UE_DIR_ANY,
-		.mh.bufsize =	sizeof(struct usb2_device_request),
-		.mh.callback =	&ubt_bulk_read_clear_stall_callback,
-		.mh.timeout =	1000,	/* 1 second */
-		.mh.interval =	50,	/* 50ms */
-	},
-	/*
-	 * Outgoing control transfer to clear stall on incoming
-	 * interrupt transfer
-	 */
-	[UBT_IF_0_INTR_CS_RD] = {
-		.type =		UE_CONTROL,
-		.endpoint =	0x00,	/* control pipe */
-		.direction =	UE_DIR_ANY,
-		.mh.bufsize =	sizeof(struct usb2_device_request),
-		.mh.callback =	&ubt_intr_read_clear_stall_callback,
-		.mh.timeout =	1000,	/* 1 second */
-		.mh.interval =	50,	/* 50ms */
-	},
 
 	/*
 	 * Interface #1
@@ -353,6 +321,7 @@ static const struct usb2_config		ubt_con
 		.type =		UE_ISOCHRONOUS,
 		.endpoint =	UE_ADDR_ANY,
 		.direction =	UE_DIR_IN,
+		.if_index = 	1,
 		.mh.bufsize =	0,	/* use "wMaxPacketSize * frames" */
 		.mh.frames =	UBT_ISOC_NFRAMES,
 		.mh.flags =	{ .short_xfer_ok = 1, },
@@ -363,6 +332,7 @@ static const struct usb2_config		ubt_con
 		.type =		UE_ISOCHRONOUS,
 		.endpoint =	UE_ADDR_ANY,
 		.direction =	UE_DIR_IN,
+		.if_index = 	1,
 		.mh.bufsize =	0,	/* use "wMaxPacketSize * frames" */
 		.mh.frames =	UBT_ISOC_NFRAMES,
 		.mh.flags =	{ .short_xfer_ok = 1, },
@@ -373,6 +343,7 @@ static const struct usb2_config		ubt_con
 		.type =		UE_ISOCHRONOUS,
 		.endpoint =	UE_ADDR_ANY,
 		.direction =	UE_DIR_OUT,
+		.if_index = 	1,
 		.mh.bufsize =	0,	/* use "wMaxPacketSize * frames" */
 		.mh.frames =	UBT_ISOC_NFRAMES,
 		.mh.flags =	{ .short_xfer_ok = 1, },
@@ -383,6 +354,7 @@ static const struct usb2_config		ubt_con
 		.type =		UE_ISOCHRONOUS,
 		.endpoint =	UE_ADDR_ANY,
 		.direction =	UE_DIR_OUT,
+		.if_index = 	1,
 		.mh.bufsize =	0,	/* use "wMaxPacketSize * frames" */
 		.mh.frames =	UBT_ISOC_NFRAMES,
 		.mh.flags =	{ .short_xfer_ok = 1, },
@@ -399,13 +371,16 @@ static const struct usb2_config		ubt_con
  *
  * where VENDOR_ID and PRODUCT_ID are hex numbers.
  */
-static const struct usb2_device_id ubt_ignore_devs[] = {
+
+static const struct usb2_device_id ubt_ignore_devs[] = 
+{
 	/* AVM USB Bluetooth-Adapter BlueFritz! v1.0 */
 	{ USB_VPI(USB_VENDOR_AVM, 0x2200, 0) },
 };
 
 /* List of supported bluetooth devices */
-static const struct usb2_device_id ubt_devs[] = {
+static const struct usb2_device_id ubt_devs[] =
+{
 	/* Generic Bluetooth class devices */
 	{ USB_IFACE_CLASS(UDCLASS_WIRELESS),
 	  USB_IFACE_SUBCLASS(UDSUBCLASS_RF),
@@ -450,27 +425,26 @@ ubt_attach(device_t dev)
 	struct ubt_softc		*sc = device_get_softc(dev);
 	struct usb2_endpoint_descriptor	*ed;
 	uint16_t			wMaxPacketSize;
-	uint8_t				alt_index, iface_index, i, j;
+	uint8_t				alt_index, i, j;
+	uint8_t				iface_index[2] = { 0, 1 };
 
 	device_set_usb2_desc(dev);
 
-	snprintf(sc->sc_name, sizeof(sc->sc_name),
-		"%s", device_get_nameunit(dev));
+	sc->sc_dev = dev;
+	sc->sc_debug = NG_UBT_WARN_LEVEL;
 
 	/* 
 	 * Create Netgraph node
 	 */
 
-	sc->sc_hook = NULL;
-
 	if (ng_make_node_common(&typestruct, &sc->sc_node) != 0) {
-		device_printf(dev, "could not create Netgraph node\n");
+		UBT_ALERT(sc, "could not create Netgraph node\n");
 		return (ENXIO);
 	}
 
 	/* Name Netgraph node */
-	if (ng_name_node(sc->sc_node, sc->sc_name) != 0) {
-		device_printf(dev, "could not name Netgraph node\n");
+	if (ng_name_node(sc->sc_node, device_get_nameunit(dev)) != 0) {
+		UBT_ALERT(sc, "could not name Netgraph node\n");
 		NG_NODE_UNREF(sc->sc_node);
 		return (ENXIO);
 	}
@@ -481,15 +455,9 @@ ubt_attach(device_t dev)
 	 * Initialize device softc structure
 	 */
 
-	/* state */
-	sc->sc_debug = NG_UBT_WARN_LEVEL;
-	sc->sc_flags = 0;
-	UBT_STAT_RESET(sc);
-
 	/* initialize locks */
-	mtx_init(&sc->sc_mbufq_mtx, "ubt mbufq", NULL, MTX_DEF);
-	mtx_init(&sc->sc_if_mtx[0], "ubt if0", NULL, MTX_DEF | MTX_RECURSE);
-	mtx_init(&sc->sc_if_mtx[1], "ubt if1", NULL, MTX_DEF | MTX_RECURSE);
+	mtx_init(&sc->sc_ng_mtx, "ubt ng", NULL, MTX_DEF);
+	mtx_init(&sc->sc_if_mtx, "ubt if", NULL, MTX_DEF | MTX_RECURSE);
 
 	/* initialize packet queues */
 	NG_BT_MBUFQ_INIT(&sc->sc_cmdq, UBT_DEFAULT_QLEN);
@@ -497,15 +465,12 @@ ubt_attach(device_t dev)
 	NG_BT_MBUFQ_INIT(&sc->sc_scoq, UBT_DEFAULT_QLEN);
 
 	/* initialize glue task */
-	sc->sc_task_flags = 0;
-	TASK_INIT(&sc->sc_task, 0, ubt_task, sc->sc_node);
+	TASK_INIT(&sc->sc_task, 0, ubt_task, sc);
 
 	/*
 	 * Configure Bluetooth USB device. Discover all required USB
 	 * interfaces and endpoints.
 	 *
-	 * Device is expected to be a high-speed device.
-	 *
 	 * USB device must present two interfaces:
 	 * 1) Interface 0 that has 3 endpoints
 	 *	1) Interrupt endpoint to receive HCI events
@@ -520,25 +485,9 @@ ubt_attach(device_t dev)
 	 * configurations with different packet size.
 	 */
 
-	bzero(&sc->sc_xfer, sizeof(sc->sc_xfer));
-
 	/*
-	 * Interface 0
-	 */
-
-	iface_index = 0;
-	if (usb2_transfer_setup(uaa->device, &iface_index, sc->sc_xfer,
-			ubt_config, UBT_IF_0_N_TRANSFER,
-			sc->sc_node, &sc->sc_if_mtx[0])) {
-		device_printf(dev, "could not allocate transfers for " \
-			"interface 0!\n");
-		goto detach;
-	}
-
-	/*
-	 * Interface 1
-	 * (search alternate settings, and find the descriptor with the largest
-	 *  wMaxPacketSize)
+	 * For interface #1 search alternate settings, and find
+	 * the descriptor with the largest wMaxPacketSize
 	 */
 
 	wMaxPacketSize = 0;
@@ -572,21 +521,18 @@ ubt_attach(device_t dev)
 		j ++;
 	}
 
-	/* Set alt configuration only if we found it */
+	/* Set alt configuration on interface #1 only if we found it */
 	if (wMaxPacketSize > 0 &&
 	    usb2_set_alt_interface_index(uaa->device, 1, alt_index)) {
-		device_printf(dev, "could not set alternate setting %d " \
+		UBT_ALERT(sc, "could not set alternate setting %d " \
 			"for interface 1!\n", alt_index);
 		goto detach;
 	}
 
-	iface_index = 1;
-	if (usb2_transfer_setup(uaa->device, &iface_index,
-			&sc->sc_xfer[UBT_IF_0_N_TRANSFER],
-			&ubt_config[UBT_IF_0_N_TRANSFER], UBT_IF_1_N_TRANSFER,
-			sc->sc_node, &sc->sc_if_mtx[1])) {
-		device_printf(dev, "could not allocate transfers for " \
-			"interface 1!\n");
+	/* Setup transfers for both interfaces */
+	if (usb2_transfer_setup(uaa->device, iface_index, sc->sc_xfer,
+			ubt_config, UBT_N_TRANSFER, sc, &sc->sc_if_mtx)) {
+		UBT_ALERT(sc, "could not allocate transfers\n");
 		goto detach;
 	}
 
@@ -616,29 +562,25 @@ ubt_detach(device_t dev)
 	/* Destroy Netgraph node */
 	if (node != NULL) {
 		sc->sc_node = NULL;
-
-		NG_NODE_SET_PRIVATE(node, NULL);
 		NG_NODE_REALLY_DIE(node);
-		NG_NODE_REF(node);
 		ng_rmnode_self(node);
 	}
 
+	/* Make sure ubt_task in gone */
+	taskqueue_drain(taskqueue_swi, &sc->sc_task);
+
 	/* Free USB transfers, if any */
 	usb2_transfer_unsetup(sc->sc_xfer, UBT_N_TRANSFER);
 
-	if (node != NULL)
-		NG_NODE_UNREF(node);
-
 	/* Destroy queues */
-	UBT_MBUFQ_LOCK(sc);
+	UBT_NG_LOCK(sc);
 	NG_BT_MBUFQ_DESTROY(&sc->sc_cmdq);
 	NG_BT_MBUFQ_DESTROY(&sc->sc_aclq);
 	NG_BT_MBUFQ_DESTROY(&sc->sc_scoq);
-	UBT_MBUFQ_UNLOCK(sc);
+	UBT_NG_UNLOCK(sc);
 
-	mtx_destroy(&sc->sc_if_mtx[0]);
-	mtx_destroy(&sc->sc_if_mtx[1]);
-	mtx_destroy(&sc->sc_mbufq_mtx);
+	mtx_destroy(&sc->sc_if_mtx);
+	mtx_destroy(&sc->sc_ng_mtx);
 
 	return (0);
 } /* ubt_detach */
@@ -652,42 +594,27 @@ ubt_detach(device_t dev)
 static void
 ubt_ctrl_write_callback(struct usb2_xfer *xfer)
 {
-	node_p				node = xfer->priv_sc;
-	struct ubt_softc		*sc;
+	struct ubt_softc		*sc = xfer->priv_sc;
 	struct usb2_device_request	req;
 	struct mbuf			*m;
 
-	if (NG_NODE_NOT_VALID(node)) {
-		NG_NODE_UNREF(node);
-		return; /* netgraph node is gone */
-	}
-
-	sc = NG_NODE_PRIVATE(node);
-
 	switch (USB_GET_STATE(xfer)) {
 	case USB_ST_TRANSFERRED:
-		if (xfer->error != 0)
-			UBT_STAT_OERROR(sc);
-		else {
-			UBT_INFO(sc, "sent %d bytes to control pipe\n",
-				xfer->actlen);
-
-			UBT_STAT_BYTES_SENT(sc, xfer->actlen);
-			UBT_STAT_PCKTS_SENT(sc);
-		}
+		UBT_INFO(sc, "sent %d bytes to control pipe\n", xfer->actlen);
+		UBT_STAT_BYTES_SENT(sc, xfer->actlen);
+		UBT_STAT_PCKTS_SENT(sc);
 		/* FALLTHROUGH */
 
 	case USB_ST_SETUP:
 send_next:
 		/* Get next command mbuf, if any */
-		UBT_MBUFQ_LOCK(sc);
+		UBT_NG_LOCK(sc);
 		NG_BT_MBUFQ_DEQUEUE(&sc->sc_cmdq, m);
-		UBT_MBUFQ_UNLOCK(sc);
+		UBT_NG_UNLOCK(sc);
 
 		if (m == NULL) {
 			UBT_INFO(sc, "HCI command queue is empty\n");
-			NG_NODE_UNREF(node);
-			return;
+			break;	/* transfer complete */
 		}
 
 		/* Initialize a USB control request and then schedule it */
@@ -720,7 +647,7 @@ send_next:
 			goto send_next;
 		}
 
-		NG_NODE_UNREF(node); /* cancelled */
+		/* transfer cancelled */
 		break;
 	}
 } /* ubt_ctrl_write_callback */
@@ -734,24 +661,9 @@ send_next:
 static void
 ubt_intr_read_callback(struct usb2_xfer *xfer)
 {
-	node_p			node = xfer->priv_sc;
-	struct ubt_softc	*sc;
+	struct ubt_softc	*sc = xfer->priv_sc;
 	struct mbuf		*m;
 	ng_hci_event_pkt_t	*hdr;
-	int			error;
-
-	if (NG_NODE_NOT_VALID(node)) {
-		NG_NODE_UNREF(node);
-		return; /* netgraph node is gone */
-	}
-
-	sc = NG_NODE_PRIVATE(node);
-
-	if ((sc->sc_hook == NULL) || NG_HOOK_NOT_VALID(sc->sc_hook)) {
-		UBT_INFO(sc, "no upstream hook\n");
-		NG_NODE_UNREF(node);
-		return; /* upstream hook is gone */
-	}
 
 	m = NULL;
 
@@ -809,10 +721,7 @@ ubt_intr_read_callback(struct usb2_xfer 
 		UBT_STAT_PCKTS_RECV(sc);
 		UBT_STAT_BYTES_RECV(sc, m->m_pkthdr.len);
 
-		NG_SEND_DATA_ONLY(error, sc->sc_hook, m);
-		if (error != 0)
-			UBT_STAT_IERROR(sc);
-
+		ubt_fwd_mbuf_up(sc, &m);
 		/* m == NULL at this point */
 		/* FALLTHROUGH */
 
@@ -820,12 +729,8 @@ ubt_intr_read_callback(struct usb2_xfer 
 submit_next:
 		NG_FREE_M(m); /* checks for m != NULL */
 
-		if (sc->sc_flags & UBT_FLAG_INTR_STALL)
-			usb2_transfer_start(sc->sc_xfer[UBT_IF_0_INTR_CS_RD]);
-		else {
-			xfer->frlengths[0] = xfer->max_data_length;
-			usb2_start_hardware(xfer);
-		}
+		xfer->frlengths[0] = xfer->max_data_length;
+		usb2_start_hardware(xfer);
 		break;
 
 	default: /* Error */
@@ -834,44 +739,15 @@ submit_next:
 				usb2_errstr(xfer->error));
 
 			/* Try to clear stall first */
-			sc->sc_flags |= UBT_FLAG_INTR_STALL;
-			usb2_transfer_start(sc->sc_xfer[UBT_IF_0_INTR_CS_RD]);
-		} else
-			NG_NODE_UNREF(node); /* cancelled */
+			xfer->flags.stall_pipe = 1;
+			goto submit_next;
+		}
+			/* transfer cancelled */
 		break;
 	}
 } /* ubt_intr_read_callback */
 
 /*
- * Called when outgoing control transfer initiated to clear stall on
- * interrupt pipe has completed.
- * USB context.
- */
-
-static void
-ubt_intr_read_clear_stall_callback(struct usb2_xfer *xfer)
-{
-	node_p			node = xfer->priv_sc;
-	struct ubt_softc	*sc;
-	struct usb2_xfer	*xfer_other;
-
-	if (NG_NODE_NOT_VALID(node)) {
-		NG_NODE_UNREF(node);
-		return; /* netgraph node is gone */
-	}
-
-	sc = NG_NODE_PRIVATE(node);
-	xfer_other = sc->sc_xfer[UBT_IF_0_INTR_DT_RD];
-
-	if (usb2_clear_stall_callback(xfer, xfer_other)) {
-		DPRINTF("stall cleared\n");
-		sc->sc_flags &= ~UBT_FLAG_INTR_STALL;
-		usb2_transfer_start(xfer_other);
-	} else
-		NG_NODE_UNREF(node); /* cant clear stall */
-} /* ubt_intr_read_clear_stall_callback */
-
-/*
  * Called when incoming bulk transfer (ACL packet) has completed, i.e.
  * ACL packet was received from the device.
  * USB context.
@@ -880,25 +756,10 @@ ubt_intr_read_clear_stall_callback(struc
 static void
 ubt_bulk_read_callback(struct usb2_xfer *xfer)
 {
-	node_p			node = xfer->priv_sc;
-	struct ubt_softc	*sc;
+	struct ubt_softc	*sc = xfer->priv_sc;
 	struct mbuf		*m;
 	ng_hci_acldata_pkt_t	*hdr;
 	uint16_t		len;
-	int			error;
-
-	if (NG_NODE_NOT_VALID(node)) {
-		NG_NODE_UNREF(node);
-		return; /* netgraph node is gone */
-	}
-
-	sc = NG_NODE_PRIVATE(node);
-
-	if ((sc->sc_hook == NULL) || NG_HOOK_NOT_VALID(sc->sc_hook)) {
-		UBT_INFO(sc, "no upstream hook\n");
-		NG_NODE_UNREF(node);
-		return; /* upstream hook is gone */
-	}
 
 	m = NULL;
 
@@ -956,10 +817,7 @@ ubt_bulk_read_callback(struct usb2_xfer 
 		UBT_STAT_PCKTS_RECV(sc);
 		UBT_STAT_BYTES_RECV(sc, m->m_pkthdr.len);
 
-		NG_SEND_DATA_ONLY(error, sc->sc_hook, m);
-		if (error != 0)
-			UBT_STAT_IERROR(sc);
-
+		ubt_fwd_mbuf_up(sc, &m);
 		/* m == NULL at this point */
 		/* FALLTHOUGH */
 
@@ -967,12 +825,8 @@ ubt_bulk_read_callback(struct usb2_xfer 
 submit_next:
 		NG_FREE_M(m); /* checks for m != NULL */
 
-		if (sc->sc_flags & UBT_FLAG_READ_STALL)
-			usb2_transfer_start(sc->sc_xfer[UBT_IF_0_BULK_CS_RD]);
-		else {
-			xfer->frlengths[0] = xfer->max_data_length;
-			usb2_start_hardware(xfer);
-		}
+		xfer->frlengths[0] = xfer->max_data_length;
+		usb2_start_hardware(xfer);
 		break;
 
 	default: /* Error */
@@ -981,44 +835,15 @@ submit_next:
 				usb2_errstr(xfer->error));
 
 			/* Try to clear stall first */
-			sc->sc_flags |= UBT_FLAG_READ_STALL;
-			usb2_transfer_start(sc->sc_xfer[UBT_IF_0_BULK_CS_RD]);
-		} else
-			NG_NODE_UNREF(node); /* cancelled */
+			xfer->flags.stall_pipe = 1;
+			goto submit_next;
+		}
+			/* transfer cancelled */
 		break;
 	}
 } /* ubt_bulk_read_callback */
 
 /*
- * Called when outgoing control transfer initiated to clear stall on
- * incoming bulk pipe has completed.
- * USB context.
- */
-
-static void
-ubt_bulk_read_clear_stall_callback(struct usb2_xfer *xfer)
-{
-	node_p			node = xfer->priv_sc;
-	struct ubt_softc	*sc;
-	struct usb2_xfer	*xfer_other;
-
-	if (NG_NODE_NOT_VALID(node)) {
-		NG_NODE_UNREF(node);
-		return; /* netgraph node is gone */
-	}
-
-	sc = NG_NODE_PRIVATE(node);
-	xfer_other = sc->sc_xfer[UBT_IF_0_BULK_DT_RD];
-
-	if (usb2_clear_stall_callback(xfer, xfer_other)) {
-		DPRINTF("stall cleared\n");
-		sc->sc_flags &= ~UBT_FLAG_READ_STALL;
-		usb2_transfer_start(xfer_other);
-	} else
-		NG_NODE_UNREF(node); /* cant clear stall */
-} /* ubt_bulk_read_clear_stall_callback */
-
-/*
  * Called when outgoing bulk transfer (ACL packet) has completed, i.e.
  * ACL packet was sent to the device.
  * USB context.
@@ -1027,40 +852,26 @@ ubt_bulk_read_clear_stall_callback(struc
 static void
 ubt_bulk_write_callback(struct usb2_xfer *xfer)
 {
-	node_p			node = xfer->priv_sc;
-	struct ubt_softc	*sc;
+	struct ubt_softc	*sc = xfer->priv_sc;
 	struct mbuf		*m;
 
-	if (NG_NODE_NOT_VALID(node)) {
-		NG_NODE_UNREF(node);
-		return; /* netgraph node is gone */
-	}
-
-	sc = NG_NODE_PRIVATE(node);
-
 	switch (USB_GET_STATE(xfer)) {
 	case USB_ST_TRANSFERRED:
-		if (xfer->error != 0)
-			UBT_STAT_OERROR(sc);
-		else {
-			UBT_INFO(sc, "sent %d bytes to bulk-out pipe\n",
-				xfer->actlen);
-
-			UBT_STAT_BYTES_SENT(sc, xfer->actlen);
-			UBT_STAT_PCKTS_SENT(sc);
-		}
+		UBT_INFO(sc, "sent %d bytes to bulk-out pipe\n", xfer->actlen);
+		UBT_STAT_BYTES_SENT(sc, xfer->actlen);
+		UBT_STAT_PCKTS_SENT(sc);
 		/* FALLTHROUGH */
 
 	case USB_ST_SETUP:
+send_next:
 		/* Get next mbuf, if any */
-		UBT_MBUFQ_LOCK(sc);
+		UBT_NG_LOCK(sc);
 		NG_BT_MBUFQ_DEQUEUE(&sc->sc_aclq, m);
-		UBT_MBUFQ_UNLOCK(sc);
+		UBT_NG_UNLOCK(sc);
 
 		if (m == NULL) {
 			UBT_INFO(sc, "ACL data queue is empty\n");
-			NG_NODE_UNREF(node);
-			return; /* transfer completed */
+			break; /* transfer completed */
 		}
 
 		/*
@@ -1087,44 +898,15 @@ ubt_bulk_write_callback(struct usb2_xfer
 			UBT_STAT_OERROR(sc);
 
 			/* try to clear stall first */
-			sc->sc_flags |= UBT_FLAG_WRITE_STALL;
-			usb2_transfer_start(sc->sc_xfer[UBT_IF_0_BULK_CS_WR]);
-		} else
-			NG_NODE_UNREF(node); /* cancelled */
+			xfer->flags.stall_pipe = 1;
+			goto send_next;
+		}
+			/* transfer cancelled */
 		break;
 	}
 } /* ubt_bulk_write_callback */
 
 /*
- * Called when outgoing control transfer initiated to clear stall on
- * outgoing bulk pipe has completed.
- * USB context.
- */
-
-static void
-ubt_bulk_write_clear_stall_callback(struct usb2_xfer *xfer)
-{
-	node_p			node = xfer->priv_sc;
-	struct ubt_softc	*sc;
-	struct usb2_xfer	*xfer_other;
-
-	if (NG_NODE_NOT_VALID(node)) {
-		NG_NODE_UNREF(node);
-		return; /* netgraph node is gone */
-	}
-
-	sc = NG_NODE_PRIVATE(node);
-	xfer_other = sc->sc_xfer[UBT_IF_0_BULK_DT_WR];
-
-	if (usb2_clear_stall_callback(xfer, xfer_other)) {
-		DPRINTF("stall cleared\n");
-		sc->sc_flags &= ~UBT_FLAG_WRITE_STALL;
-		usb2_transfer_start(xfer_other);
-	} else
-		NG_NODE_UNREF(node); /* cant clear stall */
-} /* ubt_bulk_write_clear_stall_callback */
-
-/*
  * Called when incoming isoc transfer (SCO packet) has completed, i.e.
  * SCO packet was received from the device.
  * USB context.
@@ -1133,23 +915,9 @@ ubt_bulk_write_clear_stall_callback(stru
 static void
 ubt_isoc_read_callback(struct usb2_xfer *xfer)
 {
-	node_p			node = xfer->priv_sc;
-	struct ubt_softc	*sc;
+	struct ubt_softc	*sc = xfer->priv_sc;
 	int			n;
 
-	if (NG_NODE_NOT_VALID(node)) {
-		NG_NODE_UNREF(node);
-		return; /* netgraph node is gone */
-	}
-
-	sc = NG_NODE_PRIVATE(node);
-
-	if ((sc->sc_hook == NULL) || NG_HOOK_NOT_VALID(sc->sc_hook)) {
-		UBT_INFO(sc, "no upstream hook\n");
-		NG_NODE_UNREF(node);
-		return; /* upstream hook is gone */
-	}
-
 	switch (USB_GET_STATE(xfer)) {
 	case USB_ST_TRANSFERRED:
 		for (n = 0; n < xfer->nframes; n ++)
@@ -1169,10 +937,9 @@ read_next:
                 if (xfer->error != USB_ERR_CANCELLED) {
                         UBT_STAT_IERROR(sc);
                         goto read_next;
-			/* NOT REACHED */
                 }
 
-		NG_NODE_UNREF(node); /* cancelled */
+		/* transfer cancelled */
 		break;
 	}
 } /* ubt_isoc_read_callback */
@@ -1188,7 +955,7 @@ ubt_isoc_read_one_frame(struct usb2_xfer
 {
 	struct ubt_softc	*sc = xfer->priv_sc;
 	struct mbuf		*m;
-	int			len, want, got, error;
+	int			len, want, got;
 
 	/* Get existing SCO reassembly buffer */
 	m = sc->sc_isoc_in_buffer;
@@ -1250,10 +1017,7 @@ ubt_isoc_read_one_frame(struct usb2_xfer
 		UBT_STAT_PCKTS_RECV(sc);
 		UBT_STAT_BYTES_RECV(sc, m->m_pkthdr.len);
 
-		NG_SEND_DATA_ONLY(error, sc->sc_hook, m);
-		if (error != 0)
-			UBT_STAT_IERROR(sc);
-
+		ubt_fwd_mbuf_up(sc, &m);
 		/* m == NULL at this point */
 	}
 
@@ -1272,29 +1036,15 @@ ubt_isoc_read_one_frame(struct usb2_xfer
 static void
 ubt_isoc_write_callback(struct usb2_xfer *xfer)
 {
-	node_p			node = xfer->priv_sc;
-	struct ubt_softc	*sc;
+	struct ubt_softc	*sc = xfer->priv_sc;
 	struct mbuf		*m;
 	int			n, space, offset;
 
-	if (NG_NODE_NOT_VALID(node)) {
-		NG_NODE_UNREF(node);
-		return; /* netgraph node is gone */
-	}
-
-	sc = NG_NODE_PRIVATE(node);
-
 	switch (USB_GET_STATE(xfer)) {
 	case USB_ST_TRANSFERRED:
-		if (xfer->error)
-			UBT_STAT_OERROR(sc);
-		else {
-			UBT_INFO(sc, "sent %d bytes to isoc-out pipe\n",
-				xfer->actlen);
-
-			UBT_STAT_BYTES_SENT(sc, xfer->actlen);
-			UBT_STAT_PCKTS_SENT(sc);
-		}
+		UBT_INFO(sc, "sent %d bytes to isoc-out pipe\n", xfer->actlen);
+		UBT_STAT_BYTES_SENT(sc, xfer->actlen);
+		UBT_STAT_PCKTS_SENT(sc);
 		/* FALLTHROUGH */
 
 	case USB_ST_SETUP:
@@ -1305,9 +1055,9 @@ send_next:
 
 		while (space > 0) {
 			if (m == NULL) {
-				UBT_MBUFQ_LOCK(sc);
+				UBT_NG_LOCK(sc);
 				NG_BT_MBUFQ_DEQUEUE(&sc->sc_scoq, m);
-				UBT_MBUFQ_UNLOCK(sc);
+				UBT_NG_UNLOCK(sc);
 
 				if (m == NULL)
 					break;
@@ -1328,9 +1078,9 @@ send_next:
 
 		/* Put whatever is left from mbuf back on queue */
 		if (m != NULL) {
-			UBT_MBUFQ_LOCK(sc);
+			UBT_NG_LOCK(sc);
 			NG_BT_MBUFQ_PREPEND(&sc->sc_scoq, m);
-			UBT_MBUFQ_UNLOCK(sc);
+			UBT_NG_UNLOCK(sc);
 		}
 
 		/*
@@ -1353,14 +1103,60 @@ send_next:
 		if (xfer->error != USB_ERR_CANCELLED) {
 			UBT_STAT_OERROR(sc);
 			goto send_next;
-			/* NOT REACHED */
 		}
 
-		NG_NODE_UNREF(node); /* cancelled */
+		/* transfer cancelled */
 		break;
 	}
 }
 
+/*
+ * Utility function to forward provided mbuf upstream (i.e. up the stack).
+ * Modifies value of the mbuf pointer (sets it to NULL).
+ * Save to call from any context.
+ */
+
+static int
+ubt_fwd_mbuf_up(ubt_softc_p sc, struct mbuf **m)
+{
+	hook_p	hook;
+	int	error;
+
+	/*
+	 * Close the race with Netgraph hook newhook/disconnect methods.
+	 * Save the hook pointer atomically. Two cases are possible:
+	 *
+	 * 1) The hook pointer is NULL. It means disconnect method got
+	 *    there first. In this case we are done.
+	 *
+	 * 2) The hook pointer is not NULL. It means that hook pointer
+	 *    could be either in valid or invalid (i.e. in the process
+	 *    of disconnect) state. In any case grab an extra reference
+	 *    to protect the hook pointer.
+	 *
+	 * It is ok to pass hook in invalid state to NG_SEND_DATA_ONLY() as
+	 * it checks for it. Drop extra reference after NG_SEND_DATA_ONLY().
+	 */
+
+	UBT_NG_LOCK(sc);
+	if ((hook = sc->sc_hook) != NULL)
+		NG_HOOK_REF(hook);
+	UBT_NG_UNLOCK(sc);
+
+	if (hook == NULL) {
+		NG_FREE_M(*m);
+		return (ENETDOWN);
+	}
+
+	NG_SEND_DATA_ONLY(error, hook, *m);
+	NG_HOOK_UNREF(hook);
+
+	if (error != 0)
+		UBT_STAT_IERROR(sc);
+
+	return (error);
+} /* ubt_fwd_mbuf_up */
+
 /****************************************************************************
  ****************************************************************************
  **                                 Glue 
@@ -1368,47 +1164,49 @@ send_next:
  ****************************************************************************/
 
 /*
- * Schedule glue task. Should be called with sc_mbufq_mtx held.
+ * Schedule glue task. Should be called with sc_ng_mtx held. 
  * Netgraph context.
  */
 
-static int
+static void
 ubt_task_schedule(ubt_softc_p sc, int action)
 {
-	mtx_assert(&sc->sc_mbufq_mtx, MA_OWNED);
+	mtx_assert(&sc->sc_ng_mtx, MA_OWNED);
 
-	if ((sc->sc_task_flags & action) == 0) {
-		/*
-		 * Try to handle corner case when "start all" and "stop all"
-		 * actions can both be set before task is executed.
-		 *
-		 * Assume the following:
-		 * 1) "stop all" after "start all" cancels "start all", and,
-		 *    keeps "stop all"
-		 *
-		 * 2) "start all" after "stop all" is fine because task is
-		 *    executing "stop all" first
-		 */
+	/*
+	 * Try to handle corner case when "start all" and "stop all"
+	 * actions can both be set before task is executed.
+	 *
+	 * The rules are
+	 *
+	 * sc_task_flags	action		new sc_task_flags
+	 * ------------------------------------------------------
+	 * 0			start		start
+	 * 0			stop		stop
+	 * start		start		start
+	 * start		stop		stop
+	 * stop			start		stop|start
+	 * stop			stop		stop

*** DIFF OUTPUT TRUNCATED AT 1000 LINES ***



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200901262059.n0QKxfl8062890>