f2fs: handle error of f2fs_iget correctly

In recover_orphan_inode, if f2fs_iget failed, we change to report the error number to its caller instead of bug_on.

Signed-off-by Chao Yu <[email protected]>

---
 fs/f2fs/checkpoint.c | 25 ++++++++++++++++++-------
 fs/f2fs/f2fs.h       |  2 +-
 fs/f2fs/super.c      |  4 +++-
 3 files changed, 22 insertions(+), 9 deletions(-)

fs/f2fs/checkpoint.c +18 -7

index c311176..e2def90 100644
@@ -468,22 +468,28 @@ void remove_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
 	__remove_ino_entry(sbi, ino, ORPHAN_INO);
 }
 
-static void recover_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
+static int recover_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
 {
-	struct inode *inode = f2fs_iget(sbi->sb, ino);
-	f2fs_bug_on(sbi, IS_ERR(inode));
+	struct inode *inode;
+
+	inode = f2fs_iget(sbi->sb, ino);
+	if (IS_ERR(inode))
+		return PTR_ERR(inode);
+
 	clear_nlink(inode);
 
 	/* truncate all the data during iput */
 	iput(inode);
+	return 0;
 }
 
-void recover_orphan_inodes(struct f2fs_sb_info *sbi)
+int recover_orphan_inodes(struct f2fs_sb_info *sbi)
 {
 	block_t start_blk, orphan_blocks, i, j;
+	int err;
 
 	if (!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG))
-		return;
+		return 0;
 
 	set_sbi_flag(sbi, SBI_POR_DOING);
 

@@ -499,14 +505,19 @@ void recover_orphan_inodes(struct f2fs_sb_info *sbi)
 		orphan_blk = (struct f2fs_orphan_block *)page_address(page);
 		for (j = 0; j < le32_to_cpu(orphan_blk->entry_count); j++) {
 			nid_t ino = le32_to_cpu(orphan_blk->ino[j]);
-			recover_orphan_inode(sbi, ino);
+			err = recover_orphan_inode(sbi, ino);
+			if (err) {
+				f2fs_put_page(page, 1);
+				clear_sbi_flag(sbi, SBI_POR_DOING);
+				return err;
+			}
 		}
 		f2fs_put_page(page, 1);
 	}
 	/* clear Orphan Flag */
 	clear_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG);
 	clear_sbi_flag(sbi, SBI_POR_DOING);
-	return;
+	return 0;
 }
 
 static void write_orphan_inodes(struct f2fs_sb_info *sbi, block_t start_blk)

fs/f2fs/f2fs.h +1 -1

index 311960c..4a6f69b 100644
@@ -1750,7 +1750,7 @@ int acquire_orphan_inode(struct f2fs_sb_info *);
 void release_orphan_inode(struct f2fs_sb_info *);
 void add_orphan_inode(struct f2fs_sb_info *, nid_t);
 void remove_orphan_inode(struct f2fs_sb_info *, nid_t);
-void recover_orphan_inodes(struct f2fs_sb_info *);
+int recover_orphan_inodes(struct f2fs_sb_info *);
 int get_valid_checkpoint(struct f2fs_sb_info *);
 void update_dirty_page(struct inode *, struct page *);
 void add_dirty_dir_inode(struct inode *);

fs/f2fs/super.c +3 -1

index 12eb69d..e5efb53 100644
@@ -1246,7 +1246,9 @@ try_onemore:
 	f2fs_join_shrinker(sbi);
 
 	/* if there are nt orphan nodes free them */
-	recover_orphan_inodes(sbi);
+	err = recover_orphan_inodes(sbi);
+	if (err)
+		goto free_node_inode;
 
 	/* read root inode and dentry */
 	root = f2fs_iget(sb, F2FS_ROOT_INO(sbi));

Signature · 1 line

2.4.2

-----Original Message-----
From: Jaegeuk Kim [mailto:[email protected]]
Sent: Tuesday, August 11, 2015 3:32 AM
To: Chao Yu
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 3/4] f2fs: handle error of f2fs_iget correctly

Hi Chao,

Quoted 2 lines · Chao Yu

In recover_orphan_inode, if f2fs_iget failed, we change to report the error number to its caller instead of bug_on.

Let's keep this in order to catch any bugs.
Or, is there another issue on this?

From 05068a0068a9d0a60296c492967ef2433ad0e35f Mon Sep 17 00:00:00 2001

Hi Jaegeuk,

Quoted 14 lines

-----Original Message-----
From: Jaegeuk Kim [mailto:[email protected]]
Sent: Tuesday, August 11, 2015 3:32 AM
To: Chao Yu
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 3/4] f2fs: handle error of f2fs_iget correctly

Hi Chao,

Quoted 2 lines · Chao Yu

In recover_orphan_inode, if f2fs_iget failed, we change to report the error number to its caller instead of bug_on.

Quoted 2 lines

Let's keep this in order to catch any bugs.
Or, is there another issue on this?

f2fs_iget can fail due to a lot of reason, like out of memory, IMO, it's better to report such error number to user instead of make kernel panic. So how about keeping bug_on when finding no entry for orphan inode, otherwise reporting error to caller?

Quoted 2 lines

From 05068a0068a9d0a60296c492967ef2433ad0e35f Mon Sep 17 00:00:00 2001

From: Chao Yu <[email protected]>
Date: Fri, 7 Aug 2015 17:58:43 +0800
Subject: [PATCH v2 3/4] f2fs: handle error of f2fs_iget correctly

In recover_orphan_inode, whenever f2fs_iget fail, we will make kernel panic, but it's not reasonable, because f2fs_iget can fail due to a lot of reasons including out of memory.

So we change error handling method as below:
a) when finding no entry for the orphan inode, bug_on for catching bugs;
b) for other reasons, report it to caller.

Signed-off-by Chao Yu <[email protected]>

---
 fs/f2fs/checkpoint.c | 31 ++++++++++++++++++++++++-------
 fs/f2fs/f2fs.h       |  2 +-
 fs/f2fs/super.c      |  4 +++-
 3 files changed, 28 insertions(+), 9 deletions(-)

fs/f2fs/checkpoint.c +24 -7

index c311176..0958c83 100644
@@ -468,22 +468,34 @@ void remove_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
 	__remove_ino_entry(sbi, ino, ORPHAN_INO);
 }
 
-static void recover_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
+static int recover_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
 {
-	struct inode *inode = f2fs_iget(sbi->sb, ino);
-	f2fs_bug_on(sbi, IS_ERR(inode));
+	struct inode *inode;
+
+	inode = f2fs_iget(sbi->sb, ino);
+	if (IS_ERR(inode)) {
+		/*
+		 * there should be a bug that we can't find the entry
+		 * to orphan inode.
+		 */
+		f2fs_bug_on(sbi, PTR_ERR(inode) == -ENOENT);
+		return PTR_ERR(inode);
+	}
+
 	clear_nlink(inode);
 
 	/* truncate all the data during iput */
 	iput(inode);
+	return 0;
 }
 
-void recover_orphan_inodes(struct f2fs_sb_info *sbi)
+int recover_orphan_inodes(struct f2fs_sb_info *sbi)
 {
 	block_t start_blk, orphan_blocks, i, j;
+	int err;
 
 	if (!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG))
-		return;
+		return 0;
 
 	set_sbi_flag(sbi, SBI_POR_DOING);
 

@@ -499,14 +511,19 @@ void recover_orphan_inodes(struct f2fs_sb_info *sbi)
 		orphan_blk = (struct f2fs_orphan_block *)page_address(page);
 		for (j = 0; j < le32_to_cpu(orphan_blk->entry_count); j++) {
 			nid_t ino = le32_to_cpu(orphan_blk->ino[j]);
-			recover_orphan_inode(sbi, ino);
+			err = recover_orphan_inode(sbi, ino);
+			if (err) {
+				f2fs_put_page(page, 1);
+				clear_sbi_flag(sbi, SBI_POR_DOING);
+				return err;
+			}
 		}
 		f2fs_put_page(page, 1);
 	}
 	/* clear Orphan Flag */
 	clear_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG);
 	clear_sbi_flag(sbi, SBI_POR_DOING);
-	return;
+	return 0;
 }
 
 static void write_orphan_inodes(struct f2fs_sb_info *sbi, block_t start_blk)

fs/f2fs/f2fs.h +1 -1

index f18d31e..d0e9b70 100644
@@ -1748,7 +1748,7 @@ int acquire_orphan_inode(struct f2fs_sb_info *);
 void release_orphan_inode(struct f2fs_sb_info *);
 void add_orphan_inode(struct f2fs_sb_info *, nid_t);
 void remove_orphan_inode(struct f2fs_sb_info *, nid_t);
-void recover_orphan_inodes(struct f2fs_sb_info *);
+int recover_orphan_inodes(struct f2fs_sb_info *);
 int get_valid_checkpoint(struct f2fs_sb_info *);
 void update_dirty_page(struct inode *, struct page *);
 void add_dirty_dir_inode(struct inode *);

fs/f2fs/super.c +3 -1

index a79b6b5..4db5cd9 100644
@@ -1245,7 +1245,9 @@ try_onemore:
 	f2fs_join_shrinker(sbi);
 
 	/* if there are nt orphan nodes free them */
-	recover_orphan_inodes(sbi);
+	err = recover_orphan_inodes(sbi);
+	if (err)
+		goto free_node_inode;
 
 	/* read root inode and dentry */
 	root = f2fs_iget(sb, F2FS_ROOT_INO(sbi));

Signature · 1 line

2.4.2