On Tue, Dec 08, 2020 at 08:19:59PM +0800, Gao Xiang wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Introduce a helper to make the on-disk inode allocation rolling
> logic clearer in preparation of the following cleanup.
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Signed-off-by: Gao Xiang <hsiangkao@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_ialloc.c | 43 ++++++++++++++++++++++++++++++++++++++
> fs/xfs/libxfs/xfs_ialloc.h | 5 +++++
> fs/xfs/xfs_inode.c | 37 +-------------------------------
> 3 files changed, 49 insertions(+), 36 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 45cf7e55f5ee..23e94d43acb2 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -1682,6 +1682,49 @@ xfs_dialloc_ag(
> return error;
> }
>
> +int
> +xfs_dialloc_roll(
> + struct xfs_trans **tpp,
> + struct xfs_buf *agibp)
> +{
> + struct xfs_trans *tp = *tpp;
> + struct xfs_dquot_acct *dqinfo = NULL;
> + unsigned int tflags = 0;
> + int error;
> +
> + /*
> + * Hold to on to the agibp across the commit so no other allocation can
> + * come in and take the free inodes we just allocated for our caller.
> + */
> + xfs_trans_bhold(tp, agibp);
> +
> + /*
> + * We want the quota changes to be associated with the next transaction,
> + * NOT this one. So, detach the dqinfo from this and attach it to the
> + * next transaction.
> + */
> + if (tp->t_dqinfo) {
> + dqinfo = tp->t_dqinfo;
> + tp->t_dqinfo = NULL;
> + tflags = tp->t_flags & XFS_TRANS_DQ_DIRTY;
> + tp->t_flags &= ~XFS_TRANS_DQ_DIRTY;
FWIW, one of xiakaixu's cleanup patches removes XFS_TRANS_DQ_DIRTY on
the grounds that there seemed to be a 1:1 correlation between the dqinfo
being set and the flag being set. That creates a minor merge conflict
that I can fix at commit time. The rest looks fine, so
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
--D
> + }
> +
> + error = xfs_trans_roll(&tp);
> +
> + /* Re-attach the quota info that we detached from prev trx. */
> + if (dqinfo) {
> + tp->t_dqinfo = dqinfo;
> + tp->t_flags |= tflags;
> + }
> +
> + *tpp = tp;
> + if (error)
> + return error;
> + xfs_trans_bjoin(tp, agibp);
> + return 0;
> +}
> +
> /*
> * Allocate an inode on disk.
> *
> diff --git a/fs/xfs/libxfs/xfs_ialloc.h b/fs/xfs/libxfs/xfs_ialloc.h
> index 72b3468b97b1..bd6e0db9e23c 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.h
> +++ b/fs/xfs/libxfs/xfs_ialloc.h
> @@ -32,6 +32,11 @@ xfs_make_iptr(struct xfs_mount *mp, struct xfs_buf *b, int o)
> return xfs_buf_offset(b, o << (mp)->m_sb.sb_inodelog);
> }
>
> +int
> +xfs_dialloc_roll(
> + struct xfs_trans **tpp,
> + struct xfs_buf *agibp);
> +
> /*
> * Allocate an inode on disk.
> * Mode is used to tell whether the new inode will need space, and whether
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 2bfbcf28b1bd..76282da7a05c 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -958,8 +958,6 @@ xfs_dir_ialloc(
> xfs_inode_t *ip;
> xfs_buf_t *ialloc_context = NULL;
> int code;
> - void *dqinfo;
> - uint tflags;
>
> tp = *tpp;
> ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES);
> @@ -1003,46 +1001,13 @@ xfs_dir_ialloc(
> * to succeed the second time.
> */
> if (ialloc_context) {
> - /*
> - * Normally, xfs_trans_commit releases all the locks.
> - * We call bhold to hang on to the ialloc_context across
> - * the commit. Holding this buffer prevents any other
> - * processes from doing any allocations in this
> - * allocation group.
> - */
> - xfs_trans_bhold(tp, ialloc_context);
> -
> - /*
> - * We want the quota changes to be associated with the next
> - * transaction, NOT this one. So, detach the dqinfo from this
> - * and attach it to the next transaction.
> - */
> - dqinfo = NULL;
> - tflags = 0;
> - if (tp->t_dqinfo) {
> - dqinfo = (void *)tp->t_dqinfo;
> - tp->t_dqinfo = NULL;
> - tflags = tp->t_flags & XFS_TRANS_DQ_DIRTY;
> - tp->t_flags &= ~(XFS_TRANS_DQ_DIRTY);
> - }
> -
> - code = xfs_trans_roll(&tp);
> -
> - /*
> - * Re-attach the quota info that we detached from prev trx.
> - */
> - if (dqinfo) {
> - tp->t_dqinfo = dqinfo;
> - tp->t_flags |= tflags;
> - }
> -
> + code = xfs_dialloc_roll(&tp, ialloc_context);
> if (code) {
> xfs_buf_relse(ialloc_context);
> *tpp = tp;
> *ipp = NULL;
> return code;
> }
> - xfs_trans_bjoin(tp, ialloc_context);
>
> /*
> * Call ialloc again. Since we've locked out all
> --
> 2.18.4
>
