| Message ID | 20210506072054.271157-18-david@fromorbit.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: initial agnumber -> perag conversions for shrink | expand |
On Thu, May 06, 2021 at 05:20:49PM +1000, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > The only caller of xfs_dialloc_select_ag() will always return > -ENOSPC to it's caller if the agbp returned from > xfs_dialloc_select_ag() is NULL. IOWs, failure to find a candidate > AGI we can allocate inodes from is always an ENOSPC condition, so > move this logic up into xfs_dialloc_select_ag() so we can simplify > the return logic in this function. > > xfs_dialloc_select_ag() now only ever returns 0 with a locked > agbp, or an error with no agbp. > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > --- Reviewed-by: Brian Foster <bfoster@redhat.com> > fs/xfs/libxfs/xfs_ialloc.c | 23 ++++++++--------------- > fs/xfs/xfs_inode.c | 3 --- > 2 files changed, 8 insertions(+), 18 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c > index 4540fbcd68a3..872591e8f5cb 100644 > --- a/fs/xfs/libxfs/xfs_ialloc.c > +++ b/fs/xfs/libxfs/xfs_ialloc.c > @@ -1717,7 +1717,7 @@ xfs_dialloc_roll( > * This function will ensure that the selected AG has free inodes available to > * allocate from. The selected AGI will be returned locked to the caller, and it > * will allocate more free inodes if required. If no free inodes are found or > - * can be allocated, no AGI will be returned. > + * can be allocated, -ENOSPC be returned. > */ > int > xfs_dialloc_select_ag( > @@ -1730,7 +1730,6 @@ xfs_dialloc_select_ag( > struct xfs_buf *agbp; > xfs_agnumber_t agno; > int error; > - bool noroom = false; > xfs_agnumber_t start_agno; > struct xfs_perag *pag; > struct xfs_ino_geometry *igeo = M_IGEO(mp); > @@ -1744,7 +1743,7 @@ xfs_dialloc_select_ag( > */ > start_agno = xfs_ialloc_ag_select(*tpp, parent, mode); > if (start_agno == NULLAGNUMBER) > - return 0; > + return -ENOSPC; > > /* > * If we have already hit the ceiling of inode blocks then clear > @@ -1757,7 +1756,6 @@ xfs_dialloc_select_ag( > if (igeo->maxicount && > percpu_counter_read_positive(&mp->m_icount) + igeo->ialloc_inos > > igeo->maxicount) { > - noroom = true; > okalloc = false; > } > > @@ -1794,10 +1792,8 @@ xfs_dialloc_select_ag( > if (error) > break; > > - if (pag->pagi_freecount) { > - xfs_perag_put(pag); > + if (pag->pagi_freecount) > goto found_ag; > - } > > if (!okalloc) > goto nextag_relse_buffer; > @@ -1805,9 +1801,6 @@ xfs_dialloc_select_ag( > error = xfs_ialloc_ag_alloc(*tpp, agbp, pag); > if (error < 0) { > xfs_trans_brelse(*tpp, agbp); > - > - if (error == -ENOSPC) > - error = 0; > break; > } > > @@ -1818,12 +1811,11 @@ xfs_dialloc_select_ag( > * allocate one of the new inodes. > */ > ASSERT(pag->pagi_freecount > 0); > - xfs_perag_put(pag); > > error = xfs_dialloc_roll(tpp, agbp); > if (error) { > xfs_buf_relse(agbp); > - return error; > + break; > } > goto found_ag; > } > @@ -1831,16 +1823,17 @@ xfs_dialloc_select_ag( > nextag_relse_buffer: > xfs_trans_brelse(*tpp, agbp); > nextag: > - xfs_perag_put(pag); > if (++agno == mp->m_sb.sb_agcount) > agno = 0; > if (agno == start_agno) > - return noroom ? -ENOSPC : 0; > + break; > + xfs_perag_put(pag); > } > > xfs_perag_put(pag); > - return error; > + return error ? error : -ENOSPC; > found_ag: > + xfs_perag_put(pag); > *IO_agbp = agbp; > return 0; > } > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > index 25910b145d70..3918c99fa95b 100644 > --- a/fs/xfs/xfs_inode.c > +++ b/fs/xfs/xfs_inode.c > @@ -923,9 +923,6 @@ xfs_dir_ialloc( > if (error) > return error; > > - if (!agibp) > - return -ENOSPC; > - > /* Allocate an inode from the selected AG */ > error = xfs_dialloc_ag(*tpp, agibp, parent_ino, &ino); > if (error) > -- > 2.31.1 >
On Thu, May 06, 2021 at 05:20:49PM +1000, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > The only caller of xfs_dialloc_select_ag() will always return > -ENOSPC to it's caller if the agbp returned from > xfs_dialloc_select_ag() is NULL. IOWs, failure to find a candidate > AGI we can allocate inodes from is always an ENOSPC condition, so > move this logic up into xfs_dialloc_select_ag() so we can simplify > the return logic in this function. > > xfs_dialloc_select_ag() now only ever returns 0 with a locked > agbp, or an error with no agbp. > > Signed-off-by: Dave Chinner <dchinner@redhat.com> Woo nice cleanup! Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/xfs/libxfs/xfs_ialloc.c | 23 ++++++++--------------- > fs/xfs/xfs_inode.c | 3 --- > 2 files changed, 8 insertions(+), 18 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c > index 4540fbcd68a3..872591e8f5cb 100644 > --- a/fs/xfs/libxfs/xfs_ialloc.c > +++ b/fs/xfs/libxfs/xfs_ialloc.c > @@ -1717,7 +1717,7 @@ xfs_dialloc_roll( > * This function will ensure that the selected AG has free inodes available to > * allocate from. The selected AGI will be returned locked to the caller, and it > * will allocate more free inodes if required. If no free inodes are found or > - * can be allocated, no AGI will be returned. > + * can be allocated, -ENOSPC be returned. > */ > int > xfs_dialloc_select_ag( > @@ -1730,7 +1730,6 @@ xfs_dialloc_select_ag( > struct xfs_buf *agbp; > xfs_agnumber_t agno; > int error; > - bool noroom = false; > xfs_agnumber_t start_agno; > struct xfs_perag *pag; > struct xfs_ino_geometry *igeo = M_IGEO(mp); > @@ -1744,7 +1743,7 @@ xfs_dialloc_select_ag( > */ > start_agno = xfs_ialloc_ag_select(*tpp, parent, mode); > if (start_agno == NULLAGNUMBER) > - return 0; > + return -ENOSPC; > > /* > * If we have already hit the ceiling of inode blocks then clear > @@ -1757,7 +1756,6 @@ xfs_dialloc_select_ag( > if (igeo->maxicount && > percpu_counter_read_positive(&mp->m_icount) + igeo->ialloc_inos > > igeo->maxicount) { > - noroom = true; > okalloc = false; > } > > @@ -1794,10 +1792,8 @@ xfs_dialloc_select_ag( > if (error) > break; > > - if (pag->pagi_freecount) { > - xfs_perag_put(pag); > + if (pag->pagi_freecount) > goto found_ag; > - } > > if (!okalloc) > goto nextag_relse_buffer; > @@ -1805,9 +1801,6 @@ xfs_dialloc_select_ag( > error = xfs_ialloc_ag_alloc(*tpp, agbp, pag); > if (error < 0) { > xfs_trans_brelse(*tpp, agbp); > - > - if (error == -ENOSPC) > - error = 0; > break; > } > > @@ -1818,12 +1811,11 @@ xfs_dialloc_select_ag( > * allocate one of the new inodes. > */ > ASSERT(pag->pagi_freecount > 0); > - xfs_perag_put(pag); > > error = xfs_dialloc_roll(tpp, agbp); > if (error) { > xfs_buf_relse(agbp); > - return error; > + break; > } > goto found_ag; > } > @@ -1831,16 +1823,17 @@ xfs_dialloc_select_ag( > nextag_relse_buffer: > xfs_trans_brelse(*tpp, agbp); > nextag: > - xfs_perag_put(pag); > if (++agno == mp->m_sb.sb_agcount) > agno = 0; > if (agno == start_agno) > - return noroom ? -ENOSPC : 0; > + break; > + xfs_perag_put(pag); > } > > xfs_perag_put(pag); > - return error; > + return error ? error : -ENOSPC; > found_ag: > + xfs_perag_put(pag); > *IO_agbp = agbp; > return 0; > } > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > index 25910b145d70..3918c99fa95b 100644 > --- a/fs/xfs/xfs_inode.c > +++ b/fs/xfs/xfs_inode.c > @@ -923,9 +923,6 @@ xfs_dir_ialloc( > if (error) > return error; > > - if (!agibp) > - return -ENOSPC; > - > /* Allocate an inode from the selected AG */ > error = xfs_dialloc_ag(*tpp, agibp, parent_ino, &ino); > if (error) > -- > 2.31.1 >
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c index 4540fbcd68a3..872591e8f5cb 100644 --- a/fs/xfs/libxfs/xfs_ialloc.c +++ b/fs/xfs/libxfs/xfs_ialloc.c @@ -1717,7 +1717,7 @@ xfs_dialloc_roll( * This function will ensure that the selected AG has free inodes available to * allocate from. The selected AGI will be returned locked to the caller, and it * will allocate more free inodes if required. If no free inodes are found or - * can be allocated, no AGI will be returned. + * can be allocated, -ENOSPC be returned. */ int xfs_dialloc_select_ag( @@ -1730,7 +1730,6 @@ xfs_dialloc_select_ag( struct xfs_buf *agbp; xfs_agnumber_t agno; int error; - bool noroom = false; xfs_agnumber_t start_agno; struct xfs_perag *pag; struct xfs_ino_geometry *igeo = M_IGEO(mp); @@ -1744,7 +1743,7 @@ xfs_dialloc_select_ag( */ start_agno = xfs_ialloc_ag_select(*tpp, parent, mode); if (start_agno == NULLAGNUMBER) - return 0; + return -ENOSPC; /* * If we have already hit the ceiling of inode blocks then clear @@ -1757,7 +1756,6 @@ xfs_dialloc_select_ag( if (igeo->maxicount && percpu_counter_read_positive(&mp->m_icount) + igeo->ialloc_inos > igeo->maxicount) { - noroom = true; okalloc = false; } @@ -1794,10 +1792,8 @@ xfs_dialloc_select_ag( if (error) break; - if (pag->pagi_freecount) { - xfs_perag_put(pag); + if (pag->pagi_freecount) goto found_ag; - } if (!okalloc) goto nextag_relse_buffer; @@ -1805,9 +1801,6 @@ xfs_dialloc_select_ag( error = xfs_ialloc_ag_alloc(*tpp, agbp, pag); if (error < 0) { xfs_trans_brelse(*tpp, agbp); - - if (error == -ENOSPC) - error = 0; break; } @@ -1818,12 +1811,11 @@ xfs_dialloc_select_ag( * allocate one of the new inodes. */ ASSERT(pag->pagi_freecount > 0); - xfs_perag_put(pag); error = xfs_dialloc_roll(tpp, agbp); if (error) { xfs_buf_relse(agbp); - return error; + break; } goto found_ag; } @@ -1831,16 +1823,17 @@ xfs_dialloc_select_ag( nextag_relse_buffer: xfs_trans_brelse(*tpp, agbp); nextag: - xfs_perag_put(pag); if (++agno == mp->m_sb.sb_agcount) agno = 0; if (agno == start_agno) - return noroom ? -ENOSPC : 0; + break; + xfs_perag_put(pag); } xfs_perag_put(pag); - return error; + return error ? error : -ENOSPC; found_ag: + xfs_perag_put(pag); *IO_agbp = agbp; return 0; } diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index 25910b145d70..3918c99fa95b 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -923,9 +923,6 @@ xfs_dir_ialloc( if (error) return error; - if (!agibp) - return -ENOSPC; - /* Allocate an inode from the selected AG */ error = xfs_dialloc_ag(*tpp, agibp, parent_ino, &ino); if (error)