diff mbox

[1/3] btrfs-progs: check: release path in repair_extent_data_item()

Message ID 20171127031353.9338-2-suy.fnst@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Su Yue Nov. 27, 2017, 3:13 a.m. UTC
In repair_extent_data_item(), path is not be released if some
errors occurs which causes extent buffer leak.

So release path in end of the function.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Qu Wenruo Nov. 27, 2017, 10:21 a.m. UTC | #1
On 2017年11月27日 11:13, Su Yue wrote:
> In repair_extent_data_item(), path is not be released if some
> errors occurs which causes extent buffer leak.
> 
> So release path in end of the function.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>  cmds-check.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/cmds-check.c b/cmds-check.c
> index e746ee7b281d..3a72f8e3877d 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -11974,7 +11974,6 @@ static int repair_extent_data_item(struct btrfs_trans_handle *trans,
>  		btrfs_mark_buffer_dirty(eb);
>  		ret = btrfs_update_block_group(trans, extent_root, disk_bytenr,
>  					       num_bytes, 1, 0);
> -		btrfs_release_path(&path);

Here btrfs_release_path() should not be removed.

Lines below this, btrfs_inc_extent_ref() is called.
In which we will alloc a new path and modify extent tree.

Thanks to the fact that there is no tree locking implemented in
btrfs-progs, it won't cause dead lock, but it's a good habit to make
path allocation deadlock free.

So please keep the path release here.

(And if you find some code holding conflicting path on the same tree,
feel free to fix them, even it won't cause real problem in btrfs right now)

>  	}
>  
>  	if (nrefs->full_backref[0])
> @@ -11998,6 +11997,7 @@ static int repair_extent_data_item(struct btrfs_trans_handle *trans,
>  
>  	err &= ~BACKREF_MISSING;
>  out:
> +	btrfs_release_path(&path);

On the other hand, an empty path can be released as many times as you wish.
So double releasing the path in out branch is not a problem.

Thanks,
Q

>  	if (ret)
>  		error("can't repair root %llu extent data item[%llu %llu]",
>  		      root->objectid, disk_bytenr, num_bytes);
>
diff mbox

Patch

diff --git a/cmds-check.c b/cmds-check.c
index e746ee7b281d..3a72f8e3877d 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -11974,7 +11974,6 @@  static int repair_extent_data_item(struct btrfs_trans_handle *trans,
 		btrfs_mark_buffer_dirty(eb);
 		ret = btrfs_update_block_group(trans, extent_root, disk_bytenr,
 					       num_bytes, 1, 0);
-		btrfs_release_path(&path);
 	}
 
 	if (nrefs->full_backref[0])
@@ -11998,6 +11997,7 @@  static int repair_extent_data_item(struct btrfs_trans_handle *trans,
 
 	err &= ~BACKREF_MISSING;
 out:
+	btrfs_release_path(&path);
 	if (ret)
 		error("can't repair root %llu extent data item[%llu %llu]",
 		      root->objectid, disk_bytenr, num_bytes);