Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/dev/usb usb: Provisionally release bus lock around ubm_r...
details: https://anonhg.NetBSD.org/src/rev/982e452d3bbd
branches: trunk
changeset: 363396:982e452d3bbd
user: riastradh <riastradh%NetBSD.org@localhost>
date: Wed Mar 09 22:17:41 2022 +0000
description:
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.
diffstat:
sys/arch/mips/adm5120/dev/ahci.c | 6 ++----
sys/dev/ic/sl811hs.c | 6 ++----
sys/dev/usb/ehci.c | 12 +++++++++---
sys/dev/usb/motg.c | 6 ++----
sys/dev/usb/ohci.c | 6 ++----
sys/dev/usb/uhci.c | 6 ++----
sys/dev/usb/usbdivar.h | 4 ++--
sys/dev/usb/usbroothub.c | 16 ++++++++++++++--
sys/dev/usb/xhci.c | 6 ++----
9 files changed, 37 insertions(+), 31 deletions(-)
diffs (258 lines):
diff -r 7b50b24f46c6 -r 982e452d3bbd sys/arch/mips/adm5120/dev/ahci.c
--- a/sys/arch/mips/adm5120/dev/ahci.c Wed Mar 09 17:53:39 2022 +0000
+++ b/sys/arch/mips/adm5120/dev/ahci.c Wed Mar 09 22:17:41 2022 +0000
@@ -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 @@
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);
diff -r 7b50b24f46c6 -r 982e452d3bbd sys/dev/ic/sl811hs.c
--- a/sys/dev/ic/sl811hs.c Wed Mar 09 17:53:39 2022 +0000
+++ b/sys/dev/ic/sl811hs.c Wed Mar 09 22:17:41 2022 +0000
@@ -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 @@
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);
diff -r 7b50b24f46c6 -r 982e452d3bbd sys/dev/usb/ehci.c
--- a/sys/dev/usb/ehci.c Wed Mar 09 17:53:39 2022 +0000
+++ b/sys/dev/usb/ehci.c Wed Mar 09 22:17:41 2022 +0000
@@ -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 @@
*
* 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 @@
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;
diff -r 7b50b24f46c6 -r 982e452d3bbd sys/dev/usb/motg.c
--- a/sys/dev/usb/motg.c Wed Mar 09 17:53:39 2022 +0000
+++ b/sys/dev/usb/motg.c Wed Mar 09 22:17:41 2022 +0000
@@ -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 @@
MOTGHIST_FUNC(); MOTGHIST_CALLED();
- KASSERT(bus->ub_usepolling || mutex_owned(bus->ub_lock));
-
if (sc->sc_dying)
return -1;
diff -r 7b50b24f46c6 -r 982e452d3bbd sys/dev/usb/ohci.c
--- a/sys/dev/usb/ohci.c Wed Mar 09 17:53:39 2022 +0000
+++ b/sys/dev/usb/ohci.c Wed Mar 09 22:17:41 2022 +0000
@@ -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 @@
OHCIHIST_FUNC(); OHCIHIST_CALLED();
- KASSERT(bus->ub_usepolling || mutex_owned(bus->ub_lock));
-
if (sc->sc_dying)
return -1;
diff -r 7b50b24f46c6 -r 982e452d3bbd sys/dev/usb/uhci.c
--- a/sys/dev/usb/uhci.c Wed Mar 09 17:53:39 2022 +0000
+++ b/sys/dev/usb/uhci.c Wed Mar 09 22:17:41 2022 +0000
@@ -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 @@
UHCIHIST_FUNC(); UHCIHIST_CALLED();
- KASSERT(bus->ub_usepolling || mutex_owned(bus->ub_lock));
-
if (sc->sc_dying)
return -1;
diff -r 7b50b24f46c6 -r 982e452d3bbd sys/dev/usb/usbdivar.h
--- a/sys/dev/usb/usbdivar.h Wed Mar 09 17:53:39 2022 +0000
+++ b/sys/dev/usb/usbdivar.h Wed Mar 09 22:17:41 2022 +0000
@@ -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
* ----------------------- ------- -------------------------
diff -r 7b50b24f46c6 -r 982e452d3bbd sys/dev/usb/usbroothub.c
--- a/sys/dev/usb/usbroothub.c Wed Mar 09 17:53:39 2022 +0000
+++ b/sys/dev/usb/usbroothub.c Wed Mar 09 22:17:41 2022 +0000
@@ -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 @@
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;
diff -r 7b50b24f46c6 -r 982e452d3bbd sys/dev/usb/xhci.c
--- a/sys/dev/usb/xhci.c Wed Mar 09 17:53:39 2022 +0000
+++ b/sys/dev/usb/xhci.c Wed Mar 09 22:17:41 2022 +0000
@@ -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 @@
XHCIHIST_FUNC();
- KASSERT(bus->ub_usepolling || mutex_owned(bus->ub_lock));
-
if (sc->sc_dying)
return -1;
Home |
Main Index |
Thread Index |
Old Index