Revert: "f2fs: check last page index in cached bio to decide submission"
authorChao Yu <yuchao0@huawei.com>
Thu, 27 Sep 2018 15:41:16 +0000 (23:41 +0800)
committerJaegeuk Kim <jaegeuk@kernel.org>
Mon, 1 Oct 2018 01:39:54 +0000 (18:39 -0700)
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>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
fs/f2fs/checkpoint.c
fs/f2fs/data.c
fs/f2fs/f2fs.h
fs/f2fs/node.c
fs/f2fs/segment.c

index f4b9b81..97b429b 100644 (file)
@@ -276,8 +276,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);
 
index 41102c2..ea16cad 100644 (file)
@@ -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)
@@ -1952,7 +1952,7 @@ out:
                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;
@@ -2010,10 +2010,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);
 
@@ -2116,7 +2116,7 @@ continue_unlock:
                                done = 1;
                                break;
                        } else if (submitted) {
-                               last_idx = page->index;
+                               nwritten++;
                        }
 
                        if (--wbc->nr_to_write <= 0 &&
@@ -2138,9 +2138,9 @@ continue_unlock:
        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;
 }
index 668836c..f633662 100644 (file)
@@ -3032,8 +3032,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);
index acb819b..b1e3ff8 100644 (file)
@@ -1565,8 +1565,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;
        }
 
@@ -1631,13 +1630,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);
@@ -1715,7 +1714,7 @@ continue_unlock:
                                f2fs_put_page(last_page, 0);
                                break;
                        } else if (submitted) {
-                               last_idx = page->index;
+                               nwritten++;
                        }
 
                        if (page == last_page) {
@@ -1741,8 +1740,8 @@ continue_unlock:
                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;
 }
 
index 3fa8870..e16dae0 100644 (file)
@@ -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 @@ retry:
                        }
                        /* 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) {
                /*
@@ -3181,8 +3181,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