> -----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.

> 
> 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

Reply via email to