2013-05-20 03:33:37

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 01/15] f2fs: remove redundant assignment

We don't need to assign a value redundantly.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/recovery.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 60c8a50..2941987 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -126,7 +126,6 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head)

entry = get_fsync_inode(head, ino_of_node(page));
if (entry) {
- entry->blkaddr = blkaddr;
if (IS_INODE(page) && is_dent_dnode(page))
set_inode_flag(F2FS_I(entry->inode),
FI_INC_LINK);
@@ -150,10 +149,10 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head)
kmem_cache_free(fsync_entry_slab, entry);
goto unlock_out;
}
-
list_add_tail(&entry->list, head);
- entry->blkaddr = blkaddr;
}
+ entry->blkaddr = blkaddr;
+
if (IS_INODE(page)) {
err = recover_inode(entry->inode, page);
if (err == -ENOENT) {
--
1.8.1.3.566.gaa39828


2013-05-20 03:33:45

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 07/15] f2fs: change get_new_data_page to pass a locked node page

This patch is for passing a locked node page to get_dnode_of_data.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/data.c | 12 +++++++-----
fs/f2fs/dir.c | 4 ++--
fs/f2fs/f2fs.h | 2 +-
fs/f2fs/file.c | 2 +-
4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 05fb5c6..af74549 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -280,8 +280,8 @@ repeat:
* Also, caller should grab and release a mutex by calling mutex_lock_op() and
* mutex_unlock_op().
*/
-struct page *get_new_data_page(struct inode *inode, pgoff_t index,
- bool new_i_size)
+struct page *get_new_data_page(struct inode *inode,
+ struct page *npage, pgoff_t index, bool new_i_size)
{
struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
struct address_space *mapping = inode->i_mapping;
@@ -289,18 +289,20 @@ struct page *get_new_data_page(struct inode *inode, pgoff_t index,
struct dnode_of_data dn;
int err;

- set_new_dnode(&dn, inode, NULL, NULL, 0);
+ set_new_dnode(&dn, inode, npage, npage, 0);
err = get_dnode_of_data(&dn, index, ALLOC_NODE);
if (err)
return ERR_PTR(err);

if (dn.data_blkaddr == NULL_ADDR) {
if (reserve_new_block(&dn)) {
- f2fs_put_dnode(&dn);
+ if (!npage)
+ f2fs_put_dnode(&dn);
return ERR_PTR(-ENOSPC);
}
}
- f2fs_put_dnode(&dn);
+ if (!npage)
+ f2fs_put_dnode(&dn);
repeat:
page = grab_cache_page(mapping, index);
if (!page)
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 1ac6b93..7db6e58 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -287,7 +287,7 @@ static int make_empty_dir(struct inode *inode, struct inode *parent)
struct f2fs_dir_entry *de;
void *kaddr;

- dentry_page = get_new_data_page(inode, 0, true);
+ dentry_page = get_new_data_page(inode, NULL, 0, true);
if (IS_ERR(dentry_page))
return PTR_ERR(dentry_page);

@@ -448,7 +448,7 @@ start:
bidx = dir_block_index(level, (le32_to_cpu(dentry_hash) % nbucket));

for (block = bidx; block <= (bidx + nblock - 1); block++) {
- dentry_page = get_new_data_page(dir, block, true);
+ dentry_page = get_new_data_page(dir, NULL, block, true);
if (IS_ERR(dentry_page))
return PTR_ERR(dentry_page);

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index ef6cac8..cbae2b6 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1027,7 +1027,7 @@ int reserve_new_block(struct dnode_of_data *);
void update_extent_cache(block_t, struct dnode_of_data *);
struct page *find_data_page(struct inode *, pgoff_t, bool);
struct page *get_lock_data_page(struct inode *, pgoff_t);
-struct page *get_new_data_page(struct inode *, pgoff_t, bool);
+struct page *get_new_data_page(struct inode *, struct page *, pgoff_t, bool);
int f2fs_readpage(struct f2fs_sb_info *, struct page *, block_t, int);
int do_write_data_page(struct page *);

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 1cae864..b8e34db 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -387,7 +387,7 @@ static void fill_zero(struct inode *inode, pgoff_t index,
f2fs_balance_fs(sbi);

ilock = mutex_lock_op(sbi);
- page = get_new_data_page(inode, index, false);
+ page = get_new_data_page(inode, NULL, index, false);
mutex_unlock_op(sbi, ilock);

if (!IS_ERR(page)) {
--
1.8.1.3.566.gaa39828

2013-05-20 03:33:57

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 09/15] f2fs: add debug msgs in the recovery routine

This patch adds some trivial debugging messages in the recovery process.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/node.c | 1 -
fs/f2fs/recovery.c | 44 +++++++++++++++++++++++++-------------------
2 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index b41482d..5a59780 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1495,7 +1495,6 @@ int recover_inode_page(struct f2fs_sb_info *sbi, struct page *page)
WARN_ON(1);
set_node_addr(sbi, &new_ni, NEW_ADDR);
inc_valid_inode_count(sbi);
-
f2fs_put_page(ipage, 1);
return 0;
}
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index c573944..774efdb 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -49,9 +49,6 @@ static int recover_dentry(struct page *ipage, struct inode *inode)
struct inode *dir;
int err = 0;

- if (!is_dent_dnode(ipage))
- goto out;
-
dir = check_dirty_dir_inode(F2FS_SB(inode->i_sb), pino);
if (!dir) {
dir = f2fs_iget(inode->i_sb, pino);
@@ -73,6 +70,9 @@ static int recover_dentry(struct page *ipage, struct inode *inode)
err = __f2fs_add_link(dir, &name, inode);
}
out:
+ f2fs_msg(inode->i_sb, KERN_NOTICE, "recover_inode and its dentry: "
+ "ino = %x, name = %s, dir = %lx, err = %d",
+ ino_of_node(ipage), raw_inode->i_name, dir->i_ino, err);
kunmap(ipage);
return err;
}
@@ -83,6 +83,9 @@ static int recover_inode(struct inode *inode, struct page *node_page)
struct f2fs_node *raw_node = (struct f2fs_node *)kaddr;
struct f2fs_inode *raw_inode = &(raw_node->i);

+ if (!IS_INODE(node_page))
+ return 0;
+
inode->i_mode = le16_to_cpu(raw_inode->i_mode);
i_size_write(inode, le64_to_cpu(raw_inode->i_size));
inode->i_atime.tv_sec = le64_to_cpu(raw_inode->i_mtime);
@@ -92,7 +95,12 @@ static int recover_inode(struct inode *inode, struct page *node_page)
inode->i_ctime.tv_nsec = le32_to_cpu(raw_inode->i_ctime_nsec);
inode->i_mtime.tv_nsec = le32_to_cpu(raw_inode->i_mtime_nsec);

- return recover_dentry(node_page, inode);
+ if (is_dent_dnode(node_page))
+ return recover_dentry(node_page, inode);
+
+ f2fs_msg(inode->i_sb, KERN_NOTICE, "recover_inode: ino = %x, name = %s",
+ ino_of_node(node_page), raw_inode->i_name);
+ return 0;
}

static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head)
@@ -123,7 +131,7 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head)
lock_page(page);

if (cp_ver != cpver_of_node(page))
- goto unlock_out;
+ break;

if (!is_fsync_dnode(page))
goto next;
@@ -133,40 +141,33 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head)
if (IS_INODE(page) && is_dent_dnode(page)) {
err = recover_inode_page(sbi, page);
if (err)
- goto unlock_out;
+ break;
}

/* add this fsync inode to the list */
entry = kmem_cache_alloc(fsync_entry_slab, GFP_NOFS);
if (!entry) {
err = -ENOMEM;
- goto unlock_out;
+ break;
}

entry->inode = f2fs_iget(sbi->sb, ino_of_node(page));
if (IS_ERR(entry->inode)) {
err = PTR_ERR(entry->inode);
kmem_cache_free(fsync_entry_slab, entry);
- goto unlock_out;
+ break;
}
list_add_tail(&entry->list, head);
}
entry->blkaddr = blkaddr;

- if (IS_INODE(page)) {
- err = recover_inode(entry->inode, page);
- if (err == -ENOENT) {
- goto next;
- } else if (err) {
- err = -EINVAL;
- goto unlock_out;
- }
- }
+ err = recover_inode(entry->inode, page);
+ if (err && err != -ENOENT)
+ break;
next:
/* check next segment */
blkaddr = next_blkaddr_of_node(page);
}
-unlock_out:
unlock_page(page);
out:
__free_pages(page, 0);
@@ -244,7 +245,7 @@ static int do_recover_data(struct f2fs_sb_info *sbi, struct inode *inode,
struct dnode_of_data dn;
struct f2fs_summary sum;
struct node_info ni;
- int err = 0;
+ int err = 0, recovered = 0;
int ilock;

start = start_bidx_of_node(ofs_of_node(page));
@@ -289,6 +290,7 @@ static int do_recover_data(struct f2fs_sb_info *sbi, struct inode *inode,
/* write dummy data page */
recover_data_page(sbi, NULL, &sum, src, dest);
update_extent_cache(dest, &dn);
+ recovered++;
}
dn.ofs_in_node++;
}
@@ -306,6 +308,10 @@ static int do_recover_data(struct f2fs_sb_info *sbi, struct inode *inode,
recover_node_page(sbi, dn.node_page, &sum, &ni, blkaddr);
f2fs_put_dnode(&dn);
mutex_unlock_op(sbi, ilock);
+
+ f2fs_msg(sbi->sb, KERN_NOTICE, "recover_data: ino = %lx, "
+ "recovered_data = %d blocks",
+ inode->i_ino, recovered);
return 0;
}

--
1.8.1.3.566.gaa39828

2013-05-20 03:33:43

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 04/15] f2fs: fix BUG_ON during f2fs_evict_inode(dir)

During the dentry recovery routine, recover_inode() triggers __f2fs_add_link
with its directory inode.

In the following scenario, a bug is captured.
1. dir = f2fs_iget(pino)
2. __f2fs_add_link(dir, name)
3. iput(dir)
-> f2fs_evict_inode() faces with BUG_ON(atomic_read(fi->dirty_dents))

Kernel BUG at ffffffffa01c0676 [verbose debug info unavailable]
[<ffffffffa01c0676>] f2fs_evict_inode+0x276/0x300 [f2fs]
Call Trace:
[<ffffffff8118ea00>] evict+0xb0/0x1b0
[<ffffffff8118f1c5>] iput+0x105/0x190
[<ffffffffa01d2dac>] recover_fsync_data+0x3bc/0x1070 [f2fs]
[<ffffffff81692e8a>] ? io_schedule+0xaa/0xd0
[<ffffffff81690acb>] ? __wait_on_bit_lock+0x7b/0xc0
[<ffffffff8111a0e7>] ? __lock_page+0x67/0x70
[<ffffffff81165e21>] ? kmem_cache_alloc+0x31/0x140
[<ffffffff8118a502>] ? __d_instantiate+0x92/0xf0
[<ffffffff812a949b>] ? security_d_instantiate+0x1b/0x30
[<ffffffff8118a5b4>] ? d_instantiate+0x54/0x70

This means that we should flush all the dentry pages between iget and iput().
But, during the recovery routine, it is unallowed due to consistency, so we
have to wait the whole recovery process.
And then, write_checkpoint flushes all the dirty dentry blocks, and nicely we
can put the stale dir inodes from the dirty_dir_inode_list.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/checkpoint.c | 23 +++++++++++++++++++++++
fs/f2fs/f2fs.h | 2 ++
fs/f2fs/recovery.c | 14 +++++++++-----
3 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index b1de01d..3d11449 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -514,6 +514,29 @@ void remove_dirty_dir_inode(struct inode *inode)
}
out:
spin_unlock(&sbi->dir_inode_lock);
+
+ /* Only from the recovery routine */
+ if (is_inode_flag_set(F2FS_I(inode), FI_DELAY_IPUT))
+ iput(inode);
+}
+
+struct inode *check_dirty_dir_inode(struct f2fs_sb_info *sbi, nid_t ino)
+{
+ struct list_head *head = &sbi->dir_inode_list;
+ struct list_head *this;
+ struct inode *inode = NULL;
+
+ spin_lock(&sbi->dir_inode_lock);
+ list_for_each(this, head) {
+ struct dir_inode_entry *entry;
+ entry = list_entry(this, struct dir_inode_entry, list);
+ if (entry->inode->i_ino == ino) {
+ inode = entry->inode;
+ break;
+ }
+ }
+ spin_unlock(&sbi->dir_inode_lock);
+ return inode;
}

void sync_dirty_dir_inodes(struct f2fs_sb_info *sbi)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 20aab02..ef6cac8 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -846,6 +846,7 @@ enum {
FI_INC_LINK, /* need to increment i_nlink */
FI_ACL_MODE, /* indicate acl mode */
FI_NO_ALLOC, /* should not allocate any blocks */
+ FI_DELAY_IPUT, /* used for the recovery */
};

static inline void set_inode_flag(struct f2fs_inode_info *fi, int flag)
@@ -1012,6 +1013,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *);
int get_valid_checkpoint(struct f2fs_sb_info *);
void set_dirty_dir_page(struct inode *, struct page *);
void remove_dirty_dir_inode(struct inode *);
+struct inode *check_dirty_dir_inode(struct f2fs_sb_info *, nid_t);
void sync_dirty_dir_inodes(struct f2fs_sb_info *);
void write_checkpoint(struct f2fs_sb_info *, bool);
void init_orphan_info(struct f2fs_sb_info *);
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index f77aedd..c573944 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -42,6 +42,7 @@ static int recover_dentry(struct page *ipage, struct inode *inode)
{
struct f2fs_node *raw_node = (struct f2fs_node *)kmap(ipage);
struct f2fs_inode *raw_inode = &(raw_node->i);
+ nid_t pino = le32_to_cpu(raw_inode->i_pino);
struct qstr name;
struct f2fs_dir_entry *de;
struct page *page;
@@ -51,10 +52,14 @@ static int recover_dentry(struct page *ipage, struct inode *inode)
if (!is_dent_dnode(ipage))
goto out;

- dir = f2fs_iget(inode->i_sb, le32_to_cpu(raw_inode->i_pino));
- if (IS_ERR(dir)) {
- err = PTR_ERR(dir);
- goto out;
+ dir = check_dirty_dir_inode(F2FS_SB(inode->i_sb), pino);
+ if (!dir) {
+ dir = f2fs_iget(inode->i_sb, pino);
+ if (IS_ERR(dir)) {
+ err = PTR_ERR(dir);
+ goto out;
+ }
+ set_inode_flag(F2FS_I(dir), FI_DELAY_IPUT);
}

name.len = le32_to_cpu(raw_inode->i_namelen);
@@ -67,7 +72,6 @@ static int recover_dentry(struct page *ipage, struct inode *inode)
} else {
err = __f2fs_add_link(dir, &name, inode);
}
- iput(dir);
out:
kunmap(ipage);
return err;
--
1.8.1.3.566.gaa39828

2013-05-20 03:34:23

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 08/15] f2fs: update inode page after creation

I found a bug when testing power-off-recovery as follows.

[Bug Scenario]
1. create a file
2. fsync the file
3. reboot w/o any sync
4. try to recover the file
- found its fsync mark
- found its dentry mark
: try to recover its dentry
- get its file name
- get its parent inode number
: here we got zero value

The reason why we get the wrong parent inode number is that we didn't
synchronize the inode page with its newly created inode information perfectly.

Especially, previous f2fs stores fi->i_pino and writes it to the cached
node page in a wrong order, which incurs the zero-valued i_pino during the
recovery.

So, this patch modifies the creation flow to fix the synchronization order of
inode page with its inode.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/data.c | 1 +
fs/f2fs/dir.c | 85 +++++++++++++++++++++++++++++++---------------------------
fs/f2fs/f2fs.h | 3 +--
fs/f2fs/node.c | 12 +++------
4 files changed, 51 insertions(+), 50 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index af74549..c320f7f 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -279,6 +279,7 @@ repeat:
*
* Also, caller should grab and release a mutex by calling mutex_lock_op() and
* mutex_unlock_op().
+ * Note that, npage is set only by make_empty_dir.
*/
struct page *get_new_data_page(struct inode *inode,
struct page *npage, pgoff_t index, bool new_i_size)
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 7db6e58..fc1dacf 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -264,15 +264,10 @@ void f2fs_set_link(struct inode *dir, struct f2fs_dir_entry *de,
f2fs_put_page(page, 1);
}

-void init_dent_inode(const struct qstr *name, struct page *ipage)
+static void init_dent_inode(const struct qstr *name, struct page *ipage)
{
struct f2fs_node *rn;

- if (IS_ERR(ipage))
- return;
-
- wait_on_page_writeback(ipage);
-
/* copy name info. to this inode page */
rn = (struct f2fs_node *)page_address(ipage);
rn->i.i_namelen = cpu_to_le32(name->len);
@@ -280,14 +275,15 @@ void init_dent_inode(const struct qstr *name, struct page *ipage)
set_page_dirty(ipage);
}

-static int make_empty_dir(struct inode *inode, struct inode *parent)
+static int make_empty_dir(struct inode *inode,
+ struct inode *parent, struct page *page)
{
struct page *dentry_page;
struct f2fs_dentry_block *dentry_blk;
struct f2fs_dir_entry *de;
void *kaddr;

- dentry_page = get_new_data_page(inode, NULL, 0, true);
+ dentry_page = get_new_data_page(inode, page, 0, true);
if (IS_ERR(dentry_page))
return PTR_ERR(dentry_page);

@@ -317,42 +313,47 @@ static int make_empty_dir(struct inode *inode, struct inode *parent)
return 0;
}

-static int init_inode_metadata(struct inode *inode,
+static struct page *init_inode_metadata(struct inode *inode,
struct inode *dir, const struct qstr *name)
{
+ struct page *page;
+ int err;
+
if (is_inode_flag_set(F2FS_I(inode), FI_NEW_INODE)) {
- int err;
- err = new_inode_page(inode, name);
- if (err)
- return err;
+ page = new_inode_page(inode, name);
+ if (IS_ERR(page))
+ return page;

if (S_ISDIR(inode->i_mode)) {
- err = make_empty_dir(inode, dir);
- if (err) {
- remove_inode_page(inode);
- return err;
- }
+ err = make_empty_dir(inode, dir, page);
+ if (err)
+ goto error;
}

err = f2fs_init_acl(inode, dir);
- if (err) {
- remove_inode_page(inode);
- return err;
- }
+ if (err)
+ goto error;
+
+ wait_on_page_writeback(page);
} else {
- struct page *ipage;
- ipage = get_node_page(F2FS_SB(dir->i_sb), inode->i_ino);
- if (IS_ERR(ipage))
- return PTR_ERR(ipage);
- set_cold_node(inode, ipage);
- init_dent_inode(name, ipage);
- f2fs_put_page(ipage, 1);
+ page = get_node_page(F2FS_SB(dir->i_sb), inode->i_ino);
+ if (IS_ERR(page))
+ return page;
+
+ wait_on_page_writeback(page);
+ set_cold_node(inode, page);
}
- if (is_inode_flag_set(F2FS_I(inode), FI_INC_LINK)) {
+
+ init_dent_inode(name, page);
+
+ if (is_inode_flag_set(F2FS_I(inode), FI_INC_LINK))
inc_nlink(inode);
- update_inode_page(inode);
- }
- return 0;
+ return page;
+
+error:
+ f2fs_put_page(page, 1);
+ remove_inode_page(inode);
+ return ERR_PTR(err);
}

static void update_parent_metadata(struct inode *dir, struct inode *inode,
@@ -423,6 +424,7 @@ int __f2fs_add_link(struct inode *dir, const struct qstr *name, struct inode *in
struct page *dentry_page = NULL;
struct f2fs_dentry_block *dentry_blk = NULL;
int slots = GET_DENTRY_SLOTS(namelen);
+ struct page *page;
int err = 0;
int i;

@@ -465,12 +467,13 @@ start:
++level;
goto start;
add_dentry:
- err = init_inode_metadata(inode, dir, name);
- if (err)
- goto fail;
-
wait_on_page_writeback(dentry_page);

+ page = init_inode_metadata(inode, dir, name);
+ if (IS_ERR(page)) {
+ err = PTR_ERR(page);
+ goto fail;
+ }
de = &dentry_blk->dentry[bit_pos];
de->hash_code = dentry_hash;
de->name_len = cpu_to_le16(namelen);
@@ -481,10 +484,12 @@ add_dentry:
test_and_set_bit_le(bit_pos + i, &dentry_blk->dentry_bitmap);
set_page_dirty(dentry_page);

- update_parent_metadata(dir, inode, current_depth);
-
- /* update parent inode number before releasing dentry page */
+ /* we don't need to mark_inode_dirty now */
F2FS_I(inode)->i_pino = dir->i_ino;
+ update_inode(inode, page);
+ f2fs_put_page(page, 1);
+
+ update_parent_metadata(dir, inode, current_depth);
fail:
kunmap(dentry_page);
f2fs_put_page(dentry_page, 1);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index cbae2b6..9360a03 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -914,7 +914,6 @@ struct f2fs_dir_entry *f2fs_parent_dir(struct inode *, struct page **);
ino_t f2fs_inode_by_name(struct inode *, struct qstr *);
void f2fs_set_link(struct inode *, struct f2fs_dir_entry *,
struct page *, struct inode *);
-void init_dent_inode(const struct qstr *, struct page *);
int __f2fs_add_link(struct inode *, const struct qstr *, struct inode *);
void f2fs_delete_entry(struct f2fs_dir_entry *, struct page *, struct inode *);
int f2fs_make_empty(struct inode *, struct inode *);
@@ -949,7 +948,7 @@ void get_node_info(struct f2fs_sb_info *, nid_t, struct node_info *);
int get_dnode_of_data(struct dnode_of_data *, pgoff_t, int);
int truncate_inode_blocks(struct inode *, pgoff_t);
int remove_inode_page(struct inode *);
-int new_inode_page(struct inode *, const struct qstr *);
+struct page *new_inode_page(struct inode *, const struct qstr *);
struct page *new_node_page(struct dnode_of_data *, unsigned int);
void ra_node_page(struct f2fs_sb_info *, nid_t);
struct page *get_node_page(struct f2fs_sb_info *, pgoff_t);
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index f63f0a4..b41482d 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -806,19 +806,15 @@ int remove_inode_page(struct inode *inode)
return 0;
}

-int new_inode_page(struct inode *inode, const struct qstr *name)
+struct page *new_inode_page(struct inode *inode, const struct qstr *name)
{
- struct page *page;
struct dnode_of_data dn;

/* allocate inode page for new inode */
set_new_dnode(&dn, inode, NULL, NULL, inode->i_ino);
- page = new_node_page(&dn, 0);
- init_dent_inode(name, page);
- if (IS_ERR(page))
- return PTR_ERR(page);
- f2fs_put_page(page, 1);
- return 0;
+
+ /* caller should f2fs_put_page(page, 1); */
+ return new_node_page(&dn, 0);
}

struct page *new_node_page(struct dnode_of_data *dn, unsigned int ofs)
--
1.8.1.3.566.gaa39828

2013-05-20 03:34:47

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 06/15] f2fs: skip get_node_page if locked node page is passed

If get_dnode_of_data gets a locked node page, let's skip redundant
get_node_page calls.
This is for the futher enhancement.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/node.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 9641534..f63f0a4 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -408,10 +408,13 @@ int get_dnode_of_data(struct dnode_of_data *dn, pgoff_t index, int mode)
level = get_node_path(index, offset, noffset);

nids[0] = dn->inode->i_ino;
- npage[0] = get_node_page(sbi, nids[0]);
- if (IS_ERR(npage[0]))
- return PTR_ERR(npage[0]);
+ npage[0] = dn->inode_page;

+ if (!npage[0]) {
+ npage[0] = get_node_page(sbi, nids[0]);
+ if (IS_ERR(npage[0]))
+ return PTR_ERR(npage[0]);
+ }
parent = npage[0];
if (level != 0)
nids[1] = get_nid(parent, offset[0], true);
--
1.8.1.3.566.gaa39828

2013-05-20 03:33:40

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 03/15] f2fs: fix por_doing variable coverage

The reason of using sbi->por_doing is to alleviate data writes during the
recovery.
The find_fsync_dnodes() produces some dirty dentry pages, so we should
cover it too with sbi->por_doing.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/recovery.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 993b601..f77aedd 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -377,6 +377,7 @@ int recover_fsync_data(struct f2fs_sb_info *sbi)
INIT_LIST_HEAD(&inode_list);

/* step #1: find fsynced inode numbers */
+ sbi->por_doing = 1;
err = find_fsync_dnodes(sbi, &inode_list);
if (err)
goto out;
@@ -385,13 +386,12 @@ int recover_fsync_data(struct f2fs_sb_info *sbi)
goto out;

/* step #2: recover data */
- sbi->por_doing = 1;
err = recover_data(sbi, &inode_list, CURSEG_WARM_NODE);
- sbi->por_doing = 0;
BUG_ON(!list_empty(&inode_list));
out:
destroy_fsync_dnodes(sbi, &inode_list);
kmem_cache_destroy(fsync_entry_slab);
+ sbi->por_doing = 0;
write_checkpoint(sbi, false);
return err;
}
--
1.8.1.3.566.gaa39828

2013-05-20 03:35:17

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 05/15] f2fs: remove unnecessary por_doing check

This por_doing check is totally not related to the recovery process.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/namei.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 47abc97..729b285 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -149,8 +149,7 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t mode,

alloc_nid_done(sbi, ino);

- if (!sbi->por_doing)
- d_instantiate(dentry, inode);
+ d_instantiate(dentry, inode);
unlock_new_inode(inode);
return 0;
out:
--
1.8.1.3.566.gaa39828

2013-05-20 03:35:55

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 02/15] f2fs: remove unnecessary flag set

If an inode is recovered with its dentry, it will not invoke __f2fs_add_link,
since the recovery routine checks its dentry before calling __f2fs_add_link.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/recovery.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 2941987..993b601 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -125,11 +125,7 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head)
goto next;

entry = get_fsync_inode(head, ino_of_node(page));
- if (entry) {
- if (IS_INODE(page) && is_dent_dnode(page))
- set_inode_flag(F2FS_I(entry->inode),
- FI_INC_LINK);
- } else {
+ if (!entry) {
if (IS_INODE(page) && is_dent_dnode(page)) {
err = recover_inode_page(sbi, page);
if (err)
--
1.8.1.3.566.gaa39828

2013-05-20 07:32:41

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH 02/15] f2fs: remove unnecessary flag set

Sorry for the noise.
This patch should be ignored.
Thanks,

2013-05-20 (월), 12:32 +0900, Jaegeuk Kim:
> If an inode is recovered with its dentry, it will not invoke __f2fs_add_link,
> since the recovery routine checks its dentry before calling __f2fs_add_link.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> fs/f2fs/recovery.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> index 2941987..993b601 100644
> --- a/fs/f2fs/recovery.c
> +++ b/fs/f2fs/recovery.c
> @@ -125,11 +125,7 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head)
> goto next;
>
> entry = get_fsync_inode(head, ino_of_node(page));
> - if (entry) {
> - if (IS_INODE(page) && is_dent_dnode(page))
> - set_inode_flag(F2FS_I(entry->inode),
> - FI_INC_LINK);
> - } else {
> + if (!entry) {
> if (IS_INODE(page) && is_dent_dnode(page)) {
> err = recover_inode_page(sbi, page);
> if (err)

--
Jaegeuk Kim
Samsung


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part