Source-Changes-HG archive

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

[src/trunk]: src/sys Revert "constty(4): Make MP-safe."



details:   https://anonhg.NetBSD.org/src/rev/308ddb6fa122
branches:  trunk
changeset: 371730:308ddb6fa122
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Tue Oct 04 05:20:01 2022 +0000

description:
Revert "constty(4): Make MP-safe."

Something appears to be wrong with this.

diffstat:

 sys/dev/cons.c      |  107 ++++++++++++++----------------------------------
 sys/kern/subr_prf.c |   32 ++++----------
 sys/kern/tty.c      |  114 ++++++++-------------------------------------------
 sys/sys/tty.h       |    7 +--
 4 files changed, 60 insertions(+), 200 deletions(-)

diffs (truncated from 539 to 300 lines):

diff -r c35c5dffb29c -r 308ddb6fa122 sys/dev/cons.c
--- a/sys/dev/cons.c    Tue Oct 04 05:19:30 2022 +0000
+++ b/sys/dev/cons.c    Tue Oct 04 05:20:01 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cons.c,v 1.85 2022/10/04 05:19:30 riastradh Exp $      */
+/*     $NetBSD: cons.c,v 1.86 2022/10/04 05:20:01 riastradh Exp $      */
 
 /*
  * Copyright (c) 1988 University of Utah.
@@ -39,7 +39,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cons.c,v 1.85 2022/10/04 05:19:30 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cons.c,v 1.86 2022/10/04 05:20:01 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/proc.h>
@@ -54,8 +54,6 @@
 #include <sys/kauth.h>
 #include <sys/mutex.h>
 #include <sys/module.h>
-#include <sys/atomic.h>
-#include <sys/pserialize.h>
 
 #include <dev/cons.h>
 
@@ -69,8 +67,7 @@
 dev_type_poll(cnpoll);
 dev_type_kqfilter(cnkqfilter);
 
-static bool cn_redirect(dev_t *, int, int *, struct tty **);
-static void cn_release(struct tty *);
+static bool cn_redirect(dev_t *, int, int *);
 
 const struct cdevsw cons_cdevsw = {
        .d_open = cnopen,
@@ -89,7 +86,7 @@
 
 static struct kmutex cn_lock;
 
-struct tty *volatile constty;  /* virtual console output device */
+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. */
 
@@ -202,7 +199,6 @@
 int
 cnread(dev_t dev, struct uio *uio, int flag)
 {
-       struct tty *ctp = NULL;
        int error;
 
        /*
@@ -212,31 +208,25 @@
         * input (except a shell in single-user mode, but then,
         * one wouldn't TIOCCONS then).
         */
-       if (!cn_redirect(&dev, 1, &error, &ctp))
+       if (!cn_redirect(&dev, 1, &error))
                return error;
-       error = cdev_read(dev, uio, flag);
-       cn_release(ctp);
-       return error;
+       return cdev_read(dev, uio, flag);
 }
 
 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, &ctp))
+       if (!cn_redirect(&dev, 0, &error))
                return error;
-       error = cdev_write(dev, uio, flag);
-       cn_release(ctp);
-       return error;
+       return cdev_write(dev, uio, flag);
 }
 
 int
 cnioctl(dev_t dev, u_long cmd, void *data, int flag, struct lwp *l)
 {
-       struct tty *ctp = NULL;
        int error;
 
        error = 0;
@@ -245,41 +235,29 @@
         * Superuser can always use this to wrest control of console
         * output from the "virtual" console.
         */
-       if (cmd == TIOCCONS) {
-               struct tty *tp;
+       if (cmd == TIOCCONS && constty != NULL) {
+               error = kauth_authorize_device_tty(l->l_cred,
+                   KAUTH_DEVICE_TTY_VIRTUAL, constty);
+               if (!error)
+                       constty = NULL;
+               return (error);
+       }
 
-               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, tp);
-               if (!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, &ctp))
+       if (!cn_redirect(&dev, 0, &error))
                return error;
-       error = cdev_ioctl(dev, cmd, data, flag, l);
-       cn_release(ctp);
-       return error;
+       return cdev_ioctl(dev, cmd, data, flag, l);
 }
 
 /*ARGSUSED*/
 int
 cnpoll(dev_t dev, int events, struct lwp *l)
 {
-       struct tty *ctp = NULL;
        int error;
 
        /*
@@ -287,18 +265,15 @@
         * I don't want to think of the possible side effects
         * of console redirection here.
         */
-       if (!cn_redirect(&dev, 0, &error, &ctp))
+       if (!cn_redirect(&dev, 0, &error))
                return POLLHUP;
-       error = cdev_poll(dev, events, l);
-       cn_release(ctp);
-       return error;
+       return cdev_poll(dev, events, l);
 }
 
 /*ARGSUSED*/
 int
 cnkqfilter(dev_t dev, struct knote *kn)
 {
-       struct tty *ctp = NULL;
        int error;
 
        /*
@@ -306,11 +281,9 @@
         * I don't want to think of the possible side effects
         * of console redirection here.
         */
-       if (!cn_redirect(&dev, 0, &error, &ctp))
+       if (!cn_redirect(&dev, 0, &error))
                return error;
-       error = cdev_kqfilter(dev, kn);
-       cn_release(ctp);
-       return error;
+       return cdev_kqfilter(dev, kn);
 }
 
 int
@@ -456,46 +429,28 @@
 /*
  * 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, struct tty **ctpp)
+cn_redirect(dev_t *devp, int is_read, int *error)
 {
        dev_t dev = *devp;
-       struct tty *ctp;
-       int s;
-       bool ok;
 
        *error = ENXIO;
-       *ctpp = NULL;
-       s = pserialize_read_enter();
-       if ((ctp = atomic_load_consume(&constty)) != NULL && minor(dev) == 0 &&
+       if (constty != NULL && minor(dev) == 0 &&
            (cn_tab == NULL || (cn_tab->cn_pri != CN_REMOTE))) {
                if (is_read) {
                        *error = 0;
-                       ok = false;
-                       goto out;
+                       return false;
                }
-               tty_acquire(ctp);
-               *ctpp = ctp;
-               dev = ctp->t_dev;
-       } else if (cn_tab == NULL) {
-               ok = false;
-               goto out;
-       } else {
+               dev = constty->t_dev;
+       } else if (cn_tab == NULL)
+               return false;
+       else
                dev = cn_tab->cn_dev;
-       }
        *devp = dev;
-out:   pserialize_read_exit(s);
-       return ok;
-}
-
-static void
-cn_release(struct tty *ctp)
-{
-
-       if (ctp == NULL)
-               return;
-       tty_release(ctp);
+       return true;
 }
 
 MODULE(MODULE_CLASS_DRIVER, cons, NULL);
diff -r c35c5dffb29c -r 308ddb6fa122 sys/kern/subr_prf.c
--- a/sys/kern/subr_prf.c       Tue Oct 04 05:19:30 2022 +0000
+++ b/sys/kern/subr_prf.c       Tue Oct 04 05:20:01 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: subr_prf.c,v 1.190 2022/10/03 19:57:06 riastradh Exp $ */
+/*     $NetBSD: subr_prf.c,v 1.191 2022/10/04 05:20:01 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.190 2022/10/03 19:57:06 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: subr_prf.c,v 1.191 2022/10/04 05:20:01 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -103,6 +103,7 @@
  * 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). */
@@ -400,35 +401,22 @@
 static void
 putone(int c, int flags, struct tty *tp)
 {
-       struct tty *ctp;
-       int s;
+       if (panicstr)
+               constty = NULL;
 
-       /*
-        * 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;
+       if ((flags & TOCONS) && tp == NULL && constty) {
+               tp = constty;
                flags |= TOTTY;
        }
        if ((flags & TOTTY) && tp &&
            tputchar(c, flags, tp) < 0 &&
-           (flags & TOCONS))
-               atomic_cas_ptr(&constty, tp, NULL);
+           (flags & TOCONS) && tp == constty)
+               constty = NULL;
        if ((flags & TOLOG) &&
            c != '\0' && c != '\r' && c != 0177)
                logputchar(c);
-       if ((flags & TOCONS) && ctp != NULL && c != '\0')
+       if ((flags & TOCONS) && constty == NULL && c != '\0')
                (*v_putc)(c);
-
-       pserialize_read_exit(s);
 }



Home | Main Index | Thread Index | Old Index