* [PATCH 2/4] f2fs: inject to produce some orphan inodes
2016-05-27 0:25 [PATCH 1/4] f2fs: propagate error given by f2fs_find_entry Jaegeuk Kim
@ 2016-05-27 0:25 ` Jaegeuk Kim
2016-05-27 0:25 ` [PATCH 3/4] f2fs: do not skip writing data pages Jaegeuk Kim
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2016-05-27 0:25 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, linux-f2fs-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] f2fs: do not skip writing data pages
2016-05-27 0:25 [PATCH 1/4] f2fs: propagate error given by f2fs_find_entry Jaegeuk Kim
2016-05-27 0:25 ` [PATCH 2/4] f2fs: inject to produce some orphan inodes Jaegeuk Kim
@ 2016-05-27 0:25 ` Jaegeuk Kim
2016-05-27 0:25 ` [PATCH 4/4] f2fs: remove two steps to flush dirty " Jaegeuk Kim
2016-05-27 4:48 ` [f2fs-dev] [PATCH 1/4] f2fs: propagate error given by f2fs_find_entry He YunLei
3 siblings, 0 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2016-05-27 0:25 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, linux-f2fs-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] f2fs: remove two steps to flush dirty data pages
2016-05-27 0:25 [PATCH 1/4] f2fs: propagate error given by f2fs_find_entry Jaegeuk Kim
2016-05-27 0:25 ` [PATCH 2/4] f2fs: inject to produce some orphan inodes Jaegeuk Kim
2016-05-27 0:25 ` [PATCH 3/4] f2fs: do not skip writing data pages Jaegeuk Kim
@ 2016-05-27 0:25 ` Jaegeuk Kim
2016-05-27 4:48 ` [f2fs-dev] [PATCH 1/4] f2fs: propagate error given by f2fs_find_entry He YunLei
3 siblings, 0 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2016-05-27 0:25 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, linux-f2fs-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [f2fs-dev] [PATCH 1/4] f2fs: propagate error given by f2fs_find_entry
2016-05-27 0:25 [PATCH 1/4] f2fs: propagate error given by f2fs_find_entry Jaegeuk Kim
` (2 preceding siblings ...)
2016-05-27 0:25 ` [PATCH 4/4] f2fs: remove two steps to flush dirty " Jaegeuk Kim
@ 2016-05-27 4:48 ` He YunLei
2016-05-30 2:39 ` Jaegeuk Kim
3 siblings, 1 reply; 6+ messages in thread
From: He YunLei @ 2016-05-27 4:48 UTC (permalink / raw)
To: Jaegeuk Kim, linux-kernel, linux-fsdevel, linux-f2fs-devel
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);
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [f2fs-dev] [PATCH 1/4] f2fs: propagate error given by f2fs_find_entry
2016-05-27 4:48 ` [f2fs-dev] [PATCH 1/4] f2fs: propagate error given by f2fs_find_entry He YunLei
@ 2016-05-30 2:39 ` Jaegeuk Kim
0 siblings, 0 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2016-05-30 2:39 UTC (permalink / raw)
To: He YunLei; +Cc: linux-kernel, linux-fsdevel, linux-f2fs-devel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread