Btrfs devel - Dec 2011 - [PATCH 2/2] Btrfs: fix wrong i_size when truncating a file to a larger size

Btrfsck report error 100 after the 83th case of xfstests was run, it means
the i_size of the file is wrong.

The reason of this bug is that:
Btrfs increased i_size of the file at the beginning, but it failed to expand
the file, and failed to update the i_size to the old size because there is no
enough space in the file system, so we found a wrong i_size.

This patch fixes this bug by updating the i_size just when we pass the file
expanding and get enough space to update i-node.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/inode.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 960818b..f1c4bce 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3373,6 +3373,8 @@ int btrfs_cont_expand(struct inode *inode, loff_t oldsize,
loff_t size)
 
 static int btrfs_setsize(struct inode *inode, loff_t newsize)
 {
+	struct btrfs_root *root = BTRFS_I(inode)->root;
+	struct btrfs_trans_handle *trans;
 	loff_t oldsize = i_size_read(inode);
 	int ret;
 
@@ -3380,16 +3382,20 @@ static int btrfs_setsize(struct inode *inode, loff_t
newsize)
 		return 0;
 
 	if (newsize > oldsize) {
-		i_size_write(inode, newsize);
-		btrfs_ordered_update_i_size(inode, i_size_read(inode), NULL);
 		truncate_pagecache(inode, oldsize, newsize);
 		ret = btrfs_cont_expand(inode, oldsize, newsize);
-		if (ret) {
-			btrfs_setsize(inode, oldsize);
+		if (ret)
 			return ret;
-		}
 
-		mark_inode_dirty(inode);
+		trans = btrfs_start_transaction(root, 1);
+		if (IS_ERR(trans))
+			return PTR_ERR(trans);
+
+		i_size_write(inode, newsize);
+		btrfs_ordered_update_i_size(inode, i_size_read(inode), NULL);
+		ret = btrfs_update_inode(trans, root, inode);
+
+		btrfs_end_transaction_throttle(trans, root);
 	} else {
 
 		/*
-- 
1.7.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, Dec 14, 2011 at 03:15:53PM +0800, Miao Xie
wrote:
> Btrfsck report error 100 after the 83th case of xfstests was run, it means
> the i_size of the file is wrong.
> 
> The reason of this bug is that:
> Btrfs increased i_size of the file at the beginning, but it failed to
expand
> the file, and failed to update the i_size to the old size because there is
no
> enough space in the file system, so we found a wrong i_size.
> 
> This patch fixes this bug by updating the i_size just when we pass the file
> expanding and get enough space to update i-node.
Hmmm, have you tested this one with fsx-linux?  It should be ok to call
truncate_pagecache before bumping i_size in this case, since we
shouldn''t have pages past i_size.

But, truncate_pagecache does expect i_size to be accurate before the
call.

-chris
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, 14 Dec 2011 11:51:47 -0500, Chris Mason wrote:
> On Wed, Dec 14, 2011 at 03:15:53PM +0800, Miao Xie wrote:
>> Btrfsck report error 100 after the 83th case of xfstests was run, it
means
>> the i_size of the file is wrong.
>>
>> The reason of this bug is that:
>> Btrfs increased i_size of the file at the beginning, but it failed to
expand
>> the file, and failed to update the i_size to the old size because there
is no
>> enough space in the file system, so we found a wrong i_size.
>>
>> This patch fixes this bug by updating the i_size just when we pass the
file
>> expanding and get enough space to update i-node.
> 
> Hmmm, have you tested this one with fsx-linux?  It should be ok to call
> truncate_pagecache before bumping i_size in this case, since we
> shouldn''t have pages past i_size.
Yes, I have tested it. Everything is OK.
> But, truncate_pagecache does expect i_size to be accurate before the
> call.
OK, I will modify my patch.

Thanks for your comment.
Miao
> 
> -chris
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Btrfsck report error 100 after the 83th case of xfstests was run, it means
the i_size of the file is wrong.

The reason of this bug is that:
Btrfs increased i_size of the file at the beginning, but it failed to expand
the file, and failed to update the i_size to the old size because there is no
enough space in the file system, so we found a wrong i_size.

This patch fixes this bug by updating the i_size just when we pass the file
expanding.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
Changes v1 -> v2:
- make i_size be accurate before truncate_pagecache() is called
  (use truncate_setsize())
---
 fs/btrfs/inode.c |   20 ++++++++++++++------
 1 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 960818b..21051fc 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3373,6 +3373,8 @@ int btrfs_cont_expand(struct inode *inode, loff_t oldsize,
loff_t size)
 
 static int btrfs_setsize(struct inode *inode, loff_t newsize)
 {
+	struct btrfs_root *root = BTRFS_I(inode)->root;
+	struct btrfs_trans_handle *trans;
 	loff_t oldsize = i_size_read(inode);
 	int ret;
 
@@ -3380,16 +3382,22 @@ static int btrfs_setsize(struct inode *inode, loff_t
newsize)
 		return 0;
 
 	if (newsize > oldsize) {
-		i_size_write(inode, newsize);
-		btrfs_ordered_update_i_size(inode, i_size_read(inode), NULL);
-		truncate_pagecache(inode, oldsize, newsize);
 		ret = btrfs_cont_expand(inode, oldsize, newsize);
-		if (ret) {
-			btrfs_setsize(inode, oldsize);
+		if (ret)
 			return ret;
+
+		truncate_setsize(inode, newsize);
+
+		trans = btrfs_start_transaction(root, 1);
+		if (IS_ERR(trans)) {
+			truncate_setsize(inode, oldsize);
+			return PTR_ERR(trans);
 		}
 
-		mark_inode_dirty(inode);
+		btrfs_ordered_update_i_size(inode, i_size_read(inode), NULL);
+		ret = btrfs_update_inode(trans, root, inode);
+
+		btrfs_end_transaction_throttle(trans, root);
 	} else {
 
 		/*
-- 
1.7.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html