Re: [PATCH 18/22] xfs: collapse AG selection for inode allocation

[Dave Chinner <david@fromorbit.com>]

On Wed, May 12, 2021 at 08:52:25AM -0400, Brian Foster wrote:
> On Thu, May 06, 2021 at 05:20:50PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > xfs_dialloc_select_ag() does a lot of repetitive work. It first
> > calls xfs_ialloc_ag_select() to select the AG to start allocation
> > attempts in, which can do up to two entire loops across the perags
> > that inodes can be allocated in. This is simply checking if there is
> > spce available to allocate inodes in an AG, and it returns when it
> > finds the first candidate AG.
> > 
> > xfs_dialloc_select_ag() then does it's own iterative walk across
> > all the perags locking the AGIs and trying to allocate inodes from
> > the locked AG. It also doesn't limit the search to mp->m_maxagi,
> > so it will walk all AGs whether they can allocate inodes or not.
> > 
> > Hence if we are really low on inodes, we could do almost 3 entire
> > walks across the whole perag range before we find an allocation
> > group we can allocate inodes in or report ENOSPC.
> > 
> > Because xfs_ialloc_ag_select() returns on the first candidate AG it
> > finds, we can simply do these checks directly in
> > xfs_dialloc_select_ag() before we lock and try to allocate inodes.
> > This reduces the inode allocation pass down to 2 perag sweeps at
> > most - one for aligned inode cluster allocation and if we can't
> > allocate full, aligned inode clusters anywhere we'll do another pass
> > trying to do sparse inode cluster allocation.
> > 
> > This also removes a big chunk of duplicate code.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ---
> >  fs/xfs/libxfs/xfs_ialloc.c | 221 +++++++++++++------------------------
> >  1 file changed, 75 insertions(+), 146 deletions(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> > index 872591e8f5cb..b22556556bba 100644
> > --- a/fs/xfs/libxfs/xfs_ialloc.c
> > +++ b/fs/xfs/libxfs/xfs_ialloc.c
> ...
> > @@ -1778,10 +1669,41 @@ xfs_dialloc_select_ag(
> >  				break;
> >  		}
> >  
> > +		if (!pag->pagi_freecount)
> > +			goto nextag;
> 
> It looks like this would never allow for allocation of new inode
> chunks..?
> 
> > +		if (!okalloc)
> > +			goto nextag;

I guess I never tested this patch in isolation, and I fixed the bug
in one of the following patches. This should be:

		if (!pag->pagi_freecount && !okalloc)
			goto nextag;

Fixed.

-Dave.
-- 
Dave Chinner
david@fromorbit.com