On 2024/8/20 10:44, Sunmin Jeong wrote:
Hello Chao Yu,SQLite App GC Thread Shrinker - f2fs_ioc_start_atomic_write - f2fs_ioc_commit_atomic_write - f2fs_commit_atomic_write - filemap_write_and_wait_range : write atomic_file's data to cow_inode echo 3 > drop_caches - f2fs_gc - gc_data_segment - move_data_pageWe modified the code to make GC of the atomic file go to move_data_block in commit b40a2b003709 (f2fs: use meta inode for GC of atomic file). Could you please check if this patch is still necessary?
Sunmin, Yes, you're right. Also, I've checked f2fs/003 of xfstests w/ commit b40a2b003709, it can fix the regression issue as well, so let's drop it. Thanks for pointing it out. :) Thanks,
Thanks,- set_page_dirty : it may load data of previous transaction into pagecache. - f2fs_down_write(&fi->i_gc_rwsem[WRITE]) - __f2fs_commit_atomic_write - f2fs_up_write(&fi->i_gc_rwsem[WRITE]) During committing atomic_file, GC may be triggered to migrate atomic_file's block, so it may contain data of previous transaction in page cache, we should drop atomic_file's cache once it was migrated by GC. And also, we should writeback atomic_file and cow_file's data w/ i_gc_rwsem lock held, in order to avoid block address change during __f2fs_commit_atomic_write(). Meahwhile, this patch adds f2fs_wait_on_block_writeback_range() to wait completion of block migration. Fixes: 3db1de0e582c ("f2fs: change the current atomic write way") Signed-off-by: Chao Yu <[email protected]> --- v2: - fix error path handling. fs/f2fs/segment.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 3aee71c9f3c6..a43054ab0cf1 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -236,6 +236,9 @@ static int __replace_atomic_write_block(struct inode *inode, pgoff_t index, return err; } + if (__is_valid_data_blkaddr(dn.data_blkaddr)) + f2fs_wait_on_block_writeback_range(inode, dn.data_blkaddr,1);+ if (recover) { /* dn.data_blkaddr is always valid */ if (!__is_valid_data_blkaddr(new_addr)) { @@ -339,6 +342,9@@static int __f2fs_commit_atomic_write(struct inode *inode) goto out; } + f2fs_wait_on_block_writeback_range(cow_inode, + blkaddr, 1); + new = f2fs_kmem_cache_alloc(revoke_entry_slab,GFP_NOFS,true, NULL); @@ -379,16 +385,29 @@ int f2fs_commit_atomic_write(struct inode *inode) struct f2fs_inode_info *fi = F2FS_I(inode); int err; + f2fs_down_write(&fi->i_gc_rwsem[WRITE]); + err = filemap_write_and_wait_range(inode->i_mapping, 0, LLONG_MAX); if (err) - return err; + goto out; - f2fs_down_write(&fi->i_gc_rwsem[WRITE]); - f2fs_lock_op(sbi); + /* writeback GCing page of cow_inode */ + err = filemap_write_and_wait_range(fi->cow_inode->i_mapping, + 0, LLONG_MAX); + if (err) + goto out; - err = __f2fs_commit_atomic_write(inode); + filemap_invalidate_lock(inode->i_mapping); + + /* don't allow clean page loaded by GC to pollute atomic_file */ + truncate_pagecache(inode, 0); + f2fs_lock_op(sbi); + err = __f2fs_commit_atomic_write(inode); f2fs_unlock_op(sbi); + + filemap_invalidate_unlock(inode->i_mapping); +out: f2fs_up_write(&fi->i_gc_rwsem[WRITE]); return err; -- 2.40.1 _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
_______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
