HAMMER VFS - Limit recursion for long directory chains, update mtime/ctime

A long chain of directory dependencies can blow out the kernel stack.  Limit
the recursion to 20 levels.  If the depth is exceeded the flushing of the
deep inodes is delayed until the higher dependencies are taken care of.

Update mtime/ctime accordingly based on the fstest stress test from FreeBSD.
Note however that we cannot update the ctime on directories for file creates,
renames, and deletes within the directory without rolling a new inode,
which is too expensive to do for that situation.  We can, and do, update the
mtime.

diff --git a/sys/vfs/hammer/hammer_inode.c b/sys/vfs/hammer/hammer_inode.c
index 4e359e5..7069c4b 100644 (file)
--- a/sys/vfs/hammer/hammer_inode.c
+++ b/sys/vfs/hammer/hammer_inode.c
@@ -47,10 +47,10 @@ static int  hammer_setup_child_callback(hammer_record_t rec, void *data);
 #if 0
 static int     hammer_syncgrp_child_callback(hammer_record_t rec, void *data);
 #endif
-static int     hammer_setup_parent_inodes(hammer_inode_t ip,
+static int     hammer_setup_parent_inodes(hammer_inode_t ip, int depth,
                                        hammer_flush_group_t flg);
 static int     hammer_setup_parent_inodes_helper(hammer_record_t record,
-                                       hammer_flush_group_t flg);
+                                       int depth, hammer_flush_group_t flg);
 static void    hammer_inode_wakereclaims(hammer_inode_t ip, int dowake);
 
 #ifdef DEBUG_TRUNCATE
@@ -1569,7 +1569,7 @@ hammer_flush_inode(hammer_inode_t ip, int flags)
                 * can't flush, 0 means there weren't any dependancies, and
                 * 1 means we have good connectivity.
                 */
-               good = hammer_setup_parent_inodes(ip, flg);
+               good = hammer_setup_parent_inodes(ip, 0, flg);
 
                if (good >= 0) {
                        /*
@@ -1621,20 +1621,47 @@ hammer_flush_inode(hammer_inode_t ip, int flags)
  *     ref/rel code later, the rel CAN block.
  */
 static int
-hammer_setup_parent_inodes(hammer_inode_t ip, hammer_flush_group_t flg)
+hammer_setup_parent_inodes(hammer_inode_t ip, int depth,
+                          hammer_flush_group_t flg)
 {
        hammer_record_t depend;
        int good;
        int r;
 
+       /*
+        * If we hit our recursion limit and we have parent dependencies
+        * We cannot continue.  Returning < 0 will cause us to be flagged
+        * for reflush.  Returning -2 cuts off additional dependency checks
+        * because they are likely to also hit the depth limit.
+        *
+        * We cannot return < 0 if there are no dependencies or there might
+        * not be anything to wakeup (ip).
+        */
+       if (depth == 20 && TAILQ_FIRST(&ip->target_list)) {
+               kprintf("HAMMER Warning: depth limit reached on "
+                       "setup recursion, inode %p %016llx\n",
+                       ip, (long long)ip->obj_id);
+               return(-2);
+       }
+
+       /*
+        * Scan dependencies
+        */
        good = 0;
        TAILQ_FOREACH(depend, &ip->target_list, target_entry) {
-               r = hammer_setup_parent_inodes_helper(depend, flg);
+               r = hammer_setup_parent_inodes_helper(depend, depth, flg);
                KKASSERT(depend->target_ip == ip);
                if (r < 0 && good == 0)
                        good = -1;
                if (r > 0)
                        good = 1;
+
+               /*
+                * If we failed due to the recursion depth limit then stop
+                * now.
+                */
+               if (r == -2)
+                       break;
        }
        return(good);
 }
@@ -1656,7 +1683,7 @@ hammer_setup_parent_inodes(hammer_inode_t ip, hammer_flush_group_t flg)
  * Return -1 if we can't resolve the dependancy and there is no connectivity.
  */
 static int
-hammer_setup_parent_inodes_helper(hammer_record_t record,
+hammer_setup_parent_inodes_helper(hammer_record_t record, int depth,
                                  hammer_flush_group_t flg)
 {
        hammer_mount_t hmp;
@@ -1699,19 +1726,25 @@ hammer_setup_parent_inodes_helper(hammer_record_t record,
        /*
         * It must be a setup record.  Try to resolve the setup dependancies
         * by recursing upwards so we can place ip on the flush list.
+        *
+        * Limit ourselves to 20 levels of recursion to avoid blowing out
+        * the kernel stack.  If we hit the recursion limit we can't flush
+        * until the parent flushes.  The parent will flush independantly
+        * on its own and ultimately a deep recursion will be resolved.
         */
        KKASSERT(record->flush_state == HAMMER_FST_SETUP);
 
-       good = hammer_setup_parent_inodes(pip, flg);
+       good = hammer_setup_parent_inodes(pip, depth + 1, flg);
 
        /*
         * If good < 0 the parent has no connectivity and we cannot safely
         * flush the directory entry, which also means we can't flush our
-        * ip.  Flag the parent and us for downward recursion once the
-        * parent's connectivity is resolved.
+        * ip.  Flag us for downward recursion once the parent's
+        * connectivity is resolved.  Flag the parent for [re]flush or it
+        * may not check for downward recursions.
         */
        if (good < 0) {
-               /* pip->flags |= HAMMER_INODE_CONN_DOWN; set by recursion */
+               pip->flags |= HAMMER_INODE_REFLUSH;
                record->target_ip->flags |= HAMMER_INODE_CONN_DOWN;
                return(good);
        }

diff --git a/sys/vfs/hammer/hammer_object.c b/sys/vfs/hammer/hammer_object.c
index 51aa41d..1ec890a 100644 (file)
--- a/sys/vfs/hammer/hammer_object.c
+++ b/sys/vfs/hammer/hammer_object.c
@@ -659,6 +659,7 @@ hammer_ip_add_directory(struct hammer_transaction *trans,
        bcopy(name, record->data->entry.name, bytes);
 
        ++ip->ino_data.nlinks;
+       ip->ino_data.ctime = trans->time;
        hammer_modify_inode(ip, HAMMER_INODE_DDIRTY);
 
        /*
@@ -823,8 +824,10 @@ hammer_ip_del_directory(struct hammer_transaction *trans,
         * on-media until we unmount.
         */
        if (error == 0) {
-               if (ip)
+               if (ip) {
                        --ip->ino_data.nlinks;  /* do before we might block */
+                       ip->ino_data.ctime = trans->time;
+               }
                dip->ino_data.mtime = trans->time;
                hammer_modify_inode(dip, HAMMER_INODE_MTIME);
                if (ip) {

diff --git a/sys/vfs/hammer/hammer_vnops.c b/sys/vfs/hammer/hammer_vnops.c
index 220a7d5..bcb481f 100644 (file)
--- a/sys/vfs/hammer/hammer_vnops.c
+++ b/sys/vfs/hammer/hammer_vnops.c
@@ -1626,6 +1626,7 @@ hammer_vop_nrename(struct vop_nrename_args *ap)
                                                ip);
                if (error == 0) {
                        ip->ino_data.parent_obj_id = tdip->obj_id;
+                       ip->ino_data.ctime = trans.time;
                        hammer_modify_inode(ip, HAMMER_INODE_DDIRTY);
                }
        }
@@ -1813,6 +1814,7 @@ hammer_vop_setattr(struct vop_setattr_args *ap)
                if (error == 0) {
                        if (ip->ino_data.uflags != flags) {
                                ip->ino_data.uflags = flags;
+                               ip->ino_data.ctime = trans.time;
                                modflags |= HAMMER_INODE_DDIRTY;
                                kflags |= NOTE_ATTRIB;
                        }
@@ -1849,8 +1851,9 @@ hammer_vop_setattr(struct vop_setattr_args *ap)
                                ip->ino_data.uid = uuid_uid;
                                ip->ino_data.gid = uuid_gid;
                                ip->ino_data.mode = cur_mode;
+                               ip->ino_data.ctime = trans.time;
+                               modflags |= HAMMER_INODE_DDIRTY;
                        }
-                       modflags |= HAMMER_INODE_DDIRTY;
                        kflags |= NOTE_ATTRIB;
                }
        }
@@ -1875,7 +1878,8 @@ hammer_vop_setattr(struct vop_setattr_args *ap)
                                kflags |= NOTE_WRITE | NOTE_EXTEND;
                        }
                        ip->ino_data.size = vap->va_size;
-                       modflags |= HAMMER_INODE_DDIRTY;
+                       ip->ino_data.mtime = trans.time;
+                       modflags |= HAMMER_INODE_MTIME | HAMMER_INODE_DDIRTY;
 
                        /*
                         * on-media truncation is cached in the inode until
@@ -1946,7 +1950,8 @@ hammer_vop_setattr(struct vop_setattr_args *ap)
                        }
                        hammer_ip_frontend_trunc(ip, vap->va_size);
                        ip->ino_data.size = vap->va_size;
-                       modflags |= HAMMER_INODE_DDIRTY;
+                       ip->ino_data.mtime = trans.time;
+                       modflags |= HAMMER_INODE_MTIME | HAMMER_INODE_DDIRTY;
                        kflags |= NOTE_ATTRIB;
                        break;
                default:
@@ -1956,14 +1961,12 @@ hammer_vop_setattr(struct vop_setattr_args *ap)
                break;
        }
        if (vap->va_atime.tv_sec != VNOVAL) {
-               ip->ino_data.atime =
-                       hammer_timespec_to_time(&vap->va_atime);
+               ip->ino_data.atime = hammer_timespec_to_time(&vap->va_atime);
                modflags |= HAMMER_INODE_ATIME;
                kflags |= NOTE_ATTRIB;
        }
        if (vap->va_mtime.tv_sec != VNOVAL) {
-               ip->ino_data.mtime =
-                       hammer_timespec_to_time(&vap->va_mtime);
+               ip->ino_data.mtime = hammer_timespec_to_time(&vap->va_mtime);
                modflags |= HAMMER_INODE_MTIME;
                kflags |= NOTE_ATTRIB;
        }
@@ -1976,6 +1979,7 @@ hammer_vop_setattr(struct vop_setattr_args *ap)
                                         cur_uid, cur_gid, &cur_mode);
                if (error == 0 && ip->ino_data.mode != cur_mode) {
                        ip->ino_data.mode = cur_mode;
+                       ip->ino_data.ctime = trans.time;
                        modflags |= HAMMER_INODE_DDIRTY;
                        kflags |= NOTE_ATTRIB;
                }