xfs
[Top] [All Lists]

Re: [PATCH] xfs: fix s_max_bytes to MAX_LFS_FILESIZE if needed

To: Jeff Liu <jeff.liu@xxxxxxxxxx>
Subject: Re: [PATCH] xfs: fix s_max_bytes to MAX_LFS_FILESIZE if needed
From: "Michael L. Semon" <mlsemon35@xxxxxxxxx>
Date: Tue, 16 Apr 2013 01:55:44 -0400
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>
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=cZwgGAqWvCclbXPpLUhc4WLV3Zd/83Ii53+54bpm3iU=; b=RmbsIjDfPWVExDU8nNoq1xPyzbPxEvM7glrMWOJUu6OU1FWeaU4l6QayKbS5v3be9w taLN+1RNftUxOBxPumDKWZ+lqZnwMgA8X5Q1PGBYbE3KAn3n3zyImr7BWJ1qcgSLas7a 1v/hN8T3BPHXybPzSu5D2r6nJfnVWqoTayra6Rjv8onOZDFwUqf3t/o6TaPbyr6BoBAK pp8Kk0BcfpaTJ5H9NLZ87+Th1B+g/NdN/zvskkslpJjfaBA8N1FH+wnr+1/a1ymxcQgr uvC4oNT8i3sTqSQkgoTfT8/V+X2gy6DCmkFNlUbBCQ/s5bXdza/CWaoY+RD1Y0Yw+xP4 kxMA==
In-reply-to: <516CE451.5010203@xxxxxxxxxx>
References: <5167E160.3020800@xxxxxxxxxx> <5169CC34.9080902@xxxxxxxxx> <516CE451.5010203@xxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130328 Thunderbird/17.0.5
You're welcome. Thanks for the explanation. Now everything makes logical sense.

I'll re-run the tests with different block sizes. The tests have been run already with a) default mkfs options and b) with an external journal and realtime device.

Your related patch, "xfs: don't return 0 if generic_segment_checks() find nothing to write," has also been applied. The suggested test case hasn't been tried, but the patch hasn't caused any additional problems, either.

Anyway, I hope you had/have a good stay here in the US.

Michael

On 04/16/2013 01:40 AM, Jeff Liu wrote:
Hi Michael,

Thanks a lot for help verifying this fix and sorry for my too late
response since I have traveled to US two days ago.

On 04/14/2013 05:20 AM, Michael L. Semon wrote:
Update:  My tests on my original hardware go exactly as they did in my
Pentium 4 test.  xfstests shared/[0-9][0-9][0-9] and xfs/003 through
xfs/136 were run against it.  No problems.  Good job.  I'm keeping the
patch.

My final version of the bug summary goes like this:

On a 32-bit x86 PC, with a Linux kernel that has CONFIG_LBDAF=y...

xfstests generic/308, by writing to a file at an address just before
2**32, causes the following conditions on an XFS filesystem:

1) CPU usage becomes very high,

2) The xfs_io process cannot be killed,

3) The best way to shut down the PC is through use of the magic SysRq keys.

4) Afterwards, attempts to mount the filesystem result in a soft oops.

5) After an `xfs_repair -L` on the filesystem, all is OK, other than for
what was lost by zeroing the log.

J. Liu wrote a patch that solves this problem, but he found the answers
with CONFIG_LBDAF=n, which is a condition for which xfstests generic/308
passes on the two test PCs used.
Ooops! it's wrong.  My answer is misleading, you can think that I drink
too much at that time.:(  Actually, it quite the reverse, i.e. this
issue can be reproduced against 32-bit kernel with CONFIG_LBADF=y, this
is the default configuration of mine.
In this case, I observed that the s_maxbytes is larger than the
MAX_LFS_FILESIZE.  Hence, the current patch is written to make sure that
the s_maxbytes should not beyond this limits.

For 32-bit kernel with CONFIG_LBADF=n, the s_maxbytes is just equal to
MAX_LFS_FILESIZE, so the test is works to me.  BTW, I only verified this
fix upon the default mkfs options. i.e, 4k blocksize, that is, mkfs.xfs
/dev/sdX.

Tests were conducted on a Pentium III (kernel 3.9-rc4 with numerous SGI
patches) and on a Pentium 4 (kernel 3.9-rc6 with numerous SGI patches).

Could you verify these things by memory (no need to retest)?
As I mentioned above.
a) With CONFIG_LBDAF=y, generic/308 caused filesystem corruption, and
In this case, the operation should be denied with -EFBIG error returned
if trying to create a huge file.

b) With CONFIG_LBDAF=n, generic/308 passed the test.
Even don't applying this patch, the test run passed for the default mkfs
setup.

c) Having CONFIG_LBDAF=n helped you to find the answers and write this
fine patch.
CONFIG_LBADF=y instead.

Thanks again!
-Jeff

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