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


Groups > linux.kernel > #1393730 > unrolled thread

[PATCH 01/11] f2fs: introduce macros for proc entries

Started byJaegeuk Kim <jaegeuk@kernel.org>
First post2016-05-03 20:30 +0200
Last post2016-05-05 05:00 +0200
Articles 8 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 01/11] f2fs: introduce macros for proc entries Jaegeuk Kim <jaegeuk@kernel.org> - 2016-05-03 20:30 +0200
    [PATCH 04/11] f2fs: use f2fs_grab_cache_page instead of grab_cache_page Jaegeuk Kim <jaegeuk@kernel.org> - 2016-05-03 20:30 +0200
    [PATCH 10/11] f2fs: fix leak of orphan inode objects Jaegeuk Kim <jaegeuk@kernel.org> - 2016-05-03 20:30 +0200
    [PATCH 07/11] f2fs: inject page allocation failures Jaegeuk Kim <jaegeuk@kernel.org> - 2016-05-03 20:30 +0200
    [PATCH 09/11] f2fs: revisit error handling flows Jaegeuk Kim <jaegeuk@kernel.org> - 2016-05-03 20:30 +0200
    [PATCH 11/11] f2fs: retry to truncate blocks in -ENOMEM case Jaegeuk Kim <jaegeuk@kernel.org> - 2016-05-03 20:30 +0200
      Re: [f2fs-dev] [PATCH 11/11] f2fs: retry to truncate blocks in -ENOMEM  case Hou Pengyang <houpengyang@huawei.com> - 2016-05-05 04:10 +0200
        Re: [f2fs-dev] [PATCH 11/11] f2fs: retry to truncate blocks in  -ENOMEM case Jaegeuk Kim <jaegeuk@kernel.org> - 2016-05-05 05:00 +0200

#1393730 — [PATCH 01/11] f2fs: introduce macros for proc entries

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-05-03 20:30 +0200
Subject[PATCH 01/11] f2fs: introduce macros for proc entries
Message-ID<ruNf3-4Gi-13@gated-at.bofh.it>
This adds macros to be used multiple proc entries.

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

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 19a85cf..b006de6 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -745,19 +745,22 @@ static int segment_info_seq_show(struct seq_file *seq, void *offset)
 	return 0;
 }
 
-static int segment_info_open_fs(struct inode *inode, struct file *file)
-{
-	return single_open(file, segment_info_seq_show, PDE_DATA(inode));
-}
-
-static const struct file_operations f2fs_seq_segment_info_fops = {
-	.owner = THIS_MODULE,
-	.open = segment_info_open_fs,
-	.read = seq_read,
-	.llseek = seq_lseek,
-	.release = single_release,
+#define F2FS_PROC_FILE_DEF(_name)					\
+static int _name##_open_fs(struct inode *inode, struct file *file)	\
+{									\
+	return single_open(file, _name##_seq_show, PDE_DATA(inode));	\
+}									\
+									\
+static const struct file_operations f2fs_seq_##_name##_fops = {		\
+	.owner = THIS_MODULE,						\
+	.open = _name##_open_fs,					\
+	.read = seq_read,						\
+	.llseek = seq_lseek,						\
+	.release = single_release,					\
 };
 
+F2FS_PROC_FILE_DEF(segment_info);
+
 static void default_options(struct f2fs_sb_info *sbi)
 {
 	/* init some FS parameters */
-- 
2.6.3

[toc] | [next] | [standalone]


#1393732 — [PATCH 04/11] f2fs: use f2fs_grab_cache_page instead of grab_cache_page

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-05-03 20:30 +0200
Subject[PATCH 04/11] f2fs: use f2fs_grab_cache_page instead of grab_cache_page
Message-ID<ruNf5-4Gi-45@gated-at.bofh.it>
In reply to#1393730
This patch converts grab_cache_page to f2fs_grab_cache_page.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/checkpoint.c | 7 ++++---
 fs/f2fs/inline.c     | 4 ++--
 fs/f2fs/node.c       | 8 ++++----
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index dc7bc72..ac87cd9 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -34,7 +34,7 @@ struct page *grab_meta_page(struct f2fs_sb_info *sbi, pgoff_t index)
 	struct address_space *mapping = META_MAPPING(sbi);
 	struct page *page = NULL;
 repeat:
-	page = grab_cache_page(mapping, index);
+	page = f2fs_grab_cache_page(mapping, index, false);
 	if (!page) {
 		cond_resched();
 		goto repeat;
@@ -64,7 +64,7 @@ static struct page *__get_meta_page(struct f2fs_sb_info *sbi, pgoff_t index,
 	if (unlikely(!is_meta))
 		fio.rw &= ~REQ_META;
 repeat:
-	page = grab_cache_page(mapping, index);
+	page = f2fs_grab_cache_page(mapping, index, false);
 	if (!page) {
 		cond_resched();
 		goto repeat;
@@ -186,7 +186,8 @@ int ra_meta_pages(struct f2fs_sb_info *sbi, block_t start, int nrpages,
 			BUG();
 		}
 
-		page = grab_cache_page(META_MAPPING(sbi), fio.new_blkaddr);
+		page = f2fs_grab_cache_page(META_MAPPING(sbi),
+						fio.new_blkaddr, false);
 		if (!page)
 			continue;
 		if (PageUptodate(page)) {
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 33830b2..8a51955 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -161,7 +161,7 @@ int f2fs_convert_inline_inode(struct inode *inode)
 	if (!f2fs_has_inline_data(inode))
 		return 0;
 
-	page = grab_cache_page(inode->i_mapping, 0);
+	page = f2fs_grab_cache_page(inode->i_mapping, 0, false);
 	if (!page)
 		return -ENOMEM;
 
@@ -358,7 +358,7 @@ static int f2fs_move_inline_dirents(struct inode *dir, struct page *ipage,
 	struct f2fs_dentry_block *dentry_blk;
 	int err;
 
-	page = grab_cache_page(dir->i_mapping, 0);
+	page = f2fs_grab_cache_page(dir->i_mapping, 0, false);
 	if (!page) {
 		f2fs_put_page(ipage, 1);
 		return -ENOMEM;
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index f80cfb6..af010f5 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -995,7 +995,7 @@ struct page *new_node_page(struct dnode_of_data *dn,
 	if (unlikely(is_inode_flag_set(F2FS_I(dn->inode), FI_NO_ALLOC)))
 		return ERR_PTR(-EPERM);
 
-	page = grab_cache_page(NODE_MAPPING(sbi), dn->nid);
+	page = f2fs_grab_cache_page(NODE_MAPPING(sbi), dn->nid, false);
 	if (!page)
 		return ERR_PTR(-ENOMEM);
 
@@ -1087,7 +1087,7 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t nid)
 	if (apage)
 		return;
 
-	apage = grab_cache_page(NODE_MAPPING(sbi), nid);
+	apage = f2fs_grab_cache_page(NODE_MAPPING(sbi), nid, false);
 	if (!apage)
 		return;
 
@@ -1128,7 +1128,7 @@ static struct page *__get_node_page(struct f2fs_sb_info *sbi, pgoff_t nid,
 		return ERR_PTR(-ENOENT);
 	f2fs_bug_on(sbi, check_nid_range(sbi, nid));
 repeat:
-	page = grab_cache_page(NODE_MAPPING(sbi), nid);
+	page = f2fs_grab_cache_page(NODE_MAPPING(sbi), nid, false);
 	if (!page)
 		return ERR_PTR(-ENOMEM);
 
@@ -2012,7 +2012,7 @@ int recover_inode_page(struct f2fs_sb_info *sbi, struct page *page)
 	if (unlikely(old_ni.blk_addr != NULL_ADDR))
 		return -EINVAL;
 
-	ipage = grab_cache_page(NODE_MAPPING(sbi), ino);
+	ipage = f2fs_grab_cache_page(NODE_MAPPING(sbi), ino, false);
 	if (!ipage)
 		return -ENOMEM;
 
-- 
2.6.3

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


#1393733 — [PATCH 10/11] f2fs: fix leak of orphan inode objects

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-05-03 20:30 +0200
Subject[PATCH 10/11] f2fs: fix leak of orphan inode objects
Message-ID<ruNf5-4Gi-49@gated-at.bofh.it>
In reply to#1393730
When unmounting filesystem, we should release all the ino entries.

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

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 9169dab..6402e93 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -449,12 +449,12 @@ bool exist_written_data(struct f2fs_sb_info *sbi, nid_t ino, int mode)
 	return e ? true : false;
 }
 
-void release_ino_entry(struct f2fs_sb_info *sbi)
+void release_ino_entry(struct f2fs_sb_info *sbi, bool all)
 {
 	struct ino_entry *e, *tmp;
 	int i;
 
-	for (i = APPEND_INO; i <= UPDATE_INO; i++) {
+	for (i = all ? ORPHAN_INO: APPEND_INO; i <= UPDATE_INO; i++) {
 		struct inode_management *im = &sbi->im[i];
 
 		spin_lock(&im->ino_lock);
@@ -1090,7 +1090,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 		invalidate_mapping_pages(META_MAPPING(sbi), discard_blk,
 								discard_blk);
 
-	release_ino_entry(sbi);
+	release_ino_entry(sbi, false);
 
 	if (unlikely(f2fs_cp_error(sbi)))
 		return -EIO;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 659fdea..6e6fd0b 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1910,7 +1910,7 @@ void ra_meta_pages_cond(struct f2fs_sb_info *, pgoff_t);
 long sync_meta_pages(struct f2fs_sb_info *, enum page_type, long);
 void add_ino_entry(struct f2fs_sb_info *, nid_t, int type);
 void remove_ino_entry(struct f2fs_sb_info *, nid_t, int type);
-void release_ino_entry(struct f2fs_sb_info *);
+void release_ino_entry(struct f2fs_sb_info *, bool);
 bool exist_written_data(struct f2fs_sb_info *, nid_t, int);
 int acquire_orphan_inode(struct f2fs_sb_info *);
 void release_orphan_inode(struct f2fs_sb_info *);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 55e8eb0..6b5f0af 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -596,7 +596,7 @@ static void f2fs_put_super(struct super_block *sb)
 	 * normally superblock is clean, so we need to release this.
 	 * In addition, EIO will skip do checkpoint, we need this as well.
 	 */
-	release_ino_entry(sbi);
+	release_ino_entry(sbi, true);
 	release_discard_addrs(sbi);
 
 	f2fs_leave_shrinker(sbi);
-- 
2.6.3

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


#1393734 — [PATCH 07/11] f2fs: inject page allocation failures

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-05-03 20:30 +0200
Subject[PATCH 07/11] f2fs: inject page allocation failures
Message-ID<ruNf5-4Gi-51@gated-at.bofh.it>
In reply to#1393730
This patch adds page allocation failures.

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

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 60098df..02c2c96 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -40,6 +40,7 @@
 #ifdef CONFIG_F2FS_FAULT_INJECTION
 enum {
 	FAULT_KMALLOC,
+	FAULT_PAGE_ALLOC,
 	FAULT_MAX,
 };
 
@@ -1296,6 +1297,14 @@ static inline unsigned int valid_inode_count(struct f2fs_sb_info *sbi)
 static inline struct page *f2fs_grab_cache_page(struct address_space *mapping,
 						pgoff_t index, bool for_write)
 {
+#ifdef CONFIG_F2FS_FAULT_INJECTION
+	struct page *page = find_lock_page(mapping, index);
+	if (page)
+		return page;
+
+	if (time_to_inject(FAULT_PAGE_ALLOC))
+		return NULL;
+#endif
 	if (!for_write)
 		return grab_cache_page(mapping, index);
 	return grab_cache_page_write_begin(mapping, index, AOP_FLAG_NOFS);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 76f4d2c..1715794 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -45,6 +45,7 @@ atomic_t f2fs_ops;
 
 char *fault_name[FAULT_MAX] = {
 	[FAULT_KMALLOC]		= "kmalloc",
+	[FAULT_PAGE_ALLOC]	= "page alloc",
 };
 #endif
 
-- 
2.6.3

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


#1393736 — [PATCH 09/11] f2fs: revisit error handling flows

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-05-03 20:30 +0200
Subject[PATCH 09/11] f2fs: revisit error handling flows
Message-ID<ruNf5-4Gi-55@gated-at.bofh.it>
In reply to#1393730
This patch fixes a couple of bugs regarding to orphan inodes when handling
errors.

This tries to
 - call alloc_nid_done with add_orphan_inode in handle_failed_inode
 - let truncate blocks in f2fs_evict_inode
 - not make a bad inode due to i_mode change

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/dir.c   | 20 ++++++++++++--------
 fs/f2fs/inode.c | 40 ++++++++++++++++------------------------
 2 files changed, 28 insertions(+), 32 deletions(-)

diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 50f42be..5373f33 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -391,9 +391,14 @@ struct page *init_inode_metadata(struct inode *inode, struct inode *dir,
 			return page;
 
 		if (S_ISDIR(inode->i_mode)) {
+			/* in order to handle error case */
+			get_page(page);
 			err = make_empty_dir(inode, dir, page);
-			if (err)
-				goto error;
+			if (err) {
+				lock_page(page);
+				goto put_error;
+			}
+			put_page(page);
 		}
 
 		err = f2fs_init_acl(inode, dir, page, dpage);
@@ -437,13 +442,12 @@ struct page *init_inode_metadata(struct inode *inode, struct inode *dir,
 	return page;
 
 put_error:
-	f2fs_put_page(page, 1);
-error:
-	/* once the failed inode becomes a bad inode, i_mode is S_IFREG */
+	/* truncate empty dir pages */
 	truncate_inode_pages(&inode->i_data, 0);
-	truncate_blocks(inode, 0, false);
-	remove_dirty_inode(inode);
-	remove_inode_page(inode);
+
+	clear_nlink(inode);
+	update_inode(inode, page);
+	f2fs_put_page(page, 1);
 	return ERR_PTR(err);
 }
 
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index cb269c4..f4ac851 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -368,10 +368,7 @@ no_delete:
 	if (is_inode_flag_set(fi, FI_UPDATE_WRITE))
 		add_ino_entry(sbi, inode->i_ino, UPDATE_INO);
 	if (is_inode_flag_set(fi, FI_FREE_NID)) {
-		if (err && err != -ENOENT)
-			alloc_nid_done(sbi, inode->i_ino);
-		else
-			alloc_nid_failed(sbi, inode->i_ino);
+		alloc_nid_failed(sbi, inode->i_ino);
 		clear_inode_flag(fi, FI_FREE_NID);
 	}
 
@@ -397,37 +394,32 @@ out_clear:
 void handle_failed_inode(struct inode *inode)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
-	int err = 0;
+	struct node_info ni;
 
-	clear_nlink(inode);
-	make_bad_inode(inode);
+	/* don't make bad inode, since it becomes a regular file. */
 	unlock_new_inode(inode);
 
-	i_size_write(inode, 0);
-	if (F2FS_HAS_BLOCKS(inode))
-		err = f2fs_truncate(inode, false);
-
-	if (!err)
-		err = remove_inode_page(inode);
-
 	/*
-	 * if we skip truncate_node in remove_inode_page bacause we failed
-	 * before, it's better to find another way to release resource of
-	 * this inode (e.g. valid block count, node block or nid). Here we
-	 * choose to add this inode to orphan list, so that we can call iput
-	 * for releasing in orphan recovery flow.
-	 *
 	 * Note: we should add inode to orphan list before f2fs_unlock_op()
 	 * so we can prevent losing this orphan when encoutering checkpoint
 	 * and following suddenly power-off.
 	 */
-	if (err && err != -ENOENT) {
-		err = acquire_orphan_inode(sbi);
-		if (!err)
+	get_node_info(sbi, inode->i_ino, &ni);
+
+	if (ni.blk_addr != NULL_ADDR) {
+		int err = acquire_orphan_inode(sbi);
+		if (err) {
+			set_sbi_flag(sbi, SBI_NEED_FSCK);
+			f2fs_msg(sbi->sb, KERN_WARNING,
+				"Too many orphan inodes, run fsck to fix.");
+		} else {
 			add_orphan_inode(sbi, inode->i_ino);
+		}
+		alloc_nid_done(sbi, inode->i_ino);
+	} else {
+		set_inode_flag(F2FS_I(inode), FI_FREE_NID);
 	}
 
-	set_inode_flag(F2FS_I(inode), FI_FREE_NID);
 	f2fs_unlock_op(sbi);
 
 	/* iput will drop the inode object */
-- 
2.6.3

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


#1393737 — [PATCH 11/11] f2fs: retry to truncate blocks in -ENOMEM case

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-05-03 20:30 +0200
Subject[PATCH 11/11] f2fs: retry to truncate blocks in -ENOMEM case
Message-ID<ruNf5-4Gi-57@gated-at.bofh.it>
In reply to#1393730
This patch modifies to retry truncating node blocks in -ENOMEM case.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/inode.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index f4ac851..5cccd7a 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -344,7 +344,7 @@ void f2fs_evict_inode(struct inode *inode)
 	sb_start_intwrite(inode->i_sb);
 	set_inode_flag(fi, FI_NO_ALLOC);
 	i_size_write(inode, 0);
-
+retry:
 	if (F2FS_HAS_BLOCKS(inode))
 		err = f2fs_truncate(inode, true);
 
@@ -374,6 +374,11 @@ no_delete:
 
 	if (err && err != -ENOENT) {
 		if (!exist_written_data(sbi, inode->i_ino, ORPHAN_INO)) {
+			/* give more chances, if ENOMEM case */
+			if (err == -ENOMEM) {
+				err = 0;
+				goto retry;
+			}
 			/*
 			 * get here because we failed to release resource
 			 * of inode previously, reminder our user to run fsck
-- 
2.6.3

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


#1394824 — Re: [f2fs-dev] [PATCH 11/11] f2fs: retry to truncate blocks in -ENOMEM case

FromHou Pengyang <houpengyang@huawei.com>
Date2016-05-05 04:10 +0200
SubjectRe: [f2fs-dev] [PATCH 11/11] f2fs: retry to truncate blocks in -ENOMEM case
Message-ID<rvgTL-7sV-5@gated-at.bofh.it>
In reply to#1393737
On 2016/5/4 2:21, Jaegeuk Kim wrote:
> This patch modifies to retry truncating node blocks in -ENOMEM case.
>
Hi, Kim. in this patch, I think there is NO chance to retry for -ENOMEM.

This is because if exist_written_data returns false, we can confirm that
this inode has been released from orphan radix-tree:
f2fs_evict_inode
  ---> remove_inode_page
     ---> truncate_node
         ---> remove_orphan_inode
On this condition, err is 0, So it won't enter:
if (err && err != -ENOENT)
{
     ...
}
sequentially, there is no chance to truncate node blocks again.
I miss something else?

How about this patch?

--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -345,6 +345,7 @@ void f2fs_evict_inode(struct inode *inode)
         set_inode_flag(fi, FI_NO_ALLOC);
         i_size_write(inode, 0);

+retry:
         if (F2FS_HAS_BLOCKS(inode))
                 err = f2fs_truncate(inode, true);

@@ -354,6 +355,11 @@ void f2fs_evict_inode(struct inode *inode)
                 f2fs_unlock_op(sbi);
         }

+       if (err == -ENOMEM) {
+               err = 0;
+               goto retry;
+       }
+
         sb_end_intwrite(inode->i_sb);
  no_delete:
         stat_dec_inline_xattr(inode);
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>   fs/f2fs/inode.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index f4ac851..5cccd7a 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -344,7 +344,7 @@ void f2fs_evict_inode(struct inode *inode)
>   	sb_start_intwrite(inode->i_sb);
>   	set_inode_flag(fi, FI_NO_ALLOC);
>   	i_size_write(inode, 0);
> -
> +retry:
>   	if (F2FS_HAS_BLOCKS(inode))
>   		err = f2fs_truncate(inode, true);
>
> @@ -374,6 +374,11 @@ no_delete:
>
>   	if (err && err != -ENOENT) {
>   		if (!exist_written_data(sbi, inode->i_ino, ORPHAN_INO)) {
> +			/* give more chances, if ENOMEM case */
> +			if (err == -ENOMEM) {
> +				err = 0;
> +				goto retry;
> +			}
>   			/*
>   			 * get here because we failed to release resource
>   			 * of inode previously, reminder our user to run fsck
>

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


#1394839 — Re: [f2fs-dev] [PATCH 11/11] f2fs: retry to truncate blocks in -ENOMEM case

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-05-05 05:00 +0200
SubjectRe: [f2fs-dev] [PATCH 11/11] f2fs: retry to truncate blocks in -ENOMEM case
Message-ID<rvhGa-82x-9@gated-at.bofh.it>
In reply to#1394824
Hi,

On Thu, May 05, 2016 at 10:00:15AM +0800, Hou Pengyang wrote:
> On 2016/5/4 2:21, Jaegeuk Kim wrote:
> >This patch modifies to retry truncating node blocks in -ENOMEM case.
> >
> Hi, Kim. in this patch, I think there is NO chance to retry for -ENOMEM.
> 
> This is because if exist_written_data returns false, we can confirm that
> this inode has been released from orphan radix-tree:
> f2fs_evict_inode
>  ---> remove_inode_page
>     ---> truncate_node
>         ---> remove_orphan_inode
> On this condition, err is 0, So it won't enter:
> if (err && err != -ENOENT)
> {
>     ...
> }
> sequentially, there is no chance to truncate node blocks again.
> I miss something else?

When I initially tested fault injection, I could hit that before.
But, now I can't hit this again. :(
It seems it was gone while I updated the error flow before.
Agreed with you, and let me take your change.

BTW, I even suspect whether this leaking condition happens or not.
If f2fs_evict_inode deals with inode deletion, that inode should be an orphan
one. So, we don't need to consider that condition actually.

So, I wrote this patch as well.
I started stress tests again. :)

Thanks,

From 8c1e4e5ca23410b8f55bbc75d64f75416d486739 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Wed, 4 May 2016 19:48:53 -0700
Subject: [PATCH] f2fs: don't worry about inode leak in evict_inode

Even if an inode failed to release its blocks, it should be kept in an orphan
inode list, so it will be released later.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/inode.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index baf3a2a..689d691 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -377,20 +377,8 @@ no_delete:
 		alloc_nid_failed(sbi, inode->i_ino);
 		clear_inode_flag(fi, FI_FREE_NID);
 	}
-
-	if (err && err != -ENOENT) {
-		if (!exist_written_data(sbi, inode->i_ino, ORPHAN_INO)) {
-			/*
-			 * get here because we failed to release resource
-			 * of inode previously, reminder our user to run fsck
-			 * for fixing.
-			 */
-			set_sbi_flag(sbi, SBI_NEED_FSCK);
-			f2fs_msg(sbi->sb, KERN_WARNING,
-				"inode (ino:%lu) resource leak, run fsck "
-				"to fix this issue!", inode->i_ino);
-		}
-	}
+	f2fs_bug_on(sbi, err &&
+		!exist_written_data(sbi, inode->i_ino, ORPHAN_INO));
 out_clear:
 	fscrypt_put_encryption_info(inode, NULL);
 	clear_inode(inode);
-- 
2.6.3

> 
> How about this patch?
> 
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -345,6 +345,7 @@ void f2fs_evict_inode(struct inode *inode)
>         set_inode_flag(fi, FI_NO_ALLOC);
>         i_size_write(inode, 0);
> 
> +retry:
>         if (F2FS_HAS_BLOCKS(inode))
>                 err = f2fs_truncate(inode, true);
> 
> @@ -354,6 +355,11 @@ void f2fs_evict_inode(struct inode *inode)
>                 f2fs_unlock_op(sbi);
>         }
> 
> +       if (err == -ENOMEM) {
> +               err = 0;
> +               goto retry;
> +       }
> +
>         sb_end_intwrite(inode->i_sb);
>  no_delete:
>         stat_dec_inline_xattr(inode);
> >Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> >---
> >  fs/f2fs/inode.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> >diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> >index f4ac851..5cccd7a 100644
> >--- a/fs/f2fs/inode.c
> >+++ b/fs/f2fs/inode.c
> >@@ -344,7 +344,7 @@ void f2fs_evict_inode(struct inode *inode)
> >  	sb_start_intwrite(inode->i_sb);
> >  	set_inode_flag(fi, FI_NO_ALLOC);
> >  	i_size_write(inode, 0);
> >-
> >+retry:
> >  	if (F2FS_HAS_BLOCKS(inode))
> >  		err = f2fs_truncate(inode, true);
> >
> >@@ -374,6 +374,11 @@ no_delete:
> >
> >  	if (err && err != -ENOENT) {
> >  		if (!exist_written_data(sbi, inode->i_ino, ORPHAN_INO)) {
> >+			/* give more chances, if ENOMEM case */
> >+			if (err == -ENOMEM) {
> >+				err = 0;
> >+				goto retry;
> >+			}
> >  			/*
> >  			 * get here because we failed to release resource
> >  			 * of inode previously, reminder our user to run fsck
> >
> 

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web