xfs
[Top] [All Lists]

Re: [PATCH v2] xfs: fix assertion failure in xfs_vm_write_failed()

To: Jeff Liu <jeff.liu@xxxxxxxxxx>
Subject: Re: [PATCH v2] xfs: fix assertion failure in xfs_vm_write_failed()
From: "Michael L. Semon" <mlsemon35@xxxxxxxxx>
Date: Mon, 18 Mar 2013 16:03:11 -0400
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>, Dave Chinner <david@xxxxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type; bh=O1TWzoroDzKyH35NqtlTx6pGz/QEzgWkhW6m8CdzKGo=; b=vgZy7vggRO31OJkOEWQ9VpWw298fcf9DseO7IFE7Pj8DprqJABLCGIqePuZe+og2rh Wkw3/5PtOABNkKFHuVTlnOc0lvbUoUwKEPMQmAamhc5X9fTQ3vzCNm3zusBeG1FZoLNK aL79z/FhUG0pNNY1a0gvvsoW2Z8T2mv4YZ6gedTEXavzke/oRVvH2k7g3W6IwhQsTUML 3RxgTdeZ6YuHS4BlxsCTt0Gl59NZd1VeTW6g/uhuK0iMY79ROi0Gs1iWLWGqkHviqZya mlOgfC1JbXpPcpBrE7gNNQ7g0NQ/CcgVRk0vMu9ZXkml7/ANLRCBRv1Ek2GzuFQxQLTW K/mA==
In-reply-to: <51469C90.1000001@xxxxxxxxxx>
References: <51469C90.1000001@xxxxxxxxxx>
Looks good in limited testing.  I set some extra debug code somewhere,
causing every failed mount in xfstests #078 to show up, so the
original test cases were double-checked, both on the main partition
(mid-upgrade) and on a backup boot partition:

Before Liu Patch v2:

#078, 32-bit Linux, larger partitions: PASS
#078, 32-bit Linux, smaller partitions: soft oops before test could fail

After Liu Patch v2:

#078, 32-bit Linux, larger partitions: PASS
#078, 32-bit Linux, smaller partitions: FAIL, no space left on device (CORRECT)

64-bit cases were not tested.  Additionally, tests 001-005 were run
(for my sake), and test 112 was run (pass).

The main partition on this PC for / is XFS, and I still have a file
system, so that's a good sign.  A gcc compile was left running because
I had to go to work, and I'll let you know if I don't have a file
system when I get back home.

Conclusion: Success on 32-bit, and failure is not seen for the near
future.  Testing in the next 24 hours may change this conclusion.

Thanks!

Michael

On Mon, Mar 18, 2013 at 12:48 AM, Jeff Liu <jeff.liu@xxxxxxxxxx> wrote:
> Hello,
>
> Here is the v2 patch for fixing ASSERTION failed at xfs_vm_write_failed() 
> according
> to Dave's comments, so I added Dave as SOB for credit.
>
> Hi Michael,
> Please kindly try at your at your convenience.
>
> Thanks,
> -Jeff

> This patch fix the block_offset evaluation to clear the lower 12 bits as:
> block_offset = pos >> PAGE_CACHE_SHIFT) << PAGE_CACHE_SHIFT
> Hence, the ASSERTION should be correct because the from offset in a page
> is evaluated to have the lower 12 bits only.
>
> Thanks Dave Chinner for pointing this out.
>
> Reported-by: Michael L. Semon <mlsemon35@xxxxxxxxx>
> Signed-off-by: Jie Liu <jeff.liu@xxxxxxxxxx>
> Signed-off-by: Dave Chinner <david@xxxxxxxxxxxxx>
> ---
>  fs/xfs/xfs_aops.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index 5f707e5..a418e17 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -1494,7 +1494,8 @@ xfs_vm_write_failed(
>         loff_t                  pos,
>         unsigned                len)
>  {
> -       loff_t                  block_offset = pos & PAGE_MASK;
> +       loff_t                  block_offset = (pos >> PAGE_CACHE_SHIFT) <<
> +                                               PAGE_CACHE_SHIFT;
>         loff_t                  block_start;
>         loff_t                  block_end;
>         loff_t                  from = pos & (PAGE_CACHE_SIZE - 1);
> --
> 1.7.9.5

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