[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <002e01daf2aa$f22d5430$d687fc90$@samsung.com>
Date: Tue, 20 Aug 2024 11:44:58 +0900
From: "Sunmin Jeong" <s_min.jeong@...sung.com>
To: "'Chao Yu'" <chao@...nel.org>, <jaegeuk@...nel.org>
Cc: <linux-kernel@...r.kernel.org>, <linux-f2fs-devel@...ts.sourceforge.net>
Subject: RE: [f2fs-dev] [PATCH v2 2/4] f2fs: atomic: fix to not allow GC to
pollute atomic_file
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_page
We 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?
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 <chao@...nel.org>
>---
>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
>Linux-f2fs-devel@...ts.sourceforge.net
>https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Powered by blists - more mailing lists