Re: [PATCH] f2fs: provide f2fs_balance_fs to __write_data_page for dentry pages

Mon, 31 Jul 2017 23:11:24 -0700 

Hi Jay,
The SBI_BLOCK_OPS can not cover all the case, once SBI_BLOCK_OPS is set, all the normal writeback (before the SBI_BLOCK_OPS is clear) of dentry pages which do not come from write_checkpoint flow will 
totally miss all the f2fs_balance_fs check.

On 2017/8/1 8:44, Jaegeuk Kim wrote:

Hi Yunlong,

On 07/29, Yunlong Song wrote:

f2fs_balance_fs of dentry pages is skipped in __write_data_page due to deadlock
of gc_mutex in write_checkpoint flow. This patch enables f2fs_balance_fs for
normal dentry page writeback to ensure there are always enough free segments.

So, how about adding SBI_BLOCK_OPS

/* For s_flag in struct f2fs_sb_info */
enum {
        SBI_IS_DIRTY,                           /* dirty flag for checkpoint */
        SBI_IS_CLOSE,                           /* specify unmounting */
        SBI_NEED_FSCK,                          /* need fsck.f2fs to fix */
        SBI_POR_DOING,                          /* recovery is doing or not */
        SBI_NEED_SB_WRITE,                      /* need to recover superblock */
        SBI_NEED_CP,                            /* need to checkpoint */
===
        SBI_BLOCK_OPS,                          /* doing block_operations */
===
};

How aboud avoiding f2fs_balance_fs() in __write_data_page() by:

        if (!is_sbi_flag_set(sbi, SBI_BLOCK_OPS) || !S_ISDIR(inode->i_mode))
                f2fs_balance_fs();

In block_operations(),
        set_sbi_flag(sbi, SBI_BLOCK_OPS);
        ...
        clear_sbi_flag(sbi, SBI_BLOCK_OPS);

Thanks,


Reported-by: Chao Yu <yuch...@huawei.com>
Signed-off-by: Yunlong Song <yunlong.s...@huawei.com>
---
  fs/f2fs/checkpoint.c |  2 +-
  fs/f2fs/data.c       | 67 +++++++++++++++++++++++++++++++++++++++++++++-------
  fs/f2fs/f2fs.h       |  1 +
  3 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 3c84a25..2882878 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -904,7 +904,7 @@ int sync_dirty_inodes(struct f2fs_sb_info *sbi, enum 
inode_type type)
        if (inode) {
                unsigned long cur_ino = inode->i_ino;
- filemap_fdatawrite(inode->i_mapping); 
+               f2fs_filemap_fdatawrite(inode->i_mapping, is_dir);
                iput(inode);
                /* We need to give cpu to another writers. */
                if (ino == cur_ino) {
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index aefc2a5..ed7efa5 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1475,7 +1475,7 @@ int do_write_data_page(struct f2fs_io_info *fio)
  }
static int __write_data_page(struct page *page, bool *submitted, 
-                               struct writeback_control *wbc)
+                               struct writeback_control *wbc, bool do_balance)
  {
        struct inode *inode = page->mapping->host;
        struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
@@ -1578,7 +1578,7 @@ static int __write_data_page(struct page *page, bool 
*submitted,
        }
unlock_page(page); 
-       if (!S_ISDIR(inode->i_mode))
+       if (do_balance)
                f2fs_balance_fs(sbi, need_balance_fs);
if (unlikely(f2fs_cp_error(sbi))) { 
@@ -1602,7 +1602,7 @@ static int __write_data_page(struct page *page, bool 
*submitted,
  static int f2fs_write_data_page(struct page *page,
                                        struct writeback_control *wbc)
  {
-       return __write_data_page(page, NULL, wbc);
+       return __write_data_page(page, NULL, wbc, true);
  }
/* 
@@ -1611,7 +1611,7 @@ static int f2fs_write_data_page(struct page *page,
   * warm/hot data page.
   */
  static int f2fs_write_cache_pages(struct address_space *mapping,
-                                       struct writeback_control *wbc)
+                                       struct writeback_control *wbc, bool 
do_balance)
  {
        int ret = 0;
        int done = 0;
@@ -1701,7 +1701,7 @@ static int f2fs_write_cache_pages(struct address_space 
*mapping,
                        if (!clear_page_dirty_for_io(page))
                                goto continue_unlock;
- ret = __write_data_page(page, &submitted, wbc); 
+                       ret = __write_data_page(page, &submitted, wbc, 
do_balance);
                        if (unlikely(ret)) {
                                /*
                                 * keep nr_to_write, since vfs uses this to
@@ -1756,8 +1756,8 @@ static int f2fs_write_cache_pages(struct address_space 
*mapping,
        return ret;
  }
-static int f2fs_write_data_pages(struct address_space *mapping, 
-                           struct writeback_control *wbc)
+static int _f2fs_write_data_pages(struct address_space *mapping,
+                           struct writeback_control *wbc, bool do_balance)
  {
        struct inode *inode = mapping->host;
        struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
@@ -1794,7 +1794,7 @@ static int f2fs_write_data_pages(struct address_space 
*mapping,
                goto skip_write;
blk_start_plug(&plug); 
-       ret = f2fs_write_cache_pages(mapping, wbc);
+       ret = f2fs_write_cache_pages(mapping, wbc, do_balance);
        blk_finish_plug(&plug);
if (wbc->sync_mode == WB_SYNC_ALL) 
@@ -1813,6 +1813,57 @@ static int f2fs_write_data_pages(struct address_space 
*mapping,
        return 0;
  }
+static int f2fs_write_data_pages(struct address_space *mapping, 
+                           struct writeback_control *wbc)
+{
+       return  _f2fs_write_data_pages(mapping, wbc, true);
+}
+
+/*
+ * This function was copied from do_writepages from mm/page-writeback.c.
+ * The major change is changing writepages to _f2fs_write_data_pages.
+ */
+static int f2fs_do_writepages(struct address_space *mapping,
+                               struct writeback_control *wbc, bool is_dir)
+{
+       int ret;
+
+       if (wbc->nr_to_write <= 0)
+               return 0;
+       while (1) {
+               ret = _f2fs_write_data_pages(mapping, wbc, !is_dir);
+               if ((ret != -ENOMEM) || (wbc->sync_mode != WB_SYNC_ALL))
+                       break;
+               cond_resched();
+               congestion_wait(BLK_RW_ASYNC, HZ/50);
+       }
+       return ret;
+}
+
+/*
+ * This function was copied from __filemap_fdatawrite_range from
+ * mm/filemap.c. The major change is changing do_writepages to
+ * f2fs_do_writepages.
+ */
+int f2fs_filemap_fdatawrite(struct address_space *mapping, bool is_dir)
+{
+       int ret;
+       struct writeback_control wbc = {
+               .sync_mode = WB_SYNC_ALL,
+               .nr_to_write = LONG_MAX,
+               .range_start = 0,
+               .range_end = LLONG_MAX,
+       };
+
+       if (!mapping_cap_writeback_dirty(mapping))
+               return 0;
+
+       wbc_attach_fdatawrite_inode(&wbc, mapping->host);
+       ret = f2fs_do_writepages(mapping, &wbc, is_dir);
+       wbc_detach_inode(&wbc);
+       return ret;
+}
+
  static void f2fs_write_failed(struct address_space *mapping, loff_t to)
  {
        struct inode *inode = mapping->host;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 9280283..ea9bebb 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2572,6 +2572,7 @@ void f2fs_invalidate_page(struct page *page, unsigned int 
offset,
  int f2fs_migrate_page(struct address_space *mapping, struct page *newpage,
                        struct page *page, enum migrate_mode mode);
  #endif
+int f2fs_filemap_fdatawrite(struct address_space *mapping, bool is_dir);
/* 
   * gc.c
--
1.8.5.2

.



--
Thanks,
Yunlong Song

Reply via email to