Source-Changes-HG archive

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

[src/trunk]: src/sys/ufs/lfs Remove the DIROP macros. They are evil, especial...



details:   https://anonhg.NetBSD.org/src/rev/907470314253
branches:  trunk
changeset: 329259:907470314253
user:      dholland <dholland%NetBSD.org@localhost>
date:      Sat May 17 07:08:35 2014 +0000

description:
Remove the DIROP macros. They are evil, especially the CREATE ones.

This results in some duplicate logic in the creation vnops (symlink,
mknod, create, mkdir) but we will probably be able to factor it out in
a more sensible way later.

Now the creation vnops call getnewvnode explicitly instead of under
multiple layers of obscure gunk. Then we explicitly do lfs_set_dirop,
and afterwards lfs_unset_dirop.

diffstat:

 sys/ufs/lfs/lfs_rename.c |   16 +-
 sys/ufs/lfs/lfs_vnops.c  |  318 +++++++++++++++++++++++++++++++++++++++-------
 sys/ufs/lfs/ulfs_inode.h |   51 +-------
 3 files changed, 278 insertions(+), 107 deletions(-)

diffs (truncated from 566 to 300 lines):

diff -r 6c5d6b0af6fb -r 907470314253 sys/ufs/lfs/lfs_rename.c
--- a/sys/ufs/lfs/lfs_rename.c  Sat May 17 05:50:46 2014 +0000
+++ b/sys/ufs/lfs/lfs_rename.c  Sat May 17 07:08:35 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: lfs_rename.c,v 1.6 2014/02/06 10:57:12 hannken Exp $   */
+/*     $NetBSD: lfs_rename.c,v 1.7 2014/05/17 07:08:35 dholland Exp $  */
 /*  from NetBSD: ufs_rename.c,v 1.6 2013/01/22 09:39:18 dholland Exp  */
 
 /*-
@@ -89,7 +89,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: lfs_rename.c,v 1.6 2014/02/06 10:57:12 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: lfs_rename.c,v 1.7 2014/05/17 07:08:35 dholland Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -1079,7 +1079,7 @@
        KASSERT(VOP_ISLOCKED(tdvp) == LK_EXCLUSIVE);
        KASSERT((tvp == NULL) || (VOP_ISLOCKED(tvp) == LK_EXCLUSIVE));
 
-       error = SET_DIROP_REMOVE(tdvp, tvp);
+       error = lfs_set_dirop(tdvp, tvp);
        if (error != 0)
                return error;
 
@@ -1092,7 +1092,15 @@
 
        UNMARK_VNODE(fdvp);
        UNMARK_VNODE(fvp);
-       SET_ENDOP_REMOVE(VFSTOULFS(mp)->um_lfs, tdvp, tvp, "rename");
+       UNMARK_VNODE(tdvp);
+       if (tvp) {
+               UNMARK_VNODE(tvp);
+       }
+       lfs_unset_dirop(VFSTOULFS(mp)->um_lfs, tdvp, "rename");
+       vrele(tdvp);
+       if (tvp) {
+               vrele(tvp);
+       }
 
        return error;
 }
diff -r 6c5d6b0af6fb -r 907470314253 sys/ufs/lfs/lfs_vnops.c
--- a/sys/ufs/lfs/lfs_vnops.c   Sat May 17 05:50:46 2014 +0000
+++ b/sys/ufs/lfs/lfs_vnops.c   Sat May 17 07:08:35 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: lfs_vnops.c,v 1.263 2014/05/16 09:34:03 dholland Exp $ */
+/*     $NetBSD: lfs_vnops.c,v 1.264 2014/05/17 07:08:35 dholland Exp $ */
 
 /*-
  * Copyright (c) 1999, 2000, 2001, 2002, 2003 The NetBSD Foundation, Inc.
@@ -60,7 +60,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: lfs_vnops.c,v 1.263 2014/05/16 09:34:03 dholland Exp $");
+__KERNEL_RCSID(0, "$NetBSD: lfs_vnops.c,v 1.264 2014/05/17 07:08:35 dholland Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd.h"
@@ -473,38 +473,27 @@
 }
 
 /*
- * Get a new vnode *before* adjusting the dirop count, to avoid a deadlock
- * in getnewvnode(), if we have a stacked filesystem mounted on top
- * of us.
- *
- * NB: this means we have to clear the new vnodes on error.  Fortunately
- * SET_ENDOP is there to do that for us.
+ * Opposite of lfs_set_dirop... mostly. For now at least must call
+ * UNMARK_VNODE(dvp) explicitly first. (XXX: clean that up)
  */
-int
-lfs_set_dirop_create(struct vnode *dvp, struct vnode **vpp)
+void
+lfs_unset_dirop(struct lfs *fs, struct vnode *dvp, const char *str)
 {
-       int error;
-       struct lfs *fs;
-
-       fs = VFSTOULFS(dvp->v_mount)->um_lfs;
-       ASSERT_NO_SEGLOCK(fs);
-       if (fs->lfs_ronly)
-               return EROFS;
-       if (vpp == NULL) {
-               return lfs_set_dirop(dvp, NULL);
+       mutex_enter(&lfs_lock);
+       --fs->lfs_dirops;
+       if (!fs->lfs_dirops) {
+               if (fs->lfs_nadirop) {
+                       panic("lfs_unset_dirop: %s: no dirops but "
+                             " nadirop=%d", str,
+                             fs->lfs_nadirop);
+               }
+               wakeup(&fs->lfs_writer);
+               mutex_exit(&lfs_lock);
+               lfs_check(dvp, LFS_UNUSED_LBN, 0);
+       } else {
+               mutex_exit(&lfs_lock);
        }
-       error = getnewvnode(VT_LFS, dvp->v_mount, lfs_vnodeop_p, NULL, vpp);
-       if (error) {
-               DLOG((DLOG_ALLOC, "lfs_set_dirop_create: dvp %p error %d\n",
-                     dvp, error));
-               return error;
-       }
-       if ((error = lfs_set_dirop(dvp, NULL)) != 0) {
-               ungetnewvnode(*vpp);
-               *vpp = NULL;
-               return error;
-       }
-       return 0;
+       lfs_reserve(fs, dvp, NULL, -LFS_NRESERVE(fs));
 }
 
 void
@@ -558,13 +547,60 @@
                struct vattr *a_vap;
                char *a_target;
        } */ *ap = v;
+       struct lfs *fs;
+       struct vnode *dvp, **vpp;
        int error;
 
-       if ((error = SET_DIROP_CREATE(ap->a_dvp, ap->a_vpp)) != 0) {
+       dvp = ap->a_dvp;
+       vpp = ap->a_vpp;
+
+       KASSERT(vpp != NULL);
+       KASSERT(*vpp == NULL);
+
+       fs = VFSTOULFS(dvp->v_mount)->um_lfs;
+       ASSERT_NO_SEGLOCK(fs);
+       if (fs->lfs_ronly) {
+               return EROFS;
+       }
+
+       /*
+        * Get a new vnode *before* adjusting the dirop count, to
+        * avoid a deadlock in getnewvnode(), if we have a stacked
+        * filesystem mounted on top of us.
+        *
+        * NB: this means we have to destroy the new vnode on error.
+        */
+
+       error = getnewvnode(VT_LFS, dvp->v_mount, lfs_vnodeop_p, NULL, vpp);
+       if (error) {
+               DLOG((DLOG_ALLOC, "lfs_mkdir: dvp %p error %d\n", dvp, error));
                return error;
        }
+       KASSERT(*vpp != NULL);
+
+       error = lfs_set_dirop(dvp, NULL);
+       if (error) {
+               ungetnewvnode(*vpp);
+               *vpp = NULL;
+               return error;
+       }
+
        error = ulfs_symlink(ap);
-       SET_ENDOP_CREATE_AP(ap, "symlink");
+
+       UNMARK_VNODE(dvp);
+       /* XXX: is it even possible for the symlink to get MARK'd? */
+       UNMARK_VNODE(*vpp);
+       if (!((*vpp)->v_uflag & VU_DIROP)) {
+               KASSERT(error != 0);
+               ungetnewvnode(*vpp);
+               *vpp = NULL;
+       }
+       else {
+               KASSERT(error == 0);
+       }
+       lfs_unset_dirop(fs, dvp, "symlink");
+
+       vrele(dvp);
        return (error);
 }
 
@@ -577,31 +613,76 @@
                struct componentname *a_cnp;
                struct vattr *a_vap;
        } */ *ap = v;
+       struct lfs *fs;
+       struct vnode *dvp, **vpp;
        struct vattr *vap;
-       struct vnode **vpp;
        struct inode *ip;
        int error;
        struct mount    *mp;
        ino_t           ino;
        struct ulfs_lookup_results *ulr;
 
-       vap = ap->a_vap;
+       dvp = ap->a_dvp;
        vpp = ap->a_vpp;
+       vap = ap->a_vap;
 
+       KASSERT(vpp != NULL);
+       KASSERT(*vpp == NULL);
+       
        /* XXX should handle this material another way */
-       ulr = &VTOI(ap->a_dvp)->i_crap;
-       ULFS_CHECK_CRAPCOUNTER(VTOI(ap->a_dvp));
+       ulr = &VTOI(dvp)->i_crap;
+       ULFS_CHECK_CRAPCOUNTER(VTOI(dvp));
+
+       fs = VFSTOULFS(dvp->v_mount)->um_lfs;
+       ASSERT_NO_SEGLOCK(fs);
+       if (fs->lfs_ronly) {
+               return EROFS;
+       }
 
-       if ((error = SET_DIROP_CREATE(ap->a_dvp, ap->a_vpp)) != 0) {
+       /*
+        * Get a new vnode *before* adjusting the dirop count, to
+        * avoid a deadlock in getnewvnode(), if we have a stacked
+        * filesystem mounted on top of us.
+        *
+        * NB: this means we have to destroy the new vnode on error.
+        */
+
+       error = getnewvnode(VT_LFS, dvp->v_mount, lfs_vnodeop_p, NULL, vpp);
+       if (error) {
+               DLOG((DLOG_ALLOC, "lfs_mknod: dvp %p error %d\n", dvp, error));
+               return error;
+       }
+       KASSERT(*vpp != NULL);
+
+       error = lfs_set_dirop(dvp, NULL);
+       if (error) {
+               ungetnewvnode(*vpp);
+               *vpp = NULL;
                return error;
        }
 
        fstrans_start(ap->a_dvp->v_mount, FSTRANS_SHARED);
        error = ulfs_makeinode(MAKEIMODE(vap->va_type, vap->va_mode),
-                             ap->a_dvp, ulr, vpp, ap->a_cnp);
+                             dvp, ulr, vpp, ap->a_cnp);
 
        /* Either way we're done with the dirop at this point */
-       SET_ENDOP_CREATE_AP(ap, "mknod");
+       UNMARK_VNODE(dvp);
+       UNMARK_VNODE(*vpp);
+       if (!((*vpp)->v_uflag & VU_DIROP)) {
+               KASSERT(error != 0);
+               ungetnewvnode(*vpp);
+               *vpp = NULL;
+       }
+       else {
+               KASSERT(error == 0);
+       }
+       lfs_unset_dirop(fs, dvp, "mknod");
+       /*
+        * XXX this is where this used to be (though inside some evil
+        * macros) but it clearly should be moved further down.
+        * - dholland 20140515
+        */
+       vrele(dvp);
 
        if (error) {
                fstrans_done(ap->a_dvp->v_mount);
@@ -609,14 +690,14 @@
                return (error);
        }
 
-       VN_KNOTE(ap->a_dvp, NOTE_WRITE);
+       VN_KNOTE(dvp, NOTE_WRITE);
        ip = VTOI(*vpp);
        mp  = (*vpp)->v_mount;
        ino = ip->i_number;
        ip->i_flag |= IN_ACCESS | IN_CHANGE | IN_UPDATE;
        if (vap->va_rdev != VNOVAL) {
                struct ulfsmount *ump = ip->i_ump;
-               struct lfs *fs = ip->i_lfs;
+               KASSERT(fs == ip->i_lfs);
                /*
                 * Want to be able to use this to make badblock
                 * inodes, so don't truncate the dev number.
@@ -672,13 +753,57 @@
                struct componentname *a_cnp;
                struct vattr *a_vap;
        } */ *ap = v;
+       struct lfs *fs;
+       struct vnode *dvp, **vpp;
        int error;
 
-       if ((error = SET_DIROP_CREATE(ap->a_dvp, ap->a_vpp)) != 0) {
+       dvp = ap->a_dvp;
+       vpp = ap->a_vpp;
+
+       KASSERT(vpp != NULL);
+       KASSERT(*vpp == NULL);
+
+       fs = VFSTOULFS(dvp->v_mount)->um_lfs;
+       ASSERT_NO_SEGLOCK(fs);



Home | Main Index | Thread Index | Old Index