Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys constty(4): Make MP-safe.
details: https://anonhg.NetBSD.org/src/rev/b5376a130b6b
branches: trunk
changeset: 371758:b5376a130b6b
user: riastradh <riastradh%NetBSD.org@localhost>
date: Thu Oct 06 19:58:41 2022 +0000
description:
constty(4): Make MP-safe.
Access to the global constty variable is coordinated as follows:
1. Setting constty to nonnull, with atomic_store_release, is allowed
only under the new adaptive constty_lock in thread context. This
serializes TIOCCONS operations and ensures unlocked readers can
safely use a constty pointer read with atomic_load_consume.
2. Changing constty from nonnull to null, with atomic_cas_ptr, 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, in (1).
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.
Changes second time around:
- Fix initialization of ok in cons.c cn_redirect.
- Fix reversed sense of conditional in subr_prf.c putone.
diffstat:
sys/dev/cons.c | 97 +++++++++++++++++++++++++++++++------------
sys/kern/subr_prf.c | 32 ++++++++++----
sys/kern/tty.c | 114 +++++++++++++++++++++++++++++++++++++++++++--------
sys/sys/tty.h | 7 ++-
4 files changed, 194 insertions(+), 56 deletions(-)
diffs (truncated from 534 to 300 lines):
diff -r 45995a8ae14f -r b5376a130b6b sys/dev/cons.c
--- a/sys/dev/cons.c Thu Oct 06 19:38:54 2022 +0000
+++ b/sys/dev/cons.c Thu Oct 06 19:58:41 2022 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: cons.c,v 1.86 2022/10/04 05:20:01 riastradh Exp $ */
+/* $NetBSD: cons.c,v 1.87 2022/10/06 19:58:41 riastradh Exp $ */
/*
* Copyright (c) 1988 University of Utah.
@@ -39,7 +39,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cons.c,v 1.86 2022/10/04 05:20:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cons.c,v 1.87 2022/10/06 19:58:41 riastradh Exp $");
#include <sys/param.h>
#include <sys/proc.h>
@@ -54,6 +54,8 @@
#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_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 @@
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 @@
int
cnread(dev_t dev, struct uio *uio, int flag)
{
+ struct tty *ctp = NULL;
int error;
/*
@@ -208,25 +212,31 @@
* 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 @@
* 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 @@
* 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 @@
* 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,44 @@
/*
* 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 = false;
*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;
+ goto out;
}
- dev = constty->t_dev;
+ tty_acquire(ctp);
+ *ctpp = ctp;
+ dev = ctp->t_dev;
} else if (cn_tab == NULL)
- return false;
+ goto out;
else
dev = cn_tab->cn_dev;
+ ok = true;
*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 -r 45995a8ae14f -r b5376a130b6b sys/kern/subr_prf.c
--- a/sys/kern/subr_prf.c Thu Oct 06 19:38:54 2022 +0000
+++ b/sys/kern/subr_prf.c Thu Oct 06 19:58:41 2022 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: subr_prf.c,v 1.191 2022/10/04 05:20:01 riastradh Exp $ */
+/* $NetBSD: subr_prf.c,v 1.192 2022/10/06 19:58:41 riastradh Exp $ */
/*-
* Copyright (c) 1986, 1988, 1991, 1993
@@ -37,7 +37,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: subr_prf.c,v 1.191 2022/10/04 05:20:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: subr_prf.c,v 1.192 2022/10/06 19:58:41 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_ddb.h"
@@ -103,7 +103,6 @@
* 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 @@
static void
putone(int c, int flags, struct tty *tp)
{
- if (panicstr)
- constty = NULL;
+ struct tty *ctp;
+ int s;
- if ((flags & TOCONS) && tp == NULL && constty) {
- tp = constty;
+ /*
+ * Ensure whatever constty points to can't go away while we're
+ * trying to use it.
+ */
+ s = pserialize_read_enter();
+
+ if (panicstr)
+ atomic_store_relaxed(&constty, NULL);
+
+ 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 -r 45995a8ae14f -r b5376a130b6b sys/kern/tty.c
--- a/sys/kern/tty.c Thu Oct 06 19:38:54 2022 +0000
+++ b/sys/kern/tty.c Thu Oct 06 19:58:41 2022 +0000
@@ -1,4 +1,4 @@
Home |
Main Index |
Thread Index |
Old Index