On Mon, Mar 22, 2021 at 07:28:07AM -0400, Brian Foster wrote:
> On Fri, Mar 05, 2021 at 10:57:00AM +0800, Gao Xiang wrote:
> > Move out related logic for initializing new added AGs to a new helper
> > in preparation for shrinking. No logic changes.
> >
> > Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > Signed-off-by: Gao Xiang <hsiangkao@xxxxxxxxxx>
> > ---
> > fs/xfs/xfs_fsops.c | 107 +++++++++++++++++++++++++++------------------
> > 1 file changed, 64 insertions(+), 43 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> > index 9f9ba8bd0213..fc9e799b2ae3 100644
> > --- a/fs/xfs/xfs_fsops.c
> > +++ b/fs/xfs/xfs_fsops.c
> > @@ -20,6 +20,64 @@
> > #include "xfs_ag.h"
> > #include "xfs_ag_resv.h"
> >
> > +/*
> > + * Write new AG headers to disk. Non-transactional, but need to be
> > + * written and completed prior to the growfs transaction being logged.
> > + * To do this, we use a delayed write buffer list and wait for
> > + * submission and IO completion of the list as a whole. This allows the
> > + * IO subsystem to merge all the AG headers in a single AG into a single
> > + * IO and hide most of the latency of the IO from us.
> > + *
> > + * This also means that if we get an error whilst building the buffer
> > + * list to write, we can cancel the entire list without having written
> > + * anything.
> > + */
> > +static int
> > +xfs_resizefs_init_new_ags(
> > + struct xfs_trans *tp,
> > + struct aghdr_init_data *id,
> > + xfs_agnumber_t oagcount,
> > + xfs_agnumber_t nagcount,
> > + xfs_rfsblock_t delta,
> > + bool *lastag_resetagres)
>
> Nit: I'd just call this lastag_extended or something that otherwise
> indicates what this function reports (as opposed to trying to tell the
> caller what to do).
ok, if other people don't oppose of it.
>
> > +{
> > + struct xfs_mount *mp = tp->t_mountp;
> > + xfs_rfsblock_t nb = mp->m_sb.sb_dblocks + delta;
> > + int error;
> > +
> > + *lastag_resetagres = false;
> > +
> > + INIT_LIST_HEAD(&id->buffer_list);
> > + for (id->agno = nagcount - 1;
> > + id->agno >= oagcount;
> > + id->agno--, delta -= id->agsize) {
> > +
> > + if (id->agno == nagcount - 1)
> > + id->agsize = nb - (id->agno *
> > + (xfs_rfsblock_t)mp->m_sb.sb_agblocks);
> > + else
> > + id->agsize = mp->m_sb.sb_agblocks;
> > +
> > + error = xfs_ag_init_headers(mp, id);
> > + if (error) {
> > + xfs_buf_delwri_cancel(&id->buffer_list);
> > + return error;
> > + }
> > + }
> > +
> > + error = xfs_buf_delwri_submit(&id->buffer_list);
> > + if (error)
> > + return error;
> > +
> > + xfs_trans_agblocks_delta(tp, id->nfree);
> > +
> > + if (delta) {
> > + *lastag_resetagres = true;
> > + error = xfs_ag_extend_space(mp, tp, id, delta);
> > + }
> > + return error;
> > +}
> > +
> > /*
> > * growfs operations
> > */
> ...
> > @@ -123,9 +145,8 @@ xfs_growfs_data_private(
> > */
> > if (nagcount > oagcount)
> > xfs_trans_mod_sb(tp, XFS_TRANS_SB_AGCOUNT, nagcount - oagcount);
> > - if (nb > mp->m_sb.sb_dblocks)
> > - xfs_trans_mod_sb(tp, XFS_TRANS_SB_DBLOCKS,
> > - nb - mp->m_sb.sb_dblocks);
> > + if (delta > 0)
> > + xfs_trans_mod_sb(tp, XFS_TRANS_SB_DBLOCKS, delta);
>
> Hm.. isn't delta still unsigned as of this patch?
Not sure if some difference exists, I could update it as "if (delta)",
therefore it seems [PATCH v8 4/5] won't modify this anymore.
Thanks,
Gao Xiang
>
> Brian
>
> > if (id.nfree)
> > xfs_trans_mod_sb(tp, XFS_TRANS_SB_FDBLOCKS, id.nfree);
> >
> > @@ -152,7 +173,7 @@ xfs_growfs_data_private(
> > * If we expanded the last AG, free the per-AG reservation
> > * so we can reinitialize it with the new size.
> > */
> > - if (delta) {
> > + if (lastag_resetagres) {
> > struct xfs_perag *pag;
> >
> > pag = xfs_perag_get(mp, id.agno);
> > --
> > 2.27.0
> >
>
