[src/trunk]: src/sys/ufs/lfs Take a reference and fix assertions in lfs_flush...

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

details:   https://anonhg.NetBSD.org/src/rev/27e8cacf66e8
branches:  trunk
changeset: 745130:27e8cacf66e8
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sun Feb 23 08:40:19 2020 +0000

description:
Take a reference and fix assertions in lfs_flush_dirops.

Fixes panic:

KASSERT((ip->i_state & IN_ADIROP) == 0) at lfs_vnops.c:1670
lfs_flush_dirops
lfs_check
lfs_setattr
VOP_SETATTR
change_mode
sys_fchmod
syscall

This assertion -- and the assertion that vp->v_uflag has VU_DIROP set
-- is valid only until we release lfs_lock, because we may race with
lfs_unmark_dirop which will remove the nodes and change the flags.

Further, vp itself is valid only as long as it is referenced, which it
is as long as it's on the dchain, but lfs_unmark_dirop drops the
dchain's reference.

diffstat:

 sys/ufs/lfs/lfs_vnops.c |  49 +++++++++++++++++++++++++++++++++----------------
 1 files changed, 33 insertions(+), 16 deletions(-)

diffs (111 lines):

diff -r 2f3fc1090a03 -r 27e8cacf66e8 sys/ufs/lfs/lfs_vnops.c
--- a/sys/ufs/lfs/lfs_vnops.c   Sun Feb 23 08:40:08 2020 +0000
+++ b/sys/ufs/lfs/lfs_vnops.c   Sun Feb 23 08:40:19 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: lfs_vnops.c,v 1.328 2020/02/23 08:40:08 riastradh Exp $        */
+/*     $NetBSD: lfs_vnops.c,v 1.329 2020/02/23 08:40:19 riastradh Exp $        */
 
 /*-
  * Copyright (c) 1999, 2000, 2001, 2002, 2003 The NetBSD Foundation, Inc.
@@ -125,7 +125,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: lfs_vnops.c,v 1.328 2020/02/23 08:40:08 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: lfs_vnops.c,v 1.329 2020/02/23 08:40:19 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd.h"
@@ -1623,7 +1623,6 @@
                return EROFS;
 
        mutex_enter(&lfs_lock);
-       KASSERT(fs->lfs_writer);
        if (TAILQ_FIRST(&fs->lfs_dchainhd) == NULL) {
                mutex_exit(&lfs_lock);
                return 0;
@@ -1667,13 +1666,33 @@
                        lfs_dchain_marker_pass_flush.ev_count++;
                        continue;
                }
-               mutex_exit(&lfs_lock);
                vp = ITOV(ip);
-               mutex_enter(vp->v_interlock);
 
+               /*
+                * Prevent the vnode from going away if it's just been
+                * put out in the segment and lfs_unmark_dirop is about
+                * to release it.  While it is on the list it is always
+                * referenced, so it cannot be reclaimed until we
+                * release it.
+                */
+               vref(vp);
+
+               /*
+                * Since we hold lfs_writer, the node can't be in an
+                * active dirop.  Since it's on the list and we hold a
+                * reference to it, it can't be reclaimed now.
+                */
                KASSERT((ip->i_state & IN_ADIROP) == 0);
                KASSERT(vp->v_uflag & VU_DIROP);
-               KASSERT(vdead_check(vp, VDEAD_NOWAIT) == 0);
+
+               /*
+                * After we release lfs_lock, if we were in the middle
+                * of writing a segment, lfs_unmark_dirop may end up
+                * clearing VU_DIROP, and we have no way to stop it.
+                * That should be OK -- we'll just have less to do
+                * here.
+                */
+               mutex_exit(&lfs_lock);
 
                /*
                 * All writes to directories come from dirops; all
@@ -1683,15 +1702,6 @@
                 * directory blocks inodes and file inodes.  So we don't
                 * really need to lock.
                 */
-               if (vdead_check(vp, VDEAD_NOWAIT) != 0) {
-                       mutex_exit(vp->v_interlock);
-                       mutex_enter(&lfs_lock);
-                       continue;
-               }
-               mutex_exit(vp->v_interlock);
-               /* XXX see below
-                * waslocked = VOP_ISLOCKED(vp);
-                */
                if (vp->v_type != VREG &&
                    ((ip->i_state & IN_ALLMOD) || !VPISEMPTY(vp))) {
                        error = lfs_writefile(fs, sp, vp);
@@ -1702,6 +1712,7 @@
                                mutex_exit(&lfs_lock);
                        }
                        if (error && (sp->seg_flags & SEGM_SINGLE)) {
+                               vrele(vp);
                                mutex_enter(&lfs_lock);
                                error = EAGAIN;
                                break;
@@ -1709,8 +1720,9 @@
                }
                KASSERT(ip->i_number != LFS_IFILE_INUM);
                error = lfs_writeinode(fs, sp, ip);
-               mutex_enter(&lfs_lock);
                if (error && (sp->seg_flags & SEGM_SINGLE)) {
+                       vrele(vp);
+                       mutex_enter(&lfs_lock);
                        error = EAGAIN;
                        break;
                }
@@ -1723,7 +1735,12 @@
                 * write them.
                 */
                /* XXX only for non-directories? --KS */
+               mutex_enter(&lfs_lock);
                LFS_SET_UINO(ip, IN_MODIFIED);
+               mutex_exit(&lfs_lock);
+
+               vrele(vp);
+               mutex_enter(&lfs_lock);
        }
        TAILQ_REMOVE(&fs->lfs_dchainhd, marker, i_lfs_dchain);
        mutex_exit(&lfs_lock);