Re: [PATCH 3/5] btrfs: free qgroup pertrans rsv on trans abort

[David Sterba <dsterba@suse.cz>]

On Tue, Dec 05, 2023 at 11:45:52AM -0800, Boris Burkov wrote:
> On Tue, Dec 05, 2023 at 03:27:32PM +0100, David Sterba wrote:
> > On Fri, Dec 01, 2023 at 01:00:11PM -0800, Boris Burkov wrote:
> > > If we abort a transaction, we never run the code that frees the pertrans
> > > qgroup reservation. This results in warnings on unmount as that
> > > reservation has been leaked. The leak isn't a huge issue since the fs is
> > > read-only, but it's better to clean it up when we know we can/should. Do
> > > it during the cleanup_transaction step of aborting.
> > > 
> > > Signed-off-by: Boris Burkov <boris@bur.io>
> > > ---
> > >  fs/btrfs/disk-io.c | 28 ++++++++++++++++++++++++++++
> > >  fs/btrfs/qgroup.c  |  5 +++--
> > >  2 files changed, 31 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > > index 9317606017e2..a1f440cd6d45 100644
> > > --- a/fs/btrfs/disk-io.c
> > > +++ b/fs/btrfs/disk-io.c
> > > @@ -4775,6 +4775,32 @@ void btrfs_cleanup_dirty_bgs(struct btrfs_transaction *cur_trans,
> > >  	}
> > >  }
> > >  
> > > +static void btrfs_free_all_qgroup_pertrans(struct btrfs_fs_info *fs_info)
> > > +{
> > > +	struct btrfs_root *gang[8];
> > > +	int i;
> > > +	int ret;
> > > +
> > > +	spin_lock(&fs_info->fs_roots_radix_lock);
> > > +	while (1) {
> > > +		ret = radix_tree_gang_lookup_tag(&fs_info->fs_roots_radix,
> > > +						 (void **)gang, 0,
> > > +						 ARRAY_SIZE(gang),
> > > +						 0); // BTRFS_ROOT_TRANS_TAG
> > 
> > What does the comment mean?
> 
> Oops, I forgot about this. BTRFS_ROOT_TRANS_TAG is a #define in
> transaction.c, so it wasn't visible here in disk-io.c. I should move it
> to some header they both include. Based on the other stuff in there,
> btrfs/fs.h looks reasonable?

Maybe transaction.h