| Message ID | 20241014060516.245606-2-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [1/6] xfs: pass the exact range to initialize to xfs_initialize_perag | expand |
On Mon, Oct 14, 2024 at 08:04:50AM +0200, Christoph Hellwig wrote: > Currently only the new agcount is passed to xfs_initialize_perag, which > requires lookups of existing AGs to skip them and complicates error > handling. Also pass the previous agcount so that the range that > xfs_initialize_perag operates on is exactly defined. That way the > extra lookups can be avoided, and error handling can clean up the > exact range from the old count to the last added perag structure. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > --- > fs/xfs/libxfs/xfs_ag.c | 28 ++++++---------------------- > fs/xfs/libxfs/xfs_ag.h | 5 +++-- > fs/xfs/xfs_fsops.c | 18 ++++++++---------- > fs/xfs/xfs_log_recover.c | 5 +++-- > fs/xfs/xfs_mount.c | 4 ++-- > 5 files changed, 22 insertions(+), 38 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c > index 5f0494702e0b55..464f682eab4690 100644 > --- a/fs/xfs/libxfs/xfs_ag.c > +++ b/fs/xfs/libxfs/xfs_ag.c > @@ -296,27 +296,16 @@ xfs_free_unused_perag_range( > int > xfs_initialize_perag( > struct xfs_mount *mp, > - xfs_agnumber_t agcount, > + xfs_agnumber_t old_agcount, > + xfs_agnumber_t new_agcount, What happened to using first/end or whatever terminology here like is done in one of the later patches? I really find old/new unnecessarily confusing in this context. Otherwise looks good: Reviewed-by: Brian Foster <bfoster@redhat.com> > xfs_rfsblock_t dblocks, > xfs_agnumber_t *maxagi) > { > struct xfs_perag *pag; > xfs_agnumber_t index; > - xfs_agnumber_t first_initialised = NULLAGNUMBER; > int error; > > - /* > - * Walk the current per-ag tree so we don't try to initialise AGs > - * that already exist (growfs case). Allocate and insert all the > - * AGs we don't find ready for initialisation. > - */ > - for (index = 0; index < agcount; index++) { > - pag = xfs_perag_get(mp, index); > - if (pag) { > - xfs_perag_put(pag); > - continue; > - } > - > + for (index = old_agcount; index < new_agcount; index++) { > pag = kzalloc(sizeof(*pag), GFP_KERNEL | __GFP_RETRY_MAYFAIL); > if (!pag) { > error = -ENOMEM; > @@ -353,21 +342,17 @@ xfs_initialize_perag( > /* Active ref owned by mount indicates AG is online. */ > atomic_set(&pag->pag_active_ref, 1); > > - /* first new pag is fully initialized */ > - if (first_initialised == NULLAGNUMBER) > - first_initialised = index; > - > /* > * Pre-calculated geometry > */ > - pag->block_count = __xfs_ag_block_count(mp, index, agcount, > + pag->block_count = __xfs_ag_block_count(mp, index, new_agcount, > dblocks); > pag->min_block = XFS_AGFL_BLOCK(mp); > __xfs_agino_range(mp, pag->block_count, &pag->agino_min, > &pag->agino_max); > } > > - index = xfs_set_inode_alloc(mp, agcount); > + index = xfs_set_inode_alloc(mp, new_agcount); > > if (maxagi) > *maxagi = index; > @@ -381,8 +366,7 @@ xfs_initialize_perag( > out_free_pag: > kfree(pag); > out_unwind_new_pags: > - /* unwind any prior newly initialized pags */ > - xfs_free_unused_perag_range(mp, first_initialised, agcount); > + xfs_free_unused_perag_range(mp, old_agcount, index); > return error; > } > > diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h > index d9cccd093b60e0..69fc31e7b84728 100644 > --- a/fs/xfs/libxfs/xfs_ag.h > +++ b/fs/xfs/libxfs/xfs_ag.h > @@ -146,8 +146,9 @@ __XFS_AG_OPSTATE(agfl_needs_reset, AGFL_NEEDS_RESET) > > void xfs_free_unused_perag_range(struct xfs_mount *mp, xfs_agnumber_t agstart, > xfs_agnumber_t agend); > -int xfs_initialize_perag(struct xfs_mount *mp, xfs_agnumber_t agcount, > - xfs_rfsblock_t dcount, xfs_agnumber_t *maxagi); > +int xfs_initialize_perag(struct xfs_mount *mp, xfs_agnumber_t old_agcount, > + xfs_agnumber_t agcount, xfs_rfsblock_t dcount, > + xfs_agnumber_t *maxagi); > int xfs_initialize_perag_data(struct xfs_mount *mp, xfs_agnumber_t agno); > void xfs_free_perag(struct xfs_mount *mp); > > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c > index 3643cc843f6271..de2bf0594cb474 100644 > --- a/fs/xfs/xfs_fsops.c > +++ b/fs/xfs/xfs_fsops.c > @@ -87,6 +87,7 @@ xfs_growfs_data_private( > struct xfs_mount *mp, /* mount point for filesystem */ > struct xfs_growfs_data *in) /* growfs data input struct */ > { > + xfs_agnumber_t oagcount = mp->m_sb.sb_agcount; > struct xfs_buf *bp; > int error; > xfs_agnumber_t nagcount; > @@ -94,7 +95,6 @@ xfs_growfs_data_private( > xfs_rfsblock_t nb, nb_div, nb_mod; > int64_t delta; > bool lastag_extended = false; > - xfs_agnumber_t oagcount; > struct xfs_trans *tp; > struct aghdr_init_data id = {}; > struct xfs_perag *last_pag; > @@ -138,16 +138,14 @@ xfs_growfs_data_private( > if (delta == 0) > return 0; > > - oagcount = mp->m_sb.sb_agcount; > - /* allocate the new per-ag structures */ > - if (nagcount > oagcount) { > - error = xfs_initialize_perag(mp, nagcount, nb, &nagimax); > - if (error) > - return error; > - } else if (nagcount < oagcount) { > - /* TODO: shrinking the entire AGs hasn't yet completed */ > + /* TODO: shrinking the entire AGs hasn't yet completed */ > + if (nagcount < oagcount) > return -EINVAL; > - } > + > + /* allocate the new per-ag structures */ > + error = xfs_initialize_perag(mp, oagcount, nagcount, nb, &nagimax); > + if (error) > + return error; > > if (delta > 0) > error = xfs_trans_alloc(mp, &M_RES(mp)->tr_growdata, > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c > index d6a3ff24c327c3..60d46338f51792 100644 > --- a/fs/xfs/xfs_log_recover.c > +++ b/fs/xfs/xfs_log_recover.c > @@ -3346,6 +3346,7 @@ xlog_do_recover( > struct xfs_mount *mp = log->l_mp; > struct xfs_buf *bp = mp->m_sb_bp; > struct xfs_sb *sbp = &mp->m_sb; > + xfs_agnumber_t orig_agcount = sbp->sb_agcount; > int error; > > trace_xfs_log_recover(log, head_blk, tail_blk); > @@ -3393,8 +3394,8 @@ xlog_do_recover( > /* re-initialise in-core superblock and geometry structures */ > mp->m_features |= xfs_sb_version_to_features(sbp); > xfs_reinit_percpu_counters(mp); > - error = xfs_initialize_perag(mp, sbp->sb_agcount, sbp->sb_dblocks, > - &mp->m_maxagi); > + error = xfs_initialize_perag(mp, orig_agcount, sbp->sb_agcount, > + sbp->sb_dblocks, &mp->m_maxagi); > if (error) { > xfs_warn(mp, "Failed post-recovery per-ag init: %d", error); > return error; > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c > index 1fdd79c5bfa04e..6fa7239a4a01b6 100644 > --- a/fs/xfs/xfs_mount.c > +++ b/fs/xfs/xfs_mount.c > @@ -810,8 +810,8 @@ xfs_mountfs( > /* > * Allocate and initialize the per-ag data. > */ > - error = xfs_initialize_perag(mp, sbp->sb_agcount, mp->m_sb.sb_dblocks, > - &mp->m_maxagi); > + error = xfs_initialize_perag(mp, 0, sbp->sb_agcount, > + mp->m_sb.sb_dblocks, &mp->m_maxagi); > if (error) { > xfs_warn(mp, "Failed per-ag init: %d", error); > goto out_free_dir; > -- > 2.45.2 > >
On Tue, Oct 15, 2024 at 09:11:06AM -0400, Brian Foster wrote: > > xfs_initialize_perag( > > struct xfs_mount *mp, > > - xfs_agnumber_t agcount, > > + xfs_agnumber_t old_agcount, > > + xfs_agnumber_t new_agcount, > > What happened to using first/end or whatever terminology here like is > done in one of the later patches? I really find old/new unnecessarily > confusing in this context. I though you only wanted that for the caller. I can fix it up.
On Tue, Oct 15, 2024 at 09:11:06AM -0400, Brian Foster wrote: > What happened to using first/end or whatever terminology here like is > done in one of the later patches? I really find old/new unnecessarily > confusing in this context. Looking into it first/end is bad because it is the acount and not the indices, i.e. each of them are the last index + 1. So I'll switch to using orig instead of old as inthe caller here, and keep new.
diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c index 5f0494702e0b55..464f682eab4690 100644 --- a/fs/xfs/libxfs/xfs_ag.c +++ b/fs/xfs/libxfs/xfs_ag.c @@ -296,27 +296,16 @@ xfs_free_unused_perag_range( int xfs_initialize_perag( struct xfs_mount *mp, - xfs_agnumber_t agcount, + xfs_agnumber_t old_agcount, + xfs_agnumber_t new_agcount, xfs_rfsblock_t dblocks, xfs_agnumber_t *maxagi) { struct xfs_perag *pag; xfs_agnumber_t index; - xfs_agnumber_t first_initialised = NULLAGNUMBER; int error; - /* - * Walk the current per-ag tree so we don't try to initialise AGs - * that already exist (growfs case). Allocate and insert all the - * AGs we don't find ready for initialisation. - */ - for (index = 0; index < agcount; index++) { - pag = xfs_perag_get(mp, index); - if (pag) { - xfs_perag_put(pag); - continue; - } - + for (index = old_agcount; index < new_agcount; index++) { pag = kzalloc(sizeof(*pag), GFP_KERNEL | __GFP_RETRY_MAYFAIL); if (!pag) { error = -ENOMEM; @@ -353,21 +342,17 @@ xfs_initialize_perag( /* Active ref owned by mount indicates AG is online. */ atomic_set(&pag->pag_active_ref, 1); - /* first new pag is fully initialized */ - if (first_initialised == NULLAGNUMBER) - first_initialised = index; - /* * Pre-calculated geometry */ - pag->block_count = __xfs_ag_block_count(mp, index, agcount, + pag->block_count = __xfs_ag_block_count(mp, index, new_agcount, dblocks); pag->min_block = XFS_AGFL_BLOCK(mp); __xfs_agino_range(mp, pag->block_count, &pag->agino_min, &pag->agino_max); } - index = xfs_set_inode_alloc(mp, agcount); + index = xfs_set_inode_alloc(mp, new_agcount); if (maxagi) *maxagi = index; @@ -381,8 +366,7 @@ xfs_initialize_perag( out_free_pag: kfree(pag); out_unwind_new_pags: - /* unwind any prior newly initialized pags */ - xfs_free_unused_perag_range(mp, first_initialised, agcount); + xfs_free_unused_perag_range(mp, old_agcount, index); return error; } diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h index d9cccd093b60e0..69fc31e7b84728 100644 --- a/fs/xfs/libxfs/xfs_ag.h +++ b/fs/xfs/libxfs/xfs_ag.h @@ -146,8 +146,9 @@ __XFS_AG_OPSTATE(agfl_needs_reset, AGFL_NEEDS_RESET) void xfs_free_unused_perag_range(struct xfs_mount *mp, xfs_agnumber_t agstart, xfs_agnumber_t agend); -int xfs_initialize_perag(struct xfs_mount *mp, xfs_agnumber_t agcount, - xfs_rfsblock_t dcount, xfs_agnumber_t *maxagi); +int xfs_initialize_perag(struct xfs_mount *mp, xfs_agnumber_t old_agcount, + xfs_agnumber_t agcount, xfs_rfsblock_t dcount, + xfs_agnumber_t *maxagi); int xfs_initialize_perag_data(struct xfs_mount *mp, xfs_agnumber_t agno); void xfs_free_perag(struct xfs_mount *mp); diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c index 3643cc843f6271..de2bf0594cb474 100644 --- a/fs/xfs/xfs_fsops.c +++ b/fs/xfs/xfs_fsops.c @@ -87,6 +87,7 @@ xfs_growfs_data_private( struct xfs_mount *mp, /* mount point for filesystem */ struct xfs_growfs_data *in) /* growfs data input struct */ { + xfs_agnumber_t oagcount = mp->m_sb.sb_agcount; struct xfs_buf *bp; int error; xfs_agnumber_t nagcount; @@ -94,7 +95,6 @@ xfs_growfs_data_private( xfs_rfsblock_t nb, nb_div, nb_mod; int64_t delta; bool lastag_extended = false; - xfs_agnumber_t oagcount; struct xfs_trans *tp; struct aghdr_init_data id = {}; struct xfs_perag *last_pag; @@ -138,16 +138,14 @@ xfs_growfs_data_private( if (delta == 0) return 0; - oagcount = mp->m_sb.sb_agcount; - /* allocate the new per-ag structures */ - if (nagcount > oagcount) { - error = xfs_initialize_perag(mp, nagcount, nb, &nagimax); - if (error) - return error; - } else if (nagcount < oagcount) { - /* TODO: shrinking the entire AGs hasn't yet completed */ + /* TODO: shrinking the entire AGs hasn't yet completed */ + if (nagcount < oagcount) return -EINVAL; - } + + /* allocate the new per-ag structures */ + error = xfs_initialize_perag(mp, oagcount, nagcount, nb, &nagimax); + if (error) + return error; if (delta > 0) error = xfs_trans_alloc(mp, &M_RES(mp)->tr_growdata, diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index d6a3ff24c327c3..60d46338f51792 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -3346,6 +3346,7 @@ xlog_do_recover( struct xfs_mount *mp = log->l_mp; struct xfs_buf *bp = mp->m_sb_bp; struct xfs_sb *sbp = &mp->m_sb; + xfs_agnumber_t orig_agcount = sbp->sb_agcount; int error; trace_xfs_log_recover(log, head_blk, tail_blk); @@ -3393,8 +3394,8 @@ xlog_do_recover( /* re-initialise in-core superblock and geometry structures */ mp->m_features |= xfs_sb_version_to_features(sbp); xfs_reinit_percpu_counters(mp); - error = xfs_initialize_perag(mp, sbp->sb_agcount, sbp->sb_dblocks, - &mp->m_maxagi); + error = xfs_initialize_perag(mp, orig_agcount, sbp->sb_agcount, + sbp->sb_dblocks, &mp->m_maxagi); if (error) { xfs_warn(mp, "Failed post-recovery per-ag init: %d", error); return error; diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c index 1fdd79c5bfa04e..6fa7239a4a01b6 100644 --- a/fs/xfs/xfs_mount.c +++ b/fs/xfs/xfs_mount.c @@ -810,8 +810,8 @@ xfs_mountfs( /* * Allocate and initialize the per-ag data. */ - error = xfs_initialize_perag(mp, sbp->sb_agcount, mp->m_sb.sb_dblocks, - &mp->m_maxagi); + error = xfs_initialize_perag(mp, 0, sbp->sb_agcount, + mp->m_sb.sb_dblocks, &mp->m_maxagi); if (error) { xfs_warn(mp, "Failed per-ag init: %d", error); goto out_free_dir;