[src/trunk]: src/sys Load struct filedesc::fd_dt with atomic_load_consume.

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/211c9d414133
branches:  trunk
changeset: 968901:211c9d414133
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sat Feb 01 02:23:03 2020 +0000

description:
Load struct filedesc::fd_dt with atomic_load_consume.

Exceptions: when fd_refcnt <= 1, or when holding fd_lock.

While here:

- Restore KASSERT(mutex_owned(&fdp->fd_lock)) in fd_unused.
  => This is used only in fd_close and fd_abort, where it holds.
- Move bounds check assertion in fd_putfile to where it matters.
- Store fd_dt with atomic_store_release.
- Move load of fd_dt under lock in knote_fdclose.
- Omit membar_consumer in fdesc_readdir.
  => atomic_load_consume serves the same purpose now.
  => Was needed only on alpha anyway.

diffstat:

 sys/compat/netbsd32/netbsd32_ioctl.c |   7 ++--
 sys/ddb/db_xxx.c                     |   7 ++--
 sys/kern/kern_descrip.c              |  52 ++++++++++++++++++------------------
 sys/kern/kern_event.c                |   6 ++--
 sys/kern/kern_sig.c                  |   6 ++--
 sys/kern/subr_exec_fd.c              |   9 +++--
 sys/kern/sys_aio.c                   |   6 ++--
 sys/kern/sys_descrip.c               |   8 +++--
 sys/kern/sys_select.c                |   6 ++--
 sys/kern/uipc_socket2.c              |   6 ++--
 sys/kern/uipc_usrreq.c               |   8 +++--
 sys/miscfs/fdesc/fdesc_vnops.c       |   9 ++---
 sys/miscfs/procfs/procfs_vnops.c     |   7 ++--
 13 files changed, 72 insertions(+), 65 deletions(-)

diffs (truncated from 590 to 300 lines):

diff -r f898bdc7ff98 -r 211c9d414133 sys/compat/netbsd32/netbsd32_ioctl.c
--- a/sys/compat/netbsd32/netbsd32_ioctl.c      Fri Jan 31 23:12:13 2020 +0000
+++ b/sys/compat/netbsd32/netbsd32_ioctl.c      Sat Feb 01 02:23:03 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: netbsd32_ioctl.c,v 1.106 2019/11/18 04:17:08 rin Exp $ */
+/*     $NetBSD: netbsd32_ioctl.c,v 1.107 2020/02/01 02:23:03 riastradh Exp $   */
 
 /*
  * Copyright (c) 1998, 2001 Matthew R. Green
@@ -31,13 +31,14 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: netbsd32_ioctl.c,v 1.106 2019/11/18 04:17:08 rin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: netbsd32_ioctl.c,v 1.107 2020/02/01 02:23:03 riastradh Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_ntp.h"
 #endif
 
 #include <sys/param.h>
+#include <sys/atomic.h>
 #include <sys/systm.h>
 #include <sys/filedesc.h>
 #include <sys/ioctl.h>
@@ -1132,7 +1133,7 @@
                goto out;
        }
 
-       ff = fdp->fd_dt->dt_ff[SCARG(uap, fd)];
+       ff = atomic_load_consume(&fdp->fd_dt)->dt_ff[SCARG(uap, fd)];
        switch (com = SCARG(uap, com)) {
        case FIOCLEX:
                ff->ff_exclose = true;
diff -r f898bdc7ff98 -r 211c9d414133 sys/ddb/db_xxx.c
--- a/sys/ddb/db_xxx.c  Fri Jan 31 23:12:13 2020 +0000
+++ b/sys/ddb/db_xxx.c  Sat Feb 01 02:23:03 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: db_xxx.c,v 1.71 2015/02/27 00:47:30 ozaki-r Exp $      */
+/*     $NetBSD: db_xxx.c,v 1.72 2020/02/01 02:23:04 riastradh Exp $    */
 
 /*
  * Copyright (c) 1982, 1986, 1989, 1991, 1993
@@ -37,7 +37,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: db_xxx.c,v 1.71 2015/02/27 00:47:30 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: db_xxx.c,v 1.72 2020/02/01 02:23:04 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_kgdb.h"
@@ -50,6 +50,7 @@
 #endif
 
 #include <sys/param.h>
+#include <sys/atomic.h>
 #include <sys/systm.h>
 #include <sys/kernel.h>
 #include <sys/proc.h>
@@ -173,7 +174,7 @@
        p = (struct proc *) (uintptr_t) addr;
 
        fdp = p->p_fd;
-       dt = fdp->fd_dt;
+       dt = atomic_load_consume(&fdp->fd_dt);
        for (i = 0; i < dt->dt_nfiles; i++) {
                if ((ff = dt->dt_ff[i]) == NULL)
                        continue;
diff -r f898bdc7ff98 -r 211c9d414133 sys/kern/kern_descrip.c
--- a/sys/kern/kern_descrip.c   Fri Jan 31 23:12:13 2020 +0000
+++ b/sys/kern/kern_descrip.c   Sat Feb 01 02:23:03 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kern_descrip.c,v 1.243 2019/02/20 19:42:14 christos Exp $      */
+/*     $NetBSD: kern_descrip.c,v 1.244 2020/02/01 02:23:04 riastradh Exp $     */
 
 /*-
  * Copyright (c) 2008, 2009 The NetBSD Foundation, Inc.
@@ -70,7 +70,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_descrip.c,v 1.243 2019/02/20 19:42:14 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_descrip.c,v 1.244 2020/02/01 02:23:04 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -186,7 +186,7 @@
 {
        u_int off = fd >> NDENTRYSHIFT;
 
-       KASSERT(fd < fdp->fd_dt->dt_nfiles);
+       KASSERT(fd < atomic_load_consume(&fdp->fd_dt)->dt_nfiles);
 
        return (fdp->fd_lomap[off] & (1U << (fd & NDENTRYMASK))) != 0;
 }
@@ -201,6 +201,8 @@
        fdtab_t *dt;
        u_int fd;
 
+       KASSERT(fdp->fd_refcnt <= 1 || mutex_owned(&fdp->fd_lock));
+
        dt = fdp->fd_dt;
        if (fdp->fd_refcnt == -1) {
                /*
@@ -324,13 +326,7 @@
 
        ff = fdp->fd_dt->dt_ff[fd];
 
-       /*
-        * Don't assert the lock is held here, as we may be copying
-        * the table during exec() and it is not needed there.
-        * procfs and sysctl are locked out by proc::p_reflock.
-        *
-        * KASSERT(mutex_owned(&fdp->fd_lock));
-        */
+       KASSERT(mutex_owned(&fdp->fd_lock));
        KASSERT(ff != NULL);
        KASSERT(ff->ff_file == NULL);
        KASSERT(ff->ff_allocated);
@@ -373,7 +369,7 @@
         * We are doing this unlocked.  See fd_tryexpand().
         */
        fdp = curlwp->l_fd;
-       dt = fdp->fd_dt;
+       dt = atomic_load_consume(&fdp->fd_dt);
        if (__predict_false(fd >= dt->dt_nfiles)) {
                return NULL;
        }
@@ -426,9 +422,9 @@
        u_int u, v;
 
        fdp = curlwp->l_fd;
-       ff = fdp->fd_dt->dt_ff[fd];
+       KASSERT(fd < atomic_load_consume(&fdp->fd_dt)->dt_nfiles);
+       ff = atomic_load_consume(&fdp->fd_dt)->dt_ff[fd];
 
-       KASSERT(fd < fdp->fd_dt->dt_nfiles);
        KASSERT(ff != NULL);
        KASSERT((ff->ff_refcnt & FR_MASK) > 0);
        KASSERT(fd >= NDFDFILE || ff == (fdfile_t *)fdp->fd_dfdfile[fd]);
@@ -593,7 +589,7 @@
        l = curlwp;
        p = l->l_proc;
        fdp = l->l_fd;
-       ff = fdp->fd_dt->dt_ff[fd];
+       ff = atomic_load_consume(&fdp->fd_dt)->dt_ff[fd];
 
        KASSERT(fd >= NDFDFILE || ff == (fdfile_t *)fdp->fd_dfdfile[fd]);
 
@@ -726,6 +722,7 @@
 fd_dup(file_t *fp, int minfd, int *newp, bool exclose)
 {
        proc_t *p = curproc;
+       fdtab_t *dt;
        int error;
 
        while ((error = fd_alloc(p, minfd, newp)) != 0) {
@@ -735,7 +732,8 @@
                fd_tryexpand(p);
        }
 
-       curlwp->l_fd->fd_dt->dt_ff[*newp]->ff_exclose = exclose;
+       dt = atomic_load_consume(&curlwp->l_fd->fd_dt);
+       dt->dt_ff[*newp]->ff_exclose = exclose;
        fd_affix(p, fp, *newp);
        return 0;
 }
@@ -756,7 +754,7 @@
         * Ensure there are enough slots in the descriptor table,
         * and allocate an fdfile_t up front in case we need it.
         */
-       while (newfd >= fdp->fd_dt->dt_nfiles) {
+       while (newfd >= atomic_load_consume(&fdp->fd_dt)->dt_nfiles) {
                fd_tryexpand(curproc);
        }
        ff = pool_cache_get(fdfile_cache, PR_WAITOK);
@@ -1003,7 +1001,7 @@
        fdp = p->p_fd;
        newhimap = NULL;
        newlomap = NULL;
-       oldnfiles = fdp->fd_dt->dt_nfiles;
+       oldnfiles = atomic_load_consume(&fdp->fd_dt)->dt_nfiles;
 
        if (oldnfiles < NDEXTENT)
                numfiles = NDEXTENT;
@@ -1070,8 +1068,7 @@
         * All other modifications must become globally visible before
         * the change to fd_dt.  See fd_getfile().
         */
-       membar_producer();
-       fdp->fd_dt = newdt;
+       atomic_store_release(&fdp->fd_dt, newdt);
        KASSERT(newdt->dt_ff[0] == (fdfile_t *)fdp->fd_dfdfile[0]);
        fd_checkmaps(fdp);
        mutex_exit(&fdp->fd_lock);
@@ -1138,6 +1135,7 @@
 {
        fdfile_t *ff;
        filedesc_t *fdp;
+       fdtab_t *dt;
 
        KASSERT(p == curproc || p == &proc0);
 
@@ -1155,7 +1153,8 @@
         * current process.
         */
        fdp = p->p_fd;
-       ff = fdp->fd_dt->dt_ff[fd];
+       dt = atomic_load_consume(&fdp->fd_dt);
+       ff = dt->dt_ff[fd];
 
        KASSERT(ff != NULL);
        KASSERT(ff->ff_file == NULL);
@@ -1179,7 +1178,7 @@
        KASSERT(p == curproc || p == &proc0);
 
        fdp = p->p_fd;
-       ff = fdp->fd_dt->dt_ff[fd];
+       ff = atomic_load_consume(&fdp->fd_dt)->dt_ff[fd];
        ff->ff_exclose = false;
 
        KASSERT(fd >= NDFDFILE || ff == (fdfile_t *)fdp->fd_dfdfile[fd]);
@@ -1525,7 +1524,8 @@
        filedesc_t * const fdp = l->l_fd;
        const bool noadvlock = (l->l_proc->p_flag & PK_ADVLOCK) == 0;
 
-       KASSERT(fdp->fd_dt->dt_ff[0] == (fdfile_t *)fdp->fd_dfdfile[0]);
+       KASSERT(atomic_load_consume(&fdp->fd_dt)->dt_ff[0] ==
+           (fdfile_t *)fdp->fd_dfdfile[0]);
        KASSERT(fdp->fd_dtbuiltin.dt_nfiles == NDFILE);
        KASSERT(fdp->fd_dtbuiltin.dt_link == NULL);
 
@@ -1657,7 +1657,7 @@
                return EBADF;
        }
        fdp = curlwp->l_fd;
-       dt = fdp->fd_dt;
+       dt = atomic_load_consume(&fdp->fd_dt);
        ff = dt->dt_ff[old];
 
        /*
@@ -1730,7 +1730,7 @@
                return;
        }
        fdp->fd_exclose = false;
-       dt = fdp->fd_dt;
+       dt = atomic_load_consume(&fdp->fd_dt);
 
        for (fd = 0; fd <= fdp->fd_lastfile; fd++) {
                if ((ff = dt->dt_ff[fd]) == NULL) {
@@ -1793,7 +1793,7 @@
 fd_set_exclose(struct lwp *l, int fd, bool exclose)
 {
        filedesc_t *fdp = l->l_fd;
-       fdfile_t *ff = fdp->fd_dt->dt_ff[fd];
+       fdfile_t *ff = atomic_load_consume(&fdp->fd_dt)->dt_ff[fd];
 
        ff->ff_exclose = exclose;
        if (exclose)
@@ -1868,7 +1868,7 @@
 
        fp->f_flag = flag & FMASK;
        fdp = curproc->p_fd;
-       ff = fdp->fd_dt->dt_ff[fd];
+       ff = atomic_load_consume(&fdp->fd_dt)->dt_ff[fd];
        KASSERT(ff != NULL);
        ff->ff_exclose = (flag & O_CLOEXEC) != 0;
        fp->f_type = DTYPE_MISC;
diff -r f898bdc7ff98 -r 211c9d414133 sys/kern/kern_event.c
--- a/sys/kern/kern_event.c     Fri Jan 31 23:12:13 2020 +0000
+++ b/sys/kern/kern_event.c     Sat Feb 01 02:23:03 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kern_event.c,v 1.105 2019/10/18 19:43:49 christos Exp $        */
+/*     $NetBSD: kern_event.c,v 1.106 2020/02/01 02:23:04 riastradh Exp $       */
 
 /*-
  * Copyright (c) 2008, 2009 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_event.c,v 1.105 2019/10/18 19:43:49 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_event.c,v 1.106 2020/02/01 02:23:04 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -1671,8 +1671,8 @@
        filedesc_t *fdp;
 
        fdp = curlwp->l_fd;
+       mutex_enter(&fdp->fd_lock);
        list = (struct klist *)&fdp->fd_dt->dt_ff[fd]->ff_knlist;
-       mutex_enter(&fdp->fd_lock);
        while ((kn = SLIST_FIRST(list)) != NULL) {
                knote_detach(kn, fdp, true);
                mutex_enter(&fdp->fd_lock);
diff -r f898bdc7ff98 -r 211c9d414133 sys/kern/kern_sig.c
--- a/sys/kern/kern_sig.c       Fri Jan 31 23:12:13 2020 +0000
+++ b/sys/kern/kern_sig.c       Sat Feb 01 02:23:03 2020 +0000