Skip to content

Commit

Permalink
Revert: "f2fs: check last page index in cached bio to decide submission"
Browse files Browse the repository at this point in the history
There is one case that we can leave bio in f2fs, result in hanging
page writeback waiter.

Thread A				Thread B
- f2fs_write_cache_pages
 - f2fs_submit_page_write
 page #0 cached in bio #0 of cold log
 - f2fs_submit_page_write
 page #1 cached in bio #1 of warm log
					- f2fs_write_cache_pages
					 - f2fs_submit_page_write
					 bio is full, submit bio #1 contain page #1
 - f2fs_submit_merged_write_cond(, page #1)
 fail to submit bio #0 due to page #1 is not in any cached bios.

Signed-off-by: Chao Yu <[email protected]>
Signed-off-by: Jaegeuk Kim <[email protected]>
  • Loading branch information
chaseyu authored and Jaegeuk Kim committed Oct 31, 2018
1 parent 622634c commit d1e5e63
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 35 deletions.
3 changes: 1 addition & 2 deletions fs/f2fs/checkpoint.c
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,7 @@ static int __f2fs_write_meta_page(struct page *page,
dec_page_count(sbi, F2FS_DIRTY_META);

if (wbc->for_reclaim)
f2fs_submit_merged_write_cond(sbi, page->mapping->host,
0, page->index, META);
f2fs_submit_merged_write_cond(sbi, NULL, page, 0, META);

unlock_page(page);

Expand Down
38 changes: 19 additions & 19 deletions fs/f2fs/data.c
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,8 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
io->bio = NULL;
}

static bool __has_merged_page(struct f2fs_bio_info *io,
struct inode *inode, nid_t ino, pgoff_t idx)
static bool __has_merged_page(struct f2fs_bio_info *io, struct inode *inode,
struct page *page, nid_t ino)
{
struct bio_vec *bvec;
struct page *target;
Expand All @@ -330,7 +330,7 @@ static bool __has_merged_page(struct f2fs_bio_info *io,
if (!io->bio)
return false;

if (!inode && !ino)
if (!inode && !page && !ino)
return true;

bio_for_each_segment_all(bvec, io->bio, i) {
Expand All @@ -340,11 +340,10 @@ static bool __has_merged_page(struct f2fs_bio_info *io,
else
target = fscrypt_control_page(bvec->bv_page);

if (idx != target->index)
continue;

if (inode && inode == target->mapping->host)
return true;
if (page && page == target)
return true;
if (ino && ino == ino_of_node(target))
return true;
}
Expand All @@ -353,7 +352,8 @@ static bool __has_merged_page(struct f2fs_bio_info *io,
}

static bool has_merged_page(struct f2fs_sb_info *sbi, struct inode *inode,
nid_t ino, pgoff_t idx, enum page_type type)
struct page *page, nid_t ino,
enum page_type type)
{
enum page_type btype = PAGE_TYPE_OF_BIO(type);
enum temp_type temp;
Expand All @@ -364,7 +364,7 @@ static bool has_merged_page(struct f2fs_sb_info *sbi, struct inode *inode,
io = sbi->write_io[btype] + temp;

down_read(&io->io_rwsem);
ret = __has_merged_page(io, inode, ino, idx);
ret = __has_merged_page(io, inode, page, ino);
up_read(&io->io_rwsem);

/* TODO: use HOT temp only for meta pages now. */
Expand Down Expand Up @@ -395,12 +395,12 @@ static void __f2fs_submit_merged_write(struct f2fs_sb_info *sbi,
}

static void __submit_merged_write_cond(struct f2fs_sb_info *sbi,
struct inode *inode, nid_t ino, pgoff_t idx,
enum page_type type, bool force)
struct inode *inode, struct page *page,
nid_t ino, enum page_type type, bool force)
{
enum temp_type temp;

if (!force && !has_merged_page(sbi, inode, ino, idx, type))
if (!force && !has_merged_page(sbi, inode, page, ino, type))
return;

for (temp = HOT; temp < NR_TEMP_TYPE; temp++) {
Expand All @@ -419,10 +419,10 @@ void f2fs_submit_merged_write(struct f2fs_sb_info *sbi, enum page_type type)
}

void f2fs_submit_merged_write_cond(struct f2fs_sb_info *sbi,
struct inode *inode, nid_t ino, pgoff_t idx,
enum page_type type)
struct inode *inode, struct page *page,
nid_t ino, enum page_type type)
{
__submit_merged_write_cond(sbi, inode, ino, idx, type, false);
__submit_merged_write_cond(sbi, inode, page, ino, type, false);
}

void f2fs_flush_merged_writes(struct f2fs_sb_info *sbi)
Expand Down Expand Up @@ -1925,7 +1925,7 @@ static int __write_data_page(struct page *page, bool *submitted,
ClearPageUptodate(page);

if (wbc->for_reclaim) {
f2fs_submit_merged_write_cond(sbi, inode, 0, page->index, DATA);
f2fs_submit_merged_write_cond(sbi, NULL, page, 0, DATA);
clear_inode_flag(inode, FI_HOT_DATA);
f2fs_remove_dirty_inode(inode);
submitted = NULL;
Expand Down Expand Up @@ -1983,10 +1983,10 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
pgoff_t index;
pgoff_t end; /* Inclusive */
pgoff_t done_index;
pgoff_t last_idx = ULONG_MAX;
int cycled;
int range_whole = 0;
int tag;
int nwritten = 0;

pagevec_init(&pvec, 0);

Expand Down Expand Up @@ -2089,7 +2089,7 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
done = 1;
break;
} else if (submitted) {
last_idx = page->index;
nwritten++;
}

if (--wbc->nr_to_write <= 0 &&
Expand All @@ -2111,9 +2111,9 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
if (wbc->range_cyclic || (range_whole && wbc->nr_to_write > 0))
mapping->writeback_index = done_index;

if (last_idx != ULONG_MAX)
if (nwritten)
f2fs_submit_merged_write_cond(F2FS_M_SB(mapping), mapping->host,
0, last_idx, DATA);
NULL, 0, DATA);

return ret;
}
Expand Down
4 changes: 2 additions & 2 deletions fs/f2fs/f2fs.h
Original file line number Diff line number Diff line change
Expand Up @@ -3169,8 +3169,8 @@ int f2fs_init_post_read_processing(void);
void f2fs_destroy_post_read_processing(void);
void f2fs_submit_merged_write(struct f2fs_sb_info *sbi, enum page_type type);
void f2fs_submit_merged_write_cond(struct f2fs_sb_info *sbi,
struct inode *inode, nid_t ino, pgoff_t idx,
enum page_type type);
struct inode *inode, struct page *page,
nid_t ino, enum page_type type);
void f2fs_flush_merged_writes(struct f2fs_sb_info *sbi);
int f2fs_submit_page_bio(struct f2fs_io_info *fio);
void f2fs_submit_page_write(struct f2fs_io_info *fio);
Expand Down
11 changes: 5 additions & 6 deletions fs/f2fs/node.c
Original file line number Diff line number Diff line change
Expand Up @@ -1566,8 +1566,7 @@ static int __write_node_page(struct page *page, bool atomic, bool *submitted,
up_read(&sbi->node_write);

if (wbc->for_reclaim) {
f2fs_submit_merged_write_cond(sbi, page->mapping->host, 0,
page->index, NODE);
f2fs_submit_merged_write_cond(sbi, NULL, page, 0, NODE);
submitted = NULL;
}

Expand Down Expand Up @@ -1632,13 +1631,13 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
unsigned int *seq_id)
{
pgoff_t index;
pgoff_t last_idx = ULONG_MAX;
struct pagevec pvec;
int ret = 0;
struct page *last_page = NULL;
bool marked = false;
nid_t ino = inode->i_ino;
int nr_pages;
int nwritten = 0;

if (atomic) {
last_page = last_fsync_dnode(sbi, ino);
Expand Down Expand Up @@ -1716,7 +1715,7 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
f2fs_put_page(last_page, 0);
break;
} else if (submitted) {
last_idx = page->index;
nwritten++;
}

if (page == last_page) {
Expand All @@ -1742,8 +1741,8 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
goto retry;
}
out:
if (last_idx != ULONG_MAX)
f2fs_submit_merged_write_cond(sbi, NULL, ino, last_idx, NODE);
if (nwritten)
f2fs_submit_merged_write_cond(sbi, NULL, NULL, ino, NODE);
return ret ? -EIO: 0;
}

Expand Down
11 changes: 5 additions & 6 deletions fs/f2fs/segment.c
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ static int __f2fs_commit_inmem_pages(struct inode *inode)
.io_type = FS_DATA_IO,
};
struct list_head revoke_list;
pgoff_t last_idx = ULONG_MAX;
bool submit_bio = false;
int err = 0;

INIT_LIST_HEAD(&revoke_list);
Expand Down Expand Up @@ -406,14 +406,14 @@ static int __f2fs_commit_inmem_pages(struct inode *inode)
}
/* record old blkaddr for revoking */
cur->old_addr = fio.old_blkaddr;
last_idx = page->index;
submit_bio = true;
}
unlock_page(page);
list_move_tail(&cur->list, &revoke_list);
}

if (last_idx != ULONG_MAX)
f2fs_submit_merged_write_cond(sbi, inode, 0, last_idx, DATA);
if (submit_bio)
f2fs_submit_merged_write_cond(sbi, inode, NULL, 0, DATA);

if (err) {
/*
Expand Down Expand Up @@ -3263,8 +3263,7 @@ void f2fs_wait_on_page_writeback(struct page *page,
if (PageWriteback(page)) {
struct f2fs_sb_info *sbi = F2FS_P_SB(page);

f2fs_submit_merged_write_cond(sbi, page->mapping->host,
0, page->index, type);
f2fs_submit_merged_write_cond(sbi, NULL, page, 0, type);
if (ordered)
wait_on_page_writeback(page);
else
Expand Down

0 comments on commit d1e5e63

Please sign in to comment.