diff mbox series

[f2fs-dev,2/2] f2fs: truncate page cache before clearing flags when aborting atomic write

Message ID 20240313112620.1061463-2-s_min.jeong@samsung.com (mailing list archive)
State Accepted
Commit 74b0ebcbdde4c7fe23c979e4cfc2fdbf349c39a3
Headers show
Series [f2fs-dev,1/2] f2fs: mark inode dirty for FI_ATOMIC_COMMITTED flag | expand

Commit Message

Sunmin Jeong March 13, 2024, 11:26 a.m. UTC 
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(-)

Comments

Daeho Jeong March 13, 2024, 3:46 p.m. UTC | #1 
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
Chao Yu March 14, 2024, 10:39 a.m. UTC | #2 
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,
diff mbox series

Patch

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;
 	}