Message ID | 20200417144021.9319-1-fdmanana@kernel.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/2] Btrfs: fix memory leak of transaction when deleting unused block group | expand |
On Fri, Apr 17, 2020 at 03:40:21PM +0100, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > At clean_pinned_extents(), whether we end up returning success or failure, > we pretty much have to do the same things: > > 1) unlock unused_bg_unpin_mutex > 2) decrement reference count on the previous transaction > > We also call btrfs_dec_block_group_ro() in case of failure, but that is > better done in its caller, btrfs_delete_unused_bgs(), since its the > caller that calls inc_block_group_ro(), so it should be responsible for > the decrement operation, as it is in case any of the other functions it > calls fail. > > So move the call to btrfs_dec_block_group_ro() from clean_pinned_extents() > into btrfs_delete_unused_bgs() and unify the error and success return > paths for clean_pinned_extents(), reducing duplicated code and making it > simpler. > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Added to misc-next, thanks.
On 17.04.20 г. 17:40 ч., fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > At clean_pinned_extents(), whether we end up returning success or failure, > we pretty much have to do the same things: > > 1) unlock unused_bg_unpin_mutex > 2) decrement reference count on the previous transaction > > We also call btrfs_dec_block_group_ro() in case of failure, but that is > better done in its caller, btrfs_delete_unused_bgs(), since its the > caller that calls inc_block_group_ro(), so it should be responsible for > the decrement operation, as it is in case any of the other functions it > calls fail. > > So move the call to btrfs_dec_block_group_ro() from clean_pinned_extents() > into btrfs_delete_unused_bgs() and unify the error and success return > paths for clean_pinned_extents(), reducing duplicated code and making it > simpler. > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c index 93c180ffcb80..a615d8585df4 100644 --- a/fs/btrfs/block-group.c +++ b/fs/btrfs/block-group.c @@ -1280,25 +1280,17 @@ static bool clean_pinned_extents(struct btrfs_trans_handle *trans, ret = clear_extent_bits(&prev_trans->pinned_extents, start, end, EXTENT_DIRTY); if (ret) - goto err; + goto out; } ret = clear_extent_bits(&trans->transaction->pinned_extents, start, end, EXTENT_DIRTY); - if (ret) - goto err; +out: mutex_unlock(&fs_info->unused_bg_unpin_mutex); if (prev_trans) refcount_dec(&prev_trans->use_count); - return true; - -err: - mutex_unlock(&fs_info->unused_bg_unpin_mutex); - if (prev_trans) - refcount_dec(&prev_trans->use_count); - btrfs_dec_block_group_ro(bg); - return false; + return ret == 0; } /* @@ -1396,8 +1388,10 @@ void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info) * We could have pending pinned extents for this block group, * just delete them, we don't care about them anymore. */ - if (!clean_pinned_extents(trans, block_group)) + if (!clean_pinned_extents(trans, block_group)) { + btrfs_dec_block_group_ro(block_group); goto end_trans; + } /* * At this point, the block_group is read only and should fail