Module Name: src Committed By: riastradh Date: Wed Mar 9 22:17:41 UTC 2022
Modified Files: src/sys/arch/mips/adm5120/dev: ahci.c src/sys/dev/ic: sl811hs.c src/sys/dev/usb: ehci.c motg.c ohci.c uhci.c usbdivar.h usbroothub.c xhci.c Log Message: usb: Provisionally release bus lock around ubm_rhctrl. This isn't quite correct, but it avoids a deadlock: - *_roothub_ctrl holds bus lock, waits in usb_delay_ms for kpause - softint waits for bus lock, holds up kpause wakeup The deadlock is new since recent changes to hold the bus lock over upm_start/upm_transfer. Making this change regresses to other problems: - *_suspend/resume and *_roothub_ctrl often touch the same portsc registers - roothub_ctrl_abort needs to wait for ubm_rhctrl to complete. When the bus lock was held across both, a noop served here, but we can't hold the bus lock across both, so that doesn't work. However, these problems -- which we've had for a long time -- seem to be less bad than the deadlock. So let's avoid the deadlock for now and then work out another way to serialize suspend/resume/rhctrl and aborts. Candidate fix for PR kern/56739. To generate a diff of this commit: cvs rdiff -u -r1.30 -r1.31 src/sys/arch/mips/adm5120/dev/ahci.c cvs rdiff -u -r1.110 -r1.111 src/sys/dev/ic/sl811hs.c cvs rdiff -u -r1.305 -r1.306 src/sys/dev/usb/ehci.c cvs rdiff -u -r1.40 -r1.41 src/sys/dev/usb/motg.c cvs rdiff -u -r1.321 -r1.322 src/sys/dev/usb/ohci.c cvs rdiff -u -r1.311 -r1.312 src/sys/dev/usb/uhci.c cvs rdiff -u -r1.134 -r1.135 src/sys/dev/usb/usbdivar.h cvs rdiff -u -r1.13 -r1.14 src/sys/dev/usb/usbroothub.c cvs rdiff -u -r1.158 -r1.159 src/sys/dev/usb/xhci.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/arch/mips/adm5120/dev/ahci.c diff -u src/sys/arch/mips/adm5120/dev/ahci.c:1.30 src/sys/arch/mips/adm5120/dev/ahci.c:1.31 --- src/sys/arch/mips/adm5120/dev/ahci.c:1.30 Thu Mar 3 06:12:11 2022 +++ src/sys/arch/mips/adm5120/dev/ahci.c Wed Mar 9 22:17:41 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: ahci.c,v 1.30 2022/03/03 06:12:11 riastradh Exp $ */ +/* $NetBSD: ahci.c,v 1.31 2022/03/09 22:17:41 riastradh Exp $ */ /*- * Copyright (c) 2007 Ruslan Ermilov and Vsevolod Lobko. @@ -64,7 +64,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: ahci.c,v 1.30 2022/03/03 06:12:11 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ahci.c,v 1.31 2022/03/09 22:17:41 riastradh Exp $"); #include <sys/param.h> #include <sys/systm.h> @@ -569,8 +569,6 @@ ahci_roothub_ctrl(struct usbd_bus *bus, DPRINTF(D_TRACE, ("SLRCstart ")); - KASSERT(bus->ub_polling || mutex_owned(bus->ub_lock)); - len = UGETW(req->wLength); value = UGETW(req->wValue); index = UGETW(req->wIndex); Index: src/sys/dev/ic/sl811hs.c diff -u src/sys/dev/ic/sl811hs.c:1.110 src/sys/dev/ic/sl811hs.c:1.111 --- src/sys/dev/ic/sl811hs.c:1.110 Thu Mar 3 06:12:11 2022 +++ src/sys/dev/ic/sl811hs.c Wed Mar 9 22:17:41 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: sl811hs.c,v 1.110 2022/03/03 06:12:11 riastradh Exp $ */ +/* $NetBSD: sl811hs.c,v 1.111 2022/03/09 22:17:41 riastradh Exp $ */ /* * Not (c) 2007 Matthew Orgass @@ -68,7 +68,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: sl811hs.c,v 1.110 2022/03/03 06:12:11 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: sl811hs.c,v 1.111 2022/03/09 22:17:41 riastradh Exp $"); #ifdef _KERNEL_OPT #include "opt_slhci.h" @@ -3195,8 +3195,6 @@ slhci_roothub_ctrl(struct usbd_bus *bus, uint8_t type; int actlen = 0; - KASSERT(bus->ub_usepolling || mutex_owned(bus->ub_lock)); - len = UGETW(req->wLength); value = UGETW(req->wValue); index = UGETW(req->wIndex); Index: src/sys/dev/usb/ehci.c diff -u src/sys/dev/usb/ehci.c:1.305 src/sys/dev/usb/ehci.c:1.306 --- src/sys/dev/usb/ehci.c:1.305 Thu Mar 3 06:12:11 2022 +++ src/sys/dev/usb/ehci.c Wed Mar 9 22:17:41 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: ehci.c,v 1.305 2022/03/03 06:12:11 riastradh Exp $ */ +/* $NetBSD: ehci.c,v 1.306 2022/03/09 22:17:41 riastradh Exp $ */ /* * Copyright (c) 2004-2012,2016,2020 The NetBSD Foundation, Inc. @@ -54,7 +54,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.305 2022/03/03 06:12:11 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.306 2022/03/09 22:17:41 riastradh Exp $"); #include "ohci.h" #include "uhci.h" @@ -1438,6 +1438,9 @@ ehci_activate(device_t self, enum devact * * Note that this power handler isn't to be registered directly; the * bus glue needs to call out to it. + * + * XXX This should be serialized with ehci_roothub_ctrl's access to the + * portsc registers. */ bool ehci_suspend(device_t dv, const pmf_qual_t *qual) @@ -2369,7 +2372,10 @@ ehci_roothub_ctrl(struct usbd_bus *bus, EHCIHIST_FUNC(); EHCIHIST_CALLED(); - KASSERT(bus->ub_usepolling || mutex_owned(bus->ub_lock)); + /* + * XXX This should be serialized with ehci_suspend/resume's + * access to the portsc registers. + */ if (sc->sc_dying) return -1; Index: src/sys/dev/usb/motg.c diff -u src/sys/dev/usb/motg.c:1.40 src/sys/dev/usb/motg.c:1.41 --- src/sys/dev/usb/motg.c:1.40 Thu Mar 3 06:12:11 2022 +++ src/sys/dev/usb/motg.c Wed Mar 9 22:17:41 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: motg.c,v 1.40 2022/03/03 06:12:11 riastradh Exp $ */ +/* $NetBSD: motg.c,v 1.41 2022/03/09 22:17:41 riastradh Exp $ */ /* * Copyright (c) 1998, 2004, 2011, 2012, 2014 The NetBSD Foundation, Inc. @@ -40,7 +40,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: motg.c,v 1.40 2022/03/03 06:12:11 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: motg.c,v 1.41 2022/03/09 22:17:41 riastradh Exp $"); #ifdef _KERNEL_OPT #include "opt_usb.h" @@ -806,8 +806,6 @@ motg_roothub_ctrl(struct usbd_bus *bus, MOTGHIST_FUNC(); MOTGHIST_CALLED(); - KASSERT(bus->ub_usepolling || mutex_owned(bus->ub_lock)); - if (sc->sc_dying) return -1; Index: src/sys/dev/usb/ohci.c diff -u src/sys/dev/usb/ohci.c:1.321 src/sys/dev/usb/ohci.c:1.322 --- src/sys/dev/usb/ohci.c:1.321 Thu Mar 3 06:12:11 2022 +++ src/sys/dev/usb/ohci.c Wed Mar 9 22:17:41 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: ohci.c,v 1.321 2022/03/03 06:12:11 riastradh Exp $ */ +/* $NetBSD: ohci.c,v 1.322 2022/03/09 22:17:41 riastradh Exp $ */ /* * Copyright (c) 1998, 2004, 2005, 2012, 2016, 2020 The NetBSD Foundation, Inc. @@ -42,7 +42,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: ohci.c,v 1.321 2022/03/03 06:12:11 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ohci.c,v 1.322 2022/03/09 22:17:41 riastradh Exp $"); #ifdef _KERNEL_OPT #include "opt_usb.h" @@ -2431,8 +2431,6 @@ ohci_roothub_ctrl(struct usbd_bus *bus, OHCIHIST_FUNC(); OHCIHIST_CALLED(); - KASSERT(bus->ub_usepolling || mutex_owned(bus->ub_lock)); - if (sc->sc_dying) return -1; Index: src/sys/dev/usb/uhci.c diff -u src/sys/dev/usb/uhci.c:1.311 src/sys/dev/usb/uhci.c:1.312 --- src/sys/dev/usb/uhci.c:1.311 Thu Mar 3 06:12:11 2022 +++ src/sys/dev/usb/uhci.c Wed Mar 9 22:17:41 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: uhci.c,v 1.311 2022/03/03 06:12:11 riastradh Exp $ */ +/* $NetBSD: uhci.c,v 1.312 2022/03/09 22:17:41 riastradh Exp $ */ /* * Copyright (c) 1998, 2004, 2011, 2012, 2016, 2020 The NetBSD Foundation, Inc. @@ -42,7 +42,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: uhci.c,v 1.311 2022/03/03 06:12:11 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: uhci.c,v 1.312 2022/03/09 22:17:41 riastradh Exp $"); #ifdef _KERNEL_OPT #include "opt_usb.h" @@ -3589,8 +3589,6 @@ uhci_roothub_ctrl(struct usbd_bus *bus, UHCIHIST_FUNC(); UHCIHIST_CALLED(); - KASSERT(bus->ub_usepolling || mutex_owned(bus->ub_lock)); - if (sc->sc_dying) return -1; Index: src/sys/dev/usb/usbdivar.h diff -u src/sys/dev/usb/usbdivar.h:1.134 src/sys/dev/usb/usbdivar.h:1.135 --- src/sys/dev/usb/usbdivar.h:1.134 Thu Mar 3 06:12:11 2022 +++ src/sys/dev/usb/usbdivar.h Wed Mar 9 22:17:41 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: usbdivar.h,v 1.134 2022/03/03 06:12:11 riastradh Exp $ */ +/* $NetBSD: usbdivar.h,v 1.135 2022/03/09 22:17:41 riastradh Exp $ */ /* * Copyright (c) 1998, 2012 The NetBSD Foundation, Inc. @@ -51,7 +51,7 @@ * ubm_abortx x must not release/reacquire lock * ubm_getlock - Called at attach time * ubm_newdev - Will take lock - * ubm_rhctrl x + * ubm_rhctrl - * * PIPE METHOD LOCK NOTES * ----------------------- ------- ------------------------- Index: src/sys/dev/usb/usbroothub.c diff -u src/sys/dev/usb/usbroothub.c:1.13 src/sys/dev/usb/usbroothub.c:1.14 --- src/sys/dev/usb/usbroothub.c:1.13 Thu Mar 3 06:12:11 2022 +++ src/sys/dev/usb/usbroothub.c Wed Mar 9 22:17:41 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: usbroothub.c,v 1.13 2022/03/03 06:12:11 riastradh Exp $ */ +/* $NetBSD: usbroothub.c,v 1.14 2022/03/09 22:17:41 riastradh Exp $ */ /*- * Copyright (c) 1998, 2004, 2011, 2012 The NetBSD Foundation, Inc. @@ -58,7 +58,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: usbroothub.c,v 1.13 2022/03/03 06:12:11 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: usbroothub.c,v 1.14 2022/03/09 22:17:41 riastradh Exp $"); #include <sys/param.h> #include <sys/systm.h> /* for ostype */ @@ -549,7 +549,19 @@ roothub_ctrl_start(struct usbd_xfer *xfe break; } + /* + * XXX This needs some mechanism for concurrent + * roothub_ctrl_abort to wait for ubm_rhctrl to finish. We + * can't use the bus lock because many ubm_rhctrl methods do + * usb_delay_ms and many bus locks are taken in softint + * context, leading to deadlock in the softclock needed to wake + * up usb_delay_ms. + */ + if (!bus->ub_usepolling) + mutex_exit(bus->ub_lock); actlen = bus->ub_methods->ubm_rhctrl(bus, req, buf, buflen); + if (!bus->ub_usepolling) + mutex_enter(bus->ub_lock); if (actlen < 0) goto fail; Index: src/sys/dev/usb/xhci.c diff -u src/sys/dev/usb/xhci.c:1.158 src/sys/dev/usb/xhci.c:1.159 --- src/sys/dev/usb/xhci.c:1.158 Thu Mar 3 06:12:11 2022 +++ src/sys/dev/usb/xhci.c Wed Mar 9 22:17:41 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: xhci.c,v 1.158 2022/03/03 06:12:11 riastradh Exp $ */ +/* $NetBSD: xhci.c,v 1.159 2022/03/09 22:17:41 riastradh Exp $ */ /* * Copyright (c) 2013 Jonathan A. Kollasch @@ -34,7 +34,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.158 2022/03/03 06:12:11 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.159 2022/03/09 22:17:41 riastradh Exp $"); #ifdef _KERNEL_OPT #include "opt_usb.h" @@ -3856,8 +3856,6 @@ xhci_roothub_ctrl(struct usbd_bus *bus, XHCIHIST_FUNC(); - KASSERT(bus->ub_usepolling || mutex_owned(bus->ub_lock)); - if (sc->sc_dying) return -1;