Re: [PATCH v2 1/2] f2fs: move the conditional statement after holding the inode lock in f2fs_move_file_range()

From: Chao Yu
Date: Tue May 16 2023 - 21:58:14 EST


On 2023/5/6 22:42, Yangtao Li wrote:
For judging the inode flag state, the inode lock must be held.
BTW, add compressd file check and to avoid 'if' nesting.

Please describe what's the detail problem if we check the flag w/o inode
lock.

Can we use one single patch to fix all similar issues?

Thanks,


Cc: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
Fixes: 4dd6f977fc77 ("f2fs: support an ioctl to move a range of data blocks")
Signed-off-by: Yangtao Li <frank.li@xxxxxxxx>
---
v2:
-add unlock
fs/f2fs/file.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 78aa8cff4b41..42a9b683118c 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2790,9 +2790,6 @@ static int f2fs_move_file_range(struct file *file_in, loff_t pos_in,
if (!S_ISREG(src->i_mode) || !S_ISREG(dst->i_mode))
return -EINVAL;
- if (IS_ENCRYPTED(src) || IS_ENCRYPTED(dst))
- return -EOPNOTSUPP;
-
if (pos_out < 0 || pos_in < 0)
return -EINVAL;
@@ -2804,10 +2801,19 @@ static int f2fs_move_file_range(struct file *file_in, loff_t pos_in,
}
inode_lock(src);
- if (src != dst) {
+ if (src != dst && !inode_trylock(dst)) {
ret = -EBUSY;
- if (!inode_trylock(dst))
- goto out;
+ goto out;
+ }
+
+ if (IS_ENCRYPTED(src) || IS_ENCRYPTED(dst)) {
+ ret = -EOPNOTSUPP;
+ goto out_unlock;
+ }
+
+ if (f2fs_compressed_file(src) || f2fs_compressed_file(dst)) {
+ ret = -EOPNOTSUPP;
+ goto out_unlock;
}
ret = -EINVAL;