Fix a double-flush which was occuring for every unlinked inode, resulting
authorMatthew Dillon <dillon@dragonflybsd.org>
Tue, 23 Sep 2008 22:28:56 +0000 (22:28 +0000)
committerMatthew Dillon <dillon@dragonflybsd.org>
Tue, 23 Sep 2008 22:28:56 +0000 (22:28 +0000)
in very inefficient flushing operations which also prevented any inode from
being reclaimed until the entire flush batch (which could be upwards of 40,000
inodes) completed.

The double-flush was caused by the HAMMER_INODE_DELETING flag being
improperly set twice.

Clear the HAMMER_INODE_REFLUSH flag if a reflush was queued but the
modmask is cleared by the prior flush.

Adjust the directory mtime before flushing an underlying inode, avoiding
a double flush of the directory inode.

Reported-by: Hasso Tepper <hasso@estpak.ee>
sys/vfs/hammer/hammer_inode.c
sys/vfs/hammer/hammer_object.c
sys/vfs/hammer/hammer_vnops.c

index 333b037..b928738 100644 (file)
@@ -31,7 +31,7 @@
  * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  * 
- * $DragonFly: src/sys/vfs/hammer/hammer_inode.c,v 1.112 2008/09/23 21:03:52 dillon Exp $
+ * $DragonFly: src/sys/vfs/hammer/hammer_inode.c,v 1.113 2008/09/23 22:28:56 dillon Exp $
  */
 
 #include "hammer.h"
@@ -1159,7 +1159,7 @@ retry:
 void
 hammer_rel_inode(struct hammer_inode *ip, int flush)
 {
-       hammer_mount_t hmp = ip->hmp;
+       /*hammer_mount_t hmp = ip->hmp;*/
 
        /*
         * Handle disposition when dropping the last ref.
@@ -1173,12 +1173,7 @@ hammer_rel_inode(struct hammer_inode *ip, int flush)
                        KKASSERT(ip->vp == NULL);
                        hammer_inode_unloadable_check(ip, 0);
                        if (ip->flags & HAMMER_INODE_MODMASK) {
-                               if (hmp->rsv_inodes > desiredvnodes) {
-                                       hammer_flush_inode(ip,
-                                                          HAMMER_FLUSH_SIGNAL);
-                               } else {
-                                       hammer_flush_inode(ip, 0);
-                               }
+                               hammer_flush_inode(ip, 0);
                        } else if (ip->lock.refs == 1) {
                                hammer_unload_inode(ip);
                                break;
@@ -2062,9 +2057,17 @@ hammer_flush_inode_done(hammer_inode_t ip, int error)
        /*
         * Do not lose track of inodes which no longer have vnode
         * assocations, otherwise they may never get flushed again.
+        *
+        * The reflush flag can be set superfluously, causing extra pain
+        * for no reason.  If the inode is no longer modified it no longer
+        * needs to be flushed.
         */
-       if ((ip->flags & HAMMER_INODE_MODMASK) && ip->vp == NULL)
-               ip->flags |= HAMMER_INODE_REFLUSH;
+       if (ip->flags & HAMMER_INODE_MODMASK) {
+               if (ip->vp == NULL)
+                       ip->flags |= HAMMER_INODE_REFLUSH;
+       } else {
+               ip->flags &= ~HAMMER_INODE_REFLUSH;
+       }
 
        /*
         * Adjust the flush state.
@@ -2644,9 +2647,14 @@ hammer_inode_unloadable_check(hammer_inode_t ip, int getvp)
         * The backend will clear DELETING (a mod flag) and set DELETED
         * (a state flag) when it is actually able to perform the
         * operation.
+        *
+        * Don't reflag the deletion if the flusher is currently syncing
+        * one that was already flagged.  A previously set DELETING flag
+        * may bounce around flags and sync_flags until the operation is
+        * completely done.
         */
        if (ip->ino_data.nlinks == 0 &&
-           (ip->flags & (HAMMER_INODE_DELETING|HAMMER_INODE_DELETED)) == 0) {
+           ((ip->flags | ip->sync_flags) & (HAMMER_INODE_DELETING|HAMMER_INODE_DELETED)) == 0) {
                ip->flags |= HAMMER_INODE_DELETING;
                ip->flags |= HAMMER_INODE_TRUNCATED;
                ip->trunc_off = 0;
index 3e70fb6..7d08acb 100644 (file)
@@ -31,7 +31,7 @@
  * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  * 
- * $DragonFly: src/sys/vfs/hammer/hammer_object.c,v 1.96 2008/08/09 07:04:16 dillon Exp $
+ * $DragonFly: src/sys/vfs/hammer/hammer_object.c,v 1.97 2008/09/23 22:28:56 dillon Exp $
  */
 
 #include "hammer.h"
@@ -778,6 +778,8 @@ hammer_ip_del_directory(struct hammer_transaction *trans,
         */
        if (error == 0) {
                --ip->ino_data.nlinks;
+               dip->ino_data.mtime = trans->time;
+               hammer_modify_inode(dip, HAMMER_INODE_MTIME);
                hammer_modify_inode(ip, HAMMER_INODE_DDIRTY);
                if (ip->ino_data.nlinks == 0 &&
                    (ip->vp == NULL || (ip->vp->v_flag & VINACTIVE))) {
@@ -785,8 +787,6 @@ hammer_ip_del_directory(struct hammer_transaction *trans,
                        hammer_inode_unloadable_check(ip, 1);
                        hammer_flush_inode(ip, 0);
                }
-               dip->ino_data.mtime = trans->time;
-               hammer_modify_inode(dip, HAMMER_INODE_MTIME);
 
        }
        return(error);
index 951a44a..27a1eb5 100644 (file)
@@ -31,7 +31,7 @@
  * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  * 
- * $DragonFly: src/sys/vfs/hammer/hammer_vnops.c,v 1.98 2008/09/23 21:03:52 dillon Exp $
+ * $DragonFly: src/sys/vfs/hammer/hammer_vnops.c,v 1.99 2008/09/23 22:28:56 dillon Exp $
  */
 
 #include <sys/param.h>
@@ -2578,8 +2578,13 @@ hammer_vop_strategy_write(struct vop_strategy_args *ap)
         * topology visibility).  If we queue new IO while trying to
         * destroy the inode we can deadlock the vtrunc call in
         * hammer_inode_unloadable_check().
+        *
+        * Besides, there's no point flushing a bp associated with an
+        * inode that is being destroyed on-media and has no kernel
+        * references.
         */
-       if (ip->flags & (HAMMER_INODE_DELETING|HAMMER_INODE_DELETED)) {
+       if ((ip->flags | ip->sync_flags) &
+           (HAMMER_INODE_DELETING|HAMMER_INODE_DELETED)) {
                bp->b_resid = 0;
                biodone(ap->a_bio);
                return(0);