xfs
[Top] [All Lists]

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

To: Jeff Liu <jeff.liu@xxxxxxxxxx>
Subject: Re: [PATCH] xfs: fix ASSERTION failure in xfs_vm_write_failed()
From: "Michael L. Semon" <mlsemon35@xxxxxxxxx>
Date: Mon, 18 Mar 2013 00:17:14 -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=x-received:message-id:date:from:user-agent:mime-version:to:cc :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=0bj4FQu3usGLZgNjbi8FrU7iH1FEN23nc6ECFm2xl/Q=; b=qCjhJyP63Y4+5hPQnFt3sG+zzSojW1YPH+T+vHsR5eFPnO+Ddf3K7+1AOhXoZ+CfAV s5CXVUnpzxIbXhsLrRJRH12wZ98mR2F+UxQPZCtUP7WpilgUwht0WNq0pA66CqDzEZ4S A6uMyHRxj8AaZrG9ycrdeoP7QQdaI9cJLe5CXqo1ZqANv4Kavk1SJVaNfo/m4kUgaaqh cxaRpHMjRtLye5BUS2saFrBPyhp9xm6VxvBnIGGn+bdGB3kzoKI+vVGaKtLwRQsTDViv nboJQvDPGsrDsNbEQrLSF6+Diaso5N2iekKfVnIIs8ZItrZ61o6YwPfpmlE/8oYs7CCS Wgtg==
In-reply-to: <5145DAB4.40007@xxxxxxxxxx>
References: <5145DAB4.40007@xxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130307 Thunderbird/17.0.4
On 03/17/2013 11:01 AM, Jeff Liu wrote:
On 32-bit system, if the request pos is 64-bit and evaluate block_offset
with (pos & PAGE_MASK) will result in overflows, therefore the assertion
will failed.  We have to check the write offset against (pos & ~0UL) to
avoid this issue as it can evaluate the highest 20 bits on 32-bit correctly
if the pos request is 64-bit and keep the expected result of 64-bit pos request
on 64-bit system unchanged.

Signed-off-by: Jie Liu <jeff.liu@xxxxxxxxxx>
Reported-by: Michael L. Semon <mlsemon35@xxxxxxxxx>
Cc: Dave Chinner <david@xxxxxxxxxxxxx>
---
  fs/xfs/xfs_aops.c |    7 ++++++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 5f707e5..2fc7367 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -1501,7 +1501,12 @@ xfs_vm_write_failed(
        loff_t                  to = from + len;
        struct buffer_head      *bh, *head;

-       ASSERT(block_offset + from == pos);
+       /*
+        * Evaluate block_offset via (pos & PAGE_MASK) on 32-bit system
+        * can cause overflow if the request pos is 64-bit.  Hence we
+        * have to verify the write offset with (pos & ~0UL) to avoid it.
+        */
+       ASSERT(block_offset + from == (pos & ~0UL));

        head = page_buffers(page);
        block_start = 0;

Thanks! I can't help but admire the effort. That stated, I did read Dave's review and now understand the "..." that he left as comments to the original bug report...

My original reason for writing was to refine the test case a little bit. On this 32-bit Pentium III PC, xfstests #078 succeeds on a 560MB device-mapper linear target (1146880 sectors), but it fails with an oops on a 544MB dm-linear target (1114112 sectors). Looking at the output of the `df` command over and over during the test, the data does stop growing at a point between those two numbers, proving Dave's initial observation correct.

Michael

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