Re: [PATCH 2/6] xfs: actually set aside enough space to handle a bmbt split

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, Mar 18, 2022 at 08:17:53AM -0400, Brian Foster wrote:
> On Thu, Mar 17, 2022 at 02:21:06PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > The comment for xfs_alloc_set_aside indicates that we want to set aside
> > enough space to handle a bmap btree split.  The code, unfortunately,
> > hardcodes this to 4.
> > 
> > This is incorrect, since file bmap btrees can be taller than that:
> > 
> > xfs_db> btheight bmapbt -n 4294967296 -b 512
> > bmapbt: worst case per 512-byte block: 13 records (leaf) / 13 keyptrs (node)
> > level 0: 4294967296 records, 330382100 blocks
> > level 1: 330382100 records, 25414008 blocks
> > level 2: 25414008 records, 1954924 blocks
> > level 3: 1954924 records, 150379 blocks
> > level 4: 150379 records, 11568 blocks
> > level 5: 11568 records, 890 blocks
> > level 6: 890 records, 69 blocks
> > level 7: 69 records, 6 blocks
> > level 8: 6 records, 1 block
> > 9 levels, 357913945 blocks total
> > 
> > Fix this by using the actual bmap btree maxlevel value for the
> > set-aside.  We subtract one because the root is always in the inode and
> > hence never splits.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  fs/xfs/libxfs/xfs_alloc.c |    7 +++++--
> >  fs/xfs/libxfs/xfs_sb.c    |    2 --
> >  fs/xfs/xfs_mount.c        |    7 +++++++
> >  3 files changed, 12 insertions(+), 4 deletions(-)
> > 
> > 
> ...
> > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> > index bed73e8002a5..9336176dc706 100644
> > --- a/fs/xfs/xfs_mount.c
> > +++ b/fs/xfs/xfs_mount.c
> > @@ -652,6 +652,13 @@ xfs_mountfs(
> >  
> >  	xfs_agbtree_compute_maxlevels(mp);
> >  
> > +	/*
> > +	 * Compute the amount of space to set aside to handle btree splits now
> > +	 * that we have calculated the btree maxlevels.
> > +	 */
> 
> "... to handle btree splits near -ENOSPC ..." ?

Fixed; thanks for the review!

--D

> Otherwise LGTM:
> 
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> 
> > +	mp->m_alloc_set_aside = xfs_alloc_set_aside(mp);
> > +	mp->m_ag_max_usable = xfs_alloc_ag_max_usable(mp);
> > +
> >  	/*
> >  	 * Check if sb_agblocks is aligned at stripe boundary.  If sb_agblocks
> >  	 * is NOT aligned turn off m_dalign since allocator alignment is within
> > 
>