Re: [PATCH] xfs_repair: estimate per-AG btree slack better

[Carlos Maiolino <cem@kernel.org>]

On Wed, May 03, 2023 at 08:07:03AM -0700, Darrick J. Wong wrote:
> On Wed, May 03, 2023 at 10:48:32AM +0200, Carlos Maiolino wrote:
> > On Tue, May 02, 2023 at 08:38:16AM -0700, Darrick J. Wong wrote:
> > > On Tue, May 02, 2023 at 12:49:44PM +0200, Carlos Maiolino wrote:
> > > > Hi.
> > > >
> > > > On Thu, Apr 27, 2023 at 03:45:21PM -0700, Darrick J. Wong wrote:
> > > > > From: Darrick J. Wong <djwong@kernel.org>
> > > > >
> > > > > The slack calculation for per-AG btrees is a bit inaccurate because it
> > > > > only disables slack space in the new btrees when the amount of free
> > > > > space in the AG (not counting the btrees) is less than 3/32ths of the
> > > > > AG.  In other words, it assumes that the btrees will fit in less than 9
> > > > > percent of the space.
> > > > .
> > > > .
> > > > .
> > > > >
> > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > >
> > > > This looks fine, with a small caveat below...
> > > >
> > > > .
> > > > .
> > > > .
> > > >
> > > > > +
> > > > > +static xfs_extlen_t
> > > > > +estimate_allocbt_blocks(
> > > > > +	struct xfs_perag	*pag,
> > > > > +	unsigned int		nr_extents)
> > > > > +{
> > > > > +	return libxfs_allocbt_calc_size(pag->pag_mount, nr_extents) * 2;
> > > > > +}
> > > >
> > > > Forgive my ignorance here, but what's the reason of the magic number? It seems
> > > > to me by multiplying by 2 here, you are considering a split of every single
> > > > leaf for the calculated btree size, but I'm not sure if that's the intention,
> > > > could you please confirm or correct me? :)
> > >
> > > Ah, I should document that better...
> > >
> > > 	/* Account for space consumed by both free space btrees */
> > > 	return libxfs_allocbt_calc_size(...) * 2;
> >
> > Thanks, can I update your patch with the above comment, or do you want to send
> > it again?
> 
> You can add it, if that'll save time.  I don't have any other changes
> pending for that patch.

Ok, won't take much of my time and will prevent you to need to send it again.

> 
> --D
> 
> > >
> > > --D
> > >
> > > > Other than that, the patch looks good
> > > >
> > > > Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
> > > >
> > > > --
> > > > Carlos Maiolino
> >
> > --
> > Carlos Maiolino

-- 
Carlos Maiolino