xfs
[Top] [All Lists]

Re: [PATCH 3/3] xfs: prevent needless mount warning causing test failure

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 3/3] xfs: prevent needless mount warning causing test failures
From: Ben Myers <bpm@xxxxxxx>
Date: Tue, 8 May 2012 11:29:42 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1335519922-14371-4-git-send-email-david@xxxxxxxxxxxxx>
References: <1335519922-14371-1-git-send-email-david@xxxxxxxxxxxxx> <1335519922-14371-4-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Fri, Apr 27, 2012 at 07:45:22PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> Often mounting small filesystem with small logs will emit a warning
> such as:
> 
> XFS (vdb): Invalid block length (0x2000) for buffer
> 
> during log recovery. This causes tests to randomly fail because this
> output causes the clean filesystem checks on test completion to
> think the filesystem is inconsistent.
> 
> The cause of the error is simply that log recovery is asking for a
> buffer size that is larger than the log when zeroing the tail. This
> is because the buffer size is rounded up, and if the right head and
> tail conditions exist then the buffer size can be larger than the log.
> Limit the variable size xlog_get_bp() callers to requesting buffers
> smaller than the log.
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
>  fs/xfs/xfs_log_recover.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index d7abe5f..ca38690 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -441,6 +441,8 @@ xlog_find_verify_cycle(
>        * a log sector, or we're out of luck.
>        */
>       bufblks = 1 << ffs(nbblks);
> +     while (bufblks > log->l_logBBsize)
> +             bufblks >>= 1;

AFAICS you don't need a loop here.  The following would be sufficient to make
xlog_buf_bbcount_valid return 0. 

if (bufblks > log->l_logBBsize)
        bufblks = log->l_logBBsize;

It is a bit more obviously correct.

-Ben

<Prev in Thread] Current Thread [Next in Thread>