[PATCH 0/2] cleanups when walking backref tree

[PATCH 0/2] cleanups when walking backref tree

[Wang Shilong]

From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>

The first patch removes some BUG_ONs() when walking backref tree.
These BUG_ON()s can only be triggered because of ENOMEM, and the caller
has checked the return value about it. So we just return directly rather
than trigger BUG_ON() when out of memory happens.

The second patch just makes some functions' return value be void, Since
these functions always return 0.

Wang Shilong (2):
  Btrfs: remove some BUG_ONs() when walking backref tree
  Btrfs: make some functions return type be void in backref.c

 fs/btrfs/backref.c |   28 ++++++++++++----------------
 1 files changed, 12 insertions(+), 16 deletions(-)

-- 
1.7.7.6

[PATCH 1/2] Btrfs: remove some BUG_ONs() when walking backref tree

[Wang Shilong]

From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>

The only error return value of __add_prelim_ref() is -ENOMEM,
just return errors rather than trigger BUG_ON().

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
---
 fs/btrfs/backref.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index bd605c8..dc200f6 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -582,7 +582,8 @@ static int __add_delayed_refs(struct btrfs_delayed_ref_head *head, u64 seq,
 		default:
 			WARN_ON(1);
 		}
-		BUG_ON(ret);
+		if (ret)
+			return ret;
 	}
 
 	return 0;
@@ -680,7 +681,8 @@ static int __add_inline_refs(struct btrfs_fs_info *fs_info,
 		default:
 			WARN_ON(1);
 		}
-		BUG_ON(ret);
+		if (ret)
+			return ret;
 		ptr += btrfs_extent_inline_ref_size(type);
 	}
 
@@ -762,7 +764,9 @@ static int __add_keyed_refs(struct btrfs_fs_info *fs_info,
 		default:
 			WARN_ON(1);
 		}
-		BUG_ON(ret);
+		if (ret)
+			return ret;
+
 	}
 
 	return ret;
-- 
1.7.7.6

[PATCH 2/2] Btrfs: make some functions return type be void in backref.c

[Wang Shilong]

From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>

__merge_refs() and __add_missing_keys() always return 0, it is unnecessary
for the caller to check the return value.

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
---
 fs/btrfs/backref.c |   18 +++++-------------
 1 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index dc200f6..3521206 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -404,7 +404,7 @@ static inline int ref_for_same_block(struct __prelim_ref *ref1,
 /*
  * read tree blocks and add keys where required.
  */
-static int __add_missing_keys(struct btrfs_fs_info *fs_info,
+static void __add_missing_keys(struct btrfs_fs_info *fs_info,
 			      struct list_head *head)
 {
 	struct list_head *pos;
@@ -430,7 +430,6 @@ static int __add_missing_keys(struct btrfs_fs_info *fs_info,
 		btrfs_tree_read_unlock(eb);
 		free_extent_buffer(eb);
 	}
-	return 0;
 }
 
 /*
@@ -443,7 +442,7 @@ static int __add_missing_keys(struct btrfs_fs_info *fs_info,
  *           having a parent).
  * mode = 2: merge identical parents
  */
-static int __merge_refs(struct list_head *head, int mode)
+static void __merge_refs(struct list_head *head, int mode)
 {
 	struct list_head *pos1;
 
@@ -489,7 +488,6 @@ static int __merge_refs(struct list_head *head, int mode)
 		}
 
 	}
-	return 0;
 }
 
 /*
@@ -880,22 +878,16 @@ again:
 
 	list_splice_init(&prefs_delayed, &prefs);
 
-	ret = __add_missing_keys(fs_info, &prefs);
-	if (ret)
-		goto out;
+	__add_missing_keys(fs_info, &prefs);
 
-	ret = __merge_refs(&prefs, 1);
-	if (ret)
-		goto out;
+	__merge_refs(&prefs, 1);
 
 	ret = __resolve_indirect_refs(fs_info, search_commit_root, time_seq,
 				      &prefs, extent_item_pos);
 	if (ret)
 		goto out;
 
-	ret = __merge_refs(&prefs, 2);
-	if (ret)
-		goto out;
+	__merge_refs(&prefs, 2);
 
 	while (!list_empty(&prefs)) {
 		ref = list_first_entry(&prefs, struct __prelim_ref, list);
-- 
1.7.7.6

Re: [PATCH 1/2] Btrfs: remove some BUG_ONs() when walking backref tree

[David Sterba]

On Wed, Apr 10, 2013 at 07:22:50PM +0800, Wang Shilong wrote:
> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> 
> The only error return value of __add_prelim_ref() is -ENOMEM,
> just return errors rather than trigger BUG_ON().
> 
> Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>

Reviewed-by: David Sterba <dsterba@suse.cz>

Re: [PATCH 2/2] Btrfs: make some functions return type be void in backref.c

[David Sterba]

On Wed, Apr 10, 2013 at 07:22:51PM +0800, Wang Shilong wrote:
> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> 
> __merge_refs() and __add_missing_keys() always return 0, it is unnecessary
> for the caller to check the return value.

ok for __merge_refs, nak for __add_missing_keys: there's unhandled
BUG_ON from read_tree_block

 422                 eb = read_tree_block(fs_info->tree_root, ref->wanted_disk_byte,
 423                                      fs_info->tree_root->leafsize, 0);
 424                 BUG_ON(!eb);

this should become a proper error handling someday and use the int return
value. Keep the callers aware of that.

david

Re: [PATCH 2/2] Btrfs: make some functions return type be void in backref.c

[Wang Shilong]

Hello, David

Patch V2 has been sent out,many thanks for your review ^_^

Thanks,
Wang

> On Wed, Apr 10, 2013 at 07:22:51PM +0800, Wang Shilong wrote:
>> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
>>
>> __merge_refs() and __add_missing_keys() always return 0, it is unnecessary
>> for the caller to check the return value.
> 
> ok for __merge_refs, nak for __add_missing_keys: there's unhandled
> BUG_ON from read_tree_block
> 
>  422                 eb = read_tree_block(fs_info->tree_root, ref->wanted_disk_byte,
>  423                                      fs_info->tree_root->leafsize, 0);
>  424                 BUG_ON(!eb);
> 
> this should become a proper error handling someday and use the int return
> value. Keep the callers aware of that.
> 
> david
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
>