Message ID | 20210305025703.3069469-3-hsiangkao@redhat.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | xfs: support shrinking free space in the last AG | expand |
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@oracle.com> > Signed-off-by: Gao Xiang <hsiangkao@redhat.com> > --- > 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). > +{ > + 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? 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 >
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@oracle.com> > > Signed-off-by: Gao Xiang <hsiangkao@redhat.com> > > --- > > 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 > > >
On Mon, Mar 22, 2021 at 07:53:49PM +0800, Gao Xiang wrote: > 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@oracle.com> > > > Signed-off-by: Gao Xiang <hsiangkao@redhat.com> > > > --- > > > 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 ... > > > @@ -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. > Yeah, not sure it's a problem, it just stands out a bit with it being in a separate patch from the change to the variable type. Brian > 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 > > > > > >
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) +{ + 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 */ @@ -34,6 +92,7 @@ xfs_growfs_data_private( xfs_agnumber_t nagimax = 0; xfs_rfsblock_t nb, nb_div, nb_mod; xfs_rfsblock_t delta; + bool lastag_resetagres; xfs_agnumber_t oagcount; struct xfs_trans *tp; struct aghdr_init_data id = {}; @@ -74,48 +133,11 @@ xfs_growfs_data_private( if (error) return error; - /* - * 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. - */ - 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); - goto out_trans_cancel; - } - } - error = xfs_buf_delwri_submit(&id.buffer_list); + error = xfs_resizefs_init_new_ags(tp, &id, oagcount, nagcount, + delta, &lastag_resetagres); if (error) goto out_trans_cancel; - xfs_trans_agblocks_delta(tp, id.nfree); - - /* If there are new blocks in the old last AG, extend it. */ - if (delta) { - error = xfs_ag_extend_space(mp, tp, &id, delta); - if (error) - goto out_trans_cancel; - } - /* * Update changed superblock fields transactionally. These are not * seen by the rest of the world until the transaction commit applies @@ -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); 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);