Skip to content

Commit

Permalink
f2fs: atomic: fix to avoid racing w/ GC
Browse files Browse the repository at this point in the history
Case frank-w#1:
SQLite App		GC Thread		Kworker		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
								to drop atomic_file's
								cache.
			- f2fs_gc
			 - gc_data_segment
			  - move_data_page
			   - set_page_dirty

						- writepages
						 - f2fs_do_write_data_page
						 : overwrite atomic_file's data
						   to cow_inode
  - f2fs_down_write(&fi->i_gc_rwsem[WRITE])
  - __f2fs_commit_atomic_write
  - f2fs_up_write(&fi->i_gc_rwsem[WRITE])

Case frank-w#2:
SQLite App		GC Thread		Kworker
- f2fs_ioc_start_atomic_write

						- __writeback_single_inode
						 - do_writepages
						  - f2fs_write_cache_pages
						   - f2fs_write_single_data_page
						    - f2fs_do_write_data_page
						    : write atomic_file's data to cow_inode
			- f2fs_gc
			 - gc_data_segment
			  - move_data_page
			   - set_page_dirty

						- writepages
						 - f2fs_do_write_data_page
						 : overwrite atomic_file's data to cow_inode
- f2fs_ioc_commit_atomic_write

In above cases racing in between atomic_write and GC, previous
data in atomic_file may be overwrited to cow_file, result in
data corruption.

This patch introduces PAGE_PRIVATE_ATOMIC_WRITE bit flag in page.private,
and use it to indicate that there is last dirty data in atomic file,
and the data should be writebacked into cow_file, if the flag is not
tagged in page, we should never write data across files.

Fixes: 3db1de0 ("f2fs: change the current atomic write way")
Cc: Daeho Jeong <daehojeong@google.com>
Signed-off-by: Chao Yu <chao@kernel.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
  • Loading branch information
chaseyu authored and Jaegeuk Kim committed Aug 5, 2024
1 parent d72750e commit 1a0bd28
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
10 changes: 9 additions & 1 deletion fs/f2fs/data.c
Original file line number Diff line number Diff line change
Expand Up @@ -2650,10 +2650,13 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
struct dnode_of_data dn;
struct node_info ni;
bool ipu_force = false;
bool atomic_commit;
int err = 0;

/* Use COW inode to make dnode_of_data for atomic write */
if (f2fs_is_atomic_file(inode))
atomic_commit = f2fs_is_atomic_file(inode) &&
page_private_atomic(fio->page);
if (atomic_commit)
set_new_dnode(&dn, F2FS_I(inode)->cow_inode, NULL, NULL, 0);
else
set_new_dnode(&dn, inode, NULL, NULL, 0);
Expand Down Expand Up @@ -2752,6 +2755,8 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
f2fs_outplace_write_data(&dn, fio);
trace_f2fs_do_write_data_page(page_folio(page), OPU);
set_inode_flag(inode, FI_APPEND_WRITE);
if (atomic_commit)
clear_page_private_atomic(page);
out_writepage:
f2fs_put_dnode(&dn);
out:
Expand Down Expand Up @@ -3721,6 +3726,9 @@ static int f2fs_write_end(struct file *file,

set_page_dirty(page);

if (f2fs_is_atomic_file(inode))
set_page_private_atomic(page);

if (pos + copied > i_size_read(inode) &&
!f2fs_verity_in_progress(inode)) {
f2fs_i_size_write(inode, pos + copied);
Expand Down
8 changes: 7 additions & 1 deletion fs/f2fs/f2fs.h
Original file line number Diff line number Diff line change
Expand Up @@ -1424,7 +1424,8 @@ static inline void f2fs_clear_bit(unsigned int nr, char *addr);
* bit 1 PAGE_PRIVATE_ONGOING_MIGRATION
* bit 2 PAGE_PRIVATE_INLINE_INODE
* bit 3 PAGE_PRIVATE_REF_RESOURCE
* bit 4- f2fs private data
* bit 4 PAGE_PRIVATE_ATOMIC_WRITE
* bit 5- f2fs private data
*
* Layout B: lowest bit should be 0
* page.private is a wrapped pointer.
Expand All @@ -1434,6 +1435,7 @@ enum {
PAGE_PRIVATE_ONGOING_MIGRATION, /* data page which is on-going migrating */
PAGE_PRIVATE_INLINE_INODE, /* inode page contains inline data */
PAGE_PRIVATE_REF_RESOURCE, /* dirty page has referenced resources */
PAGE_PRIVATE_ATOMIC_WRITE, /* data page from atomic write path */
PAGE_PRIVATE_MAX
};

Expand Down Expand Up @@ -2404,14 +2406,17 @@ static inline void clear_page_private_##name(struct page *page) \
PAGE_PRIVATE_GET_FUNC(nonpointer, NOT_POINTER);
PAGE_PRIVATE_GET_FUNC(inline, INLINE_INODE);
PAGE_PRIVATE_GET_FUNC(gcing, ONGOING_MIGRATION);
PAGE_PRIVATE_GET_FUNC(atomic, ATOMIC_WRITE);

PAGE_PRIVATE_SET_FUNC(reference, REF_RESOURCE);
PAGE_PRIVATE_SET_FUNC(inline, INLINE_INODE);
PAGE_PRIVATE_SET_FUNC(gcing, ONGOING_MIGRATION);
PAGE_PRIVATE_SET_FUNC(atomic, ATOMIC_WRITE);

PAGE_PRIVATE_CLEAR_FUNC(reference, REF_RESOURCE);
PAGE_PRIVATE_CLEAR_FUNC(inline, INLINE_INODE);
PAGE_PRIVATE_CLEAR_FUNC(gcing, ONGOING_MIGRATION);
PAGE_PRIVATE_CLEAR_FUNC(atomic, ATOMIC_WRITE);

static inline unsigned long get_page_private_data(struct page *page)
{
Expand Down Expand Up @@ -2443,6 +2448,7 @@ static inline void clear_page_private_all(struct page *page)
clear_page_private_reference(page);
clear_page_private_gcing(page);
clear_page_private_inline(page);
clear_page_private_atomic(page);

f2fs_bug_on(F2FS_P_SB(page), page_private(page));
}
Expand Down

0 comments on commit 1a0bd28

Please sign in to comment.