From owner-p4-projects@FreeBSD.ORG Fri Jan 23 22:25:23 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id A1F111065674; Fri, 23 Jan 2009 22:25:23 +0000 (UTC) Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5C8541065672 for ; Fri, 23 Jan 2009 22:25:23 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 4063C8FC1F for ; Fri, 23 Jan 2009 22:25:23 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id n0NMPNlP022404 for ; Fri, 23 Jan 2009 22:25:23 GMT (envelope-from hselasky@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id n0NMPNHd022402 for perforce@freebsd.org; Fri, 23 Jan 2009 22:25:23 GMT (envelope-from hselasky@FreeBSD.org) Date: Fri, 23 Jan 2009 22:25:23 GMT Message-Id: <200901232225.n0NMPNHd022402@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to hselasky@FreeBSD.org using -f From: Hans Petter Selasky To: Perforce Change Reviews Cc: Subject: PERFORCE change 156586 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 Jan 2009 22:25:24 -0000 http://perforce.freebsd.org/chv.cgi?CH=156586 Change 156586 by hselasky@hselasky_laptop001 on 2009/01/23 22:24:32 Fix: USB calls inside Netgraph callbacks must be non-blocking. Reported by: Maksim Yevmenkin Affected files ... .. //depot/projects/usb/src/sys/dev/usb2/bluetooth/ng_ubt2.c#17 edit Differences ... ==== //depot/projects/usb/src/sys/dev/usb2/bluetooth/ng_ubt2.c#17 (text+ko) ==== @@ -54,14 +54,7 @@ * 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. - * - * 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. + * grab any long-sleep lock in the Netgraph context. */ #include @@ -598,10 +591,6 @@ while (!(sc->sc_flags & UBT_FLAG_SHUTDOWN)) usb2_pause_mtx(&sc->sc_mtx, 100); UBT_UNLOCK(sc); - - /* Check if there is a leftover hook */ - if (sc->sc_hook != NULL) - NG_NODE_UNREF(node); } /* Free USB transfers, if any */ usb2_transfer_unsetup(sc->sc_xfer, UBT_N_TRANSFER); @@ -621,6 +610,33 @@ return (0); } /* ubt_detach */ +/* + * The following function will get the UBT softc pointer by the USB + * transfer pointer. This function returns NULL when the NODE is no + * longer accessible. + */ +static struct ubt_softc * +ubt_get_node_private(struct usb2_xfer *xfer) +{ + node_p node = xfer->priv_sc; + + switch (USB_GET_STATE(xfer)) { + case USB_ST_TRANSFERRED: + case USB_ST_SETUP: + /* + * Due to atomicity inside the USB stack we know that + * the node private is still in this state! + */ + return (NG_NODE_PRIVATE(node)); + + default: /* error case */ + if (xfer->error == USB_ERR_CANCELLED) + return (NULL); /* node is gone */ + else + return (NG_NODE_PRIVATE(node)); + } +} + /* * Called when outgoing control request (HCI command) has completed, i.e. * HCI command was sent to the device. @@ -630,12 +646,11 @@ static void ubt_ctrl_write_callback(struct usb2_xfer *xfer) { - node_p node = xfer->priv_sc; struct ubt_softc *sc; struct usb2_device_request req; struct mbuf *m; - sc = NG_NODE_PRIVATE(node); + sc = ubt_get_node_private(xfer); switch (USB_GET_STATE(xfer)) { case USB_ST_TRANSFERRED: @@ -698,13 +713,12 @@ static void ubt_intr_read_callback(struct usb2_xfer *xfer) { - node_p node = xfer->priv_sc; struct ubt_softc *sc; struct mbuf *m; ng_hci_event_pkt_t *hdr; int error; - sc = NG_NODE_PRIVATE(node); + sc = ubt_get_node_private(xfer); m = NULL; @@ -803,13 +817,14 @@ 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; - sc = NG_NODE_PRIVATE(node); + sc = ubt_get_node_private(xfer); + if (sc == NULL) + return; + 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; @@ -826,14 +841,14 @@ static void ubt_bulk_read_callback(struct usb2_xfer *xfer) { - node_p node = xfer->priv_sc; struct ubt_softc *sc; struct mbuf *m; ng_hci_acldata_pkt_t *hdr; uint16_t len; int error; - sc = NG_NODE_PRIVATE(node); + sc = ubt_get_node_private(xfer); + m = NULL; switch (USB_GET_STATE(xfer)) { @@ -931,13 +946,14 @@ 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; - sc = NG_NODE_PRIVATE(node); + sc = ubt_get_node_private(xfer); + if (sc == NULL) + return; + 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; @@ -954,11 +970,10 @@ static void ubt_bulk_write_callback(struct usb2_xfer *xfer) { - node_p node = xfer->priv_sc; struct ubt_softc *sc; struct mbuf *m; - sc = NG_NODE_PRIVATE(node); + sc = ubt_get_node_private(xfer); switch (USB_GET_STATE(xfer)) { case USB_ST_TRANSFERRED: @@ -1018,13 +1033,14 @@ 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; - sc = NG_NODE_PRIVATE(node); + sc = ubt_get_node_private(xfer); + if (sc == NULL) + return; + 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; @@ -1041,11 +1057,10 @@ static void ubt_isoc_read_callback(struct usb2_xfer *xfer) { - node_p node = xfer->priv_sc; struct ubt_softc *sc; int n; - sc = NG_NODE_PRIVATE(node); + sc = ubt_get_node_private(xfer); switch (USB_GET_STATE(xfer)) { case USB_ST_TRANSFERRED: @@ -1167,12 +1182,11 @@ static void ubt_isoc_write_callback(struct usb2_xfer *xfer) { - node_p node = xfer->priv_sc; struct ubt_softc *sc; struct mbuf *m; int n, space, offset; - sc = NG_NODE_PRIVATE(node); + sc = ubt_get_node_private(xfer); switch (USB_GET_STATE(xfer)) { case USB_ST_TRANSFERRED: @@ -1322,8 +1336,6 @@ return (EINVAL); } - NG_NODE_REF(sc->sc_node); - UBT_LOCK(sc); usb2_transfer_start(sc->sc_xfer[UBT_IF_0_INTR_DT_RD]); usb2_transfer_start(sc->sc_xfer[UBT_IF_0_BULK_DT_RD]); @@ -1356,17 +1368,21 @@ if (hook != sc->sc_hook) return (EINVAL); - /* - * Synchronously drain all USB transfers: - * Can take some milliseconds! + UBT_LOCK(sc); + + /* + * Netgraph cannot sleep! + * + * Asynchronously stop all USB transfers like an atomic + * operation. If an USB transfer is pending the USB transfer + * will be cancelled and not complete! This operation is + * atomic within the "sc->sc_mtx" mutex. */ for (i = 0; i != UBT_N_TRANSFER; i++) - usb2_transfer_drain(sc->sc_xfer[i]); + usb2_transfer_stop(sc->sc_xfer[i]); sc->sc_hook = NULL; - UBT_LOCK(sc); - /* Drain queues */ NG_BT_MBUFQ_DRAIN(&sc->sc_cmdq); NG_BT_MBUFQ_DRAIN(&sc->sc_aclq); @@ -1374,8 +1390,6 @@ UBT_UNLOCK(sc); - NG_NODE_UNREF(node); - return (0); } /* ng_ubt_disconnect */