From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 07/10] xfs: separate secondary sb update in growfs
Date: Fri, 11 May 2018 18:20:45 -0700 [thread overview]
Message-ID: <20180512012045.GT11261@magnolia> (raw)
In-Reply-To: <20180511225107.27171-8-david@fromorbit.com>
On Sat, May 12, 2018 at 08:51:04AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> This happens after all the transactions to update the superblock
> occur, and errors need to be handled slightly differently. Seperate
> out the code into it's own function, and clean up the error goto
> stack in the core growfs code as it is now much simpler.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/xfs/xfs_fsops.c | 159 ++++++++++++++++++++++++++-------------------
> 1 file changed, 92 insertions(+), 67 deletions(-)
>
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 0fd50c68f1b0..69facd4aa7c1 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -398,9 +398,8 @@ xfs_growfs_data_private(
> {
> xfs_agf_t *agf;
> xfs_agi_t *agi;
> - xfs_agnumber_t agno;
> xfs_buf_t *bp;
> - int error, saved_error = 0;
> + int error;
> xfs_agnumber_t nagcount;
> xfs_agnumber_t nagimax = 0;
> xfs_rfsblock_t nb, nb_mod;
> @@ -472,12 +471,12 @@ xfs_growfs_data_private(
> error = xfs_grow_ag_headers(mp, &id);
> if (error) {
> xfs_buf_delwri_cancel(&id.buffer_list);
> - goto error0;
> + goto out_trans_cancel;
> }
> }
> error = xfs_buf_delwri_submit(&id.buffer_list);
> if (error)
> - goto error0;
> + goto out_trans_cancel;
>
> xfs_trans_agblocks_delta(tp, id.nfree);
>
> @@ -491,22 +490,23 @@ xfs_growfs_data_private(
> * Change the agi length.
> */
> error = xfs_ialloc_read_agi(mp, tp, id.agno, &bp);
> - if (error) {
> - goto error0;
> - }
> + if (error)
> + goto out_trans_cancel;
> +
> ASSERT(bp);
> agi = XFS_BUF_TO_AGI(bp);
> be32_add_cpu(&agi->agi_length, new);
> ASSERT(nagcount == oagcount ||
> be32_to_cpu(agi->agi_length) == mp->m_sb.sb_agblocks);
> xfs_ialloc_log_agi(tp, bp, XFS_AGI_LENGTH);
> +
> /*
> * Change agf length.
> */
> error = xfs_alloc_read_agf(mp, tp, id.agno, 0, &bp);
> - if (error) {
> - goto error0;
> - }
> + if (error)
> + goto out_trans_cancel;
> +
> ASSERT(bp);
> agf = XFS_BUF_TO_AGF(bp);
> be32_add_cpu(&agf->agf_length, new);
> @@ -526,13 +526,13 @@ xfs_growfs_data_private(
> be32_to_cpu(agf->agf_length) - new,
> new, &oinfo);
> if (error)
> - goto error0;
> + goto out_trans_cancel;
> error = xfs_free_extent(tp,
> XFS_AGB_TO_FSB(mp, id.agno,
> be32_to_cpu(agf->agf_length) - new),
> new, &oinfo, XFS_AG_RESV_NONE);
> if (error)
> - goto error0;
> + goto out_trans_cancel;
> }
>
> /*
> @@ -569,16 +569,86 @@ xfs_growfs_data_private(
> error = xfs_ag_resv_free(pag);
> xfs_perag_put(pag);
> if (error)
> - goto out;
> + return error;
> }
>
> - /* Reserve AG metadata blocks. */
> + /*
> + * Reserve AG metadata blocks. ENOSPC here does not mean there was a
> + * growfs failure, just that there still isn't space for new user data
> + * after the grow has been run.
> + */
> error = xfs_fs_reserve_ag_blocks(mp);
> - if (error && error != -ENOSPC)
> - goto out;
> + if (error == -ENOSPC)
> + error = 0;
> + return error;
> +
> +out_trans_cancel:
> + xfs_trans_cancel(tp);
> + return error;
> +}
> +
> +static int
> +xfs_growfs_log_private(
> + xfs_mount_t *mp, /* mount point for filesystem */
> + xfs_growfs_log_t *in) /* growfs log input struct */
> +{
> + xfs_extlen_t nb;
> +
> + nb = in->newblocks;
> + if (nb < XFS_MIN_LOG_BLOCKS || nb < XFS_B_TO_FSB(mp, XFS_MIN_LOG_BYTES))
> + return -EINVAL;
> + if (nb == mp->m_sb.sb_logblocks &&
> + in->isint == (mp->m_sb.sb_logstart != 0))
> + return -EINVAL;
> + /*
> + * Moving the log is hard, need new interfaces to sync
> + * the log first, hold off all activity while moving it.
> + * Can have shorter or longer log in the same space,
> + * or transform internal to external log or vice versa.
> + */
> + return -ENOSYS;
> +}
> +
> +static int
> +xfs_growfs_imaxpct(
> + struct xfs_mount *mp,
> + __u32 imaxpct)
> +{
> + struct xfs_trans *tp;
> + int dpct;
> + int error;
> +
> + if (imaxpct > 100)
> + return -EINVAL;
> +
> + error = xfs_trans_alloc(mp, &M_RES(mp)->tr_growdata,
> + XFS_GROWFS_SPACE_RES(mp), 0, XFS_TRANS_RESERVE, &tp);
> + if (error)
> + return error;
> +
> + dpct = imaxpct - mp->m_sb.sb_imax_pct;
> + xfs_trans_mod_sb(tp, XFS_TRANS_SB_IMAXPCT, dpct);
> + xfs_trans_set_sync(tp);
> + return xfs_trans_commit(tp);
> +}
> +
> +/*
> + * After a grow operation, we need to update all the secondary superblocks
> + * to match the new state of the primary. Read/init the superblocks and update
> + * them appropriately.
> + */
> +static int
> +xfs_growfs_update_superblocks(
> + struct xfs_mount *mp,
> + xfs_agnumber_t oagcount)
> +{
> + struct xfs_buf *bp;
> + xfs_agnumber_t agno;
> + int saved_error = 0;
> + int error = 0;
>
> /* update secondary superblocks. */
> - for (agno = 1; agno < nagcount; agno++) {
> + for (agno = 1; agno < mp->m_sb.sb_agcount; agno++) {
> error = 0;
> /*
> * new secondary superblocks need to be zeroed, not read from
> @@ -628,57 +698,7 @@ xfs_growfs_data_private(
> }
> }
>
> - out:
> return saved_error ? saved_error : error;
> -
> - error0:
> - xfs_trans_cancel(tp);
> - return error;
> -}
> -
> -static int
> -xfs_growfs_log_private(
> - xfs_mount_t *mp, /* mount point for filesystem */
> - xfs_growfs_log_t *in) /* growfs log input struct */
> -{
> - xfs_extlen_t nb;
> -
> - nb = in->newblocks;
> - if (nb < XFS_MIN_LOG_BLOCKS || nb < XFS_B_TO_FSB(mp, XFS_MIN_LOG_BYTES))
> - return -EINVAL;
> - if (nb == mp->m_sb.sb_logblocks &&
> - in->isint == (mp->m_sb.sb_logstart != 0))
> - return -EINVAL;
> - /*
> - * Moving the log is hard, need new interfaces to sync
> - * the log first, hold off all activity while moving it.
> - * Can have shorter or longer log in the same space,
> - * or transform internal to external log or vice versa.
> - */
> - return -ENOSYS;
> -}
> -
> -static int
> -xfs_growfs_imaxpct(
> - struct xfs_mount *mp,
> - __u32 imaxpct)
> -{
> - struct xfs_trans *tp;
> - int64_t dpct;
> - int error;
> -
> - if (imaxpct > 100)
> - return -EINVAL;
> -
> - error = xfs_trans_alloc(mp, &M_RES(mp)->tr_growdata,
> - XFS_GROWFS_SPACE_RES(mp), 0, XFS_TRANS_RESERVE, &tp);
> - if (error)
> - return error;
> -
> - dpct = (int64_t)imaxpct - mp->m_sb.sb_imax_pct;
> - xfs_trans_mod_sb(tp, XFS_TRANS_SB_IMAXPCT, dpct);
> - xfs_trans_set_sync(tp);
> - return xfs_trans_commit(tp);
> }
>
> /*
> @@ -691,6 +711,7 @@ xfs_growfs_data(
> struct xfs_mount *mp,
> struct xfs_growfs_data *in)
> {
> + xfs_agnumber_t oagcount;
> int error = 0;
>
> if (!capable(CAP_SYS_ADMIN))
> @@ -705,6 +726,7 @@ xfs_growfs_data(
> goto out_error;
> }
>
> + oagcount = mp->m_sb.sb_agcount;
> if (in->newblocks != mp->m_sb.sb_dblocks) {
> error = xfs_growfs_data_private(mp, in);
> if (error)
> @@ -719,6 +741,9 @@ xfs_growfs_data(
> } else
> mp->m_maxicount = 0;
>
> + /* Update secondary superblocks now the physical grow has completed */
> + error = xfs_growfs_update_superblocks(mp, oagcount);
> +
> out_error:
> /*
> * Increment the generation unconditionally, the error could be from
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2018-05-12 1:20 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-11 22:50 [PATCH v2] xfs: refactor and tablise growfs Dave Chinner
2018-05-11 22:50 ` [PATCH 01/10] xfs: one-shot cached buffers Dave Chinner
2018-05-12 0:24 ` Darrick J. Wong
2018-05-12 2:06 ` Dave Chinner
2018-05-12 2:08 ` Darrick J. Wong
2018-05-11 22:50 ` [PATCH 02/10] xfs: factor out AG header initialisation from growfs core Dave Chinner
2018-05-12 0:36 ` Darrick J. Wong
2018-05-11 22:51 ` [PATCH 03/10] xfs: convert growfs AG header init to use buffer lists Dave Chinner
2018-05-12 0:39 ` Darrick J. Wong
2018-05-11 22:51 ` [PATCH 04/10] xfs: factor ag btree root block initialisation Dave Chinner
2018-05-12 0:48 ` Darrick J. Wong
2018-05-12 2:01 ` Dave Chinner
2018-05-11 22:51 ` [PATCH 05/10] xfs: turn ag header initialisation into a table driven operation Dave Chinner
2018-05-12 0:55 ` Darrick J. Wong
2018-05-12 2:03 ` Dave Chinner
2018-05-12 2:05 ` Darrick J. Wong
2018-05-11 22:51 ` [PATCH 06/10] xfs: make imaxpct changes in growfs separate Dave Chinner
2018-05-12 0:57 ` Darrick J. Wong
2018-05-11 22:51 ` [PATCH 07/10] xfs: separate secondary sb update in growfs Dave Chinner
2018-05-12 1:20 ` Darrick J. Wong [this message]
2018-05-11 22:51 ` [PATCH 08/10] xfs: rework secondary superblock updates " Dave Chinner
2018-05-12 1:47 ` Darrick J. Wong
2018-05-11 22:51 ` [PATCH 09/10] xfs: move growfs core to libxfs Dave Chinner
2018-05-12 1:48 ` Darrick J. Wong
2018-05-11 22:51 ` [PATCH 10/10] xfs: factor the ag length extension code into libxfs Dave Chinner
2018-05-12 1:48 ` Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2018-05-14 4:18 [PATCH 0/10 v3] xfs: refactor and tablise growfs Dave Chinner
2018-05-14 4:19 ` [PATCH 07/10] xfs: separate secondary sb update in growfs Dave Chinner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180512012045.GT11261@magnolia \
--to=darrick.wong@oracle.com \
--cc=david@fromorbit.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).