From: Jeff Liu <jeff.liu@oracle.com>
To: Dave Chinner <david@fromorbit.com>
Cc: "xfs@oss.sgi.com" <xfs@oss.sgi.com>
Subject: Re: [PATCH] xfs: fix the extent count when allocating an new indirection array entry
Date: Wed, 25 Sep 2013 15:38:11 +0800 [thread overview]
Message-ID: <524292E3.4010902@oracle.com> (raw)
In-Reply-To: <20130923002448.GL12541@dastard>
On 09/23/2013 08:24 AM, Dave Chinner wrote:
> On Sat, Sep 21, 2013 at 10:38:31PM +0800, Jeff Liu wrote:
>> From: Jie Liu <jeff.liu@oracle.com>
>>
>> At xfs_iext_add(), if extent(s) are being appended to the last
>> page in the indirection array and the new extent(s) don't fit
>> in the page, the number of extents(erp->er_extcount) in a new
>> allocated entry should be the minimum value between count and
>> XFS_LINEAR_EXTS, instead of count.
>
> Definitely looks like a bug, but what are the symptoms of it and how
> did you find the problem? Is there any test case that demonstrates a
> problem with the er_extcount being set incorrectly here?
Sorry for the too late response, I found this problem while reading the code.
However, I can not figure out a test case to break the kernel until now. :(
>
>> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
>> ---
>> fs/xfs/xfs_inode_fork.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_inode_fork.c b/fs/xfs/xfs_inode_fork.c
>> index 02f1083..dfb4226 100644
>> --- a/fs/xfs/xfs_inode_fork.c
>> +++ b/fs/xfs/xfs_inode_fork.c
>> @@ -1035,11 +1035,11 @@ xfs_iext_add(
>>
>> while (count) {
>> erp = xfs_iext_irec_new(ifp, erp_idx);
>> - erp->er_extcount = count;
>> - count -= MIN(count, (int)XFS_LINEAR_EXTS);
>> - if (count) {
>> + erp->er_extcount = MIN(count,
>> + (int)XFS_LINEAR_EXTS);
>> + count -= erp->er_extcount;
>
> count is declared as an int, whereas XFS_LINEAR_EXTS probably ends
> up with a type of uint because of a cast in the macro. because we
> are decrementing to zero, the count can be declared as a uint, too,
> and the cast in the MIN() can go away. Indeed, MIN() should be
> converted to min() seeing as we are touching the code here, and if
> you want to retain the current types, the min_t() is appropriate,
> not min(x, (some cast)y)....
Ok, will fix it.
Thanks,
-Jeff
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
prev parent reply other threads:[~2013-09-25 7:37 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-21 14:38 [PATCH] xfs: fix the extent count when allocating an new indirection array entry Jeff Liu
2013-09-23 0:24 ` Dave Chinner
2013-09-25 7:38 ` Jeff Liu [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=524292E3.4010902@oracle.com \
--to=jeff.liu@oracle.com \
--cc=david@fromorbit.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.