Message ID | 1601126073-32453-4-git-send-email-kaixuxia@tencent.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | xfs: random fixes for disk quota | expand |
On 9/26/20 8:14 AM, xiakaixu1987@gmail.com wrote: > From: Kaixu Xia <kaixuxia@tencent.com> > > Nowadays the only things that the XFS_TRANS_DQ_DIRTY flag seems to do > are indicates the tp->t_dqinfo->dqs[XFS_QM_TRANS_{USR,GRP,PRJ}] values > changed and check in xfs_trans_apply_dquot_deltas() and the unreserve > variant xfs_trans_unreserve_and_mod_dquots(). Actually, we also can > use the tp->t_dqinfo value instead of the XFS_TRANS_DQ_DIRTY flag, that > is to say, we allocate the new tp->t_dqinfo only when the qtrx values > changed, so the tp->t_dqinfo value isn't NULL equals the XFS_TRANS_DQ_DIRTY > flag is set, we only need to check if tp->t_dqinfo == NULL in > xfs_trans_apply_dquot_deltas() and its unreserve variant to determine > whether lock all of the dquots and join them to the transaction. > > Signed-off-by: Kaixu Xia <kaixuxia@tencent.com> > --- > fs/xfs/libxfs/xfs_shared.h | 1 - > fs/xfs/xfs_inode.c | 8 +------- > fs/xfs/xfs_trans_dquot.c | 20 ++------------------ > 3 files changed, 3 insertions(+), 26 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h > index c795ae47b3c9..8c61a461bf7b 100644 > --- a/fs/xfs/libxfs/xfs_shared.h > +++ b/fs/xfs/libxfs/xfs_shared.h > @@ -62,7 +62,6 @@ void xfs_log_get_max_trans_res(struct xfs_mount *mp, > #define XFS_TRANS_SB_DIRTY 0x02 /* superblock is modified */ > #define XFS_TRANS_PERM_LOG_RES 0x04 /* xact took a permanent log res */ > #define XFS_TRANS_SYNC 0x08 /* make commit synchronous */ > -#define XFS_TRANS_DQ_DIRTY 0x10 /* at least one dquot in trx dirty */ > #define XFS_TRANS_RESERVE 0x20 /* OK to use reserved data blocks */ > #define XFS_TRANS_NO_WRITECOUNT 0x40 /* do not elevate SB writecount */ > #define XFS_TRANS_RES_FDBLKS 0x80 /* reserve newly freed blocks */ > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > index 49624973eecc..9108eed0ea45 100644 > --- a/fs/xfs/xfs_inode.c > +++ b/fs/xfs/xfs_inode.c > @@ -941,7 +941,6 @@ xfs_dir_ialloc( > xfs_buf_t *ialloc_context = NULL; > int code; > void *dqinfo; > - uint tflags; > > tp = *tpp; > ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES); > @@ -1000,12 +999,9 @@ xfs_dir_ialloc( > * 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); > @@ -1013,10 +1009,8 @@ xfs_dir_ialloc( > /* > * Re-attach the quota info that we detached from prev trx. > */ > - if (dqinfo) { > + if (dqinfo) > tp->t_dqinfo = dqinfo; > - tp->t_flags |= tflags; > - } > > if (code) { > xfs_buf_relse(ialloc_context); > diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c > index fe45b0c3970c..0ebfd7930382 100644 > --- a/fs/xfs/xfs_trans_dquot.c > +++ b/fs/xfs/xfs_trans_dquot.c > @@ -84,13 +84,6 @@ xfs_trans_dup_dqinfo( > > xfs_trans_alloc_dqinfo(ntp); > > - /* > - * Because the quota blk reservation is carried forward, > - * it is also necessary to carry forward the DQ_DIRTY flag. > - */ > - if (otp->t_flags & XFS_TRANS_DQ_DIRTY) > - ntp->t_flags |= XFS_TRANS_DQ_DIRTY; > - > for (j = 0; j < XFS_QM_TRANS_DQTYPES; j++) { > oqa = otp->t_dqinfo->dqs[j]; > nqa = ntp->t_dqinfo->dqs[j]; > @@ -143,9 +136,6 @@ xfs_trans_mod_dquot_byino( > xfs_is_quota_inode(&mp->m_sb, ip->i_ino)) > return; > > - if (tp->t_dqinfo == NULL) > - xfs_trans_alloc_dqinfo(tp); > - I can't tell from the commit log or from a very quick read of the code why these allocations are being removed. Can we not get here with a NULL t_dqinfo? If not, why not? This seems like a change unrelated to the proposed "t_dqinfo set == XFS_TRANS_DQ_DIRTY" change. Also, while it seems clear to say that !t_dqinfo == !XFS_TRANS_DQ_DIRTY, is the converse true? Is it possible to have t_dqinfo set, but it's not dirty? I think the answer is that when we free the transaction we set t_dqinfo to NULL again, but I'm not certain, and it's not obvious from the changelog... -Eric
On 2020/9/26 23:36, Eric Sandeen wrote: > On 9/26/20 8:14 AM, xiakaixu1987@gmail.com wrote: >> From: Kaixu Xia <kaixuxia@tencent.com> >> >> Nowadays the only things that the XFS_TRANS_DQ_DIRTY flag seems to do >> are indicates the tp->t_dqinfo->dqs[XFS_QM_TRANS_{USR,GRP,PRJ}] values >> changed and check in xfs_trans_apply_dquot_deltas() and the unreserve >> variant xfs_trans_unreserve_and_mod_dquots(). Actually, we also can >> use the tp->t_dqinfo value instead of the XFS_TRANS_DQ_DIRTY flag, that >> is to say, we allocate the new tp->t_dqinfo only when the qtrx values >> changed, so the tp->t_dqinfo value isn't NULL equals the XFS_TRANS_DQ_DIRTY >> flag is set, we only need to check if tp->t_dqinfo == NULL in >> xfs_trans_apply_dquot_deltas() and its unreserve variant to determine >> whether lock all of the dquots and join them to the transaction. >> >> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com> >> --- >> fs/xfs/libxfs/xfs_shared.h | 1 - >> fs/xfs/xfs_inode.c | 8 +------- >> fs/xfs/xfs_trans_dquot.c | 20 ++------------------ >> 3 files changed, 3 insertions(+), 26 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h >> index c795ae47b3c9..8c61a461bf7b 100644 >> --- a/fs/xfs/libxfs/xfs_shared.h >> +++ b/fs/xfs/libxfs/xfs_shared.h >> @@ -62,7 +62,6 @@ void xfs_log_get_max_trans_res(struct xfs_mount *mp, >> #define XFS_TRANS_SB_DIRTY 0x02 /* superblock is modified */ >> #define XFS_TRANS_PERM_LOG_RES 0x04 /* xact took a permanent log res */ >> #define XFS_TRANS_SYNC 0x08 /* make commit synchronous */ >> -#define XFS_TRANS_DQ_DIRTY 0x10 /* at least one dquot in trx dirty */ >> #define XFS_TRANS_RESERVE 0x20 /* OK to use reserved data blocks */ >> #define XFS_TRANS_NO_WRITECOUNT 0x40 /* do not elevate SB writecount */ >> #define XFS_TRANS_RES_FDBLKS 0x80 /* reserve newly freed blocks */ >> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c >> index 49624973eecc..9108eed0ea45 100644 >> --- a/fs/xfs/xfs_inode.c >> +++ b/fs/xfs/xfs_inode.c >> @@ -941,7 +941,6 @@ xfs_dir_ialloc( >> xfs_buf_t *ialloc_context = NULL; >> int code; >> void *dqinfo; >> - uint tflags; >> >> tp = *tpp; >> ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES); >> @@ -1000,12 +999,9 @@ xfs_dir_ialloc( >> * 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); >> @@ -1013,10 +1009,8 @@ xfs_dir_ialloc( >> /* >> * Re-attach the quota info that we detached from prev trx. >> */ >> - if (dqinfo) { >> + if (dqinfo) >> tp->t_dqinfo = dqinfo; >> - tp->t_flags |= tflags; >> - } >> >> if (code) { >> xfs_buf_relse(ialloc_context); >> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c >> index fe45b0c3970c..0ebfd7930382 100644 >> --- a/fs/xfs/xfs_trans_dquot.c >> +++ b/fs/xfs/xfs_trans_dquot.c >> @@ -84,13 +84,6 @@ xfs_trans_dup_dqinfo( >> >> xfs_trans_alloc_dqinfo(ntp); >> >> - /* >> - * Because the quota blk reservation is carried forward, >> - * it is also necessary to carry forward the DQ_DIRTY flag. >> - */ >> - if (otp->t_flags & XFS_TRANS_DQ_DIRTY) >> - ntp->t_flags |= XFS_TRANS_DQ_DIRTY; >> - >> for (j = 0; j < XFS_QM_TRANS_DQTYPES; j++) { >> oqa = otp->t_dqinfo->dqs[j]; >> nqa = ntp->t_dqinfo->dqs[j]; >> @@ -143,9 +136,6 @@ xfs_trans_mod_dquot_byino( >> xfs_is_quota_inode(&mp->m_sb, ip->i_ino)) >> return; >> >> - if (tp->t_dqinfo == NULL) >> - xfs_trans_alloc_dqinfo(tp); >> - > > I can't tell from the commit log or from a very quick read of the code why these > allocations are being removed. Can we not get here with a NULL t_dqinfo? > If not, why not? This seems like a change unrelated to the proposed > "t_dqinfo set == XFS_TRANS_DQ_DIRTY" change. Yeah, remove these allocations because I want to allocate the t_dqinfo only when the tp->t_dqinfo->dqs[XFS_QM_TRANS_{USR,GRP,PRJ}] values changed, that is to say, only do the allocation in xfs_trans_mod_dquot() function. Actually, original these allocations are repeated, for example, the xfs_trans_mod_dquot_byino() function call the xfs_trans_mod_dquot(), but both of them do the allocation, so remove one of them may be reasonable. > > Also, while it seems clear to say that !t_dqinfo == !XFS_TRANS_DQ_DIRTY, is the > converse true? Is it possible to have t_dqinfo set, but it's not dirty?> > I think the answer is that when we free the transaction we set t_dqinfo to > NULL again, but I'm not certain, and it's not obvious from the changelog... Now we do the allocation in xfs_trans_mod_dquot() function and only have t_dqinfo set when it is dirty. Thanks, Kaixu > > -Eric >
On Sat, Sep 26, 2020 at 09:14:32PM +0800, xiakaixu1987@gmail.com wrote: > From: Kaixu Xia <kaixuxia@tencent.com> > > Nowadays the only things that the XFS_TRANS_DQ_DIRTY flag seems to do > are indicates the tp->t_dqinfo->dqs[XFS_QM_TRANS_{USR,GRP,PRJ}] values > changed and check in xfs_trans_apply_dquot_deltas() and the unreserve > variant xfs_trans_unreserve_and_mod_dquots(). Actually, we also can > use the tp->t_dqinfo value instead of the XFS_TRANS_DQ_DIRTY flag, that > is to say, we allocate the new tp->t_dqinfo only when the qtrx values > changed, so the tp->t_dqinfo value isn't NULL equals the XFS_TRANS_DQ_DIRTY > flag is set, we only need to check if tp->t_dqinfo == NULL in > xfs_trans_apply_dquot_deltas() and its unreserve variant to determine > whether lock all of the dquots and join them to the transaction. > > Signed-off-by: Kaixu Xia <kaixuxia@tencent.com> > --- > fs/xfs/libxfs/xfs_shared.h | 1 - > fs/xfs/xfs_inode.c | 8 +------- > fs/xfs/xfs_trans_dquot.c | 20 ++------------------ > 3 files changed, 3 insertions(+), 26 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h > index c795ae47b3c9..8c61a461bf7b 100644 > --- a/fs/xfs/libxfs/xfs_shared.h > +++ b/fs/xfs/libxfs/xfs_shared.h > @@ -62,7 +62,6 @@ void xfs_log_get_max_trans_res(struct xfs_mount *mp, > #define XFS_TRANS_SB_DIRTY 0x02 /* superblock is modified */ > #define XFS_TRANS_PERM_LOG_RES 0x04 /* xact took a permanent log res */ > #define XFS_TRANS_SYNC 0x08 /* make commit synchronous */ > -#define XFS_TRANS_DQ_DIRTY 0x10 /* at least one dquot in trx dirty */ > #define XFS_TRANS_RESERVE 0x20 /* OK to use reserved data blocks */ > #define XFS_TRANS_NO_WRITECOUNT 0x40 /* do not elevate SB writecount */ > #define XFS_TRANS_RES_FDBLKS 0x80 /* reserve newly freed blocks */ > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > index 49624973eecc..9108eed0ea45 100644 > --- a/fs/xfs/xfs_inode.c > +++ b/fs/xfs/xfs_inode.c > @@ -941,7 +941,6 @@ xfs_dir_ialloc( > xfs_buf_t *ialloc_context = NULL; > int code; > void *dqinfo; > - uint tflags; > > tp = *tpp; > ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES); > @@ -1000,12 +999,9 @@ xfs_dir_ialloc( > * 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); > @@ -1013,10 +1009,8 @@ xfs_dir_ialloc( > /* > * Re-attach the quota info that we detached from prev trx. > */ > - if (dqinfo) { > + if (dqinfo) > tp->t_dqinfo = dqinfo; > - tp->t_flags |= tflags; > - } > > if (code) { > xfs_buf_relse(ialloc_context); > diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c > index fe45b0c3970c..0ebfd7930382 100644 > --- a/fs/xfs/xfs_trans_dquot.c > +++ b/fs/xfs/xfs_trans_dquot.c > @@ -84,13 +84,6 @@ xfs_trans_dup_dqinfo( > > xfs_trans_alloc_dqinfo(ntp); > > - /* > - * Because the quota blk reservation is carried forward, > - * it is also necessary to carry forward the DQ_DIRTY flag. > - */ > - if (otp->t_flags & XFS_TRANS_DQ_DIRTY) > - ntp->t_flags |= XFS_TRANS_DQ_DIRTY; > - > for (j = 0; j < XFS_QM_TRANS_DQTYPES; j++) { > oqa = otp->t_dqinfo->dqs[j]; > nqa = ntp->t_dqinfo->dqs[j]; > @@ -143,9 +136,6 @@ xfs_trans_mod_dquot_byino( > xfs_is_quota_inode(&mp->m_sb, ip->i_ino)) > return; > > - if (tp->t_dqinfo == NULL) > - xfs_trans_alloc_dqinfo(tp); This change ought to be a separate clean patch. > - > if (XFS_IS_UQUOTA_ON(mp) && ip->i_udquot) > (void) xfs_trans_mod_dquot(tp, ip->i_udquot, field, delta); > if (XFS_IS_GQUOTA_ON(mp) && ip->i_gdquot) > @@ -273,8 +263,6 @@ xfs_trans_mod_dquot( > > if (delta) > trace_xfs_trans_mod_dquot_after(qtrx); > - > - tp->t_flags |= XFS_TRANS_DQ_DIRTY; > } > > > @@ -351,7 +339,7 @@ xfs_trans_apply_dquot_deltas( > int64_t totalbdelta; > int64_t totalrtbdelta; > > - if (!(tp->t_flags & XFS_TRANS_DQ_DIRTY)) > + if (!tp->t_dqinfo) > return; > > ASSERT(tp->t_dqinfo); > @@ -493,7 +481,7 @@ xfs_trans_unreserve_and_mod_dquots( > struct xfs_dqtrx *qtrx, *qa; > bool locked; > > - if (!tp->t_dqinfo || !(tp->t_flags & XFS_TRANS_DQ_DIRTY)) > + if (!tp->t_dqinfo) > return; > > for (j = 0; j < XFS_QM_TRANS_DQTYPES; j++) { > @@ -698,7 +686,6 @@ xfs_trans_dqresv( > * because we don't have the luxury of a transaction envelope then. > */ > if (tp) { > - ASSERT(tp->t_dqinfo); > ASSERT(flags & XFS_QMOPT_RESBLK_MASK); > if (nblks != 0) > xfs_trans_mod_dquot(tp, dqp, > @@ -752,9 +739,6 @@ xfs_trans_reserve_quota_bydquots( > if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp)) > return 0; > > - if (tp && tp->t_dqinfo == NULL) > - xfs_trans_alloc_dqinfo(tp); Um, who allocates the dqinfo in this case? --D > - > ASSERT(flags & XFS_QMOPT_RESBLK_MASK); > > if (udqp) { > -- > 2.20.0 >
On 2020/10/6 12:42, Darrick J. Wong wrote: > On Sat, Sep 26, 2020 at 09:14:32PM +0800, xiakaixu1987@gmail.com wrote: >> From: Kaixu Xia <kaixuxia@tencent.com> >> >> Nowadays the only things that the XFS_TRANS_DQ_DIRTY flag seems to do >> are indicates the tp->t_dqinfo->dqs[XFS_QM_TRANS_{USR,GRP,PRJ}] values >> changed and check in xfs_trans_apply_dquot_deltas() and the unreserve >> variant xfs_trans_unreserve_and_mod_dquots(). Actually, we also can >> use the tp->t_dqinfo value instead of the XFS_TRANS_DQ_DIRTY flag, that >> is to say, we allocate the new tp->t_dqinfo only when the qtrx values >> changed, so the tp->t_dqinfo value isn't NULL equals the XFS_TRANS_DQ_DIRTY >> flag is set, we only need to check if tp->t_dqinfo == NULL in >> xfs_trans_apply_dquot_deltas() and its unreserve variant to determine >> whether lock all of the dquots and join them to the transaction. >> >> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com> >> --- >> fs/xfs/libxfs/xfs_shared.h | 1 - >> fs/xfs/xfs_inode.c | 8 +------- >> fs/xfs/xfs_trans_dquot.c | 20 ++------------------ >> 3 files changed, 3 insertions(+), 26 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h >> index c795ae47b3c9..8c61a461bf7b 100644 >> --- a/fs/xfs/libxfs/xfs_shared.h >> +++ b/fs/xfs/libxfs/xfs_shared.h >> @@ -62,7 +62,6 @@ void xfs_log_get_max_trans_res(struct xfs_mount *mp, >> #define XFS_TRANS_SB_DIRTY 0x02 /* superblock is modified */ >> #define XFS_TRANS_PERM_LOG_RES 0x04 /* xact took a permanent log res */ >> #define XFS_TRANS_SYNC 0x08 /* make commit synchronous */ >> -#define XFS_TRANS_DQ_DIRTY 0x10 /* at least one dquot in trx dirty */ >> #define XFS_TRANS_RESERVE 0x20 /* OK to use reserved data blocks */ >> #define XFS_TRANS_NO_WRITECOUNT 0x40 /* do not elevate SB writecount */ >> #define XFS_TRANS_RES_FDBLKS 0x80 /* reserve newly freed blocks */ >> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c >> index 49624973eecc..9108eed0ea45 100644 >> --- a/fs/xfs/xfs_inode.c >> +++ b/fs/xfs/xfs_inode.c >> @@ -941,7 +941,6 @@ xfs_dir_ialloc( >> xfs_buf_t *ialloc_context = NULL; >> int code; >> void *dqinfo; >> - uint tflags; >> >> tp = *tpp; >> ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES); >> @@ -1000,12 +999,9 @@ xfs_dir_ialloc( >> * 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); >> @@ -1013,10 +1009,8 @@ xfs_dir_ialloc( >> /* >> * Re-attach the quota info that we detached from prev trx. >> */ >> - if (dqinfo) { >> + if (dqinfo) >> tp->t_dqinfo = dqinfo; >> - tp->t_flags |= tflags; >> - } >> >> if (code) { >> xfs_buf_relse(ialloc_context); >> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c >> index fe45b0c3970c..0ebfd7930382 100644 >> --- a/fs/xfs/xfs_trans_dquot.c >> +++ b/fs/xfs/xfs_trans_dquot.c >> @@ -84,13 +84,6 @@ xfs_trans_dup_dqinfo( >> >> xfs_trans_alloc_dqinfo(ntp); >> >> - /* >> - * Because the quota blk reservation is carried forward, >> - * it is also necessary to carry forward the DQ_DIRTY flag. >> - */ >> - if (otp->t_flags & XFS_TRANS_DQ_DIRTY) >> - ntp->t_flags |= XFS_TRANS_DQ_DIRTY; >> - >> for (j = 0; j < XFS_QM_TRANS_DQTYPES; j++) { >> oqa = otp->t_dqinfo->dqs[j]; >> nqa = ntp->t_dqinfo->dqs[j]; >> @@ -143,9 +136,6 @@ xfs_trans_mod_dquot_byino( >> xfs_is_quota_inode(&mp->m_sb, ip->i_ino)) >> return; >> >> - if (tp->t_dqinfo == NULL) >> - xfs_trans_alloc_dqinfo(tp); > > This change ought to be a separate clean patch. Yeah, I'll do that in the next version. > >> - >> if (XFS_IS_UQUOTA_ON(mp) && ip->i_udquot) >> (void) xfs_trans_mod_dquot(tp, ip->i_udquot, field, delta); >> if (XFS_IS_GQUOTA_ON(mp) && ip->i_gdquot) >> @@ -273,8 +263,6 @@ xfs_trans_mod_dquot( >> >> if (delta) >> trace_xfs_trans_mod_dquot_after(qtrx); >> - >> - tp->t_flags |= XFS_TRANS_DQ_DIRTY; >> } >> >> >> @@ -351,7 +339,7 @@ xfs_trans_apply_dquot_deltas( >> int64_t totalbdelta; >> int64_t totalrtbdelta; >> >> - if (!(tp->t_flags & XFS_TRANS_DQ_DIRTY)) >> + if (!tp->t_dqinfo) >> return; >> >> ASSERT(tp->t_dqinfo); >> @@ -493,7 +481,7 @@ xfs_trans_unreserve_and_mod_dquots( >> struct xfs_dqtrx *qtrx, *qa; >> bool locked; >> >> - if (!tp->t_dqinfo || !(tp->t_flags & XFS_TRANS_DQ_DIRTY)) >> + if (!tp->t_dqinfo) >> return; >> >> for (j = 0; j < XFS_QM_TRANS_DQTYPES; j++) { >> @@ -698,7 +686,6 @@ xfs_trans_dqresv( >> * because we don't have the luxury of a transaction envelope then. >> */ >> if (tp) { >> - ASSERT(tp->t_dqinfo); >> ASSERT(flags & XFS_QMOPT_RESBLK_MASK); >> if (nblks != 0) >> xfs_trans_mod_dquot(tp, dqp, >> @@ -752,9 +739,6 @@ xfs_trans_reserve_quota_bydquots( >> if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp)) >> return 0; >> >> - if (tp && tp->t_dqinfo == NULL) >> - xfs_trans_alloc_dqinfo(tp); > > Um, who allocates the dqinfo in this case? The function call chain here is xfs_trans_reserve_quota_bydquots()-> xfs_trans_dqresv()->xfs_trans_mod_dquot()(tp is non-null), thus the dqinfo will be allocated in xfs_trans_mod_dquot() function. Actually, now we do the allocation only in xfs_trans_mod_dquot() function when the tp->t_dqinfo->dqs[XFS_QM_TRANS_{USR,GRP,PRJ}] values changed. Thanks, Kaixu > > --D > >> - >> ASSERT(flags & XFS_QMOPT_RESBLK_MASK); >> >> if (udqp) { >> -- >> 2.20.0 >>
diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h index c795ae47b3c9..8c61a461bf7b 100644 --- a/fs/xfs/libxfs/xfs_shared.h +++ b/fs/xfs/libxfs/xfs_shared.h @@ -62,7 +62,6 @@ void xfs_log_get_max_trans_res(struct xfs_mount *mp, #define XFS_TRANS_SB_DIRTY 0x02 /* superblock is modified */ #define XFS_TRANS_PERM_LOG_RES 0x04 /* xact took a permanent log res */ #define XFS_TRANS_SYNC 0x08 /* make commit synchronous */ -#define XFS_TRANS_DQ_DIRTY 0x10 /* at least one dquot in trx dirty */ #define XFS_TRANS_RESERVE 0x20 /* OK to use reserved data blocks */ #define XFS_TRANS_NO_WRITECOUNT 0x40 /* do not elevate SB writecount */ #define XFS_TRANS_RES_FDBLKS 0x80 /* reserve newly freed blocks */ diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index 49624973eecc..9108eed0ea45 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -941,7 +941,6 @@ xfs_dir_ialloc( xfs_buf_t *ialloc_context = NULL; int code; void *dqinfo; - uint tflags; tp = *tpp; ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES); @@ -1000,12 +999,9 @@ xfs_dir_ialloc( * 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); @@ -1013,10 +1009,8 @@ xfs_dir_ialloc( /* * Re-attach the quota info that we detached from prev trx. */ - if (dqinfo) { + if (dqinfo) tp->t_dqinfo = dqinfo; - tp->t_flags |= tflags; - } if (code) { xfs_buf_relse(ialloc_context); diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c index fe45b0c3970c..0ebfd7930382 100644 --- a/fs/xfs/xfs_trans_dquot.c +++ b/fs/xfs/xfs_trans_dquot.c @@ -84,13 +84,6 @@ xfs_trans_dup_dqinfo( xfs_trans_alloc_dqinfo(ntp); - /* - * Because the quota blk reservation is carried forward, - * it is also necessary to carry forward the DQ_DIRTY flag. - */ - if (otp->t_flags & XFS_TRANS_DQ_DIRTY) - ntp->t_flags |= XFS_TRANS_DQ_DIRTY; - for (j = 0; j < XFS_QM_TRANS_DQTYPES; j++) { oqa = otp->t_dqinfo->dqs[j]; nqa = ntp->t_dqinfo->dqs[j]; @@ -143,9 +136,6 @@ xfs_trans_mod_dquot_byino( xfs_is_quota_inode(&mp->m_sb, ip->i_ino)) return; - if (tp->t_dqinfo == NULL) - xfs_trans_alloc_dqinfo(tp); - if (XFS_IS_UQUOTA_ON(mp) && ip->i_udquot) (void) xfs_trans_mod_dquot(tp, ip->i_udquot, field, delta); if (XFS_IS_GQUOTA_ON(mp) && ip->i_gdquot) @@ -273,8 +263,6 @@ xfs_trans_mod_dquot( if (delta) trace_xfs_trans_mod_dquot_after(qtrx); - - tp->t_flags |= XFS_TRANS_DQ_DIRTY; } @@ -351,7 +339,7 @@ xfs_trans_apply_dquot_deltas( int64_t totalbdelta; int64_t totalrtbdelta; - if (!(tp->t_flags & XFS_TRANS_DQ_DIRTY)) + if (!tp->t_dqinfo) return; ASSERT(tp->t_dqinfo); @@ -493,7 +481,7 @@ xfs_trans_unreserve_and_mod_dquots( struct xfs_dqtrx *qtrx, *qa; bool locked; - if (!tp->t_dqinfo || !(tp->t_flags & XFS_TRANS_DQ_DIRTY)) + if (!tp->t_dqinfo) return; for (j = 0; j < XFS_QM_TRANS_DQTYPES; j++) { @@ -698,7 +686,6 @@ xfs_trans_dqresv( * because we don't have the luxury of a transaction envelope then. */ if (tp) { - ASSERT(tp->t_dqinfo); ASSERT(flags & XFS_QMOPT_RESBLK_MASK); if (nblks != 0) xfs_trans_mod_dquot(tp, dqp, @@ -752,9 +739,6 @@ xfs_trans_reserve_quota_bydquots( if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp)) return 0; - if (tp && tp->t_dqinfo == NULL) - xfs_trans_alloc_dqinfo(tp); - ASSERT(flags & XFS_QMOPT_RESBLK_MASK); if (udqp) {