Re: [PATCH] Btrfs: add missing extents release on file extent cluster relocation error

[Johannes Thumshirn <jthumshirn@suse.de>]

On 09/10/2019 18:43, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> If we error out when finding a page at relocate_file_extent_cluster(), we
> need to release the outstanding extents counter on the relocation inode,
> set by the previous call to btrfs_delalloc_reserve_metadata(), otherwise
> the inode's block reserve size can never decrease to zero and metadata
> space is leaked. Therefore add a call to btrfs_delalloc_release_extents()
> in case we can't find the target page.
> 
> Fixes: 8b62f87bad9cf0 ("Btrfs: rework outstanding_extents")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  fs/btrfs/relocation.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 00504657b602..88dbc0127793 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -3277,6 +3277,8 @@ static int relocate_file_extent_cluster(struct inode *inode,
>  			if (!page) {
>  				btrfs_delalloc_release_metadata(BTRFS_I(inode),
>  							PAGE_SIZE, true);
> +				btrfs_delalloc_release_extents(BTRFS_I(inode),
> +							       PAGE_SIZE, true);

Hmm how about adding a wrapper to combine these two calls similar to
what btrfs_delalloc_release_space() is doing for
btrfs_delalloc_release_metadata() and btrfs_free_reserved_data_space()?

I count at least 3 other occurences of this pattern with a simple
git grep -C 4 btrfs_delalloc_release_metadata fs/btrfs/ | \
   grep btrfs_delalloc_release_extents

One of them being in the same function.

Otherwise
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 247165, AG München)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850