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

Brian Foster
2019-06-10T13:58:54Z

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:

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
>