Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/dev/usb ucom(4): Rework open/close/attach/detach logic.
details: https://anonhg.NetBSD.org/src/rev/cdbd531e875a
branches: trunk
changeset: 364533:cdbd531e875a
user: riastradh <riastradh%NetBSD.org@localhost>
date: Mon Mar 28 12:42:37 2022 +0000
description:
ucom(4): Rework open/close/attach/detach logic.
- Defer sleep after hangup until open.
No need to make close hang; we just need to make sure some time has
passed before we next try to open.
This changes the wchan for the sleep. Oh well.
- Use .d_cfdriver/devtounit/cancel to resolve races between attach,
detach, open, close, and revoke.
- Use a separate .sc_closing flag instead of a UCOM_CLOSING state.
ucomcancel/ucomclose owns this flag, and it may be set in any state
(except UCOM_DEAD). UCOM_OPENING remains owned by ucomopen, which
might be interrupted by cancel/close.
- Rework error branches in ucomopen. Much simpler this way.
- Nix unnecessary reference counting.
diffstat:
sys/dev/usb/ucom.c | 596 +++++++++++++++++++++-------------------------------
1 files changed, 246 insertions(+), 350 deletions(-)
diffs (truncated from 978 to 300 lines):
diff -r 02a22eb3e335 -r cdbd531e875a sys/dev/usb/ucom.c
--- a/sys/dev/usb/ucom.c Mon Mar 28 12:41:17 2022 +0000
+++ b/sys/dev/usb/ucom.c Mon Mar 28 12:42:37 2022 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ucom.c,v 1.129 2021/06/24 08:20:42 riastradh Exp $ */
+/* $NetBSD: ucom.c,v 1.130 2022/03/28 12:42:37 riastradh 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.129 2021/06/24 08:20:42 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ucom.c,v 1.130 2022/03/28 12:42:37 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -180,11 +180,10 @@
UCOM_DEAD,
UCOM_ATTACHED,
UCOM_OPENING,
- UCOM_CLOSING,
UCOM_OPEN
} sc_state;
- int sc_refcnt;
- bool sc_dying; /* disconnecting */
+ bool sc_closing; /* software is closing */
+ bool sc_dying; /* hardware is gone */
struct pps_state sc_pps_state; /* pps state */
@@ -192,10 +191,12 @@
kmutex_t sc_lock;
kcondvar_t sc_statecv;
- kcondvar_t sc_detachcv;
+
+ struct timeval sc_hup_time;
};
dev_type_open(ucomopen);
+dev_type_cancel(ucomcancel);
dev_type_close(ucomclose);
dev_type_read(ucomread);
dev_type_write(ucomwrite);
@@ -206,6 +207,7 @@
const struct cdevsw ucom_cdevsw = {
.d_open = ucomopen,
+ .d_cancel = ucomcancel,
.d_close = ucomclose,
.d_read = ucomread,
.d_write = ucomwrite,
@@ -216,16 +218,16 @@
.d_mmap = nommap,
.d_kqfilter = ttykqfilter,
.d_discard = nodiscard,
+ .d_cfdriver = &ucom_cd,
+ .d_devtounit = dev_minor_unit,
.d_flag = D_TTY | D_MPSAFE
};
-static void ucom_cleanup(struct ucom_softc *);
+static void ucom_cleanup(struct ucom_softc *, int);
static int ucomparam(struct tty *, struct termios *);
static int ucomhwiflow(struct tty *, int);
static void ucomstart(struct tty *);
static void ucom_shutdown(struct ucom_softc *);
-static int ucom_do_ioctl(struct ucom_softc *, u_long, void *,
- int, struct lwp *);
static void ucom_dtr(struct ucom_softc *, int);
static void ucom_rts(struct ucom_softc *, int);
static void ucom_break(struct ucom_softc *, int);
@@ -288,14 +290,13 @@
sc->sc_mcr = 0;
sc->sc_tx_stopped = 0;
sc->sc_swflags = 0;
- sc->sc_refcnt = 0;
+ sc->sc_closing = false;
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_statecv, "ucomstate");
- cv_init(&sc->sc_detachcv, "ucomdtch");
SIMPLEQ_INIT(&sc->sc_ibuff_empty);
SIMPLEQ_INIT(&sc->sc_ibuff_full);
@@ -366,7 +367,6 @@
aprint_error_dev(self, "couldn't establish power handler\n");
sc->sc_state = UCOM_ATTACHED;
-
return;
fail_2:
@@ -375,8 +375,8 @@
if (ub->ub_xfer)
usbd_destroy_xfer(ub->ub_xfer);
}
-
usbd_close_pipe(sc->sc_bulkout_pipe);
+ sc->sc_bulkout_pipe = NULL;
fail_1:
for (ub = &sc->sc_ibuff[0]; ub != &sc->sc_ibuff[UCOM_IN_BUFFS];
@@ -385,11 +385,10 @@
usbd_destroy_xfer(ub->ub_xfer);
}
usbd_close_pipe(sc->sc_bulkin_pipe);
+ sc->sc_bulkin_pipe = NULL;
fail_0:
aprint_error_dev(self, "attach failed, error=%d\n", error);
-
- return;
}
int
@@ -406,49 +405,21 @@
(uintptr_t)tp, 0);
DPRINTF("... pipe=%jd,%jd", sc->sc_bulkin_no, sc->sc_bulkout_no, 0, 0);
+ /* Prevent new opens from hanging. */
mutex_enter(&sc->sc_lock);
sc->sc_dying = true;
mutex_exit(&sc->sc_lock);
pmf_device_deregister(self);
- if (sc->sc_bulkin_pipe != NULL) {
- usbd_abort_pipe(sc->sc_bulkin_pipe);
- }
- if (sc->sc_bulkout_pipe != NULL) {
- 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;
- }
+ /* tty is now off. */
+ if (tp != NULL) {
+ mutex_spin_enter(&tty_lock);
+ CLR(tp->t_state, TS_CARR_ON);
+ CLR(tp->t_cflag, CLOCAL | MDMBUF);
+ mutex_spin_exit(&tty_lock);
}
- sc->sc_refcnt--;
- while (sc->sc_refcnt >= 0) {
- /* Wake up anyone waiting */
- if (tp != NULL) {
- mutex_spin_enter(&tty_lock);
- CLR(tp->t_state, TS_CARR_ON);
- CLR(tp->t_cflag, CLOCAL | MDMBUF);
- ttyflush(tp, FREAD|FWRITE);
- mutex_spin_exit(&tty_lock);
- }
- /* Wait for processes to go away. */
- if (cv_timedwait(&sc->sc_detachcv, &sc->sc_lock, hz * 60))
- aprint_error_dev(self, ": didn't detach\n");
- }
-
- softint_disestablish(sc->sc_si);
- mutex_exit(&sc->sc_lock);
-
/* locate the major number */
maj = cdevsw_lookup_major(&ucom_cdevsw);
@@ -459,6 +430,8 @@
vdevgone(maj, mn | UCOMDIALOUT_MASK, mn | UCOMDIALOUT_MASK, VCHR);
vdevgone(maj, mn | UCOMCALLUNIT_MASK, mn | UCOMCALLUNIT_MASK, VCHR);
+ softint_disestablish(sc->sc_si);
+
/* Detach and free the tty. */
if (tp != NULL) {
tty_detach(tp);
@@ -491,7 +464,6 @@
mutex_destroy(&sc->sc_lock);
cv_destroy(&sc->sc_statecv);
- cv_destroy(&sc->sc_detachcv);
return 0;
}
@@ -509,11 +481,23 @@
*/
if (ISSET(tp->t_cflag, HUPCL)) {
ucom_dtr(sc, 0);
- /* XXX will only timeout */
- (void) kpause(ttclos, false, hz, NULL);
+ mutex_enter(&sc->sc_lock);
+ microuptime(&sc->sc_hup_time);
+ sc->sc_hup_time.tv_sec++;
+ mutex_exit(&sc->sc_lock);
}
}
+/*
+ * ucomopen(dev, flag, mode, l)
+ *
+ * Called when anyone tries to open /dev/ttyU? for an existing
+ * ucom? instance that has completed attach. The attach may have
+ * failed, though, or there may be concurrent detach or close in
+ * progress, so fail if attach failed (no sc_tty) or detach has
+ * begun (sc_dying), or wait if there's a concurrent close in
+ * progress before reopening.
+ */
int
ucomopen(dev_t dev, int flag, int mode, struct lwp *l)
{
@@ -523,14 +507,11 @@
UCOMHIST_FUNC(); UCOMHIST_CALLED();
- if (sc == NULL)
- return ENXIO;
-
mutex_enter(&sc->sc_lock);
if (sc->sc_dying) {
DPRINTF("... dying", 0, 0, 0, 0);
mutex_exit(&sc->sc_lock);
- return EIO;
+ return ENXIO;
}
if (!device_is_active(sc->sc_dev)) {
@@ -539,6 +520,11 @@
}
struct tty *tp = sc->sc_tty;
+ if (tp == NULL) {
+ DPRINTF("... not attached", 0, 0, 0, 0);
+ mutex_exit(&sc->sc_lock);
+ return ENXIO;
+ }
DPRINTF("unit=%jd, tp=%#jx", unit, (uintptr_t)tp, 0, 0);
@@ -548,40 +534,64 @@
}
/*
+ * If the previous use had set DTR on close, wait up to one
+ * second for the other side to notice we hung up. After
+ * sleeping, the tty may have been revoked, so restart the
+ * whole operation.
+ *
+ * XXX The wchan is not ttclose but maybe should be.
+ */
+ if (timerisset(&sc->sc_hup_time)) {
+ struct timeval now, delta;
+ int ms, ticks;
+
+ microuptime(&now);
+ if (timercmp(&now, &sc->sc_hup_time, <)) {
+ timersub(&sc->sc_hup_time, &now, &delta);
+ ms = MIN(INT_MAX - 1000, delta.tv_sec*1000);
+ ms += howmany(delta.tv_usec, 1000);
+ ticks = MAX(1, MIN(INT_MAX, mstohz(ms)));
+ error = cv_timedwait_sig(&sc->sc_statecv, &sc->sc_lock,
+ ticks);
+ mutex_exit(&sc->sc_lock);
+ return error ? error : ERESTART;
+ }
+ }
+
+ /*
* Wait while the device is initialized by the
* first opener or cleaned up by the last closer.
*/
- 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;
+ if (state == UCOM_OPENING || sc->sc_closing) {
+ if (flag & FNONBLOCK)
+ error = EWOULDBLOCK;
+ else
+ error = cv_wait_sig(&sc->sc_statecv, &sc->sc_lock);
+ mutex_exit(&sc->sc_lock);
+ return error ? error : ERESTART;
}
Home |
Main Index |
Thread Index |
Old Index