[parent not found: <CGME20240313112706epcas1p2ee50d07f603422b0193f0b71bf1a75e6@epcas1p2.samsung.com>]
* [PATCH 2/2] f2fs: truncate page cache before clearing flags when aborting atomic write
[not found] ` <CGME20240313112706epcas1p2ee50d07f603422b0193f0b71bf1a75e6@epcas1p2.samsung.com>
@ 2024-03-13 11:26 ` Sunmin Jeong
2024-03-13 15:46 ` [f2fs-dev] " Daeho Jeong
2024-03-14 10:39 ` Chao Yu
0 siblings, 2 replies; 7+ messages in thread
From: Sunmin Jeong @ 2024-03-13 11:26 UTC (permalink / raw)
To: jaegeuk, chao, daehojeong
Cc: linux-f2fs-devel, linux-kernel, Sunmin Jeong, stable,
Sungjong Seo, Yeongjin Gil
In f2fs_do_write_data_page, FI_ATOMIC_FILE flag selects the target inode
between the original inode and COW inode. When aborting atomic write and
writeback occur simultaneously, invalid data can be written to original
inode if the FI_ATOMIC_FILE flag is cleared meanwhile.
To prevent the problem, let's truncate all pages before clearing the flag
Atomic write thread Writeback thread
f2fs_abort_atomic_write
clear_inode_flag(inode, FI_ATOMIC_FILE)
__writeback_single_inode
do_writepages
f2fs_do_write_data_page
- use dn of original inode
truncate_inode_pages_final
Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
Cc: stable@vger.kernel.org #v5.19+
Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com>
Reviewed-by: Yeongjin Gil <youngjin.gil@samsung.com>
Signed-off-by: Sunmin Jeong <s_min.jeong@samsung.com>
---
fs/f2fs/segment.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 7901ede58113..7e47b8054413 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -192,6 +192,9 @@ void f2fs_abort_atomic_write(struct inode *inode, bool clean)
if (!f2fs_is_atomic_file(inode))
return;
+ if (clean)
+ truncate_inode_pages_final(inode->i_mapping);
+
release_atomic_write_cnt(inode);
clear_inode_flag(inode, FI_ATOMIC_COMMITTED);
clear_inode_flag(inode, FI_ATOMIC_REPLACE);
@@ -201,7 +204,6 @@ void f2fs_abort_atomic_write(struct inode *inode, bool clean)
F2FS_I(inode)->atomic_write_task = NULL;
if (clean) {
- truncate_inode_pages_final(inode->i_mapping);
f2fs_i_size_write(inode, fi->original_i_size);
fi->original_i_size = 0;
}
--
2.25.1
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [f2fs-dev] [PATCH 2/2] f2fs: truncate page cache before clearing flags when aborting atomic write
2024-03-13 11:26 ` [PATCH 2/2] f2fs: truncate page cache before clearing flags when aborting atomic write Sunmin Jeong
@ 2024-03-13 15:46 ` Daeho Jeong
2024-03-14 10:39 ` Chao Yu
1 sibling, 0 replies; 7+ messages in thread
From: Daeho Jeong @ 2024-03-13 15:46 UTC (permalink / raw)
To: Sunmin Jeong
Cc: jaegeuk, chao, daehojeong, linux-f2fs-devel, stable,
linux-kernel, Sungjong Seo
Reviewed-by: Daeho Jeong <daehojeong@google.com>
On Wed, Mar 13, 2024 at 4:29 AM Sunmin Jeong <s_min.jeong@samsung.com> wrote:
>
> In f2fs_do_write_data_page, FI_ATOMIC_FILE flag selects the target inode
> between the original inode and COW inode. When aborting atomic write and
> writeback occur simultaneously, invalid data can be written to original
> inode if the FI_ATOMIC_FILE flag is cleared meanwhile.
>
> To prevent the problem, let's truncate all pages before clearing the flag
>
> Atomic write thread Writeback thread
> f2fs_abort_atomic_write
> clear_inode_flag(inode, FI_ATOMIC_FILE)
> __writeback_single_inode
> do_writepages
> f2fs_do_write_data_page
> - use dn of original inode
> truncate_inode_pages_final
>
> Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
> Cc: stable@vger.kernel.org #v5.19+
> Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com>
> Reviewed-by: Yeongjin Gil <youngjin.gil@samsung.com>
> Signed-off-by: Sunmin Jeong <s_min.jeong@samsung.com>
> ---
> fs/f2fs/segment.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 7901ede58113..7e47b8054413 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -192,6 +192,9 @@ void f2fs_abort_atomic_write(struct inode *inode, bool clean)
> if (!f2fs_is_atomic_file(inode))
> return;
>
> + if (clean)
> + truncate_inode_pages_final(inode->i_mapping);
> +
> release_atomic_write_cnt(inode);
> clear_inode_flag(inode, FI_ATOMIC_COMMITTED);
> clear_inode_flag(inode, FI_ATOMIC_REPLACE);
> @@ -201,7 +204,6 @@ void f2fs_abort_atomic_write(struct inode *inode, bool clean)
> F2FS_I(inode)->atomic_write_task = NULL;
>
> if (clean) {
> - truncate_inode_pages_final(inode->i_mapping);
> f2fs_i_size_write(inode, fi->original_i_size);
> fi->original_i_size = 0;
> }
> --
> 2.25.1
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] f2fs: truncate page cache before clearing flags when aborting atomic write
2024-03-13 11:26 ` [PATCH 2/2] f2fs: truncate page cache before clearing flags when aborting atomic write Sunmin Jeong
2024-03-13 15:46 ` [f2fs-dev] " Daeho Jeong
@ 2024-03-14 10:39 ` Chao Yu
1 sibling, 0 replies; 7+ messages in thread
From: Chao Yu @ 2024-03-14 10:39 UTC (permalink / raw)
To: Sunmin Jeong, jaegeuk, daehojeong
Cc: linux-f2fs-devel, linux-kernel, stable, Sungjong Seo, Yeongjin Gil
On 2024/3/13 19:26, Sunmin Jeong wrote:
> In f2fs_do_write_data_page, FI_ATOMIC_FILE flag selects the target inode
> between the original inode and COW inode. When aborting atomic write and
> writeback occur simultaneously, invalid data can be written to original
> inode if the FI_ATOMIC_FILE flag is cleared meanwhile.
>
> To prevent the problem, let's truncate all pages before clearing the flag
>
> Atomic write thread Writeback thread
> f2fs_abort_atomic_write
> clear_inode_flag(inode, FI_ATOMIC_FILE)
> __writeback_single_inode
> do_writepages
> f2fs_do_write_data_page
> - use dn of original inode
> truncate_inode_pages_final
>
> Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
> Cc: stable@vger.kernel.org #v5.19+
> Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com>
> Reviewed-by: Yeongjin Gil <youngjin.gil@samsung.com>
> Signed-off-by: Sunmin Jeong <s_min.jeong@samsung.com>
Reviewed-by: Chao Yu <chao@kernel.org>
Thanks,
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: mark inode dirty for FI_ATOMIC_COMMITTED flag
2024-03-13 11:26 ` [PATCH 1/2] f2fs: mark inode dirty for FI_ATOMIC_COMMITTED flag Sunmin Jeong
[not found] ` <CGME20240313112706epcas1p2ee50d07f603422b0193f0b71bf1a75e6@epcas1p2.samsung.com>
@ 2024-03-13 15:46 ` Daeho Jeong
2024-03-14 10:34 ` Chao Yu
2024-03-14 16:20 ` [f2fs-dev] " patchwork-bot+f2fs
3 siblings, 0 replies; 7+ messages in thread
From: Daeho Jeong @ 2024-03-13 15:46 UTC (permalink / raw)
To: Sunmin Jeong
Cc: jaegeuk, chao, daehojeong, linux-f2fs-devel, stable,
linux-kernel, Sungjong Seo
Reviewed-by: Daeho Jeong <daehojeong@google.com>
On Wed, Mar 13, 2024 at 4:37 AM Sunmin Jeong <s_min.jeong@samsung.com> wrote:
>
> In f2fs_update_inode, i_size of the atomic file isn't updated until
> FI_ATOMIC_COMMITTED flag is set. When committing atomic write right
> after the writeback of the inode, i_size of the raw inode will not be
> updated. It can cause the atomicity corruption due to a mismatch between
> old file size and new data.
>
> To prevent the problem, let's mark inode dirty for FI_ATOMIC_COMMITTED
>
> Atomic write thread Writeback thread
> __writeback_single_inode
> write_inode
> f2fs_update_inode
> - skip i_size update
> f2fs_ioc_commit_atomic_write
> f2fs_commit_atomic_write
> set_inode_flag(inode, FI_ATOMIC_COMMITTED)
> f2fs_do_sync_file
> f2fs_fsync_node_pages
> - skip f2fs_update_inode since the inode is clean
>
> Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
> Cc: stable@vger.kernel.org #v5.19+
> Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com>
> Reviewed-by: Yeongjin Gil <youngjin.gil@samsung.com>
> Signed-off-by: Sunmin Jeong <s_min.jeong@samsung.com>
> ---
> fs/f2fs/f2fs.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 543898482f8b..a000cb024dbe 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3039,6 +3039,7 @@ static inline void __mark_inode_dirty_flag(struct inode *inode,
> case FI_INLINE_DOTS:
> case FI_PIN_FILE:
> case FI_COMPRESS_RELEASED:
> + case FI_ATOMIC_COMMITTED:
> f2fs_mark_inode_dirty_sync(inode, true);
> }
> }
> --
> 2.25.1
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/2] f2fs: mark inode dirty for FI_ATOMIC_COMMITTED flag
2024-03-13 11:26 ` [PATCH 1/2] f2fs: mark inode dirty for FI_ATOMIC_COMMITTED flag Sunmin Jeong
[not found] ` <CGME20240313112706epcas1p2ee50d07f603422b0193f0b71bf1a75e6@epcas1p2.samsung.com>
2024-03-13 15:46 ` [f2fs-dev] [PATCH 1/2] f2fs: mark inode dirty for FI_ATOMIC_COMMITTED flag Daeho Jeong
@ 2024-03-14 10:34 ` Chao Yu
2024-03-14 16:20 ` [f2fs-dev] " patchwork-bot+f2fs
3 siblings, 0 replies; 7+ messages in thread
From: Chao Yu @ 2024-03-14 10:34 UTC (permalink / raw)
To: Sunmin Jeong, jaegeuk, daehojeong
Cc: linux-f2fs-devel, linux-kernel, stable, Sungjong Seo, Yeongjin Gil
On 2024/3/13 19:26, Sunmin Jeong wrote:
> In f2fs_update_inode, i_size of the atomic file isn't updated until
> FI_ATOMIC_COMMITTED flag is set. When committing atomic write right
> after the writeback of the inode, i_size of the raw inode will not be
> updated. It can cause the atomicity corruption due to a mismatch between
> old file size and new data.
>
> To prevent the problem, let's mark inode dirty for FI_ATOMIC_COMMITTED
>
> Atomic write thread Writeback thread
> __writeback_single_inode
> write_inode
> f2fs_update_inode
> - skip i_size update
> f2fs_ioc_commit_atomic_write
> f2fs_commit_atomic_write
> set_inode_flag(inode, FI_ATOMIC_COMMITTED)
> f2fs_do_sync_file
> f2fs_fsync_node_pages
> - skip f2fs_update_inode since the inode is clean
>
> Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
> Cc: stable@vger.kernel.org #v5.19+
> Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com>
> Reviewed-by: Yeongjin Gil <youngjin.gil@samsung.com>
> Signed-off-by: Sunmin Jeong <s_min.jeong@samsung.com>
Reviewed-by: Chao Yu <chao@kernel.org>
Thanks,
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [f2fs-dev] [PATCH 1/2] f2fs: mark inode dirty for FI_ATOMIC_COMMITTED flag
2024-03-13 11:26 ` [PATCH 1/2] f2fs: mark inode dirty for FI_ATOMIC_COMMITTED flag Sunmin Jeong
` (2 preceding siblings ...)
2024-03-14 10:34 ` Chao Yu
@ 2024-03-14 16:20 ` patchwork-bot+f2fs
3 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+f2fs @ 2024-03-14 16:20 UTC (permalink / raw)
To: Sunmin Jeong
Cc: jaegeuk, chao, daehojeong, linux-f2fs-devel, stable,
linux-kernel, sj1557.seo
Hello:
This series was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:
On Wed, 13 Mar 2024 20:26:19 +0900 you wrote:
> In f2fs_update_inode, i_size of the atomic file isn't updated until
> FI_ATOMIC_COMMITTED flag is set. When committing atomic write right
> after the writeback of the inode, i_size of the raw inode will not be
> updated. It can cause the atomicity corruption due to a mismatch between
> old file size and new data.
>
> To prevent the problem, let's mark inode dirty for FI_ATOMIC_COMMITTED
>
> [...]
Here is the summary with links:
- [f2fs-dev,1/2] f2fs: mark inode dirty for FI_ATOMIC_COMMITTED flag
https://git.kernel.org/jaegeuk/f2fs/c/4bf78322346f
- [f2fs-dev,2/2] f2fs: truncate page cache before clearing flags when aborting atomic write
https://git.kernel.org/jaegeuk/f2fs/c/74b0ebcbdde4
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 7+ messages in thread