[PATCH 3/5] xfs: don't warn on buffers not being recovered due to LSN

[Brian Foster]

On Mon, Aug 29, 2016 at 11:25:14AM +1000, Dave Chinner wrote:
> On Thu, Aug 11, 2016 at 01:11:05PM -0400, Brian Foster wrote:
> > The log recovery buffer validation function is invoked in cases where a
> > buffer update may be skipped due to LSN ordering. If the validation
> > function happens to come across directory conversion situations (e.g., a
> > dir3 block to data conversion), it may warn about seeing a buffer log
> > format of one type and a buffer with a magic number of another.
> > 
> > This warning is not valid as the buffer update is ultimately skipped.
> > This is indicated by a current_lsn of NULLCOMMITLSN provided by the
> > caller. As such, update xlog_recover_validate_buf_type() to only warn in
> > such cases when a buffer update is expected.
> > 
> > XXX: other issues here? better to not validate in such cases?
> 
> I think this is OK - we really want to ensure that buffers that are
> in cache always have the correct verifier attached to them. Hence if
> we've read the buffer in, even if we are not modifying it because
> it's more recent that what is being replayed we should still attach
> the verifiers to it.
> 
> If it changes type due to later recovery replay, we'll change the
> verifier appropriately at that point.
> 

Sounds good, thanks for commenting on this.

> > @@ -2557,6 +2542,16 @@ xlog_recover_validate_buf_type(
> >  			 xfs_blft_from_flags(buf_f));
> >  		break;
> >  	}
> > +
> > +	/*
> > +	 * A NULL current LSN indicates the buffer update is skipped due to LSN
> > +	 * ordering. Don't warn in such cases, we skip the update for a reason
> > +	 * (it's no longer valid)!
> > +	 */
> 
> I read that the first time as "we skip the update for a reason that
> is no longer valid" :P
> 

Heh, Ok.

> perhaps rework this to make it clear what is being referred to here.
> e.g. No need to warn if the the buffer contents are more recent
> that this recovery item and hence recovery did not modify the
> buffer.
> 

Yeah, I'll try to make that more clear.

Brian

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david at fromorbit.com
> 
> _______________________________________________
> xfs mailing list
> xfs at oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs