[PATCH 1/4] f2fs: propagate error given by f2fs_find_entry

[PATCH 1/4] f2fs: propagate error given by f2fs_find_entry

[Jaegeuk Kim]

If we get ENOMEM or EIO in f2fs_find_entry, we should stop right away.
Otherwise, for example, we can get duplicate directory entry by ->chash and
->clevel.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/dir.c    | 23 ++++++++++++++++-------
 fs/f2fs/inline.c |  4 +++-
 fs/f2fs/namei.c  |  5 +++++
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 24d1308..ae37543 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -185,8 +185,13 @@ static struct f2fs_dir_entry *find_in_level(struct inode *dir,
 		/* no need to allocate new dentry pages to all the indices */
 		dentry_page = find_data_page(dir, bidx);
 		if (IS_ERR(dentry_page)) {
-			room = true;
-			continue;
+			if (PTR_ERR(dentry_page) == -ENOENT) {
+				room = true;
+				continue;
+			} else {
+				*res_page = dentry_page;
+				break;
+			}
 		}
 
 		de = find_in_block(dentry_page, fname, namehash, &max_slots,
@@ -223,19 +228,22 @@ struct f2fs_dir_entry *f2fs_find_entry(struct inode *dir,
 	struct fscrypt_name fname;
 	int err;
 
-	*res_page = NULL;
-
 	err = fscrypt_setup_filename(dir, child, 1, &fname);
-	if (err)
+	if (err) {
+		*res_page = ERR_PTR(-ENOMEM);
 		return NULL;
+	}
 
 	if (f2fs_has_inline_dentry(dir)) {
+		*res_page = NULL;
 		de = find_in_inline_dir(dir, &fname, res_page);
 		goto out;
 	}
 
-	if (npages == 0)
+	if (npages == 0) {
+		*res_page = NULL;
 		goto out;
+	}
 
 	max_depth = F2FS_I(dir)->i_current_depth;
 	if (unlikely(max_depth > MAX_DIR_HASH_DEPTH)) {
@@ -247,8 +255,9 @@ struct f2fs_dir_entry *f2fs_find_entry(struct inode *dir,
 	}
 
 	for (level = 0; level < max_depth; level++) {
+		*res_page = NULL;
 		de = find_in_level(dir, level, &fname, res_page);
-		if (de)
+		if (de || IS_ERR(*res_page))
 			break;
 	}
 out:
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 77c9c24..1eb3043 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -286,8 +286,10 @@ struct f2fs_dir_entry *find_in_inline_dir(struct inode *dir,
 	f2fs_hash_t namehash;
 
 	ipage = get_node_page(sbi, dir->i_ino);
-	if (IS_ERR(ipage))
+	if (IS_ERR(ipage)) {
+		*res_page = ipage;
 		return NULL;
+	}
 
 	namehash = f2fs_dentry_hash(&name);
 
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 496f4e3..3f6119e 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -232,6 +232,9 @@ static int __recover_dot_dentries(struct inode *dir, nid_t pino)
 	if (de) {
 		f2fs_dentry_kunmap(dir, page);
 		f2fs_put_page(page, 0);
+	} else if (IS_ERR(page)) {
+		err = PTR_ERR(page);
+		goto out;
 	} else {
 		err = __f2fs_add_link(dir, &dot, NULL, dir->i_ino, S_IFDIR);
 		if (err)
@@ -242,6 +245,8 @@ static int __recover_dot_dentries(struct inode *dir, nid_t pino)
 	if (de) {
 		f2fs_dentry_kunmap(dir, page);
 		f2fs_put_page(page, 0);
+	} else if (IS_ERR(page)) {
+		err = PTR_ERR(page);
 	} else {
 		err = __f2fs_add_link(dir, &dotdot, NULL, pino, S_IFDIR);
 	}
-- 
2.6.3

[PATCH 2/4] f2fs: inject to produce some orphan inodes

[Jaegeuk Kim]

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/f2fs.h  | 3 +++
 fs/f2fs/inode.c | 5 +++++
 fs/f2fs/super.c | 1 +
 3 files changed, 9 insertions(+)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 5daba19..c4697b7 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -45,6 +45,7 @@ enum {
 	FAULT_ORPHAN,
 	FAULT_BLOCK,
 	FAULT_DIR_DEPTH,
+	FAULT_EVICT_INODE,
 	FAULT_MAX,
 };
 
@@ -74,6 +75,8 @@ static inline bool time_to_inject(int type)
 		return false;
 	else if (type == FAULT_DIR_DEPTH && !IS_FAULT_SET(type))
 		return false;
+	else if (type == FAULT_EVICT_INODE && !IS_FAULT_SET(type))
+		return false;
 
 	atomic_inc(&f2fs_fault.inject_ops);
 	if (atomic_read(&f2fs_fault.inject_ops) >= f2fs_fault.inject_rate) {
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index bdd814d..11cb60a 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -345,6 +345,11 @@ void f2fs_evict_inode(struct inode *inode)
 	if (inode->i_nlink || is_bad_inode(inode))
 		goto no_delete;
 
+#ifdef CONFIG_F2FS_FAULT_INJECTION
+	if (time_to_inject(FAULT_EVICT_INODE))
+		goto no_delete;
+#endif
+
 	sb_start_intwrite(inode->i_sb);
 	set_inode_flag(inode, FI_NO_ALLOC);
 	i_size_write(inode, 0);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index a5b5739..27f76819e 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -49,6 +49,7 @@ char *fault_name[FAULT_MAX] = {
 	[FAULT_ORPHAN]		= "orphan",
 	[FAULT_BLOCK]		= "no more block",
 	[FAULT_DIR_DEPTH]	= "too big dir depth",
+	[FAULT_EVICT_INODE]	= "evict_inode fail",
 };
 
 static void f2fs_build_fault_attr(unsigned int rate)
-- 
2.6.3

[PATCH 3/4] f2fs: do not skip writing data pages

[Jaegeuk Kim]

For data pages, let's try to flush as much as possible in background.

On /dev/pmem0,

1. dd if=/dev/zero of=/mnt/test/testfile bs=1M count=2048 conv=fsync
 Before : 800 MB/s
 After  : 1.1 GB/s

2. dd if=/dev/zero of=/mnt/test/testfile bs=1M count=2048
 Before : 1.3 GB/s
 After  : 2.2 GB/s

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c    | 11 +++++------
 fs/f2fs/segment.h |  4 +---
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 7132b02..85ceb2b 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1444,7 +1444,6 @@ static int f2fs_write_data_pages(struct address_space *mapping,
 	struct inode *inode = mapping->host;
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	int ret;
-	long diff;
 
 	/* deal with chardevs and other special file */
 	if (!mapping->a_ops->writepage)
@@ -1469,14 +1468,14 @@ static int f2fs_write_data_pages(struct address_space *mapping,
 
 	trace_f2fs_writepages(mapping->host, wbc, DATA);
 
-	diff = nr_pages_to_write(sbi, DATA, wbc);
-
 	ret = f2fs_write_cache_pages(mapping, wbc, __f2fs_writepage, mapping);
-	f2fs_submit_merged_bio_cond(sbi, inode, NULL, 0, DATA, WRITE);
+	/*
+	 * if some pages were truncated, we cannot guarantee its mapping->host
+	 * to detect pending bios.
+	 */
+	f2fs_submit_merged_bio(sbi, DATA, WRITE);
 
 	remove_dirty_inode(inode);
-
-	wbc->nr_to_write = max((long)0, wbc->nr_to_write - diff);
 	return ret;
 
 skip_write:
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 5d016a1..890bb28d 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -728,9 +728,7 @@ static inline long nr_pages_to_write(struct f2fs_sb_info *sbi, int type,
 
 	nr_to_write = wbc->nr_to_write;
 
-	if (type == DATA)
-		desired = 4096;
-	else if (type == NODE)
+	if (type == NODE)
 		desired = 3 * max_hw_blocks(sbi);
 	else
 		desired = MAX_BIO_BLOCKS(sbi);
-- 
2.6.3

[PATCH 4/4] f2fs: remove two steps to flush dirty data pages

[Jaegeuk Kim]

If there is no cold page, we don't need to do a loop to flush dirty
data pages.

On /dev/pmem0,

1. dd if=/dev/zero of=/mnt/test/testfile bs=1M count=2048 conv=fsync
 Before : 1.1 GB/s
 After  : 1.2 GB/s

2. dd if=/dev/zero of=/mnt/test/testfile bs=1M count=2048
 Before : 2.2 GB/s
 After  : 2.3 GB/s

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 85ceb2b..5dcd8db 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1326,10 +1326,9 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
 	int cycled;
 	int range_whole = 0;
 	int tag;
-	int step = 0;
 
 	pagevec_init(&pvec, 0);
-next:
+
 	if (wbc->range_cyclic) {
 		writeback_index = mapping->writeback_index; /* prev offset */
 		index = writeback_index;
@@ -1384,9 +1383,6 @@ continue_unlock:
 				goto continue_unlock;
 			}
 
-			if (step == is_cold_data(page))
-				goto continue_unlock;
-
 			if (PageWriteback(page)) {
 				if (wbc->sync_mode != WB_SYNC_NONE)
 					f2fs_wait_on_page_writeback(page,
@@ -1421,11 +1417,6 @@ continue_unlock:
 		cond_resched();
 	}
 
-	if (step < 1) {
-		step++;
-		goto next;
-	}
-
 	if (!cycled && !done) {
 		cycled = 1;
 		index = 0;
-- 
2.6.3

Re: [f2fs-dev] [PATCH 1/4] f2fs: propagate error given by f2fs_find_entry

[He YunLei]

On 2016/5/27 8:25, Jaegeuk Kim wrote:
> If we get ENOMEM or EIO in f2fs_find_entry, we should stop right away.
> Otherwise, for example, we can get duplicate directory entry by ->chash and
> ->clevel.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>   fs/f2fs/dir.c    | 23 ++++++++++++++++-------
>   fs/f2fs/inline.c |  4 +++-
>   fs/f2fs/namei.c  |  5 +++++
>   3 files changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index 24d1308..ae37543 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -185,8 +185,13 @@ static struct f2fs_dir_entry *find_in_level(struct inode *dir,
>   		/* no need to allocate new dentry pages to all the indices */
>   		dentry_page = find_data_page(dir, bidx);
>   		if (IS_ERR(dentry_page)) {
> -			room = true;
> -			continue;
> +			if (PTR_ERR(dentry_page) == -ENOENT) {
> +				room = true;
> +				continue;
> +			} else {
> +				*res_page = dentry_page;
> +				break;
> +			}
>   		}
>
>   		de = find_in_block(dentry_page, fname, namehash, &max_slots,
> @@ -223,19 +228,22 @@ struct f2fs_dir_entry *f2fs_find_entry(struct inode *dir,
>   	struct fscrypt_name fname;
>   	int err;
>
> -	*res_page = NULL;
> -
>   	err = fscrypt_setup_filename(dir, child, 1, &fname);
> -	if (err)
> +	if (err) {
> +		*res_page = ERR_PTR(-ENOMEM);
>   		return NULL;
> +	}
>
>   	if (f2fs_has_inline_dentry(dir)) {
> +		*res_page = NULL;
>   		de = find_in_inline_dir(dir, &fname, res_page);
>   		goto out;
>   	}
>
> -	if (npages == 0)
> +	if (npages == 0) {
> +		*res_page = NULL;
>   		goto out;
> +	}
>
>   	max_depth = F2FS_I(dir)->i_current_depth;
>   	if (unlikely(max_depth > MAX_DIR_HASH_DEPTH)) {
> @@ -247,8 +255,9 @@ struct f2fs_dir_entry *f2fs_find_entry(struct inode *dir,
>   	}
>
>   	for (level = 0; level < max_depth; level++) {
> +		*res_page = NULL;
>   		de = find_in_level(dir, level, &fname, res_page);
> -		if (de)
> +		if (de || IS_ERR(*res_page))
>   			break;
>   	}

Hi, kim
	Here, we return NULL for the error of find_data_page, it means
the file looked up is not exist to vfs, but may be the file has already exist
behind the block read error. So maybe we 'd better reported the error to vfs.

Thanks.

>   out:
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index 77c9c24..1eb3043 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -286,8 +286,10 @@ struct f2fs_dir_entry *find_in_inline_dir(struct inode *dir,
>   	f2fs_hash_t namehash;
>
>   	ipage = get_node_page(sbi, dir->i_ino);
> -	if (IS_ERR(ipage))
> +	if (IS_ERR(ipage)) {
> +		*res_page = ipage;
>   		return NULL;
> +	}
>
>   	namehash = f2fs_dentry_hash(&name);
>
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index 496f4e3..3f6119e 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -232,6 +232,9 @@ static int __recover_dot_dentries(struct inode *dir, nid_t pino)
>   	if (de) {
>   		f2fs_dentry_kunmap(dir, page);
>   		f2fs_put_page(page, 0);
> +	} else if (IS_ERR(page)) {
> +		err = PTR_ERR(page);
> +		goto out;
>   	} else {
>   		err = __f2fs_add_link(dir, &dot, NULL, dir->i_ino, S_IFDIR);
>   		if (err)
> @@ -242,6 +245,8 @@ static int __recover_dot_dentries(struct inode *dir, nid_t pino)
>   	if (de) {
>   		f2fs_dentry_kunmap(dir, page);
>   		f2fs_put_page(page, 0);
> +	} else if (IS_ERR(page)) {
> +		err = PTR_ERR(page);
>   	} else {
>   		err = __f2fs_add_link(dir, &dotdot, NULL, pino, S_IFDIR);
>   	}
>

Re: [f2fs-dev] [PATCH 1/4] f2fs: propagate error given by f2fs_find_entry

[Jaegeuk Kim]

On Fri, May 27, 2016 at 12:48:48PM +0800, He YunLei wrote:
> On 2016/5/27 8:25, Jaegeuk Kim wrote:
> >If we get ENOMEM or EIO in f2fs_find_entry, we should stop right away.
> >Otherwise, for example, we can get duplicate directory entry by ->chash and
> >->clevel.
> >
> >Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> >---
> >  fs/f2fs/dir.c    | 23 ++++++++++++++++-------
> >  fs/f2fs/inline.c |  4 +++-
> >  fs/f2fs/namei.c  |  5 +++++
> >  3 files changed, 24 insertions(+), 8 deletions(-)
> >
> >diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> >index 24d1308..ae37543 100644
> >--- a/fs/f2fs/dir.c
> >+++ b/fs/f2fs/dir.c
> >@@ -185,8 +185,13 @@ static struct f2fs_dir_entry *find_in_level(struct inode *dir,
> >  		/* no need to allocate new dentry pages to all the indices */
> >  		dentry_page = find_data_page(dir, bidx);
> >  		if (IS_ERR(dentry_page)) {
> >-			room = true;
> >-			continue;
> >+			if (PTR_ERR(dentry_page) == -ENOENT) {
> >+				room = true;
> >+				continue;
> >+			} else {
> >+				*res_page = dentry_page;
> >+				break;
> >+			}
> >  		}
> >
> >  		de = find_in_block(dentry_page, fname, namehash, &max_slots,
> >@@ -223,19 +228,22 @@ struct f2fs_dir_entry *f2fs_find_entry(struct inode *dir,
> >  	struct fscrypt_name fname;
> >  	int err;
> >
> >-	*res_page = NULL;
> >-
> >  	err = fscrypt_setup_filename(dir, child, 1, &fname);
> >-	if (err)
> >+	if (err) {
> >+		*res_page = ERR_PTR(-ENOMEM);
> >  		return NULL;
> >+	}
> >
> >  	if (f2fs_has_inline_dentry(dir)) {
> >+		*res_page = NULL;
> >  		de = find_in_inline_dir(dir, &fname, res_page);
> >  		goto out;
> >  	}
> >
> >-	if (npages == 0)
> >+	if (npages == 0) {
> >+		*res_page = NULL;
> >  		goto out;
> >+	}
> >
> >  	max_depth = F2FS_I(dir)->i_current_depth;
> >  	if (unlikely(max_depth > MAX_DIR_HASH_DEPTH)) {
> >@@ -247,8 +255,9 @@ struct f2fs_dir_entry *f2fs_find_entry(struct inode *dir,
> >  	}
> >
> >  	for (level = 0; level < max_depth; level++) {
> >+		*res_page = NULL;
> >  		de = find_in_level(dir, level, &fname, res_page);
> >-		if (de)
> >+		if (de || IS_ERR(*res_page))
> >  			break;
> >  	}
> 
> Hi, kim
> 	Here, we return NULL for the error of find_data_page, it means
> the file looked up is not exist to vfs, but may be the file has already exist
> behind the block read error. So maybe we 'd better reported the error to vfs.
> 

How about this? :)

>From a9098ceb560174debca8b5f06658d7a91cdd7fff Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Fri, 27 May 2016 10:10:41 -0700
Subject: [PATCH] f2fs: return error of f2fs_lookup

Now we can report an error to f2fs_lookup given by f2fs_find_entry.

Suggested-by: He YunLei <heyunlei@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/dir.c   | 2 +-
 fs/f2fs/namei.c | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index ae37543..6fbb1ed 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -230,7 +230,7 @@ struct f2fs_dir_entry *f2fs_find_entry(struct inode *dir,
 
 	err = fscrypt_setup_filename(dir, child, 1, &fname);
 	if (err) {
-		*res_page = ERR_PTR(-ENOMEM);
+		*res_page = ERR_PTR(err);
 		return NULL;
 	}
 
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 3f6119e..78efe00 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -287,8 +287,11 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
 		return ERR_PTR(-ENAMETOOLONG);
 
 	de = f2fs_find_entry(dir, &dentry->d_name, &page);
-	if (!de)
+	if (!de) {
+		if (IS_ERR(page))
+			return (struct dentry *)page;
 		return d_splice_alias(inode, dentry);
+	}
 
 	ino = le32_to_cpu(de->ino);
 	f2fs_dentry_kunmap(dir, page);
-- 
2.6.3