Re: [PATCH 02/10] xfs: convert quotacheck to use the new iwalk functions

Darrick J. Wong
2019-06-10T17:11:07Z

On Mon, Jun 10, 2019 at 09:58:52AM -0400, Brian Foster wrote:
> On Tue, Jun 04, 2019 at 02:49:40PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Convert quotacheck to use the new iwalk iterator to dig through the
> > inodes.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Reviewed-by: Dave Chinner <dchinner@redhat.com>
> > ---
> > fs/xfs/xfs_qm.c | 62 ++++++++++++++++++-------------------------------------
> > 1 file changed, 20 insertions(+), 42 deletions(-)
> >
> >
> > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> > index aa6b6db3db0e..a5b2260406a8 100644
> > --- a/fs/xfs/xfs_qm.c
> > +++ b/fs/xfs/xfs_qm.c
> ...
> > @@ -1136,20 +1135,18 @@ xfs_qm_dqusage_adjust(
> > * rootino must have its resources accounted for, not so with the quota
> > * inodes.
> > */
> > - if (xfs_is_quota_inode(&mp->m_sb, ino)) {
> > - *res = BULKSTAT_RV_NOTHING;
> > - return -EINVAL;
> > - }
> > + if (xfs_is_quota_inode(&mp->m_sb, ino))
> > + return 0;
> >
> > /*
> > * We don't _need_ to take the ilock EXCL here because quotacheck runs
> > * at mount time and therefore nobody will be racing chown/chproj.
> > */
> > - error = xfs_iget(mp, NULL, ino, XFS_IGET_DONTCACHE, 0, &ip);
> > - if (error) {
> > - *res = BULKSTAT_RV_NOTHING;
> > + error = xfs_iget(mp, tp, ino, XFS_IGET_DONTCACHE, 0, &ip);
>
> I was wondering if we should start using IGET_UNTRUSTED here, but I
> guess we're 1.) protected by quotacheck context and 2.) have the same
> record validity semantics as the existing bulkstat walker. LGTM:

Yep. There's nothing else running in the fs at quotacheck time so inode
records should not be changing while quotacheck runs.

> Reviewed-by: Brian Foster <bfoster@redhat.com>
>
> > + if (error == -EINVAL || error == -ENOENT)
> > + return 0;
> > + if (error)
> > return error;
> > - }
> >
> > ASSERT(ip->i_delayed_blks == 0);
> >
> > @@ -1157,7 +1154,7 @@ xfs_qm_dqusage_adjust(
> > struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
> >
> > if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> > - error = xfs_iread_extents(NULL, ip, XFS_DATA_FORK);
> > + error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
> > if (error)
> > goto error0;
> > }
> > @@ -1200,13 +1197,8 @@ xfs_qm_dqusage_adjust(
> > goto error0;
> > }
> >
> > - xfs_irele(ip);
> > - *res = BULKSTAT_RV_DIDONE;
> > - return 0;
> > -
> > error0:
> > xfs_irele(ip);
> > - *res = BULKSTAT_RV_GIVEUP;
> > return error;
> > }
> >
> > @@ -1270,18 +1262,13 @@ STATIC int
> > xfs_qm_quotacheck(
> > xfs_mount_t *mp)
> > {
> > - int done, count, error, error2;
> > - xfs_ino_t lastino;
> > - size_t structsz;
> > + int error, error2;
> > uint flags;
> > LIST_HEAD (buffer_list);
> > struct xfs_inode *uip = mp->m_quotainfo->qi_uquotaip;
> > struct xfs_inode *gip = mp->m_quotainfo->qi_gquotaip;
> > struct xfs_inode *pip = mp->m_quotainfo->qi_pquotaip;
> >
> > - count = INT_MAX;
> > - structsz = 1;
> > - lastino = 0;
> > flags = 0;
> >
> > ASSERT(uip || gip || pip);
> > @@ -1318,18 +1305,9 @@ xfs_qm_quotacheck(
> > flags |= XFS_PQUOTA_CHKD;
> > }
> >
> > - do {
> > - /*
> > - * Iterate thru all the inodes in the file system,
> > - * adjusting the corresponding dquot counters in core.
> > - */
> > - error = xfs_bulkstat(mp, &lastino, &count,
> > - xfs_qm_dqusage_adjust,
> > - structsz, NULL, &done);
> > - if (error)
> > - break;
> > -
> > - } while (!done);
> > + error = xfs_iwalk(mp, NULL, 0, xfs_qm_dqusage_adjust, 0, NULL);
> > + if (error)
> > + goto error_return;
> >
> > /*
> > * We've made all the changes that we need to make incore. Flush them
> >