Messages in this thread Patch in this message | | | Date | Sat, 1 Oct 2022 08:16:13 +0800 | Subject | Re: [f2fs-dev] [PATCH v2] f2fs: introduce F2FS_IOC_START_ATOMIC_REPLACE | From | Chao Yu <> |
| |
On 2022/10/1 4:01, Daeho Jeong wrote: > On Fri, Sep 30, 2022 at 9:04 AM Daeho Jeong <daeho43@gmail.com> wrote: >> >>>>>>> >>>>>>> Hi Daeho, >>>>>>> >>>>>>> isize should be updated after tagging inode as atomic_write one? >>>>>>> otherwise f2fs_mark_inode_dirty_sync() may update isize to inode page, >>>>>>> latter checkpoint may persist that change? IIUC... >>>>>>> >>>>>>> Thanks, >>>>>> >>>>>> Hi Chao, >>>>>> >>>>>> The first patch of this patchset prevents the inode page from being >>>>>> updated as dirty for atomic file cases. >>>>>> Is there any other chances for the inode page to be marked as dirty? >>>>> >>>>> I mean: >>>>> >>>>> Thread A Thread B >>>>> - f2fs_ioc_start_atomic_write >>>>> - f2fs_i_size_write(inode, 0) >>>>> - f2fs_mark_inode_dirty_sync >>>>> - checkpoint >>>>> - persist inode with incorrect zero isize >>>>> >>>>> - set_inode_flag(inode, FI_ATOMIC_FILE) >>>>> >>>>> Am I missing something? >>>>> >>>> >>>> So, f2fs_mark_inode_dirty_sync() will not work for atomic files >>>> anymore, which means it doesn't make the inode dirty. >>>> Plz, refer to the first patch of this patchset. Or I might be confused >>>> with something. :( >>> >>> I mean FI_ATOMIC_FILE was set after f2fs_i_size_write(), so inode will be set >>> as dirty. >>> >>> Thanks, >>> >> >> Oh, I was confused that f2fs_update_inode() is called in >> f2fs_mark_inode_dirty_sync(). >> That is called in f2fs_write_inode(). Let me fix this. > > Hmm, I think the issue was already there before my patch. > So, how about making the inode flushed and clean if the inode is > already dirty when starting atomic write?
What I mean is:
--- fs/f2fs/file.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index e4b6e51086a3..31b229678b1d 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -2053,6 +2053,9 @@ static int f2fs_ioc_start_atomic_write(struct file *filp, bool truncate)
isize = i_size_read(inode); fi->original_i_size = isize; + + set_inode_flag(inode, FI_ATOMIC_FILE); + if (truncate) { set_inode_flag(inode, FI_ATOMIC_REPLACE); truncate_inode_pages_final(inode->i_mapping); @@ -2063,7 +2066,6 @@ static int f2fs_ioc_start_atomic_write(struct file *filp, bool truncate)
stat_inc_atomic_inode(inode);
- set_inode_flag(inode, FI_ATOMIC_FILE); set_inode_flag(fi->cow_inode, FI_COW_FILE); clear_inode_flag(fi->cow_inode, FI_INLINE_DATA); f2fs_up_write(&fi->i_gc_rwsem[WRITE]); --
Let's set FI_ATOMIC_FILE flag before f2fs_i_size_write(inode, 0), so - f2fs_ioc_start_atomic_write - f2fs_i_size_write(, 0) - f2fs_mark_inode_dirty_sync check f2fs_is_atomic_file() and return correctly.
And for the case the inode is dirty before f2fs_i_size_write(, 0), we can call f2fs_write_inode() to flush dirty feilds into inode page, and make inode clean.
> >> >> Thanks,
| |