tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

MP-safe /dev/console and /dev/constty



The attached patch series makes /dev/console and /dev/constty MP-safe,
with careful rules around constty access so it doesn't add contention
or mutexes to the paths used by printf(9).

This passes some manual testing in qemu with TIOCCONS on a viocon(4)
device, but since this touches a very intimate part of the kernel, I
figure I should post it for review and testing before barging ahead
and committing.

So please give this a whirl on whatever systems you have that are a
pain to get diagnostics out of, or on systems with X and xconsole or
xterm that use TIOCCONS, so we can find console/constty bugs early
before you need those diagnostics for something else!
From b1dac0b91583b768a457ae946751e7c69c4554a1 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sat, 1 Oct 2022 14:00:43 +0000
Subject: [PATCH 1/5] cons(9): New function cn_set_tab.

Increment of progress toward eliminating bare access to cn_tab so we
can make more things MP-safe without the kernel lock (and maybe some
day better formalize console detection and switching).
---
 sys/dev/cons.c | 16 ++++++++++++++++
 sys/dev/cons.h |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/sys/dev/cons.c b/sys/dev/cons.c
index eef38cbbc510..270fe6e1bbee 100644
--- a/sys/dev/cons.c
+++ b/sys/dev/cons.c
@@ -87,6 +87,22 @@ struct	tty *constty = NULL;	/* virtual console output device */
 struct	consdev *cn_tab;	/* physical console device info */
 struct	vnode *cn_devvp[2];	/* vnode for underlying device. */
 
+void
+cn_set_tab(struct consdev *tab)
+{
+
+	/*
+	 * This is a point that we should have KASSERT(cold) or add
+	 * synchronization in case this can happen after cold boot.
+	 * However, cn_tab initialization is so critical to any
+	 * diagnostics or debugging that we need to tread carefully
+	 * about introducing new ways to crash.  So let's put the
+	 * assertion in only after we've audited most or all of the
+	 * cn_tab updates.
+	 */
+	cn_tab = tab;
+}
+
 int
 cnopen(dev_t dev, int flag, int mode, struct lwp *l)
 {
diff --git a/sys/dev/cons.h b/sys/dev/cons.h
index 7ccdc5d6e74c..0d0356a3e1ff 100644
--- a/sys/dev/cons.h
+++ b/sys/dev/cons.h
@@ -76,6 +76,8 @@ struct consdev {
 extern	struct consdev constab[];
 extern	struct consdev *cn_tab;
 
+void	cn_set_tab(struct consdev *);
+
 void	cninit(void);
 int	cngetc(void);
 int	cngetsn(char *, int);

From 7d7b7315a8443f9c619d332ba1b897b34ec1f65b Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sat, 1 Oct 2022 14:03:22 +0000
Subject: [PATCH 2/5] cons(9): Serialize open and close.

Kernel lock wasn't enough for this -- cdevvp, vn_lock, or VOP_OPEN
could block, allowing another thread to re-enter open.
---
 sys/dev/cons.c | 49 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 7 deletions(-)

diff --git a/sys/dev/cons.c b/sys/dev/cons.c
index 270fe6e1bbee..1bb05a90fae1 100644
--- a/sys/dev/cons.c
+++ b/sys/dev/cons.c
@@ -53,6 +53,7 @@ __KERNEL_RCSID(0, "$NetBSD: cons.c,v 1.79 2022/08/22 00:20:56 riastradh Exp $");
 #include <sys/vnode.h>
 #include <sys/kauth.h>
 #include <sys/mutex.h>
+#include <sys/module.h>
 
 #include <dev/cons.h>
 
@@ -83,6 +84,8 @@ const struct cdevsw cons_cdevsw = {
 	.d_flag = D_TTY
 };
 
+static struct kmutex cn_lock;
+
 struct	tty *constty = NULL;	/* virtual console output device */
 struct	consdev *cn_tab;	/* physical console device info */
 struct	vnode *cn_devvp[2];	/* vnode for underlying device. */
@@ -113,8 +116,12 @@ cnopen(dev_t dev, int flag, int mode, struct lwp *l)
 	if (unit > 1)
 		return ENODEV;
 
-	if (cn_tab == NULL)
-		return (0);
+	mutex_enter(&cn_lock);
+
+	if (cn_tab == NULL) {
+		error = 0;
+		goto out;
+	}
 
 	/*
 	 * always open the 'real' console device, so we don't get nailed
@@ -145,15 +152,19 @@ cnopen(dev_t dev, int flag, int mode, struct lwp *l)
 		 */
 		panic("cnopen: cn_tab->cn_dev == dev");
 	}
-	if (cn_devvp[unit] != NULLVP)
-		return 0;
+	if (cn_devvp[unit] != NULLVP) {
+		error = 0;
+		goto out;
+	}
 	if ((error = cdevvp(cndev, &cn_devvp[unit])) != 0) {
 		printf("cnopen: unable to get vnode reference\n");
-		return error;
+		goto out;
 	}
 	vn_lock(cn_devvp[unit], LK_EXCLUSIVE | LK_RETRY);
 	error = VOP_OPEN(cn_devvp[unit], flag, kauth_cred_get());
 	VOP_UNLOCK(cn_devvp[unit]);
+
+out:	mutex_exit(&cn_lock);
 	return error;
 }
 
@@ -165,8 +176,12 @@ cnclose(dev_t dev, int flag, int mode, struct lwp *l)
 
 	unit = minor(dev);
 
-	if (cn_tab == NULL)
-		return (0);
+	mutex_enter(&cn_lock);
+
+	if (cn_tab == NULL) {
+		error = 0;
+		goto out;
+	}
 
 	vp = cn_devvp[unit];
 	cn_devvp[unit] = NULL;
@@ -174,6 +189,8 @@ cnclose(dev_t dev, int flag, int mode, struct lwp *l)
 	error = VOP_CLOSE(vp, flag, kauth_cred_get());
 	VOP_UNLOCK(vp);
 	vrele(vp);
+
+out:	mutex_exit(&cn_lock);
 	return error;
 }
 
@@ -433,3 +450,21 @@ cn_redirect(dev_t *devp, int is_read, int *error)
 	*devp = dev;
 	return true;
 }
+
+MODULE(MODULE_CLASS_DRIVER, cons, NULL);
+
+static int
+cons_modcmd(modcmd_t cmd, void *arg)
+{
+
+	switch (cmd) {
+	case MODULE_CMD_INIT:
+		mutex_init(&cn_lock, MUTEX_DEFAULT, IPL_NONE);
+		return 0;
+	case MODULE_CMD_FINI:
+		mutex_destroy(&cn_lock);
+		return 0;
+	default:
+		return ENOTTY;
+	}
+}

From 2360273d41e06cc8b0676824edc9643b17b69621 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sat, 1 Oct 2022 14:06:14 +0000
Subject: [PATCH 3/5] cons(9): Check the unit number on close too.

Races between multiple opens, some of which fail, might lead to
closing a bad unit number -- not clear there's a good way to prevent
this.
---
 sys/dev/cons.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sys/dev/cons.c b/sys/dev/cons.c
index 1bb05a90fae1..483d4d5b1c32 100644
--- a/sys/dev/cons.c
+++ b/sys/dev/cons.c
@@ -175,6 +175,8 @@ cnclose(dev_t dev, int flag, int mode, struct lwp *l)
 	int unit, error;
 
 	unit = minor(dev);
+	if (unit > 1)
+		return ENODEV;
 
 	mutex_enter(&cn_lock);
 

From 50494b30028537731f7e04d5a32883d93884ae1d Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sat, 1 Oct 2022 19:54:41 +0000
Subject: [PATCH 4/5] constty(4): Make MP-safe.

Access to the global constty variable is coordinated as follows:

1. Changing constty from null to nonnull is allowed only under the
   new adaptive constty_lock in thread context.  This serializes
   TIOCCONS operations.

2. Changing constty from nonnull to null is allowed in any context --
   printf(9) uses this to disable a broken constty.

3. Reading constty under constty_lock is allowed with
   atomic_load_relaxed, because while constty_lock is held, it can
   only be made null by some other thread/CPu, never made nonnull.

4. Reading constty outside constty_lock is allowed with
   atomic_load_consume in a pserialize read section -- constty is
   only ever made nonnull with atomic_store_release under
   constty_lock.  ttyclose will wait for all these pserialize read
   sections to complete before flushing the tty.

5. To continue to use a struct tty pointer in (4) after the
   pserialize read section has completed, caller must use tty_acquire
   during the pserialize read section and then tty_release when done.
   ttyclose will wait for all these references to drain before
   returning.

These access rules allow us to serialize TIOCCONS, and safely destroy
ttys, without putting any locks on the access paths like printf(9)
that use constty.  Once we set D_MPSAFE, operations on /dev/console
will contend only with other users of the same tty as constty, which
will be an improvement over contending with all other kernel lock
users in the system.
---
 sys/dev/cons.c      |  99 ++++++++++++++++++++++++++++-----------
 sys/kern/subr_prf.c |  26 ++++++++---
 sys/kern/tty.c      | 111 +++++++++++++++++++++++++++++++++++++-------
 sys/sys/tty.h       |   5 ++
 4 files changed, 190 insertions(+), 51 deletions(-)

diff --git a/sys/dev/cons.c b/sys/dev/cons.c
index 483d4d5b1c32..b84d04dd5888 100644
--- a/sys/dev/cons.c
+++ b/sys/dev/cons.c
@@ -54,6 +54,8 @@ __KERNEL_RCSID(0, "$NetBSD: cons.c,v 1.79 2022/08/22 00:20:56 riastradh Exp $");
 #include <sys/kauth.h>
 #include <sys/mutex.h>
 #include <sys/module.h>
+#include <sys/atomic.h>
+#include <sys/pserialize.h>
 
 #include <dev/cons.h>
 
@@ -67,7 +69,8 @@ dev_type_ioctl(cnioctl);
 dev_type_poll(cnpoll);
 dev_type_kqfilter(cnkqfilter);
 
-static bool cn_redirect(dev_t *, int, int *);
+static bool cn_redirect(dev_t *, int, int *, struct tty **);
+static void cn_release(struct tty *);
 
 const struct cdevsw cons_cdevsw = {
 	.d_open = cnopen,
@@ -86,7 +89,7 @@ const struct cdevsw cons_cdevsw = {
 
 static struct kmutex cn_lock;
 
-struct	tty *constty = NULL;	/* virtual console output device */
+struct	tty *volatile constty;	/* virtual console output device */
 struct	consdev *cn_tab;	/* physical console device info */
 struct	vnode *cn_devvp[2];	/* vnode for underlying device. */
 
@@ -199,6 +202,7 @@ out:	mutex_exit(&cn_lock);
 int
 cnread(dev_t dev, struct uio *uio, int flag)
 {
+	struct tty *ctp = NULL;
 	int error;
 
 	/*
@@ -208,25 +212,31 @@ cnread(dev_t dev, struct uio *uio, int flag)
 	 * input (except a shell in single-user mode, but then,
 	 * one wouldn't TIOCCONS then).
 	 */
-	if (!cn_redirect(&dev, 1, &error))
+	if (!cn_redirect(&dev, 1, &error, &ctp))
 		return error;
-	return cdev_read(dev, uio, flag);
+	error = cdev_read(dev, uio, flag);
+	cn_release(ctp);
+	return error;
 }
 
 int
 cnwrite(dev_t dev, struct uio *uio, int flag)
 {
+	struct tty *ctp = NULL;
 	int error;
 
 	/* Redirect output, if that's appropriate. */
-	if (!cn_redirect(&dev, 0, &error))
+	if (!cn_redirect(&dev, 0, &error, &ctp))
 		return error;
-	return cdev_write(dev, uio, flag);
+	error = cdev_write(dev, uio, flag);
+	cn_release(ctp);
+	return error;
 }
 
 int
 cnioctl(dev_t dev, u_long cmd, void *data, int flag, struct lwp *l)
 {
+	struct tty *ctp = NULL;
 	int error;
 
 	error = 0;
@@ -235,29 +245,41 @@ cnioctl(dev_t dev, u_long cmd, void *data, int flag, struct lwp *l)
 	 * Superuser can always use this to wrest control of console
 	 * output from the "virtual" console.
 	 */
-	if (cmd == TIOCCONS && constty != NULL) {
+	if (cmd == TIOCCONS) {
+		struct tty *tp;
+
+		mutex_enter(&constty_lock);
+		tp = atomic_load_relaxed(&constty);
+		if (tp == NULL) {
+			mutex_exit(&constty_lock);
+			goto passthrough; /* XXX ??? */
+		}
 		error = kauth_authorize_device_tty(l->l_cred,
-		    KAUTH_DEVICE_TTY_VIRTUAL, constty);
+		    KAUTH_DEVICE_TTY_VIRTUAL, tp);
 		if (!error)
-			constty = NULL;
-		return (error);
+			atomic_store_relaxed(&constty, NULL);
+		mutex_exit(&constty_lock);
+		return error;
 	}
-
+passthrough:
 	/*
 	 * Redirect the ioctl, if that's appropriate.
 	 * Note that strange things can happen, if a program does
 	 * ioctls on /dev/console, then the console is redirected
 	 * out from under it.
 	 */
-	if (!cn_redirect(&dev, 0, &error))
+	if (!cn_redirect(&dev, 0, &error, &ctp))
 		return error;
-	return cdev_ioctl(dev, cmd, data, flag, l);
+	error = cdev_ioctl(dev, cmd, data, flag, l);
+	cn_release(ctp);
+	return error;
 }
 
 /*ARGSUSED*/
 int
 cnpoll(dev_t dev, int events, struct lwp *l)
 {
+	struct tty *ctp = NULL;
 	int error;
 
 	/*
@@ -265,15 +287,18 @@ cnpoll(dev_t dev, int events, struct lwp *l)
 	 * I don't want to think of the possible side effects
 	 * of console redirection here.
 	 */
-	if (!cn_redirect(&dev, 0, &error))
+	if (!cn_redirect(&dev, 0, &error, &ctp))
 		return POLLHUP;
-	return cdev_poll(dev, events, l);
+	error = cdev_poll(dev, events, l);
+	cn_release(ctp);
+	return error;
 }
 
 /*ARGSUSED*/
 int
 cnkqfilter(dev_t dev, struct knote *kn)
 {
+	struct tty *ctp = NULL;
 	int error;
 
 	/*
@@ -281,9 +306,11 @@ cnkqfilter(dev_t dev, struct knote *kn)
 	 * I don't want to think of the possible side effects
 	 * of console redirection here.
 	 */
-	if (!cn_redirect(&dev, 0, &error))
+	if (!cn_redirect(&dev, 0, &error, &ctp))
 		return error;
-	return cdev_kqfilter(dev, kn);
+	error = cdev_kqfilter(dev, kn);
+	cn_release(ctp);
+	return error;
 }
 
 int
@@ -429,28 +456,46 @@ cnhalt(void)
 /*
  * Redirect output, if that's appropriate.  If there's no real console,
  * return ENXIO.
- *
- * Call with tty_mutex held.
  */
 static bool
-cn_redirect(dev_t *devp, int is_read, int *error)
+cn_redirect(dev_t *devp, int is_read, int *error, struct tty **ctpp)
 {
 	dev_t dev = *devp;
+	struct tty *ctp;
+	int s;
+	bool ok;
 
 	*error = ENXIO;
-	if (constty != NULL && minor(dev) == 0 &&
+	*ctpp = NULL;
+	s = pserialize_read_enter();
+	if ((ctp = atomic_load_consume(&constty)) != NULL && minor(dev) == 0 &&
 	    (cn_tab == NULL || (cn_tab->cn_pri != CN_REMOTE))) {
 		if (is_read) {
 			*error = 0;
-			return false;
+			ok = false;
+			goto out;
 		}
-		dev = constty->t_dev;
-	} else if (cn_tab == NULL)
-		return false;
-	else
+		tty_acquire(ctp);
+		*ctpp = ctp;
+		dev = ctp->t_dev;
+	} else if (cn_tab == NULL) {
+		ok = false;
+		goto out;
+	} else {
 		dev = cn_tab->cn_dev;
+	}
 	*devp = dev;
-	return true;
+out:	pserialize_read_exit(s);
+	return ok;
+}
+
+static void
+cn_release(struct tty *ctp)
+{
+
+	if (ctp == NULL)
+		return;
+	tty_release(ctp);
 }
 
 MODULE(MODULE_CLASS_DRIVER, cons, NULL);
diff --git a/sys/kern/subr_prf.c b/sys/kern/subr_prf.c
index a41d41f5c69b..e87e6efc8501 100644
--- a/sys/kern/subr_prf.c
+++ b/sys/kern/subr_prf.c
@@ -103,7 +103,6 @@ static void	 kprintf_internal(const char *, int, void *, char *, ...);
  * globals
  */
 
-extern	struct tty *constty;	/* pointer to console "window" tty */
 extern	int log_open;	/* subr_log: is /dev/klog open? */
 const	char *panicstr; /* arg to first call to panic (used as a flag
 			   to indicate that panic has already been called). */
@@ -401,22 +400,35 @@ addlog(const char *fmt, ...)
 static void
 putone(int c, int flags, struct tty *tp)
 {
+	struct tty *ctp;
+	int s;
+
+	/*
+	 * Ensure whatever constty points to can't go away while we're
+	 * trying to use it.
+	 */
+	s = pserialize_read_enter();
+
 	if (panicstr)
-		constty = NULL;
+		atomic_store_relaxed(&constty, NULL);
 
-	if ((flags & TOCONS) && tp == NULL && constty) {
-		tp = constty;
+	if ((flags & TOCONS) &&
+	    (ctp = atomic_load_consume(&constty)) != NULL &&
+	    tp == NULL) {
+		tp = ctp;
 		flags |= TOTTY;
 	}
 	if ((flags & TOTTY) && tp &&
 	    tputchar(c, flags, tp) < 0 &&
-	    (flags & TOCONS) && tp == constty)
-		constty = NULL;
+	    (flags & TOCONS))
+		atomic_cas_ptr(&constty, tp, NULL);
 	if ((flags & TOLOG) &&
 	    c != '\0' && c != '\r' && c != 0177)
 	    	logputchar(c);
-	if ((flags & TOCONS) && constty == NULL && c != '\0')
+	if ((flags & TOCONS) && ctp != NULL && c != '\0')
 		(*v_putc)(c);
+
+	pserialize_read_exit(s);
 }
 
 static void
diff --git a/sys/kern/tty.c b/sys/kern/tty.c
index d02691b10035..2be6a2b6f356 100644
--- a/sys/kern/tty.c
+++ b/sys/kern/tty.c
@@ -99,6 +99,9 @@ __KERNEL_RCSID(0, "$NetBSD: tty.c,v 1.301 2022/04/07 21:46:51 riastradh Exp $");
 #include <sys/module.h>
 #include <sys/bitops.h>
 #include <sys/compat_stub.h>
+#include <sys/atomic.h>
+#include <sys/condvar.h>
+#include <sys/pserialize.h>
 
 #ifdef COMPAT_60
 #include <compat/sys/ttycom.h>
@@ -209,6 +212,9 @@ static void *tty_sigsih;
 struct ttylist_head ttylist = TAILQ_HEAD_INITIALIZER(ttylist);
 int tty_count;
 kmutex_t tty_lock;
+kmutex_t constty_lock;
+static struct pserialize *constty_psz;
+static kcondvar_t ttyref_cv;
 
 struct ptm_pty *ptm = NULL;
 
@@ -442,13 +448,28 @@ ttycancel(struct tty *tp)
 int
 ttyclose(struct tty *tp)
 {
-	extern struct tty *constty;	/* Temporary virtual console. */
 	struct session *sess;
 
-	mutex_spin_enter(&tty_lock);
+	/*
+	 * Make sure this is not the constty.  Without constty_lock it
+	 * is always allowed to transition from nonnull to null.
+	 */
+	(void)atomic_cas_ptr(&constty, tp, NULL);
+
+	/*
+	 * We don't know if this has _ever_ been the constty: another
+	 * thread may have kicked it out as constty before we started
+	 * to close.
+	 *
+	 * So we wait for all users that might be acquiring references
+	 * to finish doing so -- after that, no more references can be
+	 * made, at which point we can safely flush the tty, wait for
+	 * the existing references to drain, and finally free or reuse
+	 * the tty.
+	 */
+	pserialize_perform(constty_psz);
 
-	if (constty == tp)
-		constty = NULL;
+	mutex_spin_enter(&tty_lock);
 
 	ttyflush(tp, FREAD | FWRITE);
 
@@ -458,6 +479,9 @@ ttyclose(struct tty *tp)
 	sess = tp->t_session;
 	tp->t_session = NULL;
 
+	while (tp->t_refcnt)
+		cv_wait(&ttyref_cv, &tty_lock);
+
 	mutex_spin_exit(&tty_lock);
 
 	if (sess != NULL) {
@@ -473,6 +497,44 @@ ttyclose(struct tty *tp)
 		ndflush(q, (q)->c_cc);					\
 }
 
+/*
+ * tty_acquire(tp), tty_release(tp)
+ *
+ *	Acquire a reference to tp that prevents it from being closed
+ *	until released.  Caller must guarantee tp has not yet been
+ *	closed, e.g. by obtaining tp from constty during a pserialize
+ *	read section.  Caller must not hold tty_lock.
+ */
+void
+tty_acquire(struct tty *tp)
+{
+	unsigned refcnt __diagused;
+
+	refcnt = atomic_inc_uint_nv(&tp->t_refcnt);
+	KASSERT(refcnt < UINT_MAX);
+}
+
+void
+tty_release(struct tty *tp)
+{
+	unsigned old, new;
+
+	KDASSERT(mutex_ownable(&tty_lock));
+
+	do {
+		old = atomic_load_relaxed(&tp->t_refcnt);
+		if (old == 1) {
+			mutex_spin_enter(&tty_lock);
+			if (atomic_dec_uint_nv(&tp->t_refcnt) == 0)
+				cv_broadcast(&ttyref_cv);
+			mutex_spin_exit(&tty_lock);
+			return;
+		}
+		KASSERT(old != 0);
+		new = old - 1;
+	} while (atomic_cas_uint(&tp->t_refcnt, old, new) != old);
+}
+
 /*
  * This macro is used in canonical mode input processing, where a read
  * request shall not return unless a 'line delimiter' ('\n') or 'break'
@@ -941,7 +1003,6 @@ ttyoutput(int c, struct tty *tp)
 int
 ttioctl(struct tty *tp, u_long cmd, void *data, int flag, struct lwp *l)
 {
-	extern struct tty *constty;	/* Temporary virtual console. */
 	struct proc *p;
 	struct linesw	*lp;
 	int		s, error;
@@ -1044,32 +1105,46 @@ ttioctl(struct tty *tp, u_long cmd, void *data, int flag, struct lwp *l)
 		mutex_spin_exit(&tty_lock);
 		break;
 	}
-	case TIOCCONS:			/* become virtual console */
-		if (*(int *)data) {
-			if (constty && constty != tp &&
-			    ISSET(constty->t_state, TS_CARR_ON | TS_ISOPEN) ==
-			    (TS_CARR_ON | TS_ISOPEN))
-				return EBUSY;
+	case TIOCCONS: {		/* become virtual console */
+		struct tty *ctp;
+		mutex_enter(&constty_lock);
+		error = 0;
+		ctp = atomic_load_relaxed(&constty);
+		if (*(int *)data) do {
+			if (ctp != NULL && ctp != tp &&
+			    ISSET(ctp->t_state, TS_CARR_ON | TS_ISOPEN) ==
+			    (TS_CARR_ON | TS_ISOPEN)) {
+				error = EBUSY;
+				break;
+			}
 
 			pb = pathbuf_create("/dev/console");
 			if (pb == NULL) {
-				return ENOMEM;
+				error = ENOMEM;
+				break;
 			}
 			NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF, pb);
 			if ((error = namei(&nd)) != 0) {
 				pathbuf_destroy(pb);
-				return error;
+				break;
 			}
 			error = VOP_ACCESS(nd.ni_vp, VREAD, l->l_cred);
 			vput(nd.ni_vp);
 			pathbuf_destroy(pb);
 			if (error)
-				return error;
+				break;
 
-			constty = tp;
-		} else if (tp == constty)
-			constty = NULL;
+			KASSERT(atomic_load_relaxed(&constty) == tp ||
+			    atomic_load_relaxed(&constty) == NULL);
+			atomic_store_release(&constty, tp);
+		} while (0); else if (tp == ctp) {
+			atomic_store_relaxed(&constty, NULL);
+		}
+		mutex_exit(&constty_lock);
+		if (error)
+			return error;
 		break;
+	}
 	case TIOCDRAIN:			/* wait till output drained */
 		if ((error = ttywait(tp)) != 0)
 			return (error);
@@ -2969,6 +3044,8 @@ tty_init(void)
 {
 
 	mutex_init(&tty_lock, MUTEX_DEFAULT, IPL_VM);
+	mutex_init(&constty_lock, MUTEX_DEFAULT, IPL_NONE);
+	constty_psz = pserialize_create();
 	tty_sigsih = softint_establish(SOFTINT_CLOCK, ttysigintr, NULL);
 	KASSERT(tty_sigsih != NULL);
 
diff --git a/sys/sys/tty.h b/sys/sys/tty.h
index 17a3a97e5df0..8adc032164f1 100644
--- a/sys/sys/tty.h
+++ b/sys/sys/tty.h
@@ -149,6 +149,7 @@ struct tty {
 	int	t_sigcount;		/* # pending signals */
 	TAILQ_ENTRY(tty) t_sigqueue;	/* entry on pending signal list */
 	void	*t_softc;		/* pointer to driver's softc. */
+	volatile unsigned t_refcnt;	/* reference count for constty */
 };
 
 #ifdef TTY_ALLOW_PRIVATE
@@ -252,6 +253,8 @@ TAILQ_HEAD(ttylist_head, tty);		/* the ttylist is a TAILQ */
 #ifdef _KERNEL
 
 extern kmutex_t	tty_lock;
+extern kmutex_t	constty_lock;
+extern struct tty *volatile constty;
 
 extern	int tty_count;			/* number of ttys in global ttylist */
 extern	struct ttychars ttydefaults;
@@ -313,6 +316,8 @@ void	 tty_free(struct tty *);
 u_char	*firstc(struct clist *, int *);
 bool	 ttypull(struct tty *);
 int	 tty_unit(dev_t);
+void	 tty_acquire(struct tty *);
+void	 tty_release(struct tty *);
 
 int	clalloc(struct clist *, int, int);
 void	clfree(struct clist *);

From c081e0374b79c6276fabaf868625bc576b20cc1b Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sat, 1 Oct 2022 20:03:28 +0000
Subject: [PATCH 5/5] console(4), constty(4): Rip off the kernel lock.

---
 sys/dev/cons.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sys/dev/cons.c b/sys/dev/cons.c
index b84d04dd5888..f4f9a1602221 100644
--- a/sys/dev/cons.c
+++ b/sys/dev/cons.c
@@ -84,7 +84,7 @@ const struct cdevsw cons_cdevsw = {
 	.d_mmap = nommap,
 	.d_kqfilter = cnkqfilter,
 	.d_discard = nodiscard,
-	.d_flag = D_TTY
+	.d_flag = D_TTY|D_MPSAFE,
 };
 
 static struct kmutex cn_lock;


Home | Main Index | Thread Index | Old Index