[PATCH 2/2] f2fs: remove alloc_nid_done/fail for performance
From: Jaegeuk Kim
Date: Tue May 07 2013 - 20:57:50 EST
This mechanism revealed two issues: bug and performance.
Now, iput() doesn't guarantee that the inode is freed completely due to the
linked f2fs_drop_inode().
So, in the case of failure on f2fs_new_inode(), we should not add the free nid
again to the list even though iput() is called before then.
Second thing is for performance.
When allocating a free nid, we scan to find a new NID_NEW candidate, and after
allocation, we need to scan again to remove the free nid.
So, this patch resolves the issues by removing this mechanism.
Just do this with a simple basic list control.
Signed-off-by: Jaegeuk Kim <jaegeuk.kim@xxxxxxxxxxx>
---
fs/f2fs/f2fs.h | 2 --
fs/f2fs/namei.c | 15 ------------
fs/f2fs/node.c | 76 +++++++++++++--------------------------------------------
fs/f2fs/node.h | 6 -----
fs/f2fs/xattr.c | 2 --
5 files changed, 17 insertions(+), 84 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 20aab02..2fe2567 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -956,8 +956,6 @@ struct page *get_node_page_ra(struct page *, int);
void sync_inode_page(struct dnode_of_data *);
int sync_node_pages(struct f2fs_sb_info *, nid_t, struct writeback_control *);
bool alloc_nid(struct f2fs_sb_info *, nid_t *);
-void alloc_nid_done(struct f2fs_sb_info *, nid_t);
-void alloc_nid_failed(struct f2fs_sb_info *, nid_t);
void recover_node_page(struct f2fs_sb_info *, struct page *,
struct f2fs_summary *, struct node_info *, block_t);
int recover_inode_page(struct f2fs_sb_info *, struct page *);
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 4aa26e5..077ead1 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -26,7 +26,6 @@ static struct inode *f2fs_new_inode(struct inode *dir, umode_t mode)
struct f2fs_sb_info *sbi = F2FS_SB(sb);
nid_t ino;
struct inode *inode;
- bool nid_free = false;
int err, ilock;
inode = new_inode(sb);
@@ -60,7 +59,6 @@ static struct inode *f2fs_new_inode(struct inode *dir, umode_t mode)
err = insert_inode_locked(inode);
if (err) {
err = -EINVAL;
- nid_free = true;
goto out;
}
trace_f2fs_new_inode(inode, 0);
@@ -73,8 +71,6 @@ out:
fail:
trace_f2fs_new_inode(inode, err);
iput(inode);
- if (nid_free)
- alloc_nid_failed(sbi, ino);
return ERR_PTR(err);
}
@@ -146,8 +142,6 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
if (err)
goto out;
- alloc_nid_done(sbi, ino);
-
if (!sbi->por_doing)
d_instantiate(dentry, inode);
unlock_new_inode(inode);
@@ -156,7 +150,6 @@ out:
clear_nlink(inode);
unlock_new_inode(inode);
iput(inode);
- alloc_nid_failed(sbi, ino);
return err;
}
@@ -287,8 +280,6 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
goto out;
err = page_symlink(inode, symname, symlen);
- alloc_nid_done(sbi, inode->i_ino);
-
d_instantiate(dentry, inode);
unlock_new_inode(inode);
return err;
@@ -296,7 +287,6 @@ out:
clear_nlink(inode);
unlock_new_inode(inode);
iput(inode);
- alloc_nid_failed(sbi, inode->i_ino);
return err;
}
@@ -324,8 +314,6 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
if (err)
goto out_fail;
- alloc_nid_done(sbi, inode->i_ino);
-
d_instantiate(dentry, inode);
unlock_new_inode(inode);
@@ -336,7 +324,6 @@ out_fail:
clear_nlink(inode);
unlock_new_inode(inode);
iput(inode);
- alloc_nid_failed(sbi, inode->i_ino);
return err;
}
@@ -375,7 +362,6 @@ static int f2fs_mknod(struct inode *dir, struct dentry *dentry,
if (err)
goto out;
- alloc_nid_done(sbi, inode->i_ino);
d_instantiate(dentry, inode);
unlock_new_inode(inode);
return 0;
@@ -383,7 +369,6 @@ out:
clear_nlink(inode);
unlock_new_inode(inode);
iput(inode);
- alloc_nid_failed(sbi, inode->i_ino);
return err;
}
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 3df43b4..fa5e8dc 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -432,13 +432,11 @@ int get_dnode_of_data(struct dnode_of_data *dn, pgoff_t index, int mode)
dn->nid = nids[i];
npage[i] = new_node_page(dn, noffset[i]);
if (IS_ERR(npage[i])) {
- alloc_nid_failed(sbi, nids[i]);
err = PTR_ERR(npage[i]);
goto release_pages;
}
set_nid(parent, offset[i - 1], nids[i], i == 1);
- alloc_nid_done(sbi, nids[i]);
done = true;
} else if (mode == LOOKUP_NODE_RA && i == level && level > 1) {
npage[i] = get_node_page_ra(parent, offset[i - 1]);
@@ -1243,10 +1241,18 @@ static struct free_nid *__lookup_free_nid_list(nid_t n, struct list_head *head)
return NULL;
}
-static void __del_from_free_nid_list(struct free_nid *i)
+/*
+ * should be covered by free_nid_list_lock
+ */
+static void __del_from_free_nid_list(struct f2fs_nm_info *nm_i,
+ struct free_nid *i)
{
+ if (!i)
+ return;
+
list_del(&i->list);
kmem_cache_free(free_nid_slab, i);
+ nm_i->fcnt--;
}
static int add_free_nid(struct f2fs_nm_info *nm_i, nid_t nid, bool build)
@@ -1280,7 +1286,6 @@ retry:
goto retry;
}
i->nid = nid;
- i->state = NID_NEW;
spin_lock(&nm_i->free_nid_list_lock);
if (__lookup_free_nid_list(nid, &nm_i->free_nid_list)) {
@@ -1299,10 +1304,7 @@ static void remove_free_nid(struct f2fs_nm_info *nm_i, nid_t nid)
struct free_nid *i;
spin_lock(&nm_i->free_nid_list_lock);
i = __lookup_free_nid_list(nid, &nm_i->free_nid_list);
- if (i && i->state == NID_NEW) {
- __del_from_free_nid_list(i);
- nm_i->fcnt--;
- }
+ __del_from_free_nid_list(nm_i, i);
spin_unlock(&nm_i->free_nid_list_lock);
}
@@ -1382,8 +1384,6 @@ static void build_free_nids(struct f2fs_sb_info *sbi)
bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid)
{
struct f2fs_nm_info *nm_i = NM_I(sbi);
- struct free_nid *i = NULL;
- struct list_head *this;
retry:
if (sbi->total_valid_node_count + 1 >= nm_i->max_nid)
return false;
@@ -1392,17 +1392,13 @@ retry:
/* We should not use stale free nids created by build_free_nids */
if (nm_i->fcnt && !sbi->on_build_free_nids) {
- BUG_ON(list_empty(&nm_i->free_nid_list));
- list_for_each(this, &nm_i->free_nid_list) {
- i = list_entry(this, struct free_nid, list);
- if (i->state == NID_NEW)
- break;
- }
+ struct list_head *flist = &nm_i->free_nid_list;
+ struct free_nid *i;
- BUG_ON(i->state != NID_NEW);
+ BUG_ON(list_empty(flist));
+ i = list_first_entry(flist, struct free_nid, list);
*nid = i->nid;
- i->state = NID_ALLOC;
- nm_i->fcnt--;
+ __del_from_free_nid_list(nm_i, i);
spin_unlock(&nm_i->free_nid_list_lock);
return true;
}
@@ -1417,41 +1413,6 @@ retry:
goto retry;
}
-/*
- * alloc_nid() should be called prior to this function.
- */
-void alloc_nid_done(struct f2fs_sb_info *sbi, nid_t nid)
-{
- struct f2fs_nm_info *nm_i = NM_I(sbi);
- struct free_nid *i;
-
- spin_lock(&nm_i->free_nid_list_lock);
- i = __lookup_free_nid_list(nid, &nm_i->free_nid_list);
- BUG_ON(!i || i->state != NID_ALLOC);
- __del_from_free_nid_list(i);
- spin_unlock(&nm_i->free_nid_list_lock);
-}
-
-/*
- * alloc_nid() should be called prior to this function.
- */
-void alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
-{
- struct f2fs_nm_info *nm_i = NM_I(sbi);
- struct free_nid *i;
-
- spin_lock(&nm_i->free_nid_list_lock);
- i = __lookup_free_nid_list(nid, &nm_i->free_nid_list);
- BUG_ON(!i || i->state != NID_ALLOC);
- if (nm_i->fcnt > 2 * MAX_FREE_NIDS) {
- __del_from_free_nid_list(i);
- } else {
- i->state = NID_NEW;
- nm_i->fcnt++;
- }
- spin_unlock(&nm_i->free_nid_list_lock);
-}
-
void recover_node_page(struct f2fs_sb_info *sbi, struct page *page,
struct f2fs_summary *sum, struct node_info *ni,
block_t new_blkaddr)
@@ -1747,11 +1708,8 @@ void destroy_node_manager(struct f2fs_sb_info *sbi)
/* destroy free nid list */
spin_lock(&nm_i->free_nid_list_lock);
- list_for_each_entry_safe(i, next_i, &nm_i->free_nid_list, list) {
- BUG_ON(i->state == NID_ALLOC);
- __del_from_free_nid_list(i);
- nm_i->fcnt--;
- }
+ list_for_each_entry_safe(i, next_i, &nm_i->free_nid_list, list)
+ __del_from_free_nid_list(nm_i, i);
BUG_ON(nm_i->fcnt);
spin_unlock(&nm_i->free_nid_list_lock);
diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h
index 0a2d72f..2f226a4 100644
--- a/fs/f2fs/node.h
+++ b/fs/f2fs/node.h
@@ -74,15 +74,9 @@ static inline void node_info_from_raw_nat(struct node_info *ni,
/*
* For free nid mangement
*/
-enum nid_state {
- NID_NEW, /* newly added to free nid list */
- NID_ALLOC /* it is allocated */
-};
-
struct free_nid {
struct list_head list; /* for free node id list */
nid_t nid; /* node id */
- int state; /* in use or not: NID_NEW or NID_ALLOC */
};
static inline int next_free_nid(struct f2fs_sb_info *sbi, nid_t *nid)
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index 0b02dce..5675920 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -337,13 +337,11 @@ int f2fs_setxattr(struct inode *inode, int name_index, const char *name,
page = new_node_page(&dn, XATTR_NODE_OFFSET);
if (IS_ERR(page)) {
- alloc_nid_failed(sbi, fi->i_xattr_nid);
fi->i_xattr_nid = 0;
error = PTR_ERR(page);
goto exit;
}
- alloc_nid_done(sbi, fi->i_xattr_nid);
base_addr = page_address(page);
header = XATTR_HDR(base_addr);
header->h_magic = cpu_to_le32(F2FS_XATTR_MAGIC);
--
1.8.1.3.566.gaa39828
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/