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

[Dave Chinner <david@fromorbit.com>]

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.

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

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.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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