[Top] [All Lists]

Re: [PATCH 3.0-stable] xfs: fix xfstest 273 in Linux 3.0-stable

To: Mark Tinguely <tinguely@xxxxxxx>
Subject: Re: [PATCH 3.0-stable] xfs: fix xfstest 273 in Linux 3.0-stable
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 9 Jan 2013 08:48:38 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20130104171304.536607409@xxxxxxx>
References: <20130104171304.536607409@xxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Fri, Jan 04, 2013 at 11:13:00AM -0600, Mark Tinguely wrote:
> upstream commit: b64dfe4e180ab5047c59bcbe379538eb23be4d8e
> Author: Christoph Hellwig <hch@xxxxxxxxxxxxx>
> Date:   Sun Sep 18 20:40:47 2011 +0000
>     xfs: factor delalloc reservations out of xfs_bmapi
>     Move the reservation of delayed allocations, and addition of delalloc
>     regions to the extent trees into a new helper function.  For now
>     this adds some twisted goto logic to xfs_bmapi, but that will be
>     cleaned up in the following patches.
>     Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>     Signed-off-by: Alex Elder <aelder@xxxxxxx>
> Ported to Linux 3.0-stable
> Signed-off-by: Mark Tinguely <tinguely@xxxxxxx>
> ---
> xfstest 273 creates a directory of files created from /dev/zero. It then
> creates many threads, and each thread uses /bin/cp to recursively copy the
> original directory.
> xfstest 273 has been broken from Linux 2.6.39 commit fd0748 to Linux 3.1 rc1+
> commit b64dfe. The copy fails and leaves an empty file. Changing the original
> files from zeros to random values, improves the success rate of the copy.

Why? The contents of the files should make no difference to the
result as it is opaque.

> This patch is a port of patch b64dfe. b64dfe is intended to be refactor patch.
> As best that I can tell, the only difference from the original code is the
> following if statement is not in the refactor code:
>                       /*
>                        * Determine state of extent, and the filesystem.
>                        * A wasdelay extent has been initialized, so
>                        * shouldn't be flagged as unwritten.
>                        */
>                       if (wr && xfs_sb_version_hasextflgbit(&mp->m_sb)) {
>                               if (!wasdelay && (flags & XFS_BMAPI_PREALLOC))
>                                       got.br_state = XFS_EXT_UNWRITTEN;
>                       }

That code isn't touched by the patch at all - maybe you mean the
factored code jumps over it now? However, test 273 doesn't use
unwritten extents, so XFS_BMAPI_PREALLOC will never be set and hence
this code won't affect the test behaviour, anyway. Hence that
doesn't explain whatever problem you are seeing....

> With this patch, xfstest 273 has been working overnight without failure.

That may be so, but AFAICT you haven't discovered the root cause of
the test failure nor iexplained why this factoring patch seems to
fix it....


Dave Chinner

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