f2fs: do not trim preallocated blocks when truncating after i_size

PATCH In review Chao Yu 2 participants 3 messages Jun 9, 2015 – Jun 12, 2015 F2FS file system

lore

Share & tools

lore

Chao Yu

author

2015-06-09 02:27 UTC #0 lore raw

PATCH v2 1/1 f2fs: do not trim preallocated blocks when truncating after i_size

When we perform generic/092 in xfstests, output is like below:

     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
     0: [0..10239]: data
     0: [0..10239]: data
    -1: [10240..20479]: unwritten
    +1: [10240..14335]: unwritten

This is because with this testcase, we redefine the regulation for truncate in perallocated space past i_size as below:

"There was some confused about what the fs was supposed to do when you truncate at i_size with preallocated space past i_size. We decided on the following things.

1) truncate(i_size) will trim all blocks past i_size.
2) truncate(x) where x > i_size will not trim all blocks past i_size. "

This method is used in xfs, and then ext4/btrfs will follow the rule.

This patch fixes to follow the new rule for f2fs.

Signed-off-by Chao Yu <[email protected]>

---
v2:
* adjust code style pointed out by Jaegeuk Kim.

 fs/f2fs/file.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

fs/f2fs/file.c +7 -8

index 4d42d66..ec47c4a 100644
@@ -651,16 +651,15 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
 				f2fs_get_encryption_info(inode))
 			return -EACCES;
 
-		if (attr->ia_size != i_size_read(inode)) {
-			truncate_setsize(inode, attr->ia_size);
+		truncate_setsize(inode, attr->ia_size);
+
+		/*
+		 * do not trim all preallocated blocks after i_size if target
+		 * size is larger than i_size.
+		 */
+		if (attr->ia_size <= i_size_read(inode)) {
 			f2fs_truncate(inode);
 			f2fs_balance_fs(F2FS_I_SB(inode));
-		} else {
-			/*
-			 * giving a chance to truncate blocks past EOF which
-			 * are fallocated with FALLOC_FL_KEEP_SIZE.
-			 */
-			f2fs_truncate(inode);
 		}
 	}

Signature · 1 line

2.4.2

Chao Yu

author

2015-06-12 00:30 UTC #1 lore raw

Hi Jaegeuk,

Quoted 45 lines

-----Original Message-----
From: Chao Yu [mailto:[email protected]]
Sent: Tuesday, June 09, 2015 10:28 AM
To: Jaegeuk Kim; Changman Lee
Cc: [email protected]; [email protected]
Subject: [f2fs-dev] [PATCH v2] f2fs: do not trim preallocated blocks when truncating after i_size
When we perform generic/092 in xfstests, output is like below:
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
     0: [0..10239]: data
     0: [0..10239]: data
    -1: [10240..20479]: unwritten
    +1: [10240..14335]: unwritten
This is because with this testcase, we redefine the regulation for truncate in perallocated space past i_size as below:
"There was some confused about what the fs was supposed to do when you truncate at i_size with preallocated space past i_size. We decided on the following things.
1) truncate(i_size) will trim all blocks past i_size.
2) truncate(x) where x > i_size will not trim all blocks past i_size. "
This method is used in xfs, and then ext4/btrfs will follow the rule.
This patch fixes to follow the new rule for f2fs.
Signed-off-by Chao Yu <[email protected]>
---
v2:
* adjust code style pointed out by Jaegeuk Kim.
 fs/f2fs/file.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)
fs/f2fs/file.c +0 -0
index 4d42d66..ec47c4a 100644
@@ -651,16 +651,15 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
 				f2fs_get_encryption_info(inode))
 			return -EACCES;

fs/f2fs/file.c int f2fs_setattr(struct dentry *dentry, struct iattr *attr) · line 660

Quoted 10 lines

fs/f2fs/file.c +7 -2

 
-		if (attr->ia_size != i_size_read(inode)) {
-			truncate_setsize(inode, attr->ia_size);
+		truncate_setsize(inode, attr->ia_size);
+
+		/*
+		 * do not trim all preallocated blocks after i_size if target
+		 * size is larger than i_size.
+		 */
+		if (attr->ia_size <= i_size_read(inode)) {

It's wrong implementation here, because we have set i_size in inode with attr->ia_size, so that we will always go into this "truncate smaller size" branch.

Please ignore this patch, sorry for my mistake.

Jaegeuk, I'd like to use v1 patch, how do you think?

Thanks,

Quoted 21 lines

fs/f2fs/file.c +0 -7
 			f2fs_truncate(inode);
 			f2fs_balance_fs(F2FS_I_SB(inode));
-		} else {
-			/*
-			 * giving a chance to truncate blocks past EOF which
-			 * are fallocated with FALLOC_FL_KEEP_SIZE.
-			 */
-			f2fs_truncate(inode);
 		}
 	}
 
--
2.4.2
------------------------------------------------------------------------------
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Jaegeuk Kim

maintainer

2015-06-12 01:30 UTC #2 lore raw

Quoted 62 lines · Chao Yu

Hi Jaegeuk,
Quoted 45 lines
-----Original Message-----
From: Chao Yu [mailto:[email protected]]
Sent: Tuesday, June 09, 2015 10:28 AM
To: Jaegeuk Kim; Changman Lee
Cc: [email protected]; [email protected]
Subject: [f2fs-dev] [PATCH v2] f2fs: do not trim preallocated blocks when truncating after i_size
When we perform generic/092 in xfstests, output is like below:
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
     0: [0..10239]: data
     0: [0..10239]: data
    -1: [10240..20479]: unwritten
    +1: [10240..14335]: unwritten
This is because with this testcase, we redefine the regulation for truncate in perallocated space past i_size as below:
"There was some confused about what the fs was supposed to do when you truncate at i_size with preallocated space past i_size. We decided on the following things.
1) truncate(i_size) will trim all blocks past i_size.
2) truncate(x) where x > i_size will not trim all blocks past i_size. "
This method is used in xfs, and then ext4/btrfs will follow the rule.
This patch fixes to follow the new rule for f2fs.
Signed-off-by Chao Yu <[email protected]>
---
v2:
* adjust code style pointed out by Jaegeuk Kim.
 fs/f2fs/file.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)
fs/f2fs/file.c +0 -0
index 4d42d66..ec47c4a 100644
@@ -651,16 +651,15 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
 				f2fs_get_encryption_info(inode))
 			return -EACCES;
fs/f2fs/file.c int f2fs_setattr(struct dentry *dentry, struct iattr *attr) · line 660
Quoted 10 lines
fs/f2fs/file.c +7 -2
 
-		if (attr->ia_size != i_size_read(inode)) {
-			truncate_setsize(inode, attr->ia_size);
+		truncate_setsize(inode, attr->ia_size);
+
+		/*
+		 * do not trim all preallocated blocks after i_size if target
+		 * size is larger than i_size.
+		 */
+		if (attr->ia_size <= i_size_read(inode)) {
It's wrong implementation here, because we have set i_size in inode with attr->ia_size, so that we will always go into this "truncate smaller size" branch.

Quoted 3 lines

Please ignore this patch, sorry for my mistake.
Jaegeuk, I'd like to use v1 patch, how do you think?

Oh, I see.
I'll merge v1.

Thank you for pointing out this.

Quoted 24 lines

Thanks,
Quoted 21 lines
fs/f2fs/file.c +0 -7
 			f2fs_truncate(inode);
 			f2fs_balance_fs(F2FS_I_SB(inode));
-		} else {
-			/*
-			 * giving a chance to truncate blocks past EOF which
-			 * are fallocated with FALLOC_FL_KEEP_SIZE.
-			 */
-			f2fs_truncate(inode);
 		}
 	}
 
--
2.4.2
------------------------------------------------------------------------------
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel