locking around LFS_{SET,CLR}_UINO

[NAKAJIMA Yoshihiro <nakayosh%leto.eonet.ne.jp@localhost>]

Hello LFS developers,


When vmlocking2 was merged, lock and unlock were moved out of
LFS_{SET,CLR}_UINO.

: % diff -U4 lfs.h:1.{122,123} | sed -n 94,104p
:  #define LFS_SET_UINO(ip, flags) do {                                 \
: -     simple_lock(&(ip)->i_lfs->lfs_interlock);                       \
:       if (((flags) & IN_ACCESSED) && !((ip)->i_flag & IN_ACCESSED))   \
:               ++(ip)->i_lfs->lfs_uinodes;                             \
:       if (((flags) & IN_CLEANING) && !((ip)->i_flag & IN_CLEANING))   \
:               ++(ip)->i_lfs->lfs_uinodes;                             \
:       if (((flags) & IN_MODIFIED) && !((ip)->i_flag & IN_MODIFIED))   \
:               ++(ip)->i_lfs->lfs_uinodes;                             \
:       (ip)->i_flag |= (flags);                                        \
: -     simple_unlock(&(ip)->i_lfs->lfs_interlock);                     \
:  } while (0)

I guess the lock protects lfs_uinodes, when IN_ACCESSED, IN_CLEANING
or IN_MODIFIED are set.

If it is right, a lock is unnecessary and some locks are missing.  See
attached patch.


Cheers,
-- 
nakayosh

Date: Sun Dec 19 23:24:53 JST 2010
diff -u src/sys/ufs/lfs/lfs_alloc.c.ORIG src/sys/ufs/lfs/lfs_alloc.c
--- src/sys/ufs/lfs/lfs_alloc.c.ORIG    2010-06-24 22:03:19.000000000 +0900
+++ src/sys/ufs/lfs/lfs_alloc.c 2010-12-19 23:24:27.000000000 +0900
@@ -1,4 +1,4 @@
-/*     $NetBSD: lfs_alloc.c,v 1.110 2010/06/24 13:03:19 hannken Exp $  */
+/*     $NetBSD$        */
 
 /*-
  * Copyright (c) 1999, 2000, 2001, 2002, 2003, 2007 The NetBSD Foundation, Inc.
@@ -276,9 +276,7 @@
        lfs_vcreate(pvp->v_mount, new_ino, vp);
 
        ip = VTOI(vp);
-       mutex_enter(&lfs_lock);
        LFS_SET_UINO(ip, IN_CHANGE);
-       mutex_exit(&lfs_lock);
        /* on-disk structure has been zeroed out by lfs_vcreate */
        ip->i_din.ffs1_din->di_inumber = new_ino;
 
@@ -477,9 +475,8 @@
        }
 
        mutex_enter(&lfs_lock);
-       LFS_CLR_UINO(ip, IN_ACCESSED|IN_CLEANING|IN_MODIFIED);
+       LFS_CLR_UINO(ip, IN_ALLMOD);
        mutex_exit(&lfs_lock);
-       ip->i_flag &= ~IN_ALLMOD;
        ip->i_lfs_iflags |= LFSI_DELETED;
        
        /*
diff -u src/sys/ufs/lfs/lfs_segment.c.ORIG src/sys/ufs/lfs/lfs_segment.c
--- src/sys/ufs/lfs/lfs_segment.c.ORIG  2010-07-22 02:52:13.000000000 +0900
+++ src/sys/ufs/lfs/lfs_segment.c       2010-12-19 23:24:39.000000000 +0900
@@ -1,4 +1,4 @@
-/*     $NetBSD: lfs_segment.c,v 1.217 2010/07/21 17:52:13 hannken Exp $        
*/
+/*     $NetBSD$        */
 
 /*-
  * Copyright (c) 1999, 2000, 2001, 2002, 2003 The NetBSD Foundation, Inc.
@@ -324,9 +324,9 @@
                        }
                }
                mutex_exit(&bufcache_lock);
-               LFS_CLR_UINO(ip, IN_CLEANING);
-               LFS_CLR_UINO(ip, IN_MODIFIED | IN_ACCESSED);
-               ip->i_flag &= ~IN_ALLMOD;
+               mutex_enter(&lfs_lock);
+               LFS_CLR_UINO(ip, IN_ALLMOD);
+               mutex_exit(&lfs_lock);
                DLOG((DLOG_VNODE, "lfs_vflush: done not flushing ino %d\n",
                      ip->i_number));
                lfs_segunlock(fs);
@@ -767,7 +767,9 @@
                 */
                mutex_enter(&vp->v_interlock);
                if (LIST_EMPTY(&vp->v_dirtyblkhd)) {
+                       mutex_enter(&lfs_lock);
                        LFS_CLR_UINO(ip, IN_ALLMOD);
+                       mutex_exit(&lfs_lock);
                }
 #ifdef DIAGNOSTIC
                else if (do_ckp) {
@@ -1219,6 +1221,7 @@
        }
 #endif /* DIAGNOSTIC */
 
+       mutex_enter(&lfs_lock);
        if (ip->i_flag & IN_CLEANING)
                LFS_CLR_UINO(ip, IN_CLEANING);
        else {
@@ -1233,6 +1236,7 @@
                            ip->i_ffs1_blocks, ip->i_lfs_effnblks));
                }
        }
+       mutex_exit(&lfs_lock);
 
        if (ip->i_number == LFS_IFILE_INUM) {
                /* We know sp->idp == NULL */
@@ -1665,7 +1669,9 @@
        }
 
        /* This inode has been modified */
+       mutex_enter(&lfs_lock);
        LFS_SET_UINO(VTOI(vp), IN_MODIFIED);
+       mutex_exit(&lfs_lock);
 }
 
 /*
diff -u src/sys/ufs/lfs/lfs_vnops.c.ORIG src/sys/ufs/lfs/lfs_vnops.c
--- src/sys/ufs/lfs/lfs_vnops.c.ORIG    2010-12-18 09:01:46.000000000 +0900
+++ src/sys/ufs/lfs/lfs_vnops.c 2010-12-19 23:24:49.000000000 +0900
@@ -1,4 +1,4 @@
-/*     $NetBSD: lfs_vnops.c,v 1.232 2010/12/18 00:01:46 eeh Exp $      */
+/*     $NetBSD$        */
 
 /*-
  * Copyright (c) 1999, 2000, 2001, 2002, 2003 The NetBSD Foundation, Inc.
@@ -332,8 +332,11 @@
                error = VOP_IOCTL(ip->i_devvp, DIOCCACHESYNC, &l, FWRITE,
                                  curlwp->l_cred);
        }
-       if (wait && !VPISEMPTY(vp))
+       if (wait && !VPISEMPTY(vp)) {
+               mutex_enter(&lfs_lock);
                LFS_SET_UINO(ip, IN_MODIFIED);
+               mutex_exit(&lfs_lock);
+       }
 
        return error;
 }