Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1407754 > unrolled thread

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

Started byJaegeuk Kim <jaegeuk@kernel.org>
First post2016-05-27 02:30 +0200
Last post2016-05-30 04:40 +0200
Articles 5 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/4] f2fs: propagate error given by f2fs_find_entry Jaegeuk Kim <jaegeuk@kernel.org> - 2016-05-27 02:30 +0200
    [PATCH 2/4] f2fs: inject to produce some orphan inodes Jaegeuk Kim <jaegeuk@kernel.org> - 2016-05-27 02:30 +0200
    [PATCH 3/4] f2fs: do not skip writing data pages Jaegeuk Kim <jaegeuk@kernel.org> - 2016-05-27 02:30 +0200
    Re: [f2fs-dev] [PATCH 1/4] f2fs: propagate error given by  f2fs_find_entry He YunLei <heyunlei@huawei.com> - 2016-05-27 06:50 +0200
      Re: [f2fs-dev] [PATCH 1/4] f2fs: propagate error given by  f2fs_find_entry Jaegeuk Kim <jaegeuk@kernel.org> - 2016-05-30 04:40 +0200

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

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-05-27 02:30 +0200
Subject[PATCH 1/4] f2fs: propagate error given by f2fs_find_entry
Message-ID<rDdP3-4Ku-1@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

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

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-05-27 02:30 +0200
Subject[PATCH 2/4] f2fs: inject to produce some orphan inodes
Message-ID<rDdP3-4Ku-11@gated-at.bofh.it>
In reply to#1407754
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

[toc] | [prev] | [next] | [standalone]

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

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-05-27 02:30 +0200
Subject[PATCH 3/4] f2fs: do not skip writing data pages
Message-ID<rDdP3-4Ku-9@gated-at.bofh.it>
In reply to#1407754
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

[toc] | [prev] | [next] | [standalone]

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

FromHe YunLei <heyunlei@huawei.com>
Date2016-05-27 06:50 +0200
SubjectRe: [f2fs-dev] [PATCH 1/4] f2fs: propagate error given by f2fs_find_entry
Message-ID<rDhSF-7hl-11@gated-at.bofh.it>
In reply to#1407754
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);
>   	}
>

[toc] | [prev] | [next] | [standalone]

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

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-05-30 04:40 +0200
SubjectRe: [f2fs-dev] [PATCH 1/4] f2fs: propagate error given by f2fs_find_entry
Message-ID<rElhv-6xF-1@gated-at.bofh.it>
In reply to#1407812
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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web