Hi Chao Yu,
>> In case of the COW file, new updates and GC writes are already
>> separated to page caches of the atomic file and COW file. As some
>> cases that use the meta inode for GC, there are some race issues
>> between a foreground thread and GC thread.
>>
>> To handle them, we need to take care when to invalidate and wait
>> writeback of GC pages in COW files as the case of using the meta inode.
>> Also, a pointer from the COW inode to the original inode is required
>> to check the state of original pages.
>>
>> For the former, we can solve the problem by using the meta inode for
>> GC of COW files. Then let's get a page from the original inode in
>> move_data_block when GCing the COW file to avoid race condition.
>>
>> Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
>> Cc: stable@xxxxxxxxxxxxxxx #v5.19+
>> Reviewed-by: Sungjong Seo <sj1557.seo@xxxxxxxxxxx>
>> Reviewed-by: Yeongjin Gil <youngjin.gil@xxxxxxxxxxx>
>> Signed-off-by: Sunmin Jeong <s_min.jeong@xxxxxxxxxxx>
>> ---
>> fs/f2fs/data.c | 2 +-
>> fs/f2fs/f2fs.h | 7 ++++++-
>> fs/f2fs/file.c | 3 +++
>> fs/f2fs/gc.c | 12 ++++++++++--
>> fs/f2fs/inline.c | 2 +-
>> fs/f2fs/inode.c | 3 ++-
>> 6 files changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index
>> 05158f89ef32..90ff0f6f7f7f 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -2651,7 +2651,7 @@ bool f2fs_should_update_outplace(struct inode
>*inode, struct f2fs_io_info *fio)
>> return true;
>> if (IS_NOQUOTA(inode))
>> return true;
>> - if (f2fs_is_atomic_file(inode))
>> + if (f2fs_used_in_atomic_write(inode))
>> return true;
>> /* rewrite low ratio compress data w/ OPU mode to avoid
>fragmentation */
>> if (f2fs_compressed_file(inode) &&
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index
>> 59c5117e54b1..4f9fd1c1d024 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -4267,9 +4267,14 @@ static inline bool f2fs_post_read_required(struct
>inode *inode)
>> f2fs_compressed_file(inode);
>> }
>>
>> +static inline bool f2fs_used_in_atomic_write(struct inode *inode) {
>> + return f2fs_is_atomic_file(inode) || f2fs_is_cow_file(inode); }
>> +
>> static inline bool f2fs_meta_inode_gc_required(struct inode *inode)
>> {
>> - return f2fs_post_read_required(inode) || f2fs_is_atomic_file(inode);
>> + return f2fs_post_read_required(inode) ||
>> +f2fs_used_in_atomic_write(inode);
>> }
>>
>> /*
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index
>> 25b119cf3499..c9f0ba658cfd 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -2116,6 +2116,9 @@ static int f2fs_ioc_start_atomic_write(struct
>> file *filp, bool truncate)
>>
>> set_inode_flag(fi->cow_inode, FI_COW_FILE);
>> clear_inode_flag(fi->cow_inode, FI_INLINE_DATA);
>> +
>> + /* Set the COW inode's cow_inode to the atomic inode */
>> + F2FS_I(fi->cow_inode)->cow_inode = inode;
>
>How about adding a union fields as below for readability?
>
>struct f2fs_inode_info {
>...
> union {
> struct inode *cow_inode; /* copy-on-write inode for atomic
>write */
> struct inode *atomic_inode; /* point to atomic_inode,
>available only for cow_inode */
> };
>...
>};
>
>Thanks,
>
Thanks for you opinion. I'll send patch v2.
>> } else {
>> /* Reuse the already created COW inode */
>> ret = f2fs_do_truncate_blocks(fi->cow_inode, 0, true); diff -
>-git
>> a/fs/f2fs/gc.c b/fs/f2fs/gc.c index 136b9e8180a3..76854e732b35 100644
>> --- a/fs/f2fs/gc.c
>> +++ b/fs/f2fs/gc.c
>> @@ -1188,7 +1188,11 @@ static int ra_data_block(struct inode *inode,
>pgoff_t index)
>> };
>> int err;
>>
>> - page = f2fs_grab_cache_page(mapping, index, true);
>> + if (f2fs_is_cow_file(inode))
>> + page = f2fs_grab_cache_page(F2FS_I(inode)->cow_inode-
>>i_mapping,
>> + index, true);
>> + else
>> + page = f2fs_grab_cache_page(mapping, index, true);
>> if (!page)
>> return -ENOMEM;
>>
>> @@ -1287,7 +1291,11 @@ static int move_data_block(struct inode *inode,
>block_t bidx,
>> CURSEG_ALL_DATA_ATGC : CURSEG_COLD_DATA;
>>
>> /* do not read out */
>> - page = f2fs_grab_cache_page(inode->i_mapping, bidx, false);
>> + if (f2fs_is_cow_file(inode))
>> + page = f2fs_grab_cache_page(F2FS_I(inode)->cow_inode-
>>i_mapping,
>> + bidx, false);
>> + else
>> + page = f2fs_grab_cache_page(inode->i_mapping, bidx, false);
>> if (!page)
>> return -ENOMEM;
>>
>> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c index
>> ac00423f117b..0186ec049db6 100644
>> --- a/fs/f2fs/inline.c
>> +++ b/fs/f2fs/inline.c
>> @@ -16,7 +16,7 @@
>>
>> static bool support_inline_data(struct inode *inode)
>> {
>> - if (f2fs_is_atomic_file(inode))
>> + if (f2fs_used_in_atomic_write(inode))
>> return false;
>> if (!S_ISREG(inode->i_mode) && !S_ISLNK(inode->i_mode))
>> return false;
>> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c index
>> c26effdce9aa..c810304e2681 100644
>> --- a/fs/f2fs/inode.c
>> +++ b/fs/f2fs/inode.c
>> @@ -807,8 +807,9 @@ void f2fs_evict_inode(struct inode *inode)
>>
>> f2fs_abort_atomic_write(inode, true);
>>
>> - if (fi->cow_inode) {
>> + if (fi->cow_inode && f2fs_is_cow_file(fi->cow_inode)) {
>> clear_inode_flag(fi->cow_inode, FI_COW_FILE);
>> + F2FS_I(fi->cow_inode)->cow_inode = NULL;
>> iput(fi->cow_inode);
>> fi->cow_inode = NULL;
>> }
