On Mon, Aug 29, 2016 at 11:29:23AM +1000, Dave Chinner wrote:

On Thu, Aug 11, 2016 at 01:11:06PM -0400, Brian Foster wrote:

quoted

@@ -2552,6 +2562,27 @@ xlog_recover_validate_buf_type(
 		xfs_warn(mp, warnmsg);
 		ASSERT(0);
 	}
+
+	/*
+	 * We must update the metadata LSN of the buffer as it is written out to
+	 * ensure that older transactions never replay over this one and corrupt
+	 * the buffer. This can occur if log recovery is interrupted at some
+	 * point after the current transaction completes, at which point a
+	 * subsequent mount starts recovery from the beginning.
+	 *
+	 * Write verifiers update the metadata LSN from log items attached to
+	 * the buffer. Therefore, initialize a bli purely to carry the LSN to
+	 * the verifier. We'll clean it up in our ->iodone() callback.
+	 */
+	if (bp->b_ops && current_lsn != NULLCOMMITLSN) {
+		struct xfs_buf_log_item	*bip;
+
+		ASSERT(!bp->b_iodone || bp->b_iodone == xlog_recover_iodone);
+		bp->b_iodone = xlog_recover_iodone;
+		xfs_buf_item_init(bp, mp);
+		bip = bp->b_fspriv;
+		bip->bli_item.li_lsn = current_lsn;
+	}
 }

Of, so now we have two things we do when current_lsn !=
NULLCOMMITLSN. I'd change this to something like:


	ASSERT(bp->b_fspriv == NULL);
	if (current_lsn == NULLCOMMITLSN)
		return;

	if (warn) {
		....
	}

	if (!bp->b_ops)
		return

	/* add buf_item */

Ok, I may still invert the bp->b_ops check as that seems more clear to
me for whatever reason, but otherwise that looks like a nice cleanup.
Thanks.

Brian

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs