Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/nick-nhusb]: src/sys/dev/usb Reduce the scope of the softc lock further ...
details: https://anonhg.NetBSD.org/src/rev/0eb2489de028
branches: nick-nhusb
changeset: 804700:0eb2489de028
user: skrll <skrll%NetBSD.org@localhost>
date: Sun Nov 06 11:50:54 2016 +0000
description:
Reduce the scope of the softc lock further and track device state via
sc_state.
All ucom methods apart from ucom_{read,write} are called without the
softc lock held.
ucom_close is called on last close only to match ucom_open being called
on first open only.
Fix ucom_detach where refcnt wasn't being decremented.
More DEBUG.
XXX still not sure where tty_lock needs to be held.
diffstat:
sys/dev/usb/ucom.c | 309 +++++++++++++++++++++++++++++++++++++---------------
1 files changed, 217 insertions(+), 92 deletions(-)
diffs (truncated from 714 to 300 lines):
diff -r bb75f45e613c -r 0eb2489de028 sys/dev/usb/ucom.c
--- a/sys/dev/usb/ucom.c Sun Nov 06 09:36:53 2016 +0000
+++ b/sys/dev/usb/ucom.c Sun Nov 06 11:50:54 2016 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ucom.c,v 1.108.2.30 2016/11/06 09:36:53 skrll Exp $ */
+/* $NetBSD: ucom.c,v 1.108.2.31 2016/11/06 11:50:54 skrll Exp $ */
/*
* Copyright (c) 1998, 2000 The NetBSD Foundation, Inc.
@@ -34,7 +34,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ucom.c,v 1.108.2.30 2016/11/06 09:36:53 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ucom.c,v 1.108.2.31 2016/11/06 11:50:54 skrll Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -78,7 +78,7 @@
#ifndef UCOM_DEBUG
#define ucomdebug 0
#else
-int ucomdebug = 0;
+int ucomdebug = 10;
SYSCTL_SETUP(sysctl_hw_ucom_setup, "sysctl hw.ucom setup")
{
@@ -181,8 +181,13 @@
u_char sc_tx_stopped;
int sc_swflags;
- u_char sc_opening; /* lock during open */
- u_char sc_closing; /* lock during close */
+ enum ucom_state {
+ UCOM_DEAD,
+ UCOM_ATTACHED,
+ UCOM_OPENING,
+ UCOM_CLOSING,
+ UCOM_OPEN
+ } sc_state;
int sc_refcnt;
bool sc_dying; /* disconnecting */
@@ -191,7 +196,7 @@
krndsource_t sc_rndsource; /* random source */
kmutex_t sc_lock;
- kcondvar_t sc_opencv;
+ kcondvar_t sc_statecv;
kcondvar_t sc_detachcv;
};
@@ -291,14 +296,13 @@
sc->sc_mcr = 0;
sc->sc_tx_stopped = 0;
sc->sc_swflags = 0;
- sc->sc_opening = 0;
- sc->sc_closing = 0;
sc->sc_refcnt = 0;
sc->sc_dying = false;
+ sc->sc_state = UCOM_DEAD;
sc->sc_si = softint_establish(SOFTINT_USB, ucom_softintr, sc);
mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_SOFTUSB);
- cv_init(&sc->sc_opencv, "ucomopen");
+ cv_init(&sc->sc_statecv, "ucomstate");
cv_init(&sc->sc_detachcv, "ucomdtch");
SIMPLEQ_INIT(&sc->sc_ibuff_empty);
@@ -384,6 +388,9 @@
if (!pmf_device_register(self, NULL, NULL))
aprint_error_dev(self, "couldn't establish power handler\n");
+
+ sc->sc_state = UCOM_ATTACHED;
+
return;
fail_2:
@@ -434,6 +441,18 @@
usbd_abort_pipe(sc->sc_bulkout_pipe);
mutex_enter(&sc->sc_lock);
+
+ /* wait for open/close to finish */
+ while (sc->sc_state == UCOM_OPENING || sc->sc_state == UCOM_CLOSING) {
+ int error = cv_wait_sig(&sc->sc_statecv, &sc->sc_lock);
+
+ if (error) {
+ mutex_exit(&sc->sc_lock);
+ return error;
+ }
+ }
+
+ sc->sc_refcnt--;
while (sc->sc_refcnt > 0) {
/* Wake up anyone waiting */
if (tp != NULL) {
@@ -494,7 +513,7 @@
rnd_detach_source(&sc->sc_rndsource);
mutex_destroy(&sc->sc_lock);
- cv_destroy(&sc->sc_opencv);
+ cv_destroy(&sc->sc_statecv);
cv_destroy(&sc->sc_detachcv);
return 0;
@@ -544,7 +563,7 @@
{
const int unit = UCOMUNIT(dev);
struct ucom_softc * const sc = device_lookup_private(&ucom_cd, unit);
- int error;
+ int error = 0;
UCOMHIST_FUNC(); UCOMHIST_CALLED();
@@ -553,6 +572,7 @@
mutex_enter(&sc->sc_lock);
if (sc->sc_dying) {
+ DPRINTF("... dying", 0, 0, 0, 0);
mutex_exit(&sc->sc_lock);
return EIO;
}
@@ -575,29 +595,36 @@
* Wait while the device is initialized by the
* first opener or cleaned up by the last closer.
*/
- while (sc->sc_opening || sc->sc_closing) {
- error = cv_wait_sig(&sc->sc_opencv, &sc->sc_lock);
+ while (sc->sc_state == UCOM_OPENING || sc->sc_state == UCOM_CLOSING) {
+ error = cv_wait_sig(&sc->sc_statecv, &sc->sc_lock);
+
+ if (sc->sc_dying)
+ error = EIO;
if (error) {
mutex_exit(&sc->sc_lock);
return error;
}
}
+ enum ucom_state state = sc->sc_state;
- sc->sc_opening = 1;
+ KASSERTMSG(state == UCOM_OPEN || state == UCOM_ATTACHED,
+ "state is %d", state);
+ bool cleanup = false;
+ /* If this is the first open then perform the initialisation */
if (!ISSET(tp->t_state, TS_ISOPEN) && tp->t_wopen == 0) {
+ KASSERT(state == UCOM_ATTACHED);
tp->t_dev = dev;
+ cleanup = true;
+ sc->sc_state = UCOM_OPENING;
+ mutex_exit(&sc->sc_lock);
if (sc->sc_methods->ucom_open != NULL) {
error = sc->sc_methods->ucom_open(sc->sc_parent,
sc->sc_portno);
if (error) {
- ucom_cleanup(sc);
- sc->sc_opening = 0;
- cv_signal(&sc->sc_opencv);
- mutex_exit(&sc->sc_lock);
- return error;
+ goto fail_2;
}
}
@@ -644,6 +671,13 @@
ucom_dtr(sc, 1);
ucom_rts(sc, 1);
+ mutex_enter(&sc->sc_lock);
+ if (sc->sc_dying) {
+ DPRINTF("... dying", 0, 0, 0, 0);
+ error = EIO;
+ goto fail_1;
+ }
+
sc->sc_rx_unblock = 0;
sc->sc_rx_stopped = 0;
sc->sc_tx_stopped = 0;
@@ -654,14 +688,18 @@
for (size_t i = 0; i < UCOM_IN_BUFFS; i++) {
struct ucom_buffer *ub = &sc->sc_ibuff[i];
error = ucomsubmitread(sc, ub);
- if (error)
+ if (error) {
+ mutex_exit(&sc->sc_lock);
goto fail_2;
+ }
}
}
- sc->sc_opening = 0;
- cv_signal(&sc->sc_opencv);
+ sc->sc_state = UCOM_OPEN;
+ cv_signal(&sc->sc_statecv);
mutex_exit(&sc->sc_lock);
+ DPRINTF("unit=%d, tp=%p dialout %d nonblock %d", unit, tp,
+ !!UCOMDIALOUT(dev), !!ISSET(flag, O_NONBLOCK));
error = ttyopen(tp, UCOMDIALOUT(dev), ISSET(flag, O_NONBLOCK));
if (error)
goto bad;
@@ -672,30 +710,24 @@
return 0;
-fail_2:
- if (sc->sc_bulkin_no != -1)
- usbd_abort_pipe(sc->sc_bulkin_pipe);
- if (sc->sc_bulkout_no != -1)
- usbd_abort_pipe(sc->sc_bulkout_pipe);
+bad:
+ cleanup = !ISSET(tp->t_state, TS_ISOPEN) && tp->t_wopen == 0;
- mutex_enter(&sc->sc_lock);
- sc->sc_opening = 0;
- cv_signal(&sc->sc_opencv);
- mutex_exit(&sc->sc_lock);
-
- return error;
-
-bad:
- mutex_spin_enter(&tty_lock);
- CLR(tp->t_state, TS_BUSY);
- if (!ISSET(tp->t_state, TS_ISOPEN) && tp->t_wopen == 0) {
+fail_2:
+ if (cleanup) {
/*
- * We failed to open the device, and nobody else had it opened.
- * Clean up the state as appropriate.
+ * We failed to open the device, and nobody else had
+ * it opened. Clean up the state as appropriate.
*/
ucom_cleanup(sc);
}
- mutex_spin_exit(&tty_lock);
+
+ mutex_enter(&sc->sc_lock);
+
+fail_1:
+ sc->sc_state = state;
+ cv_signal(&sc->sc_statecv);
+ mutex_exit(&sc->sc_lock);
return error;
}
@@ -705,6 +737,7 @@
{
const int unit = UCOMUNIT(dev);
struct ucom_softc *sc = device_lookup_private(&ucom_cd, unit);
+ int error = 0;
UCOMHIST_FUNC(); UCOMHIST_CALLED();
@@ -714,21 +747,44 @@
return 0;
mutex_enter(&sc->sc_lock);
+ if (sc->sc_dying) {
+ DPRINTF("... dying", 0, 0, 0, 0);
+ mutex_exit(&sc->sc_lock);
+ return ENXIO;
+ }
+
+ /*
+ * Wait until any opens/closes have finished
+ */
+ while (sc->sc_state == UCOM_OPENING || sc->sc_state == UCOM_CLOSING) {
+ error = cv_wait_sig(&sc->sc_statecv, &sc->sc_lock);
+
+ if (sc->sc_dying)
+ error = EIO;
+
+ if (error) {
+ mutex_exit(&sc->sc_lock);
+ return error;
+ }
+ }
+
struct tty *tp = sc->sc_tty;
- while (sc->sc_closing)
- cv_wait(&sc->sc_opencv, &sc->sc_lock);
- sc->sc_closing = 1;
-
if (!ISSET(tp->t_state, TS_ISOPEN)) {
- goto out;
+ KASSERT(sc->sc_state == UCOM_ATTACHED);
+ mutex_exit(&sc->sc_lock);
+ return 0;
}
- sc->sc_refcnt++;
+ KASSERT(sc->sc_state == UCOM_OPEN);
+ sc->sc_state = UCOM_CLOSING;
+ mutex_exit(&sc->sc_lock);
Home |
Main Index |
Thread Index |
Old Index