f2fs: avoid unnecessary f2fs_balance_fs calls

PATCH New Jaegeuk Kim 1 participant 3 messages Jan 8, 2016

Subsystems (2)

VFS F2FS file system

lore

Share & tools

lore

Jaegeuk Kim

author maintainer

2016-01-08 01:36 UTC #0 lore raw

PATCH 1/3 f2fs: avoid unnecessary f2fs_balance_fs calls

Only when node page is newly dirtied, it needs to check whether we need to do f2fs_gc.

Signed-off-by Jaegeuk Kim <[email protected]>

---
 fs/f2fs/data.c  |  4 ++--
 fs/f2fs/f2fs.h  |  4 ++--
 fs/f2fs/inode.c | 19 ++++++++++---------
 fs/f2fs/node.c  | 26 ++++++++++++++------------
 fs/f2fs/node.h  |  4 ++--
 fs/f2fs/super.c |  2 --
 6 files changed, 30 insertions(+), 29 deletions(-)

fs/f2fs/data.c +2 -2

index 77c3bbb..3cf86fd 100644
@@ -225,8 +225,8 @@ void set_data_blkaddr(struct dnode_of_data *dn)
 	/* Get physical address of data block */
 	addr_array = blkaddr_in_node(rn);
 	addr_array[ofs_in_node] = cpu_to_le32(dn->data_blkaddr);
-	set_page_dirty(node_page);
-	dn->node_changed = true;
+	if (set_page_dirty(node_page))
+		dn->node_changed = true;
 }
 
 int reserve_new_block(struct dnode_of_data *dn)

fs/f2fs/f2fs.h +2 -2

index 882baba..461b329 100644
@@ -1674,8 +1674,8 @@ long f2fs_compat_ioctl(struct file *, unsigned int, unsigned long);
 void f2fs_set_inode_flags(struct inode *);
 struct inode *f2fs_iget(struct super_block *, unsigned long);
 int try_to_free_nats(struct f2fs_sb_info *, int);
-void update_inode(struct inode *, struct page *);
-void update_inode_page(struct inode *);
+int update_inode(struct inode *, struct page *);
+int update_inode_page(struct inode *);
 int f2fs_write_inode(struct inode *, struct writeback_control *);
 void f2fs_evict_inode(struct inode *);
 void handle_failed_inode(struct inode *);

fs/f2fs/inode.c +10 -9

index e955008..cabc1ff 100644
@@ -222,7 +222,7 @@ bad_inode:
 	return ERR_PTR(ret);
 }
 
-void update_inode(struct inode *inode, struct page *node_page)
+int update_inode(struct inode *inode, struct page *node_page)
 {
 	struct f2fs_inode *ri;

@@ -260,15 +260,16 @@ void update_inode(struct inode *inode, struct page *node_page)
 
 	__set_inode_rdev(inode, ri);
 	set_cold_node(inode, node_page);
-	set_page_dirty(node_page);
-
 	clear_inode_flag(F2FS_I(inode), FI_DIRTY_INODE);
+
+	return set_page_dirty(node_page);
 }
 
-void update_inode_page(struct inode *inode)
+int update_inode_page(struct inode *inode)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	struct page *node_page;
+	int ret = 0;
 retry:
 	node_page = get_node_page(sbi, inode->i_ino);
 	if (IS_ERR(node_page)) {

@@ -279,10 +280,11 @@ retry:
 		} else if (err != -ENOENT) {
 			f2fs_stop_checkpoint(sbi);
 		}
-		return;
+		return 0;
 	}
-	update_inode(inode, node_page);
+	ret = update_inode(inode, node_page);
 	f2fs_put_page(node_page, 1);
+	return ret;
 }
 
 int f2fs_write_inode(struct inode *inode, struct writeback_control *wbc)

@@ -300,9 +302,8 @@ int f2fs_write_inode(struct inode *inode, struct writeback_control *wbc)
 	 * We need to balance fs here to prevent from producing dirty node pages
 	 * during the urgent cleaning time when runing out of free sections.
 	 */
-	update_inode_page(inode);
-
-	f2fs_balance_fs(sbi);
+	if (update_inode_page(inode))
+		f2fs_balance_fs(sbi);
 	return 0;
 }

fs/f2fs/node.c +14 ~~-12~~

index 5a2d800..c091b75 100644
@@ -543,7 +543,6 @@ int get_dnode_of_data(struct dnode_of_data *dn, pgoff_t index, int mode)
 
 			set_nid(parent, offset[i - 1], nids[i], i == 1);
 			alloc_nid_done(sbi, nids[i]);
-			dn->node_changed = true;
 			done = true;
 		} else if (mode == LOOKUP_NODE_RA && i == level && level > 1) {
 			npage[i] = get_node_page_ra(parent, offset[i - 1]);

@@ -679,8 +678,8 @@ static int truncate_nodes(struct dnode_of_data *dn, unsigned int nofs,
 			ret = truncate_dnode(&rdn);
 			if (ret < 0)
 				goto out_err;
-			set_nid(page, i, 0, false);
-			dn->node_changed = true;
+			if (set_nid(page, i, 0, false))
+				dn->node_changed = true;
 		}
 	} else {
 		child_nofs = nofs + ofs * (NIDS_PER_BLOCK + 1) + 1;

@@ -693,8 +692,8 @@ static int truncate_nodes(struct dnode_of_data *dn, unsigned int nofs,
 			rdn.nid = child_nid;
 			ret = truncate_nodes(&rdn, child_nofs, 0, depth - 1);
 			if (ret == (NIDS_PER_BLOCK + 1)) {
-				set_nid(page, i, 0, false);
-				dn->node_changed = true;
+				if (set_nid(page, i, 0, false))
+					dn->node_changed = true;
 				child_nofs += ret;
 			} else if (ret < 0 && ret != -ENOENT) {
 				goto out_err;

@@ -755,8 +754,8 @@ static int truncate_partial_nodes(struct dnode_of_data *dn,
 		err = truncate_dnode(dn);
 		if (err < 0)
 			goto fail;
-		set_nid(pages[idx], i, 0, false);
-		dn->node_changed = true;
+		if (set_nid(pages[idx], i, 0, false))
+			dn->node_changed = true;
 	}
 
 	if (offset[idx + 1] == 0) {

@@ -981,7 +980,8 @@ struct page *new_node_page(struct dnode_of_data *dn,
 	fill_node_footer(page, dn->nid, dn->inode->i_ino, ofs, true);
 	set_cold_node(dn->inode, page);
 	SetPageUptodate(page);
-	set_page_dirty(page);
+	if (set_page_dirty(page))
+		dn->node_changed = true;
 
 	if (f2fs_has_xattr_block(ofs))
 		F2FS_I(dn->inode)->i_xattr_nid = dn->nid;

@@ -1138,18 +1138,20 @@ struct page *get_node_page_ra(struct page *parent, int start)
 
 void sync_inode_page(struct dnode_of_data *dn)
 {
+	int ret = 0;
+
 	if (IS_INODE(dn->node_page) || dn->inode_page == dn->node_page) {
-		update_inode(dn->inode, dn->node_page);
+		ret = update_inode(dn->inode, dn->node_page);
 	} else if (dn->inode_page) {
 		if (!dn->inode_page_locked)
 			lock_page(dn->inode_page);
-		update_inode(dn->inode, dn->inode_page);
+		ret = update_inode(dn->inode, dn->inode_page);
 		if (!dn->inode_page_locked)
 			unlock_page(dn->inode_page);
 	} else {
-		update_inode_page(dn->inode);
+		ret = update_inode_page(dn->inode);
 	}
-	dn->node_changed = true;
+	dn->node_changed = ret ? true: false;
 }
 
 int sync_node_pages(struct f2fs_sb_info *sbi, nid_t ino,

fs/f2fs/node.h +2 -2

index 2de759a..d4d1f63 100644
@@ -317,7 +317,7 @@ static inline bool IS_DNODE(struct page *node_page)
 	return true;
 }
 
-static inline void set_nid(struct page *p, int off, nid_t nid, bool i)
+static inline int set_nid(struct page *p, int off, nid_t nid, bool i)
 {
 	struct f2fs_node *rn = F2FS_NODE(p);

@@ -327,7 +327,7 @@ static inline void set_nid(struct page *p, int off, nid_t nid, bool i)
 		rn->i.i_nid[off - NODE_DIR1_BLOCK] = cpu_to_le32(nid);
 	else
 		rn->in.nid[off] = cpu_to_le32(nid);
-	set_page_dirty(p);
+	return set_page_dirty(p);
 }
 
 static inline nid_t get_nid(struct page *p, int off, bool i)

fs/f2fs/super.c +0 -2

index 0bbd756..f5cc790 100644
@@ -591,8 +591,6 @@ int f2fs_sync_fs(struct super_block *sb, int sync)
 		mutex_lock(&sbi->gc_mutex);
 		err = write_checkpoint(sbi, &cpc);
 		mutex_unlock(&sbi->gc_mutex);
-	} else {
-		f2fs_balance_fs(sbi);
 	}
 	f2fs_trace_ios(NULL, 1);

Signature · 1 line

2.6.3

Jaegeuk Kim

author maintainer

2016-01-08 01:36 UTC #1 lore raw

PATCH 2/3 f2fs: remove redundant calls

This patch removes redundant calls.

Signed-off-by Jaegeuk Kim <[email protected]>

---
 fs/f2fs/file.c | 3 ---
 1 file changed, 3 deletions(-)

fs/f2fs/file.c +0 -3

index e3d32f6..69bc65f 100644
@@ -267,8 +267,6 @@ sync_nodes:
 	if (need_inode_block_update(sbi, ino)) {
 		mark_inode_dirty_sync(inode);
 		f2fs_write_inode(inode, NULL);
-
-		f2fs_balance_fs(sbi);
 		goto sync_nodes;
 	}

@@ -484,7 +482,6 @@ int truncate_data_blocks_range(struct dnode_of_data *dn, int count)
 						F2FS_I(dn->inode)) + ofs;
 		f2fs_update_extent_cache_range(dn, fofs, 0, len);
 		dec_valid_block_count(sbi, dn->inode, nr_free);
-		set_page_dirty(dn->node_page);
 		sync_inode_page(dn);
 	}
 	dn->ofs_in_node = ofs;

Signature · 1 line

2.6.3

Jaegeuk Kim

author maintainer

2016-01-08 01:36 UTC #2 lore raw

PATCH 3/3 f2fs: clean up f2fs_balance_fs

In reply to Jaegeuk Kim

This patch adds one parameter to clean up all the callers of f2fs_balance_fs.

Signed-off-by Jaegeuk Kim <[email protected]>

---
 fs/f2fs/data.c    | 17 ++++++-----------
 fs/f2fs/f2fs.h    |  2 +-
 fs/f2fs/file.c    | 17 ++++++++---------
 fs/f2fs/inline.c  |  3 +--
 fs/f2fs/inode.c   |  2 +-
 fs/f2fs/namei.c   | 22 +++++++++++-----------
 fs/f2fs/segment.c |  6 ++++--
 fs/f2fs/xattr.c   |  2 +-
 8 files changed, 33 insertions(+), 38 deletions(-)

fs/f2fs/data.c +6 ~~-11~~

index 3cf86fd..6fae75d 100644
@@ -546,8 +546,7 @@ static int __allocate_data_blocks(struct inode *inode, loff_t offset,
 		f2fs_put_dnode(&dn);
 		f2fs_unlock_op(sbi);
 
-		if (dn.node_changed)
-			f2fs_balance_fs(sbi);
+		f2fs_balance_fs(sbi, dn.node_changed);
 	}
 	return err;

@@ -557,8 +556,7 @@ sync_out:
 	f2fs_put_dnode(&dn);
 out:
 	f2fs_unlock_op(sbi);
-	if (dn.node_changed)
-		f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, dn.node_changed);
 	return err;
 }

@@ -657,8 +655,7 @@ get_next:
 
 		if (create) {
 			f2fs_unlock_op(sbi);
-			if (dn.node_changed)
-				f2fs_balance_fs(sbi);
+			f2fs_balance_fs(sbi, dn.node_changed);
 			f2fs_lock_op(sbi);
 		}

@@ -718,8 +715,7 @@ put_out:
 unlock_out:
 	if (create) {
 		f2fs_unlock_op(sbi);
-		if (dn.node_changed)
-			f2fs_balance_fs(sbi);
+		f2fs_balance_fs(sbi, dn.node_changed);
 	}
 out:
 	trace_f2fs_map_blocks(inode, map, err);

@@ -1178,8 +1174,7 @@ out:
 	if (err)
 		ClearPageUptodate(page);
 	unlock_page(page);
-	if (need_balance_fs)
-		f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, need_balance_fs);
 	if (wbc->for_reclaim || unlikely(f2fs_cp_error(sbi))) {
 		f2fs_submit_merged_bio(sbi, DATA, WRITE);
 		remove_dirty_inode(inode);

@@ -1506,7 +1501,7 @@ repeat:
 
 	if (need_balance && has_not_enough_free_secs(sbi, 0)) {
 		unlock_page(page);
-		f2fs_balance_fs(sbi);
+		f2fs_balance_fs(sbi, true);
 		lock_page(page);
 		if (page->mapping != mapping) {
 			/* The page got truncated from under us */

fs/f2fs/f2fs.h +1 -1

index 461b329..4128654 100644
@@ -1780,7 +1780,7 @@ void destroy_node_manager_caches(void);
  */
 void register_inmem_page(struct inode *, struct page *);
 int commit_inmem_pages(struct inode *, bool);
-void f2fs_balance_fs(struct f2fs_sb_info *);
+void f2fs_balance_fs(struct f2fs_sb_info *, bool);
 void f2fs_balance_fs_bg(struct f2fs_sb_info *);
 int f2fs_issue_flush(struct f2fs_sb_info *);
 int create_flush_cmd_control(struct f2fs_sb_info *);

fs/f2fs/file.c +8 -9

index 69bc65f..ff06827 100644
@@ -55,8 +55,7 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma,
 	f2fs_put_dnode(&dn);
 	f2fs_unlock_op(sbi);
 
-	if (dn.node_changed)
-		f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, dn.node_changed);
 
 	file_update_time(vma->vm_file);
 	lock_page(page);

@@ -677,7 +676,7 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
 			err = f2fs_truncate(inode, true);
 			if (err)
 				return err;
-			f2fs_balance_fs(F2FS_I_SB(inode));
+			f2fs_balance_fs(F2FS_I_SB(inode), true);
 		} else {
 			/*
 			 * do not trim all blocks after i_size if target size is

@@ -732,7 +731,7 @@ static int fill_zero(struct inode *inode, pgoff_t index,
 	if (!len)
 		return 0;
 
-	f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, true);
 
 	f2fs_lock_op(sbi);
 	page = get_new_data_page(inode, NULL, index, false);

@@ -818,7 +817,7 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
 			loff_t blk_start, blk_end;
 			struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 
-			f2fs_balance_fs(sbi);
+			f2fs_balance_fs(sbi, true);
 
 			blk_start = (loff_t)pg_start << PAGE_CACHE_SHIFT;
 			blk_end = (loff_t)pg_end << PAGE_CACHE_SHIFT;

@@ -921,7 +920,7 @@ static int f2fs_do_collapse(struct inode *inode, pgoff_t start, pgoff_t end)
 	int ret = 0;
 
 	for (; end < nrpages; start++, end++) {
-		f2fs_balance_fs(sbi);
+		f2fs_balance_fs(sbi, true);
 		f2fs_lock_op(sbi);
 		ret = __exchange_data_block(inode, end, start, true);
 		f2fs_unlock_op(sbi);

@@ -1103,7 +1102,7 @@ static int f2fs_insert_range(struct inode *inode, loff_t offset, loff_t len)
 	if (ret)
 		return ret;
 
-	f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, true);
 
 	ret = truncate_blocks(inode, i_size_read(inode), true);
 	if (ret)

@@ -1155,7 +1154,7 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 	if (ret)
 		return ret;
 
-	f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, true);
 
 	pg_start = ((unsigned long long) offset) >> PAGE_CACHE_SHIFT;
 	pg_end = ((unsigned long long) offset + len) >> PAGE_CACHE_SHIFT;

@@ -1653,7 +1652,7 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
 	pg_start = range->start >> PAGE_CACHE_SHIFT;
 	pg_end = (range->start + range->len) >> PAGE_CACHE_SHIFT;
 
-	f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, true);
 
 	mutex_lock(&inode->i_mutex);

fs/f2fs/inline.c +1 -2

index 5ffbd16..c3f0b7d 100644
@@ -200,8 +200,7 @@ out:
 
 	f2fs_put_page(page, 1);
 
-	if (dn.node_changed)
-		f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, dn.node_changed);
 
 	return err;
 }

fs/f2fs/inode.c +1 -1

index cabc1ff..2ac4b78 100644
@@ -303,7 +303,7 @@ int f2fs_write_inode(struct inode *inode, struct writeback_control *wbc)
 	 * during the urgent cleaning time when runing out of free sections.
 	 */
 	if (update_inode_page(inode))
-		f2fs_balance_fs(sbi);
+		f2fs_balance_fs(sbi, true);
 	return 0;
 }

fs/f2fs/namei.c +11 ~~-11~~

index 0d61a68..53d6227 100644
@@ -140,7 +140,7 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 	ino = inode->i_ino;
 
-	f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, true);
 
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);

@@ -172,7 +172,7 @@ static int f2fs_link(struct dentry *old_dentry, struct inode *dir,
 		!f2fs_is_child_context_consistent_with_parent(dir, inode))
 		return -EPERM;
 
-	f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, true);
 
 	inode->i_ctime = CURRENT_TIME;
 	ihold(inode);

@@ -221,7 +221,7 @@ static int __recover_dot_dentries(struct inode *dir, nid_t pino)
 		return 0;
 	}
 
-	f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, true);
 
 	f2fs_lock_op(sbi);

@@ -302,7 +302,7 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
 	if (!de)
 		goto fail;
 
-	f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, true);
 
 	f2fs_lock_op(sbi);
 	err = acquire_orphan_inode(sbi);

@@ -361,7 +361,7 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 		inode->i_op = &f2fs_symlink_inode_operations;
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 
-	f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, true);
 
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);

@@ -452,7 +452,7 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 	mapping_set_gfp_mask(inode->i_mapping, GFP_F2FS_HIGH_ZERO);
 
-	f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, true);
 
 	set_inode_flag(F2FS_I(inode), FI_INC_LINK);
 	f2fs_lock_op(sbi);

@@ -498,7 +498,7 @@ static int f2fs_mknod(struct inode *dir, struct dentry *dentry,
 	init_special_inode(inode, inode->i_mode, rdev);
 	inode->i_op = &f2fs_special_inode_operations;
 
-	f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, true);
 
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);

@@ -539,7 +539,7 @@ static int __f2fs_tmpfile(struct inode *dir, struct dentry *dentry,
 		inode->i_mapping->a_ops = &f2fs_dblock_aops;
 	}
 
-	f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, true);
 
 	f2fs_lock_op(sbi);
 	err = acquire_orphan_inode(sbi);

@@ -642,7 +642,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		if (!new_entry)
 			goto out_whiteout;
 
-		f2fs_balance_fs(sbi);
+		f2fs_balance_fs(sbi, true);
 
 		f2fs_lock_op(sbi);

@@ -675,7 +675,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		update_inode_page(old_inode);
 		update_inode_page(new_inode);
 	} else {
-		f2fs_balance_fs(sbi);
+		f2fs_balance_fs(sbi, true);
 
 		f2fs_lock_op(sbi);

@@ -816,7 +816,7 @@ static int f2fs_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
 			goto out_new_dir;
 	}
 
-	f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, true);
 
 	f2fs_lock_op(sbi);

fs/f2fs/segment.c +4 -2

index a3474ba..c7bbc91 100644
@@ -213,7 +213,7 @@ int commit_inmem_pages(struct inode *inode, bool abort)
 	 * inode becomes free by iget_locked in f2fs_iget.
 	 */
 	if (!abort) {
-		f2fs_balance_fs(sbi);
+		f2fs_balance_fs(sbi, true);
 		f2fs_lock_op(sbi);
 	}

@@ -262,8 +262,10 @@ int commit_inmem_pages(struct inode *inode, bool abort)
  * This function balances dirty node and dentry pages.
  * In addition, it controls garbage collection.
  */
-void f2fs_balance_fs(struct f2fs_sb_info *sbi)
+void f2fs_balance_fs(struct f2fs_sb_info *sbi, bool need)
 {
+	if (!need)
+		return;
 	/*
 	 * We should do GC or end up with checkpoint, if there are so many dirty
 	 * dir/node pages without enough free segments.

fs/f2fs/xattr.c +1 -1

index 862368a..822a8af 100644
@@ -609,7 +609,7 @@ int f2fs_setxattr(struct inode *inode, int index, const char *name,
 	if (ipage)
 		return __f2fs_setxattr(inode, index, name, value,
 						size, ipage, flags);
-	f2fs_balance_fs(sbi);
+	f2fs_balance_fs(sbi, true);
 
 	f2fs_lock_op(sbi);
 	/* protect xattr_ver */

Signature · 1 line

2.6.3