HAMMER 40G/Many: UNDO cleanup & stabilization.
authorMatthew Dillon <dillon@dragonflybsd.org>
Sun, 4 May 2008 19:57:42 +0000 (19:57 +0000)
committerMatthew Dillon <dillon@dragonflybsd.org>
Sun, 4 May 2008 19:57:42 +0000 (19:57 +0000)
* Fix a race in the undo record allocator that could result in a
  corrupted UNDO FIFO.

* Fix improperly placed calls to hammer_test_inode().

* Properly account for nlinks when a deleted ADD record is to be
  converted to a DEL record by the flush.  In this case the frontend's
  notion of nlinks accounts for the deletion but the backend must sync
  the record anyway, so the backend needs to bump the link count by one.

sys/vfs/hammer/hammer_inode.c
sys/vfs/hammer/hammer_object.c
sys/vfs/hammer/hammer_undo.c

index 385ab1b..986d03b 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.50 2008/05/04 09:06:45 dillon Exp $
+ * $DragonFly: src/sys/vfs/hammer/hammer_inode.c,v 1.51 2008/05/04 19:57:42 dillon Exp $
  */
 
 #include "hammer.h"
@@ -281,8 +281,8 @@ retry:
                }
                ip->flags |= HAMMER_INODE_ONDISK;
        } else {
-               kprintf("hammer_get_inode: failed ip %p cursor %p error %d\n",
-                       ip, &cursor, *errorp);
+               kprintf("hammer_get_inode: failed ip %p obj_id %016llx cursor %p error %d\n",
+                       ip, ip->obj_id, &cursor, *errorp);
                /*Debugger("x");*/
                --hammer_count_inodes;
                kfree(ip, M_HAMMER);
@@ -1346,7 +1346,8 @@ hammer_sync_inode(hammer_inode_t ip)
         * purposes of synchronization to disk.
         *
         * Records which are in our flush group can be unlinked from our
-        * inode now, allowing the inode to be physically deleted.
+        * inode now, potentially allowing the inode to be physically
+        * deleted.
         */
        nlinks = ip->ino_rec.ino_nlinks;
        next = TAILQ_FIRST(&ip->target_list);
@@ -1354,10 +1355,29 @@ hammer_sync_inode(hammer_inode_t ip)
                next = TAILQ_NEXT(depend, target_entry);
                if (depend->flush_state == HAMMER_FST_FLUSH &&
                    depend->flush_group == ip->hmp->flusher_act) {
-                       TAILQ_REMOVE(&ip->target_list, depend, target_entry);
-                       depend->target_ip = NULL;
-                       /* no need to signal target_ip, it is us */
+                       /*
+                        * If this is an ADD that was deleted by the frontend
+                        * the frontend nlinks count will have already been
+                        * decremented, but the backend is going to sync its
+                        * directory entry and must account for it.  The
+                        * record will be converted to a delete-on-disk when
+                        * it gets synced.
+                        *
+                        * If the ADD was not deleted by the frontend we
+                        * can remove the dependancy from our target_list.
+                        */
+                       if (depend->flags & HAMMER_RECF_DELETED_FE) {
+                               ++nlinks;
+                       } else {
+                               TAILQ_REMOVE(&ip->target_list, depend,
+                                            target_entry);
+                               depend->target_ip = NULL;
+                       }
                } else if ((depend->flags & HAMMER_RECF_DELETED_FE) == 0) {
+                       /*
+                        * Not part of our flush group
+                        */
+                       KKASSERT((depend->flags & HAMMER_RECF_DELETED_BE) == 0);
                        switch(depend->type) {
                        case HAMMER_MEM_RECORD_ADD:
                                --nlinks;
@@ -1625,8 +1645,12 @@ hammer_inode_unloadable_check(hammer_inode_t ip, int getvp)
        struct vnode *vp;
 
        /*
-        * If the inode is on-media and the link count is 0 we MUST delete
-        * it on-media.  DELETING is a mod flag, DELETED is a state flag.
+        * Set the DELETING flag when the link count drops to 0 and the
+        * OS no longer has any opens on the inode.
+        *
+        * The backend will clear DELETING (a mod flag) and set DELETED
+        * (a state flag) when it is actually able to perform the
+        * operation.
         */
        if (ip->ino_rec.ino_nlinks == 0 &&
            (ip->flags & (HAMMER_INODE_DELETING|HAMMER_INODE_DELETED)) == 0) {
index d242914..f87672a 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.53 2008/05/03 20:21:20 dillon Exp $
+ * $DragonFly: src/sys/vfs/hammer/hammer_object.c,v 1.54 2008/05/04 19:57:42 dillon Exp $
  */
 
 #include "hammer.h"
@@ -215,10 +215,13 @@ hammer_flush_record_done(hammer_record_t record, int error)
                }
                record->flush_state = HAMMER_FST_IDLE;
        } else {
-               if (record->target_ip)
+               if (record->target_ip) {
                        record->flush_state = HAMMER_FST_SETUP;
-               else
+                       hammer_test_inode(record->ip);
+                       hammer_test_inode(record->target_ip);
+               } else {
                        record->flush_state = HAMMER_FST_IDLE;
+               }
        }
        record->flags &= ~HAMMER_RECF_INTERLOCK_BE;
        if (record->flags & HAMMER_RECF_WANTED) {
@@ -756,6 +759,7 @@ done:
        return(error);
 }
 
+#if 0
 /*
  * Sync an in-memory record to the disk.  This is called by the backend.
  * This code is responsible for actually writing a record out to the disk.
@@ -786,6 +790,8 @@ hammer_ip_sync_record(hammer_transaction_t trans, hammer_record_t record)
        return (error);
 }
 
+#endif
+
 int
 hammer_ip_sync_record_cursor(hammer_cursor_t cursor, hammer_record_t record)
 {
@@ -940,11 +946,9 @@ hammer_ip_sync_record_cursor(hammer_cursor_t cursor, hammer_record_t record)
                        KKASSERT(record->type == HAMMER_MEM_RECORD_ADD);
                        record->flags &= ~HAMMER_RECF_DELETED_FE;
                        record->type = HAMMER_MEM_RECORD_DEL;
-                       if (record->flush_state == HAMMER_FST_SETUP) {
-                               hammer_test_inode(record->ip);
-                               hammer_test_inode(record->target_ip);
-                       }
+                       KKASSERT(record->flush_state == HAMMER_FST_FLUSH);
                        record->flags &= ~HAMMER_RECF_CONVERT_DELETE;
+                       /* hammer_flush_record_done takes care of the rest */
                } else {
                        record->flags |= HAMMER_RECF_DELETED_FE;
                        record->flags |= HAMMER_RECF_DELETED_BE;
index 334b6f8..b12bd89 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_undo.c,v 1.11 2008/05/04 09:06:45 dillon Exp $
+ * $DragonFly: src/sys/vfs/hammer/hammer_undo.c,v 1.12 2008/05/04 19:57:42 dillon Exp $
  */
 
 /*
@@ -132,17 +132,15 @@ again:
        }
 
        undo = hammer_bread(trans->hmp, next_offset, &error, &buffer);
+       hammer_modify_buffer(NULL, buffer, NULL, 0);
 
        /*
         * We raced another thread, try again.
         */
-       if (undomap->next_offset != next_offset)
+       if (undomap->next_offset != next_offset) {
+               hammer_modify_buffer_done(buffer);
                goto again;
-
-       hammer_modify_buffer(NULL, buffer, NULL, 0);
-
-       /* XXX eventually goto again again, but for now catch it */
-       KKASSERT(undomap->next_offset == next_offset);
+       }
 
        /*
         * The FIFO entry would cross a buffer boundary, PAD to the end