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

Dave Chinner
2021-05-18T01:21:21Z

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