From owner-freebsd-usb@freebsd.org Mon Sep 5 09:05:54 2016 Return-Path: Delivered-To: freebsd-usb@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id B6D15A9DD7A; Mon, 5 Sep 2016 09:05:54 +0000 (UTC) (envelope-from hps@selasky.org) Received: from mail.turbocat.net (mail.turbocat.net [IPv6:2a01:4f8:d16:4514::2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 69FDF9F7; Mon, 5 Sep 2016 09:05:54 +0000 (UTC) (envelope-from hps@selasky.org) Received: from laptop015.home.selasky.org (unknown [62.141.129.119]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.turbocat.net (Postfix) with ESMTPSA id C47B11FE022; Mon, 5 Sep 2016 11:05:52 +0200 (CEST) Subject: Re: Deadlock between device_detach() and usbd_do_request_flags() To: Andriy Voskoboinyk , Adrian Chadd References: <4cf378ff-63e1-7cdc-6120-9578fceec20d@selasky.org> <55cff8ce-6132-53ec-3419-d27286dce622@selasky.org> Cc: "freebsd-wireless@freebsd.org" , "freebsd-usb@freebsd.org" From: Hans Petter Selasky Message-ID: <67777201-e078-b519-566a-7a2a29363eee@selasky.org> Date: Mon, 5 Sep 2016 11:10:34 +0200 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <55cff8ce-6132-53ec-3419-d27286dce622@selasky.org> Content-Type: multipart/mixed; boundary="------------42EA583F81AFB157F5F19237" X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Sep 2016 09:05:54 -0000 This is a multi-part message in MIME format. --------------42EA583F81AFB157F5F19237 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Hi, Can you test the attached patch? Does it solve your issue? --HPS --------------42EA583F81AFB157F5F19237 Content-Type: text/x-patch; name="usb_fix_deadlock.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="usb_fix_deadlock.diff" Index: sys/dev/usb/usb_device.c =================================================================== --- sys/dev/usb/usb_device.c (revision 304569) +++ sys/dev/usb/usb_device.c (working copy) @@ -1585,6 +1585,7 @@ /* initialise our SX-lock */ sx_init_flags(&udev->enum_sx, "USB config SX lock", SX_DUPOK); sx_init_flags(&udev->sr_sx, "USB suspend and resume SX lock", SX_NOWITNESS); + sx_init_flags(&udev->ctrl_sx, "USB control transfer SX lock", SX_DUPOK); cv_init(&udev->ctrlreq_cv, "WCTRL"); cv_init(&udev->ref_cv, "UGONE"); @@ -2195,6 +2196,7 @@ sx_destroy(&udev->enum_sx); sx_destroy(&udev->sr_sx); + sx_destroy(&udev->ctrl_sx); cv_destroy(&udev->ctrlreq_cv); cv_destroy(&udev->ref_cv); Index: sys/dev/usb/usb_device.h =================================================================== --- sys/dev/usb/usb_device.h (revision 304569) +++ sys/dev/usb/usb_device.h (working copy) @@ -183,6 +183,7 @@ struct usb_udev_msg cs_msg[2]; struct sx enum_sx; struct sx sr_sx; + struct sx ctrl_sx; struct mtx device_mtx; struct cv ctrlreq_cv; struct cv ref_cv; Index: sys/dev/usb/usb_request.c =================================================================== --- sys/dev/usb/usb_request.c (revision 304569) +++ sys/dev/usb/usb_request.c (working copy) @@ -418,7 +418,6 @@ uint16_t length; uint16_t temp; uint16_t acttemp; - uint8_t do_unlock; if (timeout < 50) { /* timeout is too small */ @@ -460,16 +459,16 @@ } /* - * Grab the USB device enumeration SX-lock serialization is - * achieved when multiple threads are involved: + * Serialize access to this function: */ - do_unlock = usbd_enum_lock(udev); + sx_xlock(&udev->ctrl_sx); /* * We need to allow suspend and resume at this point, else the * control transfer will timeout if the device is suspended! */ - usbd_sr_unlock(udev); + if (usbd_enum_is_locked(udev)) + usbd_sr_unlock(udev); hr_func = usbd_get_hr_func(udev); @@ -713,10 +712,10 @@ USB_XFER_UNLOCK(xfer); done: - usbd_sr_lock(udev); + sx_xunlock(&udev->ctrl_sx); - if (do_unlock) - usbd_enum_unlock(udev); + if (usbd_enum_is_locked(udev)) + usbd_sr_lock(udev); if ((mtx != NULL) && (mtx != &Giant)) mtx_lock(mtx); --------------42EA583F81AFB157F5F19237--