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

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1789 bytes --]



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);
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]