On 2022/3/7 15:54, 常凤楠 wrote:
-----Original Message-----
From: Chao Yu <[email protected]>
Sent: Monday, March 7, 2022 9:43 AM
To: Fengnan Chang <[email protected]>; [email protected]
Cc: [email protected]; 常凤楠
<[email protected]>
Subject: Re: [RFC PATCH] f2fs: fix compress file start atomic write may cause
data corruption
On 2022/3/5 12:00, Fengnan Chang wrote:
When compressed file has blocks, start atomic write will be failed,
but
You mean f2fs_ioc_start_atomic_write will succeed, but compressed flag will
be remained in inode?
Yes.
still set FI_ATOMIC_FILE flag, if write partial cluster and commit
atomic write will cause data corruption.
Could you please explain more about why data will be corrupted?
Step 1:
Create a compressed file ,write 64K data , call fsync(), then the blocks are
write as compressed cluster.
Step2:
iotcl(F2FS_IOC_START_ATOMIC_WRITE) --- this should be fail, but not.
write page 0 and page 3.
iotcl(F2FS_IOC_COMMIT_ATOMIC_WRITE) -- page 0 and 3 write as normal file,
Step3:
drop cache.
Read page 0-4 -- Since page 0 has a valid block address, read as
no-compressed, page 1 and 2 will be compressed data or zero.
And before f2fs: compress: remove unneeded read when rewrite whole cluster),
even Step 2 is not right, but whole
cluster will mark dirty in write_begin, and whole cluster will be re-write as
no-compressed cluster, so it's ok.
Ah, after 7eab7a696827 (f2fs: compress: remove unneeded read when rewrite whole
cluster),
we expect that f2fs_write_cache_pages() will be called, and
f2fs_prepare_compress_overwrite()
can read and set dirty left pages of compressed cluster.
However atomic commit flow won't call f2fs_write_cache_pages()...
Anyway, thanks for the explanation, and, could you please add above comments
into commit
description of this patch?
Thanks,
Thanks,
Fixes: 4c8ff7095bef (f2fs: support data compression)
Fixes: 7eab7a696827 (f2fs: compress: remove unneeded read when rewrite
whole cluster)
Signed-off-by: Fengnan Chang <[email protected]>
---
fs/f2fs/data.c | 4 +---
fs/f2fs/file.c | 3 ++-
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index
6b5f389ba998..5cbee4ed0982 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -3358,8 +3358,7 @@ static int f2fs_write_begin(struct file *file, struct
address_space *mapping,
int ret;
*fsdata = NULL;
-
- if (len == PAGE_SIZE)
+ if (len == PAGE_SIZE && !(f2fs_is_atomic_file(inode)))
goto repeat;
ret = f2fs_prepare_compress_overwrite(inode, pagep, diff --git
a/fs/f2fs/file.c b/fs/f2fs/file.c index cfdc41f87f5d..26a648ef9e1f
100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2009,7 +2009,8 @@ static int f2fs_ioc_start_atomic_write(struct
file *filp)
inode_lock(inode);
- f2fs_disable_compressed_file(inode);
+ if (!f2fs_disable_compressed_file(inode))
+ return -EINVAL;
if (f2fs_is_atomic_file(inode)) {
if (is_inode_flag_set(inode, FI_ATOMIC_REVOKE_REQUEST))
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
