All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael L. Semon" <mlsemon35@gmail.com>
To: Jeff Liu <jeff.liu@oracle.com>
Cc: "xfs@oss.sgi.com" <xfs@oss.sgi.com>
Subject: Re: [PATCH] xfs: fix ASSERTION failure in xfs_vm_write_failed()
Date: Mon, 18 Mar 2013 00:17:14 -0400	[thread overview]
Message-ID: <5146954A.20802@gmail.com> (raw)
In-Reply-To: <5145DAB4.40007@oracle.com>

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@oracle.com>
> Reported-by: Michael L. Semon <mlsemon35@gmail.com>
> Cc: Dave Chinner <david@fromorbit.com>
> ---
>   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

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

      parent reply	other threads:[~2013-03-18  4:17 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-17 15:01 [PATCH] xfs: fix ASSERTION failure in xfs_vm_write_failed() Jeff Liu
2013-03-17 23:53 ` Dave Chinner
2013-03-18  4:12   ` Jeff Liu
2013-03-18  4:17 ` Michael L. Semon [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5146954A.20802@gmail.com \
    --to=mlsemon35@gmail.com \
    --cc=jeff.liu@oracle.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.