On Tue, Dec 08, 2020 at 03:09:13PM -0800, Darrick J. Wong wrote:
> On Tue, Dec 08, 2020 at 08:19:59PM +0800, Gao Xiang wrote:
...
> >
> > +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
Yeah, it sounds good to me, thanks for reminder & help!
Thanks,
Gao Xiang
>
> 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
> >
>
