* [PATCH RESEND v2] Revert: "f2fs: check last page index in cached bio to decide submission" @ 2018-09-26 14:44 Chao Yu 2018-09-27 7:40 ` [f2fs-dev] " Sahitya Tummala 0 siblings, 1 reply; 3+ messages in thread From: Chao Yu @ 2018-09-26 14:44 UTC (permalink / raw) To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu From: Chao Yu <yuchao0@huawei.com> 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 <yuchao0@huawei.com> --- v2: - rebase to dev-test fs/f2fs/checkpoint.c | 2 +- fs/f2fs/data.c | 38 +++++++++++++++++++------------------- fs/f2fs/f2fs.h | 4 ++-- fs/f2fs/node.c | 12 ++++++------ fs/f2fs/segment.c | 11 +++++------ 5 files changed, 33 insertions(+), 34 deletions(-) diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c index d624d7983197..2f63b362ce63 100644 --- a/fs/f2fs/checkpoint.c +++ b/fs/f2fs/checkpoint.c @@ -280,7 +280,7 @@ static int __f2fs_write_meta_page(struct page *page, if (wbc->for_reclaim) f2fs_submit_merged_write_cond(sbi, page->mapping->host, - 0, page->index, META); + page, 0, META); unlock_page(page); diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index be69b6ac6870..b03f9d163175 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -322,8 +322,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; @@ -332,7 +332,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) { @@ -342,11 +342,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; } @@ -355,7 +354,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; @@ -366,7 +366,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. */ @@ -397,12 +397,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++) { @@ -421,10 +421,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) @@ -1962,7 +1962,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, inode, page, 0, DATA); clear_inode_flag(inode, FI_HOT_DATA); f2fs_remove_dirty_inode(inode); submitted = NULL; @@ -2020,10 +2020,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); @@ -2126,7 +2126,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 && @@ -2148,9 +2148,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; } diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index dc9306e87a51..464545e40729 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -3049,8 +3049,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); diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index ea151e07790f..c47ca807e245 100644 --- a/fs/f2fs/node.c +++ b/fs/f2fs/node.c @@ -1561,8 +1561,8 @@ 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, page->mapping->host, + page, 0, NODE); submitted = NULL; } @@ -1634,13 +1634,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); @@ -1718,7 +1718,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) { @@ -1744,8 +1744,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; } diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 97a4fae75651..965ef524e2a2 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -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); @@ -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) { /* @@ -3179,8 +3179,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 -- 2.18.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [f2fs-dev] [PATCH RESEND v2] Revert: "f2fs: check last page index in cached bio to decide submission" 2018-09-26 14:44 [PATCH RESEND v2] Revert: "f2fs: check last page index in cached bio to decide submission" Chao Yu @ 2018-09-27 7:40 ` Sahitya Tummala 2018-09-27 7:50 ` Chao Yu 0 siblings, 1 reply; 3+ messages in thread From: Sahitya Tummala @ 2018-09-27 7:40 UTC (permalink / raw) To: Chao Yu; +Cc: jaegeuk, linux-kernel, linux-f2fs-devel On Wed, Sep 26, 2018 at 10:44:56PM +0800, Chao Yu wrote: > From: Chao Yu <yuchao0@huawei.com> > > 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 <yuchao0@huawei.com> > --- > v2: > - rebase to dev-test > fs/f2fs/checkpoint.c | 2 +- > fs/f2fs/data.c | 38 +++++++++++++++++++------------------- > fs/f2fs/f2fs.h | 4 ++-- > fs/f2fs/node.c | 12 ++++++------ > fs/f2fs/segment.c | 11 +++++------ > 5 files changed, 33 insertions(+), 34 deletions(-) > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > index d624d7983197..2f63b362ce63 100644 > --- a/fs/f2fs/checkpoint.c > +++ b/fs/f2fs/checkpoint.c > @@ -280,7 +280,7 @@ static int __f2fs_write_meta_page(struct page *page, > > if (wbc->for_reclaim) > f2fs_submit_merged_write_cond(sbi, page->mapping->host, > - 0, page->index, META); > + page, 0, META); > > unlock_page(page); > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index be69b6ac6870..b03f9d163175 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -322,8 +322,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; > @@ -332,7 +332,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) { > @@ -342,11 +342,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 both inode and page are passed, then I think it we should check for page first followed by inode checks. What do you think? > if (ino && ino == ino_of_node(target)) > return true; > } > @@ -355,7 +354,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; > @@ -366,7 +366,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. */ > @@ -397,12 +397,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++) { > @@ -421,10 +421,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) > @@ -1962,7 +1962,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, inode, page, 0, DATA); > clear_inode_flag(inode, FI_HOT_DATA); > f2fs_remove_dirty_inode(inode); > submitted = NULL; > @@ -2020,10 +2020,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); > > @@ -2126,7 +2126,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 && > @@ -2148,9 +2148,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; > } > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index dc9306e87a51..464545e40729 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -3049,8 +3049,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); > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c > index ea151e07790f..c47ca807e245 100644 > --- a/fs/f2fs/node.c > +++ b/fs/f2fs/node.c > @@ -1561,8 +1561,8 @@ 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, page->mapping->host, > + page, 0, NODE); > submitted = NULL; > } > > @@ -1634,13 +1634,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); > @@ -1718,7 +1718,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) { > @@ -1744,8 +1744,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; > } > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 97a4fae75651..965ef524e2a2 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -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); > @@ -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) { > /* > @@ -3179,8 +3179,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 > -- > 2.18.0 > > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel -- -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH RESEND v2] Revert: "f2fs: check last page index in cached bio to decide submission" 2018-09-27 7:40 ` [f2fs-dev] " Sahitya Tummala @ 2018-09-27 7:50 ` Chao Yu 0 siblings, 0 replies; 3+ messages in thread From: Chao Yu @ 2018-09-27 7:50 UTC (permalink / raw) To: Sahitya Tummala, Chao Yu; +Cc: jaegeuk, linux-kernel, linux-f2fs-devel On 2018/9/27 15:40, Sahitya Tummala wrote: > On Wed, Sep 26, 2018 at 10:44:56PM +0800, Chao Yu wrote: >> From: Chao Yu <yuchao0@huawei.com> >> >> 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 <yuchao0@huawei.com> >> --- >> v2: >> - rebase to dev-test >> fs/f2fs/checkpoint.c | 2 +- >> fs/f2fs/data.c | 38 +++++++++++++++++++------------------- >> fs/f2fs/f2fs.h | 4 ++-- >> fs/f2fs/node.c | 12 ++++++------ >> fs/f2fs/segment.c | 11 +++++------ >> 5 files changed, 33 insertions(+), 34 deletions(-) >> >> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c >> index d624d7983197..2f63b362ce63 100644 >> --- a/fs/f2fs/checkpoint.c >> +++ b/fs/f2fs/checkpoint.c >> @@ -280,7 +280,7 @@ static int __f2fs_write_meta_page(struct page *page, >> >> if (wbc->for_reclaim) >> f2fs_submit_merged_write_cond(sbi, page->mapping->host, >> - 0, page->index, META); >> + page, 0, META); >> >> unlock_page(page); >> >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >> index be69b6ac6870..b03f9d163175 100644 >> --- a/fs/f2fs/data.c >> +++ b/fs/f2fs/data.c >> @@ -322,8 +322,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; >> @@ -332,7 +332,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) { >> @@ -342,11 +342,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 both inode and page are passed, then I think it we should check for page first > followed by inode checks. What do you think? Yes, I've changed this and would like to check in latter tests. But, it's strange, yesterday, I remember I've passed all tests, but when I retest this patch at home, I got stuck and obviously there are compiler warning... I guess I'm missing something yesterday. Thanks, > >> if (ino && ino == ino_of_node(target)) >> return true; >> } >> @@ -355,7 +354,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; >> @@ -366,7 +366,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. */ >> @@ -397,12 +397,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++) { >> @@ -421,10 +421,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) >> @@ -1962,7 +1962,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, inode, page, 0, DATA); >> clear_inode_flag(inode, FI_HOT_DATA); >> f2fs_remove_dirty_inode(inode); >> submitted = NULL; >> @@ -2020,10 +2020,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); >> >> @@ -2126,7 +2126,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 && >> @@ -2148,9 +2148,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; >> } >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >> index dc9306e87a51..464545e40729 100644 >> --- a/fs/f2fs/f2fs.h >> +++ b/fs/f2fs/f2fs.h >> @@ -3049,8 +3049,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); >> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c >> index ea151e07790f..c47ca807e245 100644 >> --- a/fs/f2fs/node.c >> +++ b/fs/f2fs/node.c >> @@ -1561,8 +1561,8 @@ 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, page->mapping->host, >> + page, 0, NODE); >> submitted = NULL; >> } >> >> @@ -1634,13 +1634,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); >> @@ -1718,7 +1718,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) { >> @@ -1744,8 +1744,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; >> } >> >> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c >> index 97a4fae75651..965ef524e2a2 100644 >> --- a/fs/f2fs/segment.c >> +++ b/fs/f2fs/segment.c >> @@ -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); >> @@ -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) { >> /* >> @@ -3179,8 +3179,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 >> -- >> 2.18.0 >> >> >> >> _______________________________________________ >> 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] 3+ messages in thread
end of thread, other threads:[~2018-09-27 7:50 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-09-26 14:44 [PATCH RESEND v2] Revert: "f2fs: check last page index in cached bio to decide submission" Chao Yu 2018-09-27 7:40 ` [f2fs-dev] " Sahitya Tummala 2018-09-27 7:50 ` Chao Yu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).