On Fri, Apr 17, 2020 at 04:36:15PM +0100, fdmanana@xxxxxxxxxx wrote: > From: Filipe Manana <fdmanana@xxxxxxxx> > > When cleaning pinned extents right before deleting an unused block group, > we check if there's still a previous transaction running and if so we > increment its reference count before using it for cleaning pinned ranges > in its pinned extents iotree. However we ended up never decrementing the > reference count after using the transaction, resulting in a memory leak. > > Fix it by decrementing the reference count. > > Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx> > --- > > V2: Use btrfs_put_transaction() and not refcount_dec(), otherwise we are > not really releasing the memory used by the transaction in case its > refcount is 1. Stupid mistake. I missed it too. As Nik pointed out on IRC, the API should be consistent so that for put there's a get, or refcount_inc/refcount_dec. In this case there's non-trivial work on the put side, so the wrappers make sense. A good example may be btrfs_get_block_group/btrfs_put_block_group, same as for the transaction. Trivial wrappers around plain _inc/_dec wouldn't be so great, but in case there's eg refcount_dec_and_test + kfree, then I think this can be considered a good pattern.
