Message ID | 1602819508-29033-2-git-send-email-kaixuxia@tencent.com (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
Series | xfs: random fixes for disk quota | expand |
On Fri, Oct 16, 2020 at 11:38:26AM +0800, xiakaixu1987@gmail.com wrote: > From: Kaixu Xia <kaixuxia@tencent.com> > > The function xfs_trans_mod_dquot_byino() wraps around > xfs_trans_mod_dquot() to account for quotas, and also there is the > function call chain xfs_trans_reserve_quota_bydquots -> xfs_trans_dqresv > -> xfs_trans_mod_dquot, both of them do the duplicated null check and > allocation. Thus we can delete the duplicated operation from them. > > Signed-off-by: Kaixu Xia <kaixuxia@tencent.com> > Reviewed-by: Brian Foster <bfoster@redhat.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> HAH this got all the way to v6, sorry I suck. :( Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/xfs_trans_dquot.c | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c > index fe45b0c3970c..67f1e275b34d 100644 > --- a/fs/xfs/xfs_trans_dquot.c > +++ b/fs/xfs/xfs_trans_dquot.c > @@ -143,9 +143,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) > @@ -698,7 +695,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 +748,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) { > -- > 2.20.0 >
On 2020/10/27 6:52, Darrick J. Wong wrote: > On Fri, Oct 16, 2020 at 11:38:26AM +0800, xiakaixu1987@gmail.com wrote: >> From: Kaixu Xia <kaixuxia@tencent.com> >> >> The function xfs_trans_mod_dquot_byino() wraps around >> xfs_trans_mod_dquot() to account for quotas, and also there is the >> function call chain xfs_trans_reserve_quota_bydquots -> xfs_trans_dqresv >> -> xfs_trans_mod_dquot, both of them do the duplicated null check and >> allocation. Thus we can delete the duplicated operation from them. >> >> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com> >> Reviewed-by: Brian Foster <bfoster@redhat.com> >> Reviewed-by: Christoph Hellwig <hch@lst.de> > > HAH this got all the way to v6, sorry I suck. :( > > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Hi Darrick, There are some patches that have been reviewed but not been merged into xfs for-next branch, I will reply to them. Sorry for the noise:) Thanks, Kaixu > > --D > >> --- >> fs/xfs/xfs_trans_dquot.c | 7 ------- >> 1 file changed, 7 deletions(-) >> >> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c >> index fe45b0c3970c..67f1e275b34d 100644 >> --- a/fs/xfs/xfs_trans_dquot.c >> +++ b/fs/xfs/xfs_trans_dquot.c >> @@ -143,9 +143,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) >> @@ -698,7 +695,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 +748,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) { >> -- >> 2.20.0 >>
On Thu, Nov 12, 2020 at 10:53:39AM +0800, kaixuxia wrote: > > > On 2020/10/27 6:52, Darrick J. Wong wrote: > > On Fri, Oct 16, 2020 at 11:38:26AM +0800, xiakaixu1987@gmail.com wrote: > >> From: Kaixu Xia <kaixuxia@tencent.com> > >> > >> The function xfs_trans_mod_dquot_byino() wraps around > >> xfs_trans_mod_dquot() to account for quotas, and also there is the > >> function call chain xfs_trans_reserve_quota_bydquots -> xfs_trans_dqresv > >> -> xfs_trans_mod_dquot, both of them do the duplicated null check and > >> allocation. Thus we can delete the duplicated operation from them. > >> > >> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com> > >> Reviewed-by: Brian Foster <bfoster@redhat.com> > >> Reviewed-by: Christoph Hellwig <hch@lst.de> > > > > HAH this got all the way to v6, sorry I suck. :( > > > > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> > > Hi Darrick, > > There are some patches that have been reviewed but not been merged > into xfs for-next branch, I will reply to them. > Sorry for the noise:) Same situation here -- these are 5.11 cleanups and I'm still working on bug fixes for 5.10. If you have time to review patches, can you please have a look at the unreviewed patches in the series "xfs: fix various scrub problems", please? --D > Thanks, > Kaixu > > > > --D > > > >> --- > >> fs/xfs/xfs_trans_dquot.c | 7 ------- > >> 1 file changed, 7 deletions(-) > >> > >> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c > >> index fe45b0c3970c..67f1e275b34d 100644 > >> --- a/fs/xfs/xfs_trans_dquot.c > >> +++ b/fs/xfs/xfs_trans_dquot.c > >> @@ -143,9 +143,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) > >> @@ -698,7 +695,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 +748,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) { > >> -- > >> 2.20.0 > >> > > -- > kaixuxia
On 2020/11/13 9:55, Darrick J. Wong wrote: > On Thu, Nov 12, 2020 at 10:53:39AM +0800, kaixuxia wrote: >> >> >> On 2020/10/27 6:52, Darrick J. Wong wrote: >>> On Fri, Oct 16, 2020 at 11:38:26AM +0800, xiakaixu1987@gmail.com wrote: >>>> From: Kaixu Xia <kaixuxia@tencent.com> >>>> >>>> The function xfs_trans_mod_dquot_byino() wraps around >>>> xfs_trans_mod_dquot() to account for quotas, and also there is the >>>> function call chain xfs_trans_reserve_quota_bydquots -> xfs_trans_dqresv >>>> -> xfs_trans_mod_dquot, both of them do the duplicated null check and >>>> allocation. Thus we can delete the duplicated operation from them. >>>> >>>> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com> >>>> Reviewed-by: Brian Foster <bfoster@redhat.com> >>>> Reviewed-by: Christoph Hellwig <hch@lst.de> >>> >>> HAH this got all the way to v6, sorry I suck. :( >>> >>> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> >> >> Hi Darrick, >> >> There are some patches that have been reviewed but not been merged >> into xfs for-next branch, I will reply to them. >> Sorry for the noise:) > > Same situation here -- these are 5.11 cleanups and I'm still working on > bug fixes for 5.10. If you have time to review patches, can you please > have a look at the unreviewed patches in the series "xfs: fix various > scrub problems", please? > Hi Darrick, Thank you for the invitation! I am very interested and happy to review these patches. I will review them when I have time:) Thanks, Kaixu > --D > >> Thanks, >> Kaixu >>> >>> --D >>> >>>> --- >>>> fs/xfs/xfs_trans_dquot.c | 7 ------- >>>> 1 file changed, 7 deletions(-) >>>> >>>> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c >>>> index fe45b0c3970c..67f1e275b34d 100644 >>>> --- a/fs/xfs/xfs_trans_dquot.c >>>> +++ b/fs/xfs/xfs_trans_dquot.c >>>> @@ -143,9 +143,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) >>>> @@ -698,7 +695,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 +748,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) { >>>> -- >>>> 2.20.0 >>>> >> >> -- >> kaixuxia
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c index fe45b0c3970c..67f1e275b34d 100644 --- a/fs/xfs/xfs_trans_dquot.c +++ b/fs/xfs/xfs_trans_dquot.c @@ -143,9 +143,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) @@ -698,7 +695,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 +748,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) {