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:
> > @@ -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@xxxxxxxxxxxxx
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
|