On Thu, May 06, 2021 at 05:20:50PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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;
> +
> + if (!pag->pagf_init) {
> + error = xfs_alloc_pagf_init(mp, *tpp, agno, flags);
> + if (error)
> + goto nextag;
> + }
> +
> /*
> - * Do a first racy fast path check if this AG is usable.
> + * Check that there is enough free space for the file plus a
> + * chunk of inodes if we need to allocate some. If this is the
> + * first pass across the AGs, take into account the potential
> + * space needed for alignment of inode chunks when checking the
> + * longest contiguous free space in the AG - this prevents us
> + * from getting ENOSPC because we have free space larger than
> + * ialloc_blks but alignment constraints prevent us from using
> + * it.
> + *
> + * If we can't find an AG with space for full alignment slack to
> + * be taken into account, we must be near ENOSPC in all AGs.
> + * Hence we don't include alignment for the second pass and so
> + * if we fail allocation due to alignment issues then it is most
> + * likely a real ENOSPC condition.
> */
> - if (!pag->pagi_freecount && !okalloc)
> + ineed = M_IGEO(mp)->ialloc_min_blks;
> + if (flags && ineed > 1)
> + ineed += M_IGEO(mp)->cluster_align;
> + longest = pag->pagf_longest;
> + if (!longest)
> + longest = pag->pagf_flcount > 0;
> +
> + if (pag->pagf_freeblks < needspace + ineed || longest < ineed)
> goto nextag;
... and here we check for enough free space in the AG for chunk
allocation purposes. The pagi_freecount check is further down, however,
so it looks like we can skip the AG even if pagi_freecount > 0 and
allocation is not necessary.
Brian
>
> /*
> @@ -1823,10 +1745,17 @@ xfs_dialloc_select_ag(
> nextag_relse_buffer:
> xfs_trans_brelse(*tpp, agbp);
> nextag:
> - if (++agno == mp->m_sb.sb_agcount)
> - agno = 0;
> - if (agno == start_agno)
> + if (XFS_FORCED_SHUTDOWN(mp)) {
> + error = -EFSCORRUPTED;
> break;
> + }
> + if (++agno == mp->m_maxagi)
> + agno = 0;
> + if (agno == start_agno) {
> + if (!flags)
> + break;
> + flags = 0;
> + }
> xfs_perag_put(pag);
> }
>
> --
> 2.31.1
>
