HAMMER 45/Many: Stabilization pass, undo sequencing.
authorMatthew Dillon <dillon@dragonflybsd.org>
Thu, 15 May 2008 03:36:40 +0000 (03:36 +0000)
committerMatthew Dillon <dillon@dragonflybsd.org>
Thu, 15 May 2008 03:36:40 +0000 (03:36 +0000)
* The flusher was improperly requesting a reflush on buffers.  The flush
  request was being defered for any buffers with active front-end references
  and then wound up being flushed by the front-end, breaking ordering
  requirements.

  Remove the reflush flag entirely.  This fixes numerous crash recovery
  cases.

* Add a missing unlock in the reblocking ioctl code which was responsible
  for a number of process lockups.

* Enhance the undo recovery kprintf.

* Validate the CRC in UNDO records

sys/vfs/hammer/hammer.h
sys/vfs/hammer/hammer_flusher.c
sys/vfs/hammer/hammer_io.c
sys/vfs/hammer/hammer_ondisk.c
sys/vfs/hammer/hammer_reblock.c
sys/vfs/hammer/hammer_recover.c
sys/vfs/hammer/hammer_undo.c

index 9a0b363..68b9cdb 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.h,v 1.67 2008/05/13 20:46:54 dillon Exp $
+ * $DragonFly: src/sys/vfs/hammer/hammer.h,v 1.68 2008/05/15 03:36:40 dillon Exp $
  */
 /*
  * This header file contains structures used internally by the HAMMERFS
@@ -386,7 +386,6 @@ struct hammer_io {
        u_int           running : 1;    /* bp write IO in progress */
        u_int           waiting : 1;    /* someone is waiting on us */
        u_int           validated : 1;  /* ondisk has been validated */
-       u_int           flush : 1;      /* flush on last release */
        u_int           waitdep : 1;    /* flush waits for dependancies */
 };
 
@@ -829,7 +828,7 @@ void hammer_io_init(hammer_io_t io, hammer_mount_t hmp,
 void hammer_io_reinit(hammer_io_t io, enum hammer_io_type type);
 int hammer_io_read(struct vnode *devvp, struct hammer_io *io);
 int hammer_io_new(struct vnode *devvp, struct hammer_io *io);
-void hammer_io_release(struct hammer_io *io);
+void hammer_io_release(struct hammer_io *io, int flush);
 void hammer_io_flush(struct hammer_io *io);
 int hammer_io_checkflush(hammer_io_t io);
 void hammer_io_clear_modify(struct hammer_io *io);
index 31fc2aa..066a2f0 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_flusher.c,v 1.15 2008/05/13 20:46:55 dillon Exp $
+ * $DragonFly: src/sys/vfs/hammer/hammer_flusher.c,v 1.16 2008/05/15 03:36:40 dillon Exp $
  */
 /*
  * HAMMER dependancy flusher thread
@@ -286,7 +286,7 @@ hammer_flusher_finalize(hammer_transaction_t trans)
                hammer_ref(&io->lock);
                KKASSERT(io->type != HAMMER_STRUCTURE_VOLUME);
                hammer_io_flush(io);
-               hammer_rel_buffer((hammer_buffer_t)io, 1);
+               hammer_rel_buffer((hammer_buffer_t)io, 0);
                ++count;
        }
        if (count)
@@ -301,7 +301,7 @@ hammer_flusher_finalize(hammer_transaction_t trans)
                hammer_ref(&io->lock);
                KKASSERT(io->type != HAMMER_STRUCTURE_VOLUME);
                hammer_io_flush(io);
-               hammer_rel_buffer((hammer_buffer_t)io, 1);
+               hammer_rel_buffer((hammer_buffer_t)io, 0);
                ++count;
        }
        if (count)
@@ -325,6 +325,13 @@ hammer_flusher_finalize(hammer_transaction_t trans)
                hammer_modify_volume_done(root_volume);
        }
 
+       if (hammer_debug_recover_faults > 0) {
+               if (--hammer_debug_recover_faults == 0) {
+                       Debugger("hammer_debug_recover_faults");
+               }
+       }
+
+
        /*
         * Update the UNDO FIFO's first_offset.  Same deal.
         */
@@ -335,7 +342,7 @@ hammer_flusher_finalize(hammer_transaction_t trans)
                hammer_crc_set_blockmap(&root_volume->ondisk->vol0_blockmap[HAMMER_ZONE_UNDO_INDEX]);
                hammer_modify_volume_done(root_volume);
        }
-       trans->hmp->flusher_undo_start = rootmap->next_offset;
+       hmp->flusher_undo_start = rootmap->next_offset;
 
        /*
         * Flush the root volume header.
@@ -367,7 +374,7 @@ hammer_flusher_finalize(hammer_transaction_t trans)
                hammer_ref(&io->lock);
                KKASSERT(io->type != HAMMER_STRUCTURE_VOLUME);
                hammer_io_flush(io);
-               hammer_rel_buffer((hammer_buffer_t)io, 1);
+               hammer_rel_buffer((hammer_buffer_t)io, 0);
                ++count;
        }
        hammer_unlock(&hmp->sync_lock);
index bc2a533..5bcab68 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_io.c,v 1.30 2008/05/06 00:21:08 dillon Exp $
+ * $DragonFly: src/sys/vfs/hammer/hammer_io.c,v 1.31 2008/05/15 03:36:40 dillon Exp $
  */
 /*
  * IO Primitives and buffer cache management
@@ -242,7 +242,7 @@ hammer_io_new(struct vnode *devvp, struct hammer_io *io)
  * by HAMMER until explicitly flushed by the backend.
  */
 void
-hammer_io_release(struct hammer_io *io)
+hammer_io_release(struct hammer_io *io, int flush)
 {
        struct buf *bp;
 
@@ -258,7 +258,7 @@ hammer_io_release(struct hammer_io *io)
         * by HAMMER.
         */
        if (io->modified) {
-               if (io->flush) {
+               if (flush) {
                        hammer_io_flush(io);
                } else if (bp->b_flags & B_LOCKED) {
                        switch(io->type) {
@@ -284,7 +284,7 @@ hammer_io_release(struct hammer_io *io)
         * that our bioops can override kernel decisions with regards to
         * the buffer).
         */
-       if (io->flush && io->modified == 0 && io->running == 0) {
+       if (flush && io->modified == 0 && io->running == 0) {
                /*
                 * Always disassociate the bp if an explicit flush
                 * was requested and the IO completed with no error
@@ -340,7 +340,6 @@ hammer_io_flush(struct hammer_io *io)
         * Degenerate case - nothing to flush if nothing is dirty.
         */
        if (io->modified == 0) {
-               io->flush = 0;
                return;
        }
 
@@ -388,7 +387,6 @@ hammer_io_flush(struct hammer_io *io)
        TAILQ_REMOVE(io->mod_list, io, mod_entry);
        io->mod_list = NULL;
        io->modified = 0;
-       io->flush = 0;
 
        /*
         * Transfer ownership to the kernel and initiate I/O.
index fddafcf..b613fb4 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_ondisk.c,v 1.44 2008/05/13 20:46:55 dillon Exp $
+ * $DragonFly: src/sys/vfs/hammer/hammer_ondisk.c,v 1.45 2008/05/15 03:36:40 dillon Exp $
  */
 /*
  * Manage HAMMER's on-disk structures.  These routines are primarily
@@ -288,9 +288,8 @@ hammer_unload_volume(hammer_volume_t volume, void *data __unused)
        /*
         * Release our buffer and flush anything left in the buffer cache.
         */
-       volume->io.flush = 1;
        volume->io.waitdep = 1;
-       hammer_io_release(&volume->io);
+       hammer_io_release(&volume->io, 1);
 
        /*
         * There should be no references on the volume, no clusters, and
@@ -452,15 +451,13 @@ hammer_load_volume(hammer_volume_t volume)
 void
 hammer_rel_volume(hammer_volume_t volume, int flush)
 {
-       if (flush)
-               volume->io.flush = 1;
        crit_enter();
        if (volume->io.lock.refs == 1) {
                ++volume->io.loading;
                hammer_lock_ex(&volume->io.lock);
                if (volume->io.lock.refs == 1) {
                        volume->ondisk = NULL;
-                       hammer_io_release(&volume->io);
+                       hammer_io_release(&volume->io, flush);
                }
                --volume->io.loading;
                hammer_unlock(&volume->io.lock);
@@ -695,14 +692,12 @@ hammer_rel_buffer(hammer_buffer_t buffer, int flush)
        hammer_volume_t volume;
        int freeme = 0;
 
-       if (flush)
-               buffer->io.flush = 1;
        crit_enter();
        if (buffer->io.lock.refs == 1) {
                ++buffer->io.loading;   /* force interlock check */
                hammer_lock_ex(&buffer->io.lock);
                if (buffer->io.lock.refs == 1) {
-                       hammer_io_release(&buffer->io);
+                       hammer_io_release(&buffer->io, flush);
                        hammer_flush_buffer_nodes(buffer);
                        KKASSERT(TAILQ_EMPTY(&buffer->clist));
 
index 7b3daa9..14d1136 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_reblock.c,v 1.14 2008/05/12 21:17:18 dillon Exp $
+ * $DragonFly: src/sys/vfs/hammer/hammer_reblock.c,v 1.15 2008/05/15 03:36:40 dillon Exp $
  */
 /*
  * HAMMER reblocker - This code frees up fragmented physical space
@@ -99,6 +99,8 @@ retry:
                /*
                 * Acquiring the sync_lock prevents the operation from
                 * crossing a synchronization boundary.
+                *
+                * NOTE: cursor.node may have changed on return.
                 */
                hammer_lock_ex(&trans->hmp->sync_lock);
                error = hammer_reblock_helper(reblock, &cursor, elm);
@@ -273,9 +275,6 @@ hammer_reblock_node(struct hammer_ioc_reblock *reblock,
 
        onode = cursor->node;
        nnode = hammer_alloc_btree(cursor->trans, &error);
-       hammer_lock_ex(&nnode->lock);
-
-       hammer_modify_node_noundo(cursor->trans, nnode);
 
        if (nnode == NULL)
                return (error);
@@ -283,6 +282,8 @@ hammer_reblock_node(struct hammer_ioc_reblock *reblock,
        /*
         * Move the node
         */
+       hammer_lock_ex(&nnode->lock);
+       hammer_modify_node_noundo(cursor->trans, nnode);
        bcopy(onode->ondisk, nnode->ondisk, sizeof(*nnode->ondisk));
 
        if (elm) {
@@ -317,8 +318,9 @@ hammer_reblock_node(struct hammer_ioc_reblock *reblock,
                        onode->node_offset, nnode->node_offset);
        }
        hammer_modify_node_done(nnode);
-
        cursor->node = nnode;
+
+       hammer_unlock(&onode->lock);
        hammer_rel_node(onode);
 
        return (error);
index 16c9b79..6b70dd3 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_recover.c,v 1.16 2008/05/06 00:21:08 dillon Exp $
+ * $DragonFly: src/sys/vfs/hammer/hammer_recover.c,v 1.17 2008/05/15 03:36:40 dillon Exp $
  */
 
 #include "hammer.h"
@@ -83,8 +83,11 @@ hammer_recover(hammer_mount_t hmp, hammer_volume_t root_volume)
                bytes = rootmap->alloc_offset - rootmap->first_offset +
                        (rootmap->next_offset & HAMMER_OFF_LONG_MASK);
        }
-       kprintf("HAMMER(%s) Start Recovery (%lld bytes of UNDO)\n",
-               root_volume->ondisk->vol_name, bytes);
+       kprintf("HAMMER(%s) Start Recovery %016llx - %016llx "
+               "(%lld bytes of UNDO)\n",
+               root_volume->ondisk->vol_name,
+               rootmap->first_offset, rootmap->next_offset,
+               bytes);
        if (bytes > (rootmap->alloc_offset & HAMMER_OFF_LONG_MASK)) {
                kprintf("Undo size is absurd, unable to mount\n");
                return(EIO);
@@ -211,6 +214,7 @@ hammer_recover_undo(hammer_mount_t hmp, hammer_fifo_undo_t undo, int bytes)
        int vol_no;
        int max_bytes;
        u_int32_t offset;
+       u_int32_t crc;
 
        /*
         * Basic sanity checks
@@ -232,11 +236,23 @@ hammer_recover_undo(hammer_mount_t hmp, hammer_fifo_undo_t undo, int bytes)
 
        /*
         * Skip PAD records.  Note that PAD records also do not require
-        * a tail.
+        * a tail and may have a truncated structure.
         */
        if (undo->head.hdr_type == HAMMER_HEAD_TYPE_PAD)
                return(0);
 
+       /*
+        * Check the CRC
+        */
+       crc = crc32(undo, HAMMER_FIFO_HEAD_CRCOFF) ^
+             crc32(&undo->head + 1, undo->head.hdr_size - sizeof(undo->head));
+       if (undo->head.hdr_crc != crc) {
+               kprintf("HAMMER: Undo record CRC failed %08x %08x\n",
+                       undo->head.hdr_crc, crc);
+               return(EIO);
+       }
+
+
        /*
         * Check the tail
         */
index 23c3a34..884c84b 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.14 2008/05/06 00:21:08 dillon Exp $
+ * $DragonFly: src/sys/vfs/hammer/hammer_undo.c,v 1.15 2008/05/15 03:36:40 dillon Exp $
  */
 
 /*
@@ -87,7 +87,6 @@ hammer_generate_undo(hammer_transaction_t trans, hammer_io_t io,
                     hammer_off_t zone_off, void *base, int len)
 {
        hammer_volume_t root_volume;
-       hammer_volume_ondisk_t ondisk;
        hammer_blockmap_t undomap;
        hammer_buffer_t buffer = NULL;
        hammer_fifo_undo_t undo;
@@ -104,7 +103,6 @@ hammer_generate_undo(hammer_transaction_t trans, hammer_io_t io,
                return(0);
 
        root_volume = trans->rootvol;
-       ondisk = root_volume->ondisk;
        undomap = &trans->hmp->blockmap[HAMMER_ZONE_UNDO_INDEX];
 
        /* no undo recursion */