From: Chao Yu <chao@kernel.org>
To: jaegeuk@kernel.org
Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net
Subject: [f2fs-dev] [PATCH v2 2/4] f2fs: atomic: fix to not allow GC to pollute atomic_file
Date: Wed, 14 Aug 2024 21:48:15 +0800 [thread overview]
Message-ID: <20240814134815.801303-1-chao@kernel.org> (raw)
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
- 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@kernel.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@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
WARNING: multiple messages have this Message-ID (diff)
From: Chao Yu <chao@kernel.org>
To: jaegeuk@kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org, Chao Yu <chao@kernel.org>
Subject: [PATCH v2 2/4] f2fs: atomic: fix to not allow GC to pollute atomic_file
Date: Wed, 14 Aug 2024 21:48:15 +0800 [thread overview]
Message-ID: <20240814134815.801303-1-chao@kernel.org> (raw)
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
- 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@kernel.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
next reply other threads:[~2024-08-14 13:48 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20240814135008epcas1p3e11d629481d51a4cc4af78dbe3e66fda@epcas1p3.samsung.com>
2024-08-14 13:48 ` Chao Yu [this message]
2024-08-14 13:48 ` [PATCH v2 2/4] f2fs: atomic: fix to not allow GC to pollute atomic_file Chao Yu
2024-08-20 2:44 ` [f2fs-dev] " Sunmin Jeong
2024-08-20 2:44 ` Sunmin Jeong
2024-09-02 6:38 ` Chao Yu via Linux-f2fs-devel
2024-09-02 6:38 ` Chao Yu
2024-08-28 15:27 ` Jaegeuk Kim via Linux-f2fs-devel
2024-08-28 15:27 ` Jaegeuk Kim
2024-09-02 6:40 ` [f2fs-dev] " Chao Yu via Linux-f2fs-devel
2024-09-02 6:40 ` Chao Yu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240814134815.801303-1-chao@kernel.org \
--to=chao@kernel.org \
--cc=jaegeuk@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.