From: Matthew Dillon <dillon@dragonflybsd.org>
Date: Thu, 15 May 2008 03:36:40 +0000 (+0000)
Subject: HAMMER 45/Many: Stabilization pass, undo sequencing.
X-Git-Tag: v2.0.1~644
X-Git-Url: https://gitweb.dragonflybsd.org/dragonfly.git/commitdiff_plain/09ac686b997c174abdd1561c4319954b6575edae

HAMMER 45/Many: Stabilization pass, undo sequencing.

* 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
---

diff --git a/sys/vfs/hammer/hammer.h b/sys/vfs/hammer/hammer.h
index 9a0b3635e5..68b9cdb526 100644
--- a/sys/vfs/hammer/hammer.h
+++ b/sys/vfs/hammer/hammer.h
@@ -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);
diff --git a/sys/vfs/hammer/hammer_flusher.c b/sys/vfs/hammer/hammer_flusher.c
index 31fc2aa357..066a2f0191 100644
--- a/sys/vfs/hammer/hammer_flusher.c
+++ b/sys/vfs/hammer/hammer_flusher.c
@@ -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);
diff --git a/sys/vfs/hammer/hammer_io.c b/sys/vfs/hammer/hammer_io.c
index bc2a533a2c..5bcab68cb8 100644
--- a/sys/vfs/hammer/hammer_io.c
+++ b/sys/vfs/hammer/hammer_io.c
@@ -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.
diff --git a/sys/vfs/hammer/hammer_ondisk.c b/sys/vfs/hammer/hammer_ondisk.c
index fddafcfad6..b613fb442e 100644
--- a/sys/vfs/hammer/hammer_ondisk.c
+++ b/sys/vfs/hammer/hammer_ondisk.c
@@ -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));
 
diff --git a/sys/vfs/hammer/hammer_reblock.c b/sys/vfs/hammer/hammer_reblock.c
index 7b3daa92c2..14d1136afc 100644
--- a/sys/vfs/hammer/hammer_reblock.c
+++ b/sys/vfs/hammer/hammer_reblock.c
@@ -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);
diff --git a/sys/vfs/hammer/hammer_recover.c b/sys/vfs/hammer/hammer_recover.c
index 16c9b79f93..6b70dd376f 100644
--- a/sys/vfs/hammer/hammer_recover.c
+++ b/sys/vfs/hammer/hammer_recover.c
@@ -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
 	 */
diff --git a/sys/vfs/hammer/hammer_undo.c b/sys/vfs/hammer/hammer_undo.c
index 23c3a34de2..884c84bfc3 100644
--- a/sys/vfs/hammer/hammer_undo.c
+++ b/sys/vfs/hammer/hammer_undo.c
@@ -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 */