[PATCH 00/13] xfs-4.18: quota refactor

[PATCH 00/13] xfs-4.18: quota refactor

[Darrick J. Wong]

Hi all,

Here's a series to refactor and clean out the incohesive mess that
xfs_qm_dqget has become.

I've distinguished three contexts in which dqget can be called, and
converted them into separate externally visible interfaces --
_qm_dqget_next, which returns the first initialized dquot with an id at
least as high as the id passed in; _qm_dqget_inode, which returns the
dquot for a given (inode, type); and so _qm_dqget now only returns the
dquot for a given (id, type).  I removed unused parameters from the
dqattach functions, renamed _qm_dqiterate (since it now only knows how
to zap dquot counters during quotacheck), and added a real function to
iterate all dquots of a given type.

This means that all _dqget variants no longer take QMOPT flags, and
QMOPT_NEXT is gone.  XFS_QMOPT_DQALLOC is now XFS_DQGET_DQALLOC and can
only be specified for functions where it makes sense.

If you're going to start using this mess, you probably ought to just
pull from my git trees.  The kernel patches[1] should apply against
4.17-rc1.  xfsprogs[2] and xfstests[3] can be found in their usual
places.  The git trees contain all four series' worth of changes.

This is an extraordinary way to eat your data.  Enjoy!
Comments and questions are, as always, welcome.

--D

[1] https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=djwong-devel
[2] https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=djwong-devel
[3] https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=djwong-devel

[PATCH 01/13] xfs: refactor XFS_QMOPT_DQNEXT out of existence

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

There's only one caller of DQNEXT and its semantics can be moved into a
separate function, so create the function and get rid of the flag.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_quota_defs.h |    1 
 fs/xfs/scrub/quota.c           |    3 -
 fs/xfs/xfs_dquot.c             |   63 ++++++++++++++------------
 fs/xfs/xfs_dquot.h             |    2 +
 fs/xfs/xfs_qm.h                |    6 ++-
 fs/xfs/xfs_qm_syscalls.c       |   96 +++++++++++++++++++++++++++-------------
 fs/xfs/xfs_quotaops.c          |    8 +--
 7 files changed, 108 insertions(+), 71 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index bb1b13a..c80b226 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -114,7 +114,6 @@ typedef uint16_t	xfs_qwarncnt_t;
 #define XFS_QMOPT_SBVERSION	0x0000040 /* change superblock version num */
 #define XFS_QMOPT_GQUOTA	0x0002000 /* group dquot requested */
 #define XFS_QMOPT_ENOSPC	0x0004000 /* enospc instead of edquot (prj) */
-#define XFS_QMOPT_DQNEXT	0x0008000 /* return next dquot >= this ID */
 
 /*
  * flags to xfs_trans_mod_dquot to indicate which field needs to be
diff --git a/fs/xfs/scrub/quota.c b/fs/xfs/scrub/quota.c
index 6ba465e..50415e8 100644
--- a/fs/xfs/scrub/quota.c
+++ b/fs/xfs/scrub/quota.c
@@ -268,8 +268,7 @@ xfs_scrub_quota(
 		if (xfs_scrub_should_terminate(sc, &error))
 			break;
 
-		error = xfs_qm_dqget(mp, NULL, id, dqtype, XFS_QMOPT_DQNEXT,
-				&dq);
+		error = xfs_qm_dqget_next(mp, id, dqtype, &dq);
 		if (error == -ENOENT)
 			break;
 		if (!xfs_scrub_fblock_process_error(sc, XFS_DATA_FORK,
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index a7daef9..79a9df6 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -728,18 +728,6 @@ xfs_qm_dqget(
 			goto restart;
 		}
 
-		/* uninit / unused quota found in radix tree, keep looking  */
-		if (flags & XFS_QMOPT_DQNEXT) {
-			if (XFS_IS_DQUOT_UNINITIALIZED(dqp)) {
-				xfs_dqunlock(dqp);
-				mutex_unlock(&qi->qi_tree_lock);
-				error = xfs_dq_get_next_id(mp, type, &id);
-				if (error)
-					return error;
-				goto restart;
-			}
-		}
-
 		dqp->q_nrefs++;
 		mutex_unlock(&qi->qi_tree_lock);
 
@@ -766,13 +754,6 @@ xfs_qm_dqget(
 	if (ip)
 		xfs_ilock(ip, XFS_ILOCK_EXCL);
 
-	/* If we are asked to find next active id, keep looking */
-	if (error == -ENOENT && (flags & XFS_QMOPT_DQNEXT)) {
-		error = xfs_dq_get_next_id(mp, type, &id);
-		if (!error)
-			goto restart;
-	}
-
 	if (error)
 		return error;
 
@@ -823,17 +804,6 @@ xfs_qm_dqget(
 	qi->qi_dquots++;
 	mutex_unlock(&qi->qi_tree_lock);
 
-	/* If we are asked to find next active id, keep looking */
-	if (flags & XFS_QMOPT_DQNEXT) {
-		if (XFS_IS_DQUOT_UNINITIALIZED(dqp)) {
-			xfs_qm_dqput(dqp);
-			error = xfs_dq_get_next_id(mp, type, &id);
-			if (error)
-				return error;
-			goto restart;
-		}
-	}
-
  dqret:
 	ASSERT((ip == NULL) || xfs_isilocked(ip, XFS_ILOCK_EXCL));
 	trace_xfs_dqget_miss(dqp);
@@ -842,6 +812,39 @@ xfs_qm_dqget(
 }
 
 /*
+ * Starting at @id and progressing upwards, look for an initialized incore
+ * dquot, lock it, and return it.
+ */
+int
+xfs_qm_dqget_next(
+	struct xfs_mount	*mp,
+	xfs_dqid_t		id,
+	uint			type,
+	struct xfs_dquot	**dqpp)
+{
+	struct xfs_dquot	*dqp;
+	int			error = 0;
+
+	*dqpp = NULL;
+	for (; !error; error = xfs_dq_get_next_id(mp, type, &id)) {
+		error = xfs_qm_dqget(mp, NULL, id, type, 0, &dqp);
+		if (error == -ENOENT)
+			continue;
+		else if (error != 0)
+			break;
+
+		if (!XFS_IS_DQUOT_UNINITIALIZED(dqp)) {
+			*dqpp = dqp;
+			return 0;
+		}
+
+		xfs_qm_dqput(dqp);
+	}
+
+	return error;
+}
+
+/*
  * Release a reference to the dquot (decrement ref-count) and unlock it.
  *
  * If there is a group quota attached to this dquot, carefully release that
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 2f536f3..303e71d 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -171,6 +171,8 @@ extern void		xfs_qm_adjust_dqlimits(struct xfs_mount *,
 					       struct xfs_dquot *);
 extern int		xfs_qm_dqget(xfs_mount_t *, xfs_inode_t *,
 					xfs_dqid_t, uint, uint, xfs_dquot_t **);
+extern int		xfs_qm_dqget_next(struct xfs_mount *mp, xfs_dqid_t id,
+					uint type, struct xfs_dquot **dqpp);
 extern void		xfs_qm_dqput(xfs_dquot_t *);
 
 extern void		xfs_dqlock2(struct xfs_dquot *, struct xfs_dquot *);
diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
index 2975a82..e3129b2 100644
--- a/fs/xfs/xfs_qm.h
+++ b/fs/xfs/xfs_qm.h
@@ -170,8 +170,10 @@ extern void		xfs_qm_dqrele_all_inodes(struct xfs_mount *, uint);
 
 /* quota ops */
 extern int		xfs_qm_scall_trunc_qfiles(struct xfs_mount *, uint);
-extern int		xfs_qm_scall_getquota(struct xfs_mount *, xfs_dqid_t *,
-					uint, struct qc_dqblk *, uint);
+extern int		xfs_qm_scall_getquota(struct xfs_mount *, xfs_dqid_t,
+					uint, struct qc_dqblk *);
+extern int		xfs_qm_scall_getquota_next(struct xfs_mount *,
+					xfs_dqid_t *, uint, struct qc_dqblk *);
 extern int		xfs_qm_scall_setqlim(struct xfs_mount *, xfs_dqid_t, uint,
 					struct qc_dqblk *);
 extern int		xfs_qm_scall_quotaon(struct xfs_mount *, uint);
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 9cb5c38..0234cfc 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -622,39 +622,14 @@ xfs_qm_log_quotaoff(
 	return error;
 }
 
-
-int
-xfs_qm_scall_getquota(
+/* Fill out the quota context. */
+static void
+xfs_qm_scall_getquota_fill_qc(
 	struct xfs_mount	*mp,
-	xfs_dqid_t		*id,
 	uint			type,
-	struct qc_dqblk		*dst,
-	uint			dqget_flags)
+	const struct xfs_dquot	*dqp,
+	struct qc_dqblk		*dst)
 {
-	struct xfs_dquot	*dqp;
-	int			error;
-
-	/*
-	 * Try to get the dquot. We don't want it allocated on disk, so
-	 * we aren't passing the XFS_QMOPT_DOALLOC flag. If it doesn't
-	 * exist, we'll get ENOENT back.
-	 */
-	error = xfs_qm_dqget(mp, NULL, *id, type, dqget_flags, &dqp);
-	if (error)
-		return error;
-
-	/*
-	 * If everything's NULL, this dquot doesn't quite exist as far as
-	 * our utility programs are concerned.
-	 */
-	if (XFS_IS_DQUOT_UNINITIALIZED(dqp)) {
-		error = -ENOENT;
-		goto out_put;
-	}
-
-	/* Fill in the ID we actually read from disk */
-	*id = be32_to_cpu(dqp->q_core.d_id);
-
 	memset(dst, 0, sizeof(*dst));
 	dst->d_spc_hardlimit =
 		XFS_FSB_TO_B(mp, be64_to_cpu(dqp->q_core.d_blk_hardlimit));
@@ -696,7 +671,7 @@ xfs_qm_scall_getquota(
 	if (((XFS_IS_UQUOTA_ENFORCED(mp) && type == XFS_DQ_USER) ||
 	     (XFS_IS_GQUOTA_ENFORCED(mp) && type == XFS_DQ_GROUP) ||
 	     (XFS_IS_PQUOTA_ENFORCED(mp) && type == XFS_DQ_PROJ)) &&
-	    *id != 0) {
+	    dqp->q_core.d_id != 0) {
 		if ((dst->d_space > dst->d_spc_softlimit) &&
 		    (dst->d_spc_softlimit > 0)) {
 			ASSERT(dst->d_spc_timer != 0);
@@ -707,11 +682,70 @@ xfs_qm_scall_getquota(
 		}
 	}
 #endif
+}
+
+/* Return the quota information for the dquot matching id. */
+int
+xfs_qm_scall_getquota(
+	struct xfs_mount	*mp,
+	xfs_dqid_t		id,
+	uint			type,
+	struct qc_dqblk		*dst)
+{
+	struct xfs_dquot	*dqp;
+	int			error;
+
+	/*
+	 * Try to get the dquot. We don't want it allocated on disk, so
+	 * we aren't passing the XFS_QMOPT_DOALLOC flag. If it doesn't
+	 * exist, we'll get ENOENT back.
+	 */
+	error = xfs_qm_dqget(mp, NULL, id, type, 0, &dqp);
+	if (error)
+		return error;
+
+	/*
+	 * If everything's NULL, this dquot doesn't quite exist as far as
+	 * our utility programs are concerned.
+	 */
+	if (XFS_IS_DQUOT_UNINITIALIZED(dqp)) {
+		error = -ENOENT;
+		goto out_put;
+	}
+
+	xfs_qm_scall_getquota_fill_qc(mp, type, dqp, dst);
+
 out_put:
 	xfs_qm_dqput(dqp);
 	return error;
 }
 
+/*
+ * Return the quota information for the first initialized dquot whose id
+ * is at least as high as id.
+ */
+int
+xfs_qm_scall_getquota_next(
+	struct xfs_mount	*mp,
+	xfs_dqid_t		*id,
+	uint			type,
+	struct qc_dqblk		*dst)
+{
+	struct xfs_dquot	*dqp;
+	int			error;
+
+	error = xfs_qm_dqget_next(mp, *id, type, &dqp);
+	if (error)
+		return error;
+
+	/* Fill in the ID we actually read from disk */
+	*id = be32_to_cpu(dqp->q_core.d_id);
+
+	xfs_qm_scall_getquota_fill_qc(mp, type, dqp, dst);
+
+	xfs_qm_dqput(dqp);
+	return error;
+}
 
 STATIC int
 xfs_dqrele_inode(
diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c
index a651085..c93fc91 100644
--- a/fs/xfs/xfs_quotaops.c
+++ b/fs/xfs/xfs_quotaops.c
@@ -239,8 +239,7 @@ xfs_fs_get_dqblk(
 		return -ESRCH;
 
 	id = from_kqid(&init_user_ns, qid);
-	return xfs_qm_scall_getquota(mp, &id,
-				      xfs_quota_type(qid.type), qdq, 0);
+	return xfs_qm_scall_getquota(mp, id, xfs_quota_type(qid.type), qdq);
 }
 
 /* Return quota info for active quota >= this qid */
@@ -260,9 +259,8 @@ xfs_fs_get_nextdqblk(
 		return -ESRCH;
 
 	id = from_kqid(&init_user_ns, *qid);
-	ret = xfs_qm_scall_getquota(mp, &id,
-				    xfs_quota_type(qid->type), qdq,
-				    XFS_QMOPT_DQNEXT);
+	ret = xfs_qm_scall_getquota_next(mp, &id, xfs_quota_type(qid->type),
+			qdq);
 	if (ret)
 		return ret;
 

[PATCH 02/13] xfs: refactor dquot cache handling

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Delegate the dquot cache handling (radix tree lookup and insertion) to
separate helper functions so that we can continue to simplify the body
of dqget.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_dquot.c |  112 ++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 78 insertions(+), 34 deletions(-)


diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 79a9df6..43b0b32 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -679,6 +679,81 @@ xfs_dq_get_next_id(
 }
 
 /*
+ * Look up the dquot in the in-core cache.  If found, the dquot is returned
+ * locked and ready to go.
+ */
+static struct xfs_dquot *
+xfs_qm_dqget_cache_lookup(
+	struct xfs_mount	*mp,
+	struct xfs_quotainfo	*qi,
+	struct radix_tree_root	*tree,
+	xfs_dqid_t		id)
+{
+	struct xfs_dquot	*dqp;
+
+restart:
+	mutex_lock(&qi->qi_tree_lock);
+	dqp = radix_tree_lookup(tree, id);
+	if (!dqp) {
+		mutex_unlock(&qi->qi_tree_lock);
+		XFS_STATS_INC(mp, xs_qm_dqcachemisses);
+		return NULL;
+	}
+
+	xfs_dqlock(dqp);
+	if (dqp->dq_flags & XFS_DQ_FREEING) {
+		xfs_dqunlock(dqp);
+		mutex_unlock(&qi->qi_tree_lock);
+		trace_xfs_dqget_freeing(dqp);
+		delay(1);
+		goto restart;
+	}
+
+	dqp->q_nrefs++;
+	mutex_unlock(&qi->qi_tree_lock);
+
+	trace_xfs_dqget_hit(dqp);
+	XFS_STATS_INC(mp, xs_qm_dqcachehits);
+	return dqp;
+}
+
+/*
+ * Try to insert a new dquot into the in-core cache.  If an error occurs the
+ * caller should throw away the dquot and start over.  Otherwise, the dquot
+ * is returned locked (and held by the cache) as if there had been a cache
+ * hit.
+ */
+static int
+xfs_qm_dqget_cache_insert(
+	struct xfs_mount	*mp,
+	struct xfs_quotainfo	*qi,
+	struct radix_tree_root	*tree,
+	xfs_dqid_t		id,
+	struct xfs_dquot	*dqp)
+{
+	int			error;
+
+	mutex_lock(&qi->qi_tree_lock);
+	error = radix_tree_insert(tree, id, dqp);
+	if (unlikely(error)) {
+		/* Duplicate found!  Caller must try again. */
+		WARN_ON(error != -EEXIST);
+		mutex_unlock(&qi->qi_tree_lock);
+		trace_xfs_dqget_dup(dqp);
+		return error;
+	}
+
+	/* Return a locked dquot to the caller, with a reference taken. */
+	xfs_dqlock(dqp);
+	dqp->q_nrefs = 1;
+
+	qi->qi_dquots++;
+	mutex_unlock(&qi->qi_tree_lock);
+
+	return 0;
+}
+
+/*
  * Given the file system, inode OR id, and type (UDQUOT/GDQUOT), return a
  * a locked dquot, doing an allocation (if requested) as needed.
  * When both an inode and an id are given, the inode's id takes precedence.
@@ -716,28 +791,11 @@ xfs_qm_dqget(
 	}
 
 restart:
-	mutex_lock(&qi->qi_tree_lock);
-	dqp = radix_tree_lookup(tree, id);
+	dqp = xfs_qm_dqget_cache_lookup(mp, qi, tree, id);
 	if (dqp) {
-		xfs_dqlock(dqp);
-		if (dqp->dq_flags & XFS_DQ_FREEING) {
-			xfs_dqunlock(dqp);
-			mutex_unlock(&qi->qi_tree_lock);
-			trace_xfs_dqget_freeing(dqp);
-			delay(1);
-			goto restart;
-		}
-
-		dqp->q_nrefs++;
-		mutex_unlock(&qi->qi_tree_lock);
-
-		trace_xfs_dqget_hit(dqp);
-		XFS_STATS_INC(mp, xs_qm_dqcachehits);
 		*O_dqpp = dqp;
 		return 0;
 	}
-	mutex_unlock(&qi->qi_tree_lock);
-	XFS_STATS_INC(mp, xs_qm_dqcachemisses);
 
 	/*
 	 * Dquot cache miss. We don't want to keep the inode lock across
@@ -779,31 +837,17 @@ xfs_qm_dqget(
 		}
 	}
 
-	mutex_lock(&qi->qi_tree_lock);
-	error = radix_tree_insert(tree, id, dqp);
-	if (unlikely(error)) {
-		WARN_ON(error != -EEXIST);
-
+	error = xfs_qm_dqget_cache_insert(mp, qi, tree, id, dqp);
+	if (error) {
 		/*
 		 * Duplicate found. Just throw away the new dquot and start
 		 * over.
 		 */
-		mutex_unlock(&qi->qi_tree_lock);
-		trace_xfs_dqget_dup(dqp);
 		xfs_qm_dqdestroy(dqp);
 		XFS_STATS_INC(mp, xs_qm_dquot_dups);
 		goto restart;
 	}
 
-	/*
-	 * We return a locked dquot to the caller, with a reference taken
-	 */
-	xfs_dqlock(dqp);
-	dqp->q_nrefs = 1;
-
-	qi->qi_dquots++;
-	mutex_unlock(&qi->qi_tree_lock);
-
  dqret:
 	ASSERT((ip == NULL) || xfs_isilocked(ip, XFS_ILOCK_EXCL));
 	trace_xfs_dqget_miss(dqp);

[PATCH 03/13] xfs: delegate dqget input checks to helper function

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Move the dqget input checks to a separate function in preparation for
splitting up the dqget functionality.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_dquot.c |   39 ++++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)


diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 43b0b32..9a3a05e 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -753,6 +753,33 @@ xfs_qm_dqget_cache_insert(
 	return 0;
 }
 
+/* Check our input parameters. */
+static int
+xfs_qm_dqget_checks(
+	struct xfs_mount	*mp,
+	uint			type)
+{
+	if (!XFS_IS_QUOTA_RUNNING(mp)) {
+		ASSERT(0);
+		return -ESRCH;
+	}
+
+	if ((!XFS_IS_UQUOTA_ON(mp) && type == XFS_DQ_USER) ||
+	    (!XFS_IS_PQUOTA_ON(mp) && type == XFS_DQ_PROJ) ||
+	    (!XFS_IS_GQUOTA_ON(mp) && type == XFS_DQ_GROUP)) {
+		return -ESRCH;
+	}
+
+	if (type != XFS_DQ_USER &&
+	    type != XFS_DQ_PROJ &&
+	    type != XFS_DQ_GROUP) {
+		ASSERT(0);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 /*
  * Given the file system, inode OR id, and type (UDQUOT/GDQUOT), return a
  * a locked dquot, doing an allocation (if requested) as needed.
@@ -775,16 +802,10 @@ xfs_qm_dqget(
 	struct xfs_dquot	*dqp;
 	int			error;
 
-	ASSERT(XFS_IS_QUOTA_RUNNING(mp));
-	if ((! XFS_IS_UQUOTA_ON(mp) && type == XFS_DQ_USER) ||
-	    (! XFS_IS_PQUOTA_ON(mp) && type == XFS_DQ_PROJ) ||
-	    (! XFS_IS_GQUOTA_ON(mp) && type == XFS_DQ_GROUP)) {
-		return -ESRCH;
-	}
+	error = xfs_qm_dqget_checks(mp, type);
+	if (error)
+		return error;
 
-	ASSERT(type == XFS_DQ_USER ||
-	       type == XFS_DQ_PROJ ||
-	       type == XFS_DQ_GROUP);
 	if (ip) {
 		ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
 		ASSERT(xfs_inode_dquot(ip, type) == NULL);

[PATCH 04/13] xfs: remove unnecessary xfs_qm_dqattach parameter

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

The flags argument is always zero, get rid of it.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_attr.c |    4 ++--
 fs/xfs/xfs_bmap_util.c   |    6 +++---
 fs/xfs/xfs_inode.c       |   10 +++++-----
 fs/xfs/xfs_iomap.c       |    4 ++--
 fs/xfs/xfs_iops.c        |    2 +-
 fs/xfs/xfs_qm.c          |    7 +++----
 fs/xfs/xfs_quota.h       |    4 ++--
 fs/xfs/xfs_reflink.c     |    2 +-
 8 files changed, 19 insertions(+), 20 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 35a1244..c3d02a6 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -236,7 +236,7 @@ xfs_attr_set(
 	args.op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
 	args.total = xfs_attr_calc_size(&args, &local);
 
-	error = xfs_qm_dqattach(dp, 0);
+	error = xfs_qm_dqattach(dp);
 	if (error)
 		return error;
 
@@ -427,7 +427,7 @@ xfs_attr_remove(
 	 */
 	args.op_flags = XFS_DA_OP_OKNOENT;
 
-	error = xfs_qm_dqattach(dp, 0);
+	error = xfs_qm_dqattach(dp);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 8cd8c41..65b2cc2 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -848,7 +848,7 @@ xfs_free_eofblocks(
 		/*
 		 * Attach the dquots to the inode up front.
 		 */
-		error = xfs_qm_dqattach(ip, 0);
+		error = xfs_qm_dqattach(ip);
 		if (error)
 			return error;
 
@@ -918,7 +918,7 @@ xfs_alloc_file_space(
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return -EIO;
 
-	error = xfs_qm_dqattach(ip, 0);
+	error = xfs_qm_dqattach(ip);
 	if (error)
 		return error;
 
@@ -1169,7 +1169,7 @@ xfs_free_file_space(
 
 	trace_xfs_free_file_space(ip);
 
-	error = xfs_qm_dqattach(ip, 0);
+	error = xfs_qm_dqattach(ip);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 2b70c8b..1da9863 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1411,11 +1411,11 @@ xfs_link(
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return -EIO;
 
-	error = xfs_qm_dqattach(sip, 0);
+	error = xfs_qm_dqattach(sip);
 	if (error)
 		goto std_return;
 
-	error = xfs_qm_dqattach(tdp, 0);
+	error = xfs_qm_dqattach(tdp);
 	if (error)
 		goto std_return;
 
@@ -1911,7 +1911,7 @@ xfs_inactive(
 	     ip->i_d.di_nextents > 0 || ip->i_delayed_blks > 0))
 		truncate = 1;
 
-	error = xfs_qm_dqattach(ip, 0);
+	error = xfs_qm_dqattach(ip);
 	if (error)
 		return;
 
@@ -2574,11 +2574,11 @@ xfs_remove(
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return -EIO;
 
-	error = xfs_qm_dqattach(dp, 0);
+	error = xfs_qm_dqattach(dp);
 	if (error)
 		goto std_return;
 
-	error = xfs_qm_dqattach(ip, 0);
+	error = xfs_qm_dqattach(ip);
 	if (error)
 		goto std_return;
 
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 046469f..58b04ea 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -224,7 +224,7 @@ xfs_iomap_write_direct(
 	 * necessary and move on to transaction setup.
 	 */
 	xfs_iunlock(ip, lockmode);
-	error = xfs_qm_dqattach(ip, 0);
+	error = xfs_qm_dqattach(ip);
 	if (error)
 		return error;
 
@@ -692,7 +692,7 @@ xfs_iomap_write_allocate(
 	/*
 	 * Make sure that the dquots are there.
 	 */
-	error = xfs_qm_dqattach(ip, 0);
+	error = xfs_qm_dqattach(ip);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index a3ed3c8..5afe3c2 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -855,7 +855,7 @@ xfs_setattr_size(
 	/*
 	 * Make sure that the dquots are attached to the inode.
 	 */
-	error = xfs_qm_dqattach(ip, 0);
+	error = xfs_qm_dqattach(ip);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index ec39ae2..d919b85 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -381,8 +381,7 @@ xfs_qm_dqattach_locked(
 
 int
 xfs_qm_dqattach(
-	struct xfs_inode	*ip,
-	uint			flags)
+	struct xfs_inode	*ip)
 {
 	int			error;
 
@@ -390,7 +389,7 @@ xfs_qm_dqattach(
 		return 0;
 
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
-	error = xfs_qm_dqattach_locked(ip, flags);
+	error = xfs_qm_dqattach_locked(ip, 0);
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 
 	return error;
@@ -1933,7 +1932,7 @@ xfs_qm_vop_rename_dqattach(
 		 */
 		if (i == 0 || ip != i_tab[i-1]) {
 			if (XFS_NOT_DQATTACHED(mp, ip)) {
-				error = xfs_qm_dqattach(ip, 0);
+				error = xfs_qm_dqattach(ip);
 				if (error)
 					return error;
 			}
diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h
index ce6506a..a4d3922 100644
--- a/fs/xfs/xfs_quota.h
+++ b/fs/xfs/xfs_quota.h
@@ -90,7 +90,7 @@ extern struct xfs_dquot *xfs_qm_vop_chown(struct xfs_trans *,
 extern int xfs_qm_vop_chown_reserve(struct xfs_trans *, struct xfs_inode *,
 		struct xfs_dquot *, struct xfs_dquot *,
 		struct xfs_dquot *, uint);
-extern int xfs_qm_dqattach(struct xfs_inode *, uint);
+extern int xfs_qm_dqattach(struct xfs_inode *);
 extern int xfs_qm_dqattach_locked(struct xfs_inode *, uint);
 extern void xfs_qm_dqdetach(struct xfs_inode *);
 extern void xfs_qm_dqrele(struct xfs_dquot *);
@@ -132,7 +132,7 @@ static inline int xfs_trans_reserve_quota_bydquots(struct xfs_trans *tp,
 #define xfs_qm_vop_rename_dqattach(it)					(0)
 #define xfs_qm_vop_chown(tp, ip, old, new)				(NULL)
 #define xfs_qm_vop_chown_reserve(tp, ip, u, g, p, fl)			(0)
-#define xfs_qm_dqattach(ip, fl)						(0)
+#define xfs_qm_dqattach(ip)						(0)
 #define xfs_qm_dqattach_locked(ip, fl)					(0)
 #define xfs_qm_dqdetach(ip)
 #define xfs_qm_dqrele(d)
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index cdbd342..fd8e6af 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -1359,7 +1359,7 @@ xfs_reflink_remap_range(
 		goto out_unlock;
 
 	/* Attach dquots to dest inode before changing block map */
-	ret = xfs_qm_dqattach(dest, 0);
+	ret = xfs_qm_dqattach(dest);
 	if (ret)
 		goto out_unlock;
 

[PATCH 05/13] xfs: split out dqget for inodes from regular dqget

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

There are two uses of dqget here -- one is to return the dquot for a
given type and id, and the other is to return the dquot for a given type
and inode.  Those are two separate things, so split them into two
smaller functions.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_dquot.c       |  148 ++++++++++++++++++++++++++++++++--------------
 fs/xfs/xfs_dquot.h       |   11 +++
 fs/xfs/xfs_qm.c          |   22 ++-----
 fs/xfs/xfs_qm_bhv.c      |    2 -
 fs/xfs/xfs_qm_syscalls.c |    4 +
 5 files changed, 124 insertions(+), 63 deletions(-)


diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 9a3a05e..62415f1 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -781,24 +781,19 @@ xfs_qm_dqget_checks(
 }
 
 /*
- * Given the file system, inode OR id, and type (UDQUOT/GDQUOT), return a
- * a locked dquot, doing an allocation (if requested) as needed.
- * When both an inode and an id are given, the inode's id takes precedence.
- * That is, if the id changes while we don't hold the ilock inside this
- * function, the new dquot is returned, not necessarily the one requested
- * in the id argument.
+ * Given the file system, id, and type (UDQUOT/GDQUOT), return a a locked
+ * dquot, doing an allocation (if requested) as needed.
  */
 int
 xfs_qm_dqget(
-	xfs_mount_t	*mp,
-	xfs_inode_t	*ip,	  /* locked inode (optional) */
-	xfs_dqid_t	id,	  /* uid/projid/gid depending on type */
-	uint		type,	  /* XFS_DQ_USER/XFS_DQ_PROJ/XFS_DQ_GROUP */
-	uint		flags,	  /* DQALLOC, DQSUSER, DQREPAIR, DOWARN */
-	xfs_dquot_t	**O_dqpp) /* OUT : locked incore dquot */
+	struct xfs_mount	*mp,
+	xfs_dqid_t		id,
+	uint			type,
+	uint			flags,	  /* DQALLOC, DQSUSER, DQREPAIR, DOWARN */
+	struct xfs_dquot	**O_dqpp)
 {
 	struct xfs_quotainfo	*qi = mp->m_quotainfo;
-	struct radix_tree_root *tree = xfs_dquot_tree(qi, type);
+	struct radix_tree_root	*tree = xfs_dquot_tree(qi, type);
 	struct xfs_dquot	*dqp;
 	int			error;
 
@@ -806,10 +801,82 @@ xfs_qm_dqget(
 	if (error)
 		return error;
 
-	if (ip) {
-		ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
-		ASSERT(xfs_inode_dquot(ip, type) == NULL);
+restart:
+	dqp = xfs_qm_dqget_cache_lookup(mp, qi, tree, id);
+	if (dqp) {
+		*O_dqpp = dqp;
+		return 0;
+	}
+
+	error = xfs_qm_dqread(mp, id, type, flags, &dqp);
+	if (error)
+		return error;
+
+	error = xfs_qm_dqget_cache_insert(mp, qi, tree, id, dqp);
+	if (error) {
+		/*
+		 * Duplicate found. Just throw away the new dquot and start
+		 * over.
+		 */
+		xfs_qm_dqdestroy(dqp);
+		XFS_STATS_INC(mp, xs_qm_dquot_dups);
+		goto restart;
+	}
+
+	trace_xfs_dqget_miss(dqp);
+	*O_dqpp = dqp;
+	return 0;
+}
+
+/* Return the quota id for a given inode and type. */
+xfs_dqid_t
+xfs_qm_id_for_quotatype(
+	struct xfs_inode	*ip,
+	uint			type)
+{
+	switch (type) {
+	case XFS_DQ_USER:
+		return ip->i_d.di_uid;
+	case XFS_DQ_GROUP:
+		return ip->i_d.di_gid;
+	case XFS_DQ_PROJ:
+		return xfs_get_projid(ip);
 	}
+	ASSERT(0);
+	return 0;
+}
+
+/*
+ * Return the dquot for a given inode and type.  If @can_alloc is true, then
+ * allocate blocks if needed.  The inode's ILOCK must be held and it must not
+ * have already had an inode attached.
+ */
+int
+xfs_qm_dqget_inode(
+	struct xfs_mount	*mp,
+	struct xfs_inode	*ip,
+	uint			type,
+	uint			can_alloc,
+	struct xfs_dquot	**O_dqpp)
+{
+	struct xfs_quotainfo	*qi = mp->m_quotainfo;
+	struct radix_tree_root	*tree = xfs_dquot_tree(qi, type);
+	struct xfs_dquot	*dqp;
+	xfs_dqid_t		id;
+	uint			flags = 0;
+	int			error;
+
+	error = xfs_qm_dqget_checks(mp, type);
+	if (error)
+		return error;
+
+	if (can_alloc)
+		flags |= XFS_QMOPT_DQALLOC;
+
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+	ASSERT(xfs_inode_dquot(ip, type) == NULL);
+
+	id = xfs_qm_id_for_quotatype(ip, type);
 
 restart:
 	dqp = xfs_qm_dqget_cache_lookup(mp, qi, tree, id);
@@ -825,37 +892,30 @@ xfs_qm_dqget(
 	 * lock here means dealing with a chown that can happen before
 	 * we re-acquire the lock.
 	 */
-	if (ip)
-		xfs_iunlock(ip, XFS_ILOCK_EXCL);
-
+	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 	error = xfs_qm_dqread(mp, id, type, flags, &dqp);
-
-	if (ip)
-		xfs_ilock(ip, XFS_ILOCK_EXCL);
-
+	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	if (error)
 		return error;
 
-	if (ip) {
-		/*
-		 * A dquot could be attached to this inode by now, since
-		 * we had dropped the ilock.
-		 */
-		if (xfs_this_quota_on(mp, type)) {
-			struct xfs_dquot	*dqp1;
-
-			dqp1 = xfs_inode_dquot(ip, type);
-			if (dqp1) {
-				xfs_qm_dqdestroy(dqp);
-				dqp = dqp1;
-				xfs_dqlock(dqp);
-				goto dqret;
-			}
-		} else {
-			/* inode stays locked on return */
+	/*
+	 * A dquot could be attached to this inode by now, since we had
+	 * dropped the ilock.
+	 */
+	if (xfs_this_quota_on(mp, type)) {
+		struct xfs_dquot	*dqp1;
+
+		dqp1 = xfs_inode_dquot(ip, type);
+		if (dqp1) {
 			xfs_qm_dqdestroy(dqp);
-			return -ESRCH;
+			dqp = dqp1;
+			xfs_dqlock(dqp);
+			goto dqret;
 		}
+	} else {
+		/* inode stays locked on return */
+		xfs_qm_dqdestroy(dqp);
+		return -ESRCH;
 	}
 
 	error = xfs_qm_dqget_cache_insert(mp, qi, tree, id, dqp);
@@ -869,8 +929,8 @@ xfs_qm_dqget(
 		goto restart;
 	}
 
- dqret:
-	ASSERT((ip == NULL) || xfs_isilocked(ip, XFS_ILOCK_EXCL));
+dqret:
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
 	trace_xfs_dqget_miss(dqp);
 	*O_dqpp = dqp;
 	return 0;
@@ -892,7 +952,7 @@ xfs_qm_dqget_next(
 
 	*dqpp = NULL;
 	for (; !error; error = xfs_dq_get_next_id(mp, type, &id)) {
-		error = xfs_qm_dqget(mp, NULL, id, type, 0, &dqp);
+		error = xfs_qm_dqget(mp, id, type, 0, &dqp);
 		if (error == -ENOENT)
 			continue;
 		else if (error != 0)
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 303e71d..1458de3 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -169,8 +169,15 @@ extern void		xfs_qm_adjust_dqtimers(xfs_mount_t *,
 					xfs_disk_dquot_t *);
 extern void		xfs_qm_adjust_dqlimits(struct xfs_mount *,
 					       struct xfs_dquot *);
-extern int		xfs_qm_dqget(xfs_mount_t *, xfs_inode_t *,
-					xfs_dqid_t, uint, uint, xfs_dquot_t **);
+extern xfs_dqid_t	xfs_qm_id_for_quotatype(struct xfs_inode *ip,
+					uint type);
+extern int		xfs_qm_dqget(struct xfs_mount *mp, xfs_dqid_t id,
+					uint type, uint flags,
+					struct xfs_dquot **dqpp);
+extern int		xfs_qm_dqget_inode(struct xfs_mount *mp,
+					struct xfs_inode *ip, uint type,
+					uint can_alloc,
+					struct xfs_dquot **dqpp);
 extern int		xfs_qm_dqget_next(struct xfs_mount *mp, xfs_dqid_t id,
 					uint type, struct xfs_dquot **dqpp);
 extern void		xfs_qm_dqput(xfs_dquot_t *);
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index d919b85..385d315 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -291,7 +291,7 @@ xfs_qm_dqattach_one(
 	 * exist on disk and we didn't ask it to allocate; ESRCH if quotas got
 	 * turned off suddenly.
 	 */
-	error = xfs_qm_dqget(ip->i_mount, ip, id, type, doalloc, &dqp);
+	error = xfs_qm_dqget_inode(ip->i_mount, ip, type, doalloc, &dqp);
 	if (error)
 		return error;
 
@@ -1074,7 +1074,7 @@ xfs_qm_quotacheck_dqadjust(
 	struct xfs_dquot	*dqp;
 	int			error;
 
-	error = xfs_qm_dqget(mp, ip, id, type, XFS_QMOPT_DQALLOC, &dqp);
+	error = xfs_qm_dqget_inode(mp, ip, type, XFS_QMOPT_DQALLOC, &dqp);
 	if (error) {
 		/*
 		 * Shouldn't be able to turn off quotas here.
@@ -1693,10 +1693,8 @@ xfs_qm_vop_dqalloc(
 			 * holding ilock.
 			 */
 			xfs_iunlock(ip, lockflags);
-			error = xfs_qm_dqget(mp, NULL, uid,
-						 XFS_DQ_USER,
-						 XFS_QMOPT_DQALLOC,
-						 &uq);
+			error = xfs_qm_dqget(mp, uid, XFS_DQ_USER,
+					XFS_QMOPT_DQALLOC, &uq);
 			if (error) {
 				ASSERT(error != -ENOENT);
 				return error;
@@ -1719,10 +1717,8 @@ xfs_qm_vop_dqalloc(
 	if ((flags & XFS_QMOPT_GQUOTA) && XFS_IS_GQUOTA_ON(mp)) {
 		if (ip->i_d.di_gid != gid) {
 			xfs_iunlock(ip, lockflags);
-			error = xfs_qm_dqget(mp, NULL, gid,
-						 XFS_DQ_GROUP,
-						 XFS_QMOPT_DQALLOC,
-						 &gq);
+			error = xfs_qm_dqget(mp, gid, XFS_DQ_GROUP,
+					XFS_QMOPT_DQALLOC, &gq);
 			if (error) {
 				ASSERT(error != -ENOENT);
 				goto error_rele;
@@ -1738,10 +1734,8 @@ xfs_qm_vop_dqalloc(
 	if ((flags & XFS_QMOPT_PQUOTA) && XFS_IS_PQUOTA_ON(mp)) {
 		if (xfs_get_projid(ip) != prid) {
 			xfs_iunlock(ip, lockflags);
-			error = xfs_qm_dqget(mp, NULL, (xfs_dqid_t)prid,
-						 XFS_DQ_PROJ,
-						 XFS_QMOPT_DQALLOC,
-						 &pq);
+			error = xfs_qm_dqget(mp, (xfs_dqid_t)prid, XFS_DQ_PROJ,
+					XFS_QMOPT_DQALLOC, &pq);
 			if (error) {
 				ASSERT(error != -ENOENT);
 				goto error_rele;
diff --git a/fs/xfs/xfs_qm_bhv.c b/fs/xfs/xfs_qm_bhv.c
index 2be6d27..531e822 100644
--- a/fs/xfs/xfs_qm_bhv.c
+++ b/fs/xfs/xfs_qm_bhv.c
@@ -72,7 +72,7 @@ xfs_qm_statvfs(
 	xfs_mount_t		*mp = ip->i_mount;
 	xfs_dquot_t		*dqp;
 
-	if (!xfs_qm_dqget(mp, NULL, xfs_get_projid(ip), XFS_DQ_PROJ, 0, &dqp)) {
+	if (!xfs_qm_dqget(mp, xfs_get_projid(ip), XFS_DQ_PROJ, 0, &dqp)) {
 		xfs_fill_statvfs_from_dquot(statp, dqp);
 		xfs_qm_dqput(dqp);
 	}
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 0234cfc..b9243f5 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -425,7 +425,7 @@ xfs_qm_scall_setqlim(
 	 * a reference to the dquot, so it's safe to do this unlock/lock without
 	 * it being reclaimed in the mean time.
 	 */
-	error = xfs_qm_dqget(mp, NULL, id, type, XFS_QMOPT_DQALLOC, &dqp);
+	error = xfs_qm_dqget(mp, id, type, XFS_QMOPT_DQALLOC, &dqp);
 	if (error) {
 		ASSERT(error != -ENOENT);
 		goto out_unlock;
@@ -700,7 +700,7 @@ xfs_qm_scall_getquota(
 	 * we aren't passing the XFS_QMOPT_DOALLOC flag. If it doesn't
 	 * exist, we'll get ENOENT back.
 	 */
-	error = xfs_qm_dqget(mp, NULL, id, type, 0, &dqp);
+	error = xfs_qm_dqget(mp, id, type, 0, &dqp);
 	if (error)
 		return error;
 

[PATCH 06/13] xfs: fetch dquots directly during quotacheck

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Quotacheck only runs during mount, which means that there are no other
processes in the system that could be doing chown or chproj.  Therefore
there's no potential for racing to attach dquots to the inode so we can
drop all the ILOCK and race detection bits from quotacheck.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_qm.c |   28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)


diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 385d315..123ea5f 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1065,16 +1065,17 @@ xfs_qm_dqiterate(
 STATIC int
 xfs_qm_quotacheck_dqadjust(
 	struct xfs_inode	*ip,
-	xfs_dqid_t		id,
 	uint			type,
 	xfs_qcnt_t		nblks,
 	xfs_qcnt_t		rtblks)
 {
 	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_dquot	*dqp;
+	xfs_dqid_t		id;
 	int			error;
 
-	error = xfs_qm_dqget_inode(mp, ip, type, XFS_QMOPT_DQALLOC, &dqp);
+	id = xfs_qm_id_for_quotatype(ip, type);
+	error = xfs_qm_dqget(mp, id, type, XFS_QMOPT_DQALLOC, &dqp);
 	if (error) {
 		/*
 		 * Shouldn't be able to turn off quotas here.
@@ -1147,13 +1148,10 @@ xfs_qm_dqusage_adjust(
 	}
 
 	/*
-	 * We don't _need_ to take the ilock EXCL. However, the xfs_qm_dqget
-	 * interface expects the inode to be exclusively locked because that's
-	 * the case in all other instances. It's OK that we do this because
-	 * quotacheck is done only at mount time.
+	 * 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, XFS_ILOCK_EXCL,
-			 &ip);
+	error = xfs_iget(mp, NULL, ino, XFS_IGET_DONTCACHE, 0, &ip);
 	if (error) {
 		*res = BULKSTAT_RV_NOTHING;
 		return error;
@@ -1188,33 +1186,31 @@ xfs_qm_dqusage_adjust(
 	 * and quotaoffs don't race. (Quotachecks happen at mount time only).
 	 */
 	if (XFS_IS_UQUOTA_ON(mp)) {
-		error = xfs_qm_quotacheck_dqadjust(ip, ip->i_d.di_uid,
-						   XFS_DQ_USER, nblks, rtblks);
+		error = xfs_qm_quotacheck_dqadjust(ip, XFS_DQ_USER, nblks,
+				rtblks);
 		if (error)
 			goto error0;
 	}
 
 	if (XFS_IS_GQUOTA_ON(mp)) {
-		error = xfs_qm_quotacheck_dqadjust(ip, ip->i_d.di_gid,
-						   XFS_DQ_GROUP, nblks, rtblks);
+		error = xfs_qm_quotacheck_dqadjust(ip, XFS_DQ_GROUP, nblks,
+				rtblks);
 		if (error)
 			goto error0;
 	}
 
 	if (XFS_IS_PQUOTA_ON(mp)) {
-		error = xfs_qm_quotacheck_dqadjust(ip, xfs_get_projid(ip),
-						   XFS_DQ_PROJ, nblks, rtblks);
+		error = xfs_qm_quotacheck_dqadjust(ip, XFS_DQ_PROJ, nblks,
+				rtblks);
 		if (error)
 			goto error0;
 	}
 
-	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 	IRELE(ip);
 	*res = BULKSTAT_RV_DIDONE;
 	return 0;
 
 error0:
-	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 	IRELE(ip);
 	*res = BULKSTAT_RV_GIVEUP;
 	return error;

[PATCH 07/13] xfs: refactor incore dquot initialization functions

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Create two incore dquot initialization functions that will help us to
disentangle dqget and dqread.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_dquot.c |   80 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 50 insertions(+), 30 deletions(-)


diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 62415f1..7154909 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -491,26 +491,14 @@ xfs_qm_dqtobp(
 	return 0;
 }
 
-
-/*
- * Read in the ondisk dquot using dqtobp() then copy it to an incore version,
- * and release the buffer immediately.
- *
- * If XFS_QMOPT_DQALLOC is set, allocate a dquot on disk if it needed.
- */
-int
-xfs_qm_dqread(
+/* Allocate and initialize everything we need for an incore dquot. */
+STATIC struct xfs_dquot *
+xfs_qm_dqinit_once(
 	struct xfs_mount	*mp,
 	xfs_dqid_t		id,
-	uint			type,
-	uint			flags,
-	struct xfs_dquot	**O_dqpp)
+	uint			type)
 {
 	struct xfs_dquot	*dqp;
-	struct xfs_disk_dquot	*ddqp;
-	struct xfs_buf		*bp;
-	struct xfs_trans	*tp = NULL;
-	int			error;
 
 	dqp = kmem_zone_zalloc(xfs_qm_dqzone, KM_SLEEP);
 
@@ -549,7 +537,52 @@ xfs_qm_dqread(
 	}
 
 	XFS_STATS_INC(mp, xs_qm_dquot);
+	return dqp;
+}
+
+/* Copy the in-core quota fields in from the on-disk buffer. */
+STATIC void
+xfs_qm_dqinit_from_buf(
+	struct xfs_dquot	*dqp,
+	struct xfs_disk_dquot	*ddqp)
+{
+	/* copy everything from disk dquot to the incore dquot */
+	memcpy(&dqp->q_core, ddqp, sizeof(xfs_disk_dquot_t));
+	xfs_qm_dquot_logitem_init(dqp);
+
+	/*
+	 * Reservation counters are defined as reservation plus current usage
+	 * to avoid having to add every time.
+	 */
+	dqp->q_res_bcount = be64_to_cpu(ddqp->d_bcount);
+	dqp->q_res_icount = be64_to_cpu(ddqp->d_icount);
+	dqp->q_res_rtbcount = be64_to_cpu(ddqp->d_rtbcount);
+
+	/* initialize the dquot speculative prealloc thresholds */
+	xfs_dquot_set_prealloc_limits(dqp);
+}
+
+/*
+ * Read in the ondisk dquot using dqtobp() then copy it to an incore version,
+ * and release the buffer immediately.
+ *
+ * If XFS_QMOPT_DQALLOC is set, allocate a dquot on disk if it needed.
+ */
+int
+xfs_qm_dqread(
+	struct xfs_mount	*mp,
+	xfs_dqid_t		id,
+	uint			type,
+	uint			flags,
+	struct xfs_dquot	**O_dqpp)
+{
+	struct xfs_dquot	*dqp;
+	struct xfs_disk_dquot	*ddqp;
+	struct xfs_buf		*bp;
+	struct xfs_trans	*tp = NULL;
+	int			error;
 
+	dqp = xfs_qm_dqinit_once(mp, id, type);
 	trace_xfs_dqread(dqp);
 
 	if (flags & XFS_QMOPT_DQALLOC) {
@@ -574,20 +607,7 @@ xfs_qm_dqread(
 		goto error1;
 	}
 
-	/* copy everything from disk dquot to the incore dquot */
-	memcpy(&dqp->q_core, ddqp, sizeof(xfs_disk_dquot_t));
-	xfs_qm_dquot_logitem_init(dqp);
-
-	/*
-	 * Reservation counters are defined as reservation plus current usage
-	 * to avoid having to add every time.
-	 */
-	dqp->q_res_bcount = be64_to_cpu(ddqp->d_bcount);
-	dqp->q_res_icount = be64_to_cpu(ddqp->d_icount);
-	dqp->q_res_rtbcount = be64_to_cpu(ddqp->d_rtbcount);
-
-	/* initialize the dquot speculative prealloc thresholds */
-	xfs_dquot_set_prealloc_limits(dqp);
+	xfs_qm_dqinit_from_buf(dqp, ddqp);
 
 	/* Mark the buf so that this will stay incore a little longer */
 	xfs_buf_set_ref(bp, XFS_DQUOT_REF);

[PATCH 08/13] xfs: refactor xfs_qm_dqtobp and xfs_qm_dqalloc

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Separate the disk dquot read and allocation functionality into
two helper functions, then refactor dqread to call them directly.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_dquot.c |  261 ++++++++++++++++++++--------------------------------
 1 file changed, 99 insertions(+), 162 deletions(-)


diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 7154909..ef4cd56 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -288,49 +288,44 @@ xfs_dquot_set_prealloc_limits(struct xfs_dquot *dqp)
 }
 
 /*
- * Allocate a block and fill it with dquots.
- * This is called when the bmapi finds a hole.
+ * Ensure that the given in-core dquot has a buffer on disk backing it, and
+ * return the buffer. This is called when the bmapi finds a hole.
  */
 STATIC int
-xfs_qm_dqalloc(
-	xfs_trans_t	**tpp,
-	xfs_mount_t	*mp,
-	xfs_dquot_t	*dqp,
-	xfs_inode_t	*quotip,
-	xfs_fileoff_t	offset_fsb,
-	xfs_buf_t	**O_bpp)
+xfs_qm_dqalloc_ondisk(
+	struct xfs_trans	**tpp,
+	struct xfs_dquot	*dqp,
+	struct xfs_buf		**bpp)
 {
-	xfs_fsblock_t	firstblock;
-	struct xfs_defer_ops dfops;
-	xfs_bmbt_irec_t map;
-	int		nmaps, error;
-	xfs_buf_t	*bp;
-	xfs_trans_t	*tp = *tpp;
-
-	ASSERT(tp != NULL);
+	struct xfs_bmbt_irec	map;
+	struct xfs_defer_ops	dfops;
+	struct xfs_mount	*mp = (*tpp)->t_mountp;
+	struct xfs_buf		*bp;
+	struct xfs_inode	*quotip;
+	xfs_fsblock_t		firstblock;
+	int			nmaps = 1;
+	int			error;
 
 	trace_xfs_dqalloc(dqp);
 
-	/*
-	 * Initialize the bmap freelist prior to calling bmapi code.
-	 */
+	quotip = xfs_quota_inode(mp, dqp->dq_flags);
 	xfs_defer_init(&dfops, &firstblock);
 	xfs_ilock(quotip, XFS_ILOCK_EXCL);
-	/*
-	 * Return if this type of quotas is turned off while we didn't
-	 * have an inode lock
-	 */
 	if (!xfs_this_quota_on(dqp->q_mount, dqp->dq_flags)) {
+		/*
+		 * Return if this type of quotas is turned off while we didn't
+		 * have an inode lock
+		 */
 		xfs_iunlock(quotip, XFS_ILOCK_EXCL);
 		return -ESRCH;
 	}
 
-	xfs_trans_ijoin(tp, quotip, XFS_ILOCK_EXCL);
-	nmaps = 1;
-	error = xfs_bmapi_write(tp, quotip, offset_fsb,
-				XFS_DQUOT_CLUSTER_SIZE_FSB, XFS_BMAPI_METADATA,
-				&firstblock, XFS_QM_DQALLOC_SPACE_RES(mp),
-				&map, &nmaps, &dfops);
+	/* Create the block mapping. */
+	xfs_trans_ijoin(*tpp, quotip, XFS_ILOCK_EXCL);
+	error = xfs_bmapi_write(*tpp, quotip, dqp->q_fileoffset,
+			XFS_DQUOT_CLUSTER_SIZE_FSB, XFS_BMAPI_METADATA,
+			&firstblock, XFS_QM_DQALLOC_SPACE_RES(mp),
+			&map, &nmaps, &dfops);
 	if (error)
 		goto error0;
 	ASSERT(map.br_blockcount == XFS_DQUOT_CLUSTER_SIZE_FSB);
@@ -344,10 +339,8 @@ xfs_qm_dqalloc(
 	dqp->q_blkno = XFS_FSB_TO_DADDR(mp, map.br_startblock);
 
 	/* now we can just get the buffer (there's nothing to read yet) */
-	bp = xfs_trans_get_buf(tp, mp->m_ddev_targp,
-			       dqp->q_blkno,
-			       mp->m_quotainfo->qi_dqchunklen,
-			       0);
+	bp = xfs_trans_get_buf(*tpp, mp->m_ddev_targp, dqp->q_blkno,
+			mp->m_quotainfo->qi_dqchunklen, 0);
 	if (!bp) {
 		error = -ENOMEM;
 		goto error1;
@@ -358,37 +351,22 @@ xfs_qm_dqalloc(
 	 * Make a chunk of dquots out of this buffer and log
 	 * the entire thing.
 	 */
-	xfs_qm_init_dquot_blk(tp, mp, be32_to_cpu(dqp->q_core.d_id),
+	xfs_qm_init_dquot_blk(*tpp, mp, be32_to_cpu(dqp->q_core.d_id),
 			      dqp->dq_flags & XFS_DQ_ALLTYPES, bp);
+	xfs_buf_set_ref(bp, XFS_DQUOT_REF);
 
 	/*
-	 * xfs_defer_finish() may commit the current transaction and
-	 * start a second transaction if the freelist is not empty.
-	 *
-	 * Since we still want to modify this buffer, we need to
-	 * ensure that the buffer is not released on commit of
-	 * the first transaction and ensure the buffer is added to the
-	 * second transaction.
-	 *
-	 * If there is only one transaction then don't stop the buffer
-	 * from being released when it commits later on.
+	 * Hold the buffer and join it to the dfops so that we'll still own
+	 * the buffer when we return to the caller.
 	 */
-
-	xfs_trans_bhold(tp, bp);
-
+	xfs_trans_bhold(*tpp, bp);
+	error = xfs_defer_bjoin(&dfops, bp);
+	if (error)
+		goto error1;
 	error = xfs_defer_finish(tpp, &dfops);
 	if (error)
 		goto error1;
-
-	/* Transaction was committed? */
-	if (*tpp != tp) {
-		tp = *tpp;
-		xfs_trans_bjoin(tp, bp);
-	} else {
-		xfs_trans_bhold_release(tp, bp);
-	}
-
-	*O_bpp = bp;
+	*bpp = bp;
 	return 0;
 
 error1:
@@ -398,32 +376,25 @@ xfs_qm_dqalloc(
 }
 
 /*
- * Maps a dquot to the buffer containing its on-disk version.
- * This returns a ptr to the buffer containing the on-disk dquot
- * in the bpp param, and a ptr to the on-disk dquot within that buffer
+ * Read in the in-core dquot's on-disk metadata and return the buffer.
+ * Returns ENOENT to signal a hole.
  */
 STATIC int
-xfs_qm_dqtobp(
-	xfs_trans_t		**tpp,
-	xfs_dquot_t		*dqp,
-	xfs_disk_dquot_t	**O_ddpp,
-	xfs_buf_t		**O_bpp,
-	uint			flags)
+xfs_qm_dqread_ondisk(
+	struct xfs_mount	*mp,
+	struct xfs_dquot	*dqp,
+	struct xfs_buf		**bpp)
 {
 	struct xfs_bmbt_irec	map;
-	int			nmaps = 1, error;
 	struct xfs_buf		*bp;
 	struct xfs_inode	*quotip;
-	struct xfs_mount	*mp = dqp->q_mount;
-	xfs_dqid_t		id = be32_to_cpu(dqp->q_core.d_id);
-	struct xfs_trans	*tp = (tpp ? *tpp : NULL);
 	uint			lock_mode;
+	int			nmaps = 1;
+	int			error;
 
-	quotip = xfs_quota_inode(dqp->q_mount, dqp->dq_flags);
-	dqp->q_fileoffset = (xfs_fileoff_t)id / mp->m_quotainfo->qi_dqperchunk;
-
+	quotip = xfs_quota_inode(mp, dqp->dq_flags);
 	lock_mode = xfs_ilock_data_map_shared(quotip);
-	if (!xfs_this_quota_on(dqp->q_mount, dqp->dq_flags)) {
+	if (!xfs_this_quota_on(mp, dqp->dq_flags)) {
 		/*
 		 * Return if this type of quotas is turned off while we
 		 * didn't have the quota inode lock.
@@ -436,57 +407,36 @@ xfs_qm_dqtobp(
 	 * Find the block map; no allocations yet
 	 */
 	error = xfs_bmapi_read(quotip, dqp->q_fileoffset,
-			       XFS_DQUOT_CLUSTER_SIZE_FSB, &map, &nmaps, 0);
-
+			XFS_DQUOT_CLUSTER_SIZE_FSB, &map, &nmaps, 0);
 	xfs_iunlock(quotip, lock_mode);
 	if (error)
 		return error;
 
 	ASSERT(nmaps == 1);
-	ASSERT(map.br_blockcount == 1);
+	ASSERT(map.br_blockcount >= 1);
+	ASSERT(map.br_startblock != DELAYSTARTBLOCK);
+	if (map.br_startblock == HOLESTARTBLOCK)
+		return -ENOENT;
+
+	trace_xfs_dqtobp_read(dqp);
 
 	/*
-	 * Offset of dquot in the (fixed sized) dquot chunk.
+	 * store the blkno etc so that we don't have to do the
+	 * mapping all the time
 	 */
-	dqp->q_bufoffset = (id % mp->m_quotainfo->qi_dqperchunk) *
-		sizeof(xfs_dqblk_t);
-
-	ASSERT(map.br_startblock != DELAYSTARTBLOCK);
-	if (map.br_startblock == HOLESTARTBLOCK) {
-		/*
-		 * We don't allocate unless we're asked to
-		 */
-		if (!(flags & XFS_QMOPT_DQALLOC))
-			return -ENOENT;
-
-		ASSERT(tp);
-		error = xfs_qm_dqalloc(tpp, mp, dqp, quotip,
-					dqp->q_fileoffset, &bp);
-		if (error)
-			return error;
-		tp = *tpp;
-	} else {
-		trace_xfs_dqtobp_read(dqp);
+	dqp->q_blkno = XFS_FSB_TO_DADDR(mp, map.br_startblock);
 
-		/*
-		 * store the blkno etc so that we don't have to do the
-		 * mapping all the time
-		 */
-		dqp->q_blkno = XFS_FSB_TO_DADDR(mp, map.br_startblock);
-
-		error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp,
-					   dqp->q_blkno,
-					   mp->m_quotainfo->qi_dqchunklen,
-					   0, &bp, &xfs_dquot_buf_ops);
-		if (error) {
-			ASSERT(bp == NULL);
-			return error;
-		}
+	error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dqp->q_blkno,
+			mp->m_quotainfo->qi_dqchunklen, 0, &bp,
+			&xfs_dquot_buf_ops);
+	if (error) {
+		ASSERT(bp == NULL);
+		return error;
 	}
 
 	ASSERT(xfs_buf_islocked(bp));
-	*O_bpp = bp;
-	*O_ddpp = bp->b_addr + dqp->q_bufoffset;
+	xfs_buf_set_ref(bp, XFS_DQUOT_REF);
+	*bpp = bp;
 
 	return 0;
 }
@@ -508,6 +458,12 @@ xfs_qm_dqinit_once(
 	INIT_LIST_HEAD(&dqp->q_lru);
 	mutex_init(&dqp->q_qlock);
 	init_waitqueue_head(&dqp->q_pinwait);
+	dqp->q_fileoffset = (xfs_fileoff_t)id / mp->m_quotainfo->qi_dqperchunk;
+	/*
+	 * Offset of dquot in the (fixed sized) dquot chunk.
+	 */
+	dqp->q_bufoffset = (id % mp->m_quotainfo->qi_dqperchunk) *
+			sizeof(xfs_dqblk_t);
 
 	/*
 	 * Because we want to use a counting completion, complete
@@ -544,8 +500,10 @@ xfs_qm_dqinit_once(
 STATIC void
 xfs_qm_dqinit_from_buf(
 	struct xfs_dquot	*dqp,
-	struct xfs_disk_dquot	*ddqp)
+	struct xfs_buf		*bp)
 {
+	struct xfs_disk_dquot	*ddqp = bp->b_addr + dqp->q_bufoffset;
+
 	/* copy everything from disk dquot to the incore dquot */
 	memcpy(&dqp->q_core, ddqp, sizeof(xfs_disk_dquot_t));
 	xfs_qm_dquot_logitem_init(dqp);
@@ -574,74 +532,53 @@ xfs_qm_dqread(
 	xfs_dqid_t		id,
 	uint			type,
 	uint			flags,
-	struct xfs_dquot	**O_dqpp)
+	struct xfs_dquot	**dqpp)
 {
 	struct xfs_dquot	*dqp;
-	struct xfs_disk_dquot	*ddqp;
 	struct xfs_buf		*bp;
-	struct xfs_trans	*tp = NULL;
+	struct xfs_trans	*tp;
 	int			error;
 
 	dqp = xfs_qm_dqinit_once(mp, id, type);
 	trace_xfs_dqread(dqp);
 
-	if (flags & XFS_QMOPT_DQALLOC) {
+	/* Try to read the buffer... */
+	error = xfs_qm_dqread_ondisk(mp, dqp, &bp);
+	if (error == -ENOENT && (flags & XFS_QMOPT_DQALLOC)) {
+		/* ...or allocate a new block and buffer. */
 		error = xfs_trans_alloc(mp, &M_RES(mp)->tr_qm_dqalloc,
 				XFS_QM_DQALLOC_SPACE_RES(mp), 0, 0, &tp);
 		if (error)
-			goto error0;
-	}
+			goto err;
 
-	/*
-	 * get a pointer to the on-disk dquot and the buffer containing it
-	 * dqp already knows its own type (GROUP/USER).
-	 */
-	error = xfs_qm_dqtobp(&tp, dqp, &ddqp, &bp, flags);
-	if (error) {
-		/*
-		 * This can happen if quotas got turned off (ESRCH),
-		 * or if the dquot didn't exist on disk and we ask to
-		 * allocate (ENOENT).
-		 */
-		trace_xfs_dqread_fail(dqp);
-		goto error1;
-	}
+		error = xfs_qm_dqalloc_ondisk(&tp, dqp, &bp);
+		if (error)
+			goto err_cancel;
 
-	xfs_qm_dqinit_from_buf(dqp, ddqp);
+		error = xfs_trans_commit(tp);
+	}
+	ASSERT(xfs_buf_islocked(bp));
+	if (error)
+		goto err;
 
-	/* Mark the buf so that this will stay incore a little longer */
-	xfs_buf_set_ref(bp, XFS_DQUOT_REF);
+	xfs_qm_dqinit_from_buf(dqp, bp);
 
 	/*
-	 * We got the buffer with a xfs_trans_read_buf() (in dqtobp())
-	 * So we need to release with xfs_trans_brelse().
-	 * The strategy here is identical to that of inodes; we lock
-	 * the dquot in xfs_qm_dqget() before making it accessible to
-	 * others. This is because dquots, like inodes, need a good level of
-	 * concurrency, and we don't want to take locks on the entire buffers
-	 * for dquot accesses.
-	 * Note also that the dquot buffer may even be dirty at this point, if
-	 * this particular dquot was repaired. We still aren't afraid to
-	 * brelse it because we have the changes incore.
+	 * At this point we should have a clean locked buffer.  Release the
+	 * buffer since the incore dquot has its own copy and locking
+	 * protocol so we needn't tie up the buffer any further.
 	 */
 	ASSERT(xfs_buf_islocked(bp));
-	xfs_trans_brelse(tp, bp);
-
-	if (tp) {
-		error = xfs_trans_commit(tp);
-		if (error)
-			goto error0;
-	}
-
-	*O_dqpp = dqp;
+	xfs_buf_relse(bp);
+	*dqpp = dqp;
 	return error;
 
-error1:
-	if (tp)
-		xfs_trans_cancel(tp);
-error0:
+err_cancel:
+	xfs_trans_cancel(tp);
+err:
+	trace_xfs_dqread_fail(dqp);
 	xfs_qm_dqdestroy(dqp);
-	*O_dqpp = NULL;
+	*dqpp = NULL;
 	return error;
 }
 

[PATCH 09/13] xfs: remove xfs_qm_dqread flags argument

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Move the _qm_dqread functionality to _qm_dqensure, then remove the flags
argument from xfs_qm_dqread since the only DQALLOC users were internal
to xfs_dquot.c anyway.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_dquot.c |   39 ++++++++++++++++++++++++---------------
 fs/xfs/xfs_dquot.h |    6 +++---
 fs/xfs/xfs_qm.c    |    4 ++--
 3 files changed, 29 insertions(+), 20 deletions(-)


diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index ef4cd56..a82d928 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -522,16 +522,15 @@ xfs_qm_dqinit_from_buf(
 
 /*
  * Read in the ondisk dquot using dqtobp() then copy it to an incore version,
- * and release the buffer immediately.
- *
- * If XFS_QMOPT_DQALLOC is set, allocate a dquot on disk if it needed.
+ * and release the buffer immediately.  If @can_alloc is specified, fill any
+ * holes in the on-disk metadata.
  */
-int
-xfs_qm_dqread(
+STATIC int
+xfs_qm_dqensure(
 	struct xfs_mount	*mp,
 	xfs_dqid_t		id,
 	uint			type,
-	uint			flags,
+	uint			can_alloc,
 	struct xfs_dquot	**dqpp)
 {
 	struct xfs_dquot	*dqp;
@@ -544,7 +543,7 @@ xfs_qm_dqread(
 
 	/* Try to read the buffer... */
 	error = xfs_qm_dqread_ondisk(mp, dqp, &bp);
-	if (error == -ENOENT && (flags & XFS_QMOPT_DQALLOC)) {
+	if (error == -ENOENT && can_alloc) {
 		/* ...or allocate a new block and buffer. */
 		error = xfs_trans_alloc(mp, &M_RES(mp)->tr_qm_dqalloc,
 				XFS_QM_DQALLOC_SPACE_RES(mp), 0, 0, &tp);
@@ -557,10 +556,10 @@ xfs_qm_dqread(
 
 		error = xfs_trans_commit(tp);
 	}
-	ASSERT(xfs_buf_islocked(bp));
 	if (error)
 		goto err;
 
+	ASSERT(xfs_buf_islocked(bp));
 	xfs_qm_dqinit_from_buf(dqp, bp);
 
 	/*
@@ -583,6 +582,20 @@ xfs_qm_dqread(
 }
 
 /*
+ * Read in the ondisk dquot using dqtobp() then copy it to an incore version,
+ * and release the buffer immediately.
+ */
+int
+xfs_qm_dqread(
+	struct xfs_mount	*mp,
+	xfs_dqid_t		id,
+	uint			type,
+	struct xfs_dquot	**dqpp)
+{
+	return xfs_qm_dqensure(mp, id, type, 0, dqpp);
+}
+
+/*
  * Advance to the next id in the current chunk, or if at the
  * end of the chunk, skip ahead to first id in next allocated chunk
  * using the SEEK_DATA interface.
@@ -746,7 +759,7 @@ xfs_qm_dqget(
 	struct xfs_mount	*mp,
 	xfs_dqid_t		id,
 	uint			type,
-	uint			flags,	  /* DQALLOC, DQSUSER, DQREPAIR, DOWARN */
+	uint			can_alloc,
 	struct xfs_dquot	**O_dqpp)
 {
 	struct xfs_quotainfo	*qi = mp->m_quotainfo;
@@ -765,7 +778,7 @@ xfs_qm_dqget(
 		return 0;
 	}
 
-	error = xfs_qm_dqread(mp, id, type, flags, &dqp);
+	error = xfs_qm_dqensure(mp, id, type, can_alloc, &dqp);
 	if (error)
 		return error;
 
@@ -820,16 +833,12 @@ xfs_qm_dqget_inode(
 	struct radix_tree_root	*tree = xfs_dquot_tree(qi, type);
 	struct xfs_dquot	*dqp;
 	xfs_dqid_t		id;
-	uint			flags = 0;
 	int			error;
 
 	error = xfs_qm_dqget_checks(mp, type);
 	if (error)
 		return error;
 
-	if (can_alloc)
-		flags |= XFS_QMOPT_DQALLOC;
-
 	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
 	ASSERT(xfs_inode_dquot(ip, type) == NULL);
 
@@ -850,7 +859,7 @@ xfs_qm_dqget_inode(
 	 * we re-acquire the lock.
 	 */
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
-	error = xfs_qm_dqread(mp, id, type, flags, &dqp);
+	error = xfs_qm_dqensure(mp, id, type, can_alloc, &dqp);
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	if (error)
 		return error;
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 1458de3..6bc5ed8 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -160,8 +160,8 @@ static inline bool xfs_dquot_lowsp(struct xfs_dquot *dqp)
 #define XFS_QM_ISPDQ(dqp)	((dqp)->dq_flags & XFS_DQ_PROJ)
 #define XFS_QM_ISGDQ(dqp)	((dqp)->dq_flags & XFS_DQ_GROUP)
 
-extern int		xfs_qm_dqread(struct xfs_mount *, xfs_dqid_t, uint,
-					uint, struct xfs_dquot	**);
+extern int		xfs_qm_dqread(struct xfs_mount *mp, xfs_dqid_t id,
+					uint type, struct xfs_dquot **dqpp);
 extern void		xfs_qm_dqdestroy(xfs_dquot_t *);
 extern int		xfs_qm_dqflush(struct xfs_dquot *, struct xfs_buf **);
 extern void		xfs_qm_dqunpin_wait(xfs_dquot_t *);
@@ -172,7 +172,7 @@ extern void		xfs_qm_adjust_dqlimits(struct xfs_mount *,
 extern xfs_dqid_t	xfs_qm_id_for_quotatype(struct xfs_inode *ip,
 					uint type);
 extern int		xfs_qm_dqget(struct xfs_mount *mp, xfs_dqid_t id,
-					uint type, uint flags,
+					uint type, uint can_alloc,
 					struct xfs_dquot **dqpp);
 extern int		xfs_qm_dqget_inode(struct xfs_mount *mp,
 					struct xfs_inode *ip, uint type,
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 123ea5f..e3d9b38 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -572,7 +572,7 @@ xfs_qm_set_defquota(
 	struct xfs_def_quota    *defq;
 	int			error;
 
-	error = xfs_qm_dqread(mp, 0, type, 0, &dqp);
+	error = xfs_qm_dqread(mp, 0, type, &dqp);
 
 	if (!error) {
 		xfs_disk_dquot_t        *ddqp = &dqp->q_core;
@@ -650,7 +650,7 @@ xfs_qm_init_quotainfo(
 			XFS_IS_UQUOTA_RUNNING(mp) ? XFS_DQ_USER :
 			 (XFS_IS_GQUOTA_RUNNING(mp) ? XFS_DQ_GROUP :
 			  XFS_DQ_PROJ),
-			0, &dqp);
+			&dqp);
 
 	if (!error) {
 		xfs_disk_dquot_t	*ddqp = &dqp->q_core;

[PATCH 10/13] xfs: replace XFS_QMOPT_DQALLOC with XFS_DQGET_{ALLOC, EXISTS}

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

DQALLOC is only ever used with xfs_qm_dqget*, and the only flag that the
_dqget family of functions cares about is DQALLOC.  Therefore, change
the name to make it clear that it's only a dqget flag.  Since these are
modes and no longer bit flags, define a XFS_DQGET_EXISTS flag and
convert all the callers.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_quota_defs.h |    4 +++
 fs/xfs/xfs_dquot.c             |   18 ++++++++-------
 fs/xfs/xfs_dquot.h             |    4 ++-
 fs/xfs/xfs_iomap.c             |    2 +-
 fs/xfs/xfs_qm.c                |   47 +++++++++++++++++++---------------------
 fs/xfs/xfs_qm_bhv.c            |    3 ++-
 fs/xfs/xfs_qm_syscalls.c       |    4 ++-
 fs/xfs/xfs_quota.h             |    3 ++-
 fs/xfs/xfs_reflink.c           |    4 ++-
 9 files changed, 45 insertions(+), 44 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index c80b226..56c8787 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -102,12 +102,14 @@ typedef uint16_t	xfs_qwarncnt_t;
 #define XFS_IS_GQUOTA_ON(mp)	((mp)->m_qflags & XFS_GQUOTA_ACTIVE)
 #define XFS_IS_PQUOTA_ON(mp)	((mp)->m_qflags & XFS_PQUOTA_ACTIVE)
 
+#define XFS_DQGET_EXISTS	(0)	/* disk dquot must already exist */
+#define XFS_DQGET_ALLOC	(1)	/* allocate dquot ondisk if needed */
+
 /*
  * Flags to tell various functions what to do. Not all of these are meaningful
  * to a single function. None of these XFS_QMOPT_* flags are meant to have
  * persistent values (ie. their values can and will change between versions)
  */
-#define XFS_QMOPT_DQALLOC	0x0000002 /* alloc dquot ondisk if needed */
 #define XFS_QMOPT_UQUOTA	0x0000004 /* user dquot requested */
 #define XFS_QMOPT_PQUOTA	0x0000008 /* project dquot requested */
 #define XFS_QMOPT_FORCE_RES	0x0000010 /* ignore quota limits */
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index a82d928..7ce3dbd 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -522,7 +522,7 @@ xfs_qm_dqinit_from_buf(
 
 /*
  * Read in the ondisk dquot using dqtobp() then copy it to an incore version,
- * and release the buffer immediately.  If @can_alloc is specified, fill any
+ * and release the buffer immediately.  If @dqmode is DQALLOC, fill any
  * holes in the on-disk metadata.
  */
 STATIC int
@@ -530,7 +530,7 @@ xfs_qm_dqensure(
 	struct xfs_mount	*mp,
 	xfs_dqid_t		id,
 	uint			type,
-	uint			can_alloc,
+	unsigned int		dqmode,
 	struct xfs_dquot	**dqpp)
 {
 	struct xfs_dquot	*dqp;
@@ -543,7 +543,7 @@ xfs_qm_dqensure(
 
 	/* Try to read the buffer... */
 	error = xfs_qm_dqread_ondisk(mp, dqp, &bp);
-	if (error == -ENOENT && can_alloc) {
+	if (error == -ENOENT && dqmode == XFS_DQGET_ALLOC) {
 		/* ...or allocate a new block and buffer. */
 		error = xfs_trans_alloc(mp, &M_RES(mp)->tr_qm_dqalloc,
 				XFS_QM_DQALLOC_SPACE_RES(mp), 0, 0, &tp);
@@ -759,7 +759,7 @@ xfs_qm_dqget(
 	struct xfs_mount	*mp,
 	xfs_dqid_t		id,
 	uint			type,
-	uint			can_alloc,
+	unsigned int		dqmode,
 	struct xfs_dquot	**O_dqpp)
 {
 	struct xfs_quotainfo	*qi = mp->m_quotainfo;
@@ -778,7 +778,7 @@ xfs_qm_dqget(
 		return 0;
 	}
 
-	error = xfs_qm_dqensure(mp, id, type, can_alloc, &dqp);
+	error = xfs_qm_dqensure(mp, id, type, dqmode, &dqp);
 	if (error)
 		return error;
 
@@ -817,7 +817,7 @@ xfs_qm_id_for_quotatype(
 }
 
 /*
- * Return the dquot for a given inode and type.  If @can_alloc is true, then
+ * Return the dquot for a given inode and type.  If @dqmode is DQALLOC, then
  * allocate blocks if needed.  The inode's ILOCK must be held and it must not
  * have already had an inode attached.
  */
@@ -826,7 +826,7 @@ xfs_qm_dqget_inode(
 	struct xfs_mount	*mp,
 	struct xfs_inode	*ip,
 	uint			type,
-	uint			can_alloc,
+	unsigned int		dqmode,
 	struct xfs_dquot	**O_dqpp)
 {
 	struct xfs_quotainfo	*qi = mp->m_quotainfo;
@@ -859,7 +859,7 @@ xfs_qm_dqget_inode(
 	 * we re-acquire the lock.
 	 */
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
-	error = xfs_qm_dqensure(mp, id, type, can_alloc, &dqp);
+	error = xfs_qm_dqensure(mp, id, type, dqmode, &dqp);
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	if (error)
 		return error;
@@ -918,7 +918,7 @@ xfs_qm_dqget_next(
 
 	*dqpp = NULL;
 	for (; !error; error = xfs_dq_get_next_id(mp, type, &id)) {
-		error = xfs_qm_dqget(mp, id, type, 0, &dqp);
+		error = xfs_qm_dqget(mp, id, type, XFS_DQGET_EXISTS, &dqp);
 		if (error == -ENOENT)
 			continue;
 		else if (error != 0)
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 6bc5ed8..fe99e43 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -172,11 +172,11 @@ extern void		xfs_qm_adjust_dqlimits(struct xfs_mount *,
 extern xfs_dqid_t	xfs_qm_id_for_quotatype(struct xfs_inode *ip,
 					uint type);
 extern int		xfs_qm_dqget(struct xfs_mount *mp, xfs_dqid_t id,
-					uint type, uint can_alloc,
+					uint type, unsigned int dqmode,
 					struct xfs_dquot **dqpp);
 extern int		xfs_qm_dqget_inode(struct xfs_mount *mp,
 					struct xfs_inode *ip, uint type,
-					uint can_alloc,
+					unsigned int dqmode,
 					struct xfs_dquot **dqpp);
 extern int		xfs_qm_dqget_next(struct xfs_mount *mp, xfs_dqid_t id,
 					uint type, struct xfs_dquot **dqpp);
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 58b04ea..b6ca5ec 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -576,7 +576,7 @@ xfs_file_iomap_begin_delay(
 		goto done;
 	}
 
-	error = xfs_qm_dqattach_locked(ip, 0);
+	error = xfs_qm_dqattach_locked(ip, XFS_DQGET_EXISTS);
 	if (error)
 		goto out_unlock;
 
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index e3d9b38..1b94da0 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -262,14 +262,14 @@ xfs_qm_unmount_quotas(
 
 STATIC int
 xfs_qm_dqattach_one(
-	xfs_inode_t	*ip,
-	xfs_dqid_t	id,
-	uint		type,
-	uint		doalloc,
-	xfs_dquot_t	**IO_idqpp)
+	struct xfs_inode	*ip,
+	xfs_dqid_t		id,
+	uint			type,
+	unsigned int		dqmode,
+	struct xfs_dquot	**IO_idqpp)
 {
-	xfs_dquot_t	*dqp;
-	int		error;
+	struct xfs_dquot	*dqp;
+	int			error;
 
 	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
 	error = 0;
@@ -291,7 +291,7 @@ xfs_qm_dqattach_one(
 	 * exist on disk and we didn't ask it to allocate; ESRCH if quotas got
 	 * turned off suddenly.
 	 */
-	error = xfs_qm_dqget_inode(ip->i_mount, ip, type, doalloc, &dqp);
+	error = xfs_qm_dqget_inode(ip->i_mount, ip, type, dqmode, &dqp);
 	if (error)
 		return error;
 
@@ -326,17 +326,17 @@ xfs_qm_need_dqattach(
 /*
  * Given a locked inode, attach dquot(s) to it, taking U/G/P-QUOTAON
  * into account.
- * If XFS_QMOPT_DQALLOC, the dquot(s) will be allocated if needed.
+ * If @dqmode is XFS_DQGET_ALLOC, the dquot(s) will be allocated if needed.
  * Inode may get unlocked and relocked in here, and the caller must deal with
  * the consequences.
  */
 int
 xfs_qm_dqattach_locked(
-	xfs_inode_t	*ip,
-	uint		flags)
+	struct xfs_inode	*ip,
+	unsigned int		dqmode)
 {
-	xfs_mount_t	*mp = ip->i_mount;
-	int		error = 0;
+	struct xfs_mount	*mp = ip->i_mount;
+	int			error = 0;
 
 	if (!xfs_qm_need_dqattach(ip))
 		return 0;
@@ -345,8 +345,7 @@ xfs_qm_dqattach_locked(
 
 	if (XFS_IS_UQUOTA_ON(mp) && !ip->i_udquot) {
 		error = xfs_qm_dqattach_one(ip, ip->i_d.di_uid, XFS_DQ_USER,
-						flags & XFS_QMOPT_DQALLOC,
-						&ip->i_udquot);
+				dqmode, &ip->i_udquot);
 		if (error)
 			goto done;
 		ASSERT(ip->i_udquot);
@@ -354,8 +353,7 @@ xfs_qm_dqattach_locked(
 
 	if (XFS_IS_GQUOTA_ON(mp) && !ip->i_gdquot) {
 		error = xfs_qm_dqattach_one(ip, ip->i_d.di_gid, XFS_DQ_GROUP,
-						flags & XFS_QMOPT_DQALLOC,
-						&ip->i_gdquot);
+				dqmode, &ip->i_gdquot);
 		if (error)
 			goto done;
 		ASSERT(ip->i_gdquot);
@@ -363,8 +361,7 @@ xfs_qm_dqattach_locked(
 
 	if (XFS_IS_PQUOTA_ON(mp) && !ip->i_pdquot) {
 		error = xfs_qm_dqattach_one(ip, xfs_get_projid(ip), XFS_DQ_PROJ,
-						flags & XFS_QMOPT_DQALLOC,
-						&ip->i_pdquot);
+				dqmode, &ip->i_pdquot);
 		if (error)
 			goto done;
 		ASSERT(ip->i_pdquot);
@@ -389,7 +386,7 @@ xfs_qm_dqattach(
 		return 0;
 
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
-	error = xfs_qm_dqattach_locked(ip, 0);
+	error = xfs_qm_dqattach_locked(ip, XFS_DQGET_EXISTS);
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 
 	return error;
@@ -1075,7 +1072,7 @@ xfs_qm_quotacheck_dqadjust(
 	int			error;
 
 	id = xfs_qm_id_for_quotatype(ip, type);
-	error = xfs_qm_dqget(mp, id, type, XFS_QMOPT_DQALLOC, &dqp);
+	error = xfs_qm_dqget(mp, id, type, XFS_DQGET_ALLOC, &dqp);
 	if (error) {
 		/*
 		 * Shouldn't be able to turn off quotas here.
@@ -1670,7 +1667,7 @@ xfs_qm_vop_dqalloc(
 	 * if necessary. The dquot(s) will not be locked.
 	 */
 	if (XFS_NOT_DQATTACHED(mp, ip)) {
-		error = xfs_qm_dqattach_locked(ip, XFS_QMOPT_DQALLOC);
+		error = xfs_qm_dqattach_locked(ip, XFS_DQGET_ALLOC);
 		if (error) {
 			xfs_iunlock(ip, lockflags);
 			return error;
@@ -1690,7 +1687,7 @@ xfs_qm_vop_dqalloc(
 			 */
 			xfs_iunlock(ip, lockflags);
 			error = xfs_qm_dqget(mp, uid, XFS_DQ_USER,
-					XFS_QMOPT_DQALLOC, &uq);
+					XFS_DQGET_ALLOC, &uq);
 			if (error) {
 				ASSERT(error != -ENOENT);
 				return error;
@@ -1714,7 +1711,7 @@ xfs_qm_vop_dqalloc(
 		if (ip->i_d.di_gid != gid) {
 			xfs_iunlock(ip, lockflags);
 			error = xfs_qm_dqget(mp, gid, XFS_DQ_GROUP,
-					XFS_QMOPT_DQALLOC, &gq);
+					XFS_DQGET_ALLOC, &gq);
 			if (error) {
 				ASSERT(error != -ENOENT);
 				goto error_rele;
@@ -1731,7 +1728,7 @@ xfs_qm_vop_dqalloc(
 		if (xfs_get_projid(ip) != prid) {
 			xfs_iunlock(ip, lockflags);
 			error = xfs_qm_dqget(mp, (xfs_dqid_t)prid, XFS_DQ_PROJ,
-					XFS_QMOPT_DQALLOC, &pq);
+					XFS_DQGET_ALLOC, &pq);
 			if (error) {
 				ASSERT(error != -ENOENT);
 				goto error_rele;
diff --git a/fs/xfs/xfs_qm_bhv.c b/fs/xfs/xfs_qm_bhv.c
index 531e822..580cdf1 100644
--- a/fs/xfs/xfs_qm_bhv.c
+++ b/fs/xfs/xfs_qm_bhv.c
@@ -72,7 +72,8 @@ xfs_qm_statvfs(
 	xfs_mount_t		*mp = ip->i_mount;
 	xfs_dquot_t		*dqp;
 
-	if (!xfs_qm_dqget(mp, xfs_get_projid(ip), XFS_DQ_PROJ, 0, &dqp)) {
+	if (!xfs_qm_dqget(mp, xfs_get_projid(ip), XFS_DQ_PROJ,
+			XFS_DQGET_EXISTS, &dqp)) {
 		xfs_fill_statvfs_from_dquot(statp, dqp);
 		xfs_qm_dqput(dqp);
 	}
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index b9243f5..285d50e 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -425,7 +425,7 @@ xfs_qm_scall_setqlim(
 	 * a reference to the dquot, so it's safe to do this unlock/lock without
 	 * it being reclaimed in the mean time.
 	 */
-	error = xfs_qm_dqget(mp, id, type, XFS_QMOPT_DQALLOC, &dqp);
+	error = xfs_qm_dqget(mp, id, type, XFS_DQGET_ALLOC, &dqp);
 	if (error) {
 		ASSERT(error != -ENOENT);
 		goto out_unlock;
@@ -700,7 +700,7 @@ xfs_qm_scall_getquota(
 	 * we aren't passing the XFS_QMOPT_DOALLOC flag. If it doesn't
 	 * exist, we'll get ENOENT back.
 	 */
-	error = xfs_qm_dqget(mp, id, type, 0, &dqp);
+	error = xfs_qm_dqget(mp, id, type, XFS_DQGET_EXISTS, &dqp);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h
index a4d3922..fb67409 100644
--- a/fs/xfs/xfs_quota.h
+++ b/fs/xfs/xfs_quota.h
@@ -91,7 +91,8 @@ extern int xfs_qm_vop_chown_reserve(struct xfs_trans *, struct xfs_inode *,
 		struct xfs_dquot *, struct xfs_dquot *,
 		struct xfs_dquot *, uint);
 extern int xfs_qm_dqattach(struct xfs_inode *);
-extern int xfs_qm_dqattach_locked(struct xfs_inode *, uint);
+extern int xfs_qm_dqattach_locked(struct xfs_inode *ip,
+		unsigned int dqmode);
 extern void xfs_qm_dqdetach(struct xfs_inode *);
 extern void xfs_qm_dqrele(struct xfs_dquot *);
 extern void xfs_qm_statvfs(struct xfs_inode *, struct kstatfs *);
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index fd8e6af..fabb61b 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -305,7 +305,7 @@ xfs_reflink_reserve_cow(
 	 * Fork all the shared blocks from our write offset until the end of
 	 * the extent.
 	 */
-	error = xfs_qm_dqattach_locked(ip, 0);
+	error = xfs_qm_dqattach_locked(ip, XFS_DQGET_EXISTS);
 	if (error)
 		return error;
 
@@ -431,7 +431,7 @@ xfs_reflink_allocate_cow(
 		if (error)
 			return error;
 
-		error = xfs_qm_dqattach_locked(ip, 0);
+		error = xfs_qm_dqattach_locked(ip, XFS_DQGET_EXISTS);
 		if (error)
 			goto out;
 		goto retry;

[PATCH 11/13] xfs: report failing address when dquot verifier fails

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Pass the failing address through to the corruption report when dquot
verifiers fail.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_dquot_buf.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index 8b7a6c3..a1e6cf1 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -229,7 +229,7 @@ xfs_dquot_buf_read_verify(
 	else {
 		fa = xfs_dquot_buf_verify(mp, bp);
 		if (fa)
-			xfs_verifier_error(bp, -EFSCORRUPTED, __this_address);
+			xfs_verifier_error(bp, -EFSCORRUPTED, fa);
 	}
 }
 
@@ -266,7 +266,7 @@ xfs_dquot_buf_write_verify(
 
 	fa = xfs_dquot_buf_verify(mp, bp);
 	if (fa)
-		xfs_verifier_error(bp, -EFSCORRUPTED, __this_address);
+		xfs_verifier_error(bp, -EFSCORRUPTED, fa);
 }
 
 const struct xfs_buf_ops xfs_dquot_buf_ops = {

[PATCH 12/13] xfs: rename on-disk dquot counter zap functions

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

The function 'xfs_qm_dqiterate' doesn't iterate dquots at all, it
iterates all dquot blocks of a quota inode and clears the counters.
Therefore, change the name to something more descriptive so that we can
introduce a real dquot iterator later.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_qm.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)


diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 1b94da0..08ec69c 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -889,7 +889,7 @@ xfs_qm_reset_dqcounts(
 }
 
 STATIC int
-xfs_qm_dqiter_bufs(
+xfs_qm_quotip_zero_bufs(
 	struct xfs_mount	*mp,
 	xfs_dqid_t		firstid,
 	xfs_fsblock_t		bno,
@@ -957,11 +957,11 @@ xfs_qm_dqiter_bufs(
 }
 
 /*
- * Iterate over all allocated USR/GRP/PRJ dquots in the system, calling a
- * caller supplied function for every chunk of dquots that we find.
+ * Iterate over all allocated dquot blocks in this quota inode, zeroing all
+ * counters for every chunk of dquots that we find.
  */
 STATIC int
-xfs_qm_dqiterate(
+xfs_qm_quotip_zero(
 	struct xfs_mount	*mp,
 	struct xfs_inode	*qip,
 	uint			flags,
@@ -1037,7 +1037,7 @@ xfs_qm_dqiterate(
 			 * Iterate thru all the blks in the extent and
 			 * reset the counters of all the dquots inside them.
 			 */
-			error = xfs_qm_dqiter_bufs(mp, firstid,
+			error = xfs_qm_quotip_zero_bufs(mp, firstid,
 						   map[i].br_startblock,
 						   map[i].br_blockcount,
 						   flags, buffer_list);
@@ -1299,7 +1299,7 @@ xfs_qm_quotacheck(
 	 * We don't log our changes till later.
 	 */
 	if (uip) {
-		error = xfs_qm_dqiterate(mp, uip, XFS_QMOPT_UQUOTA,
+		error = xfs_qm_quotip_zero(mp, uip, XFS_QMOPT_UQUOTA,
 					 &buffer_list);
 		if (error)
 			goto error_return;
@@ -1307,7 +1307,7 @@ xfs_qm_quotacheck(
 	}
 
 	if (gip) {
-		error = xfs_qm_dqiterate(mp, gip, XFS_QMOPT_GQUOTA,
+		error = xfs_qm_quotip_zero(mp, gip, XFS_QMOPT_GQUOTA,
 					 &buffer_list);
 		if (error)
 			goto error_return;
@@ -1315,7 +1315,7 @@ xfs_qm_quotacheck(
 	}
 
 	if (pip) {
-		error = xfs_qm_dqiterate(mp, pip, XFS_QMOPT_PQUOTA,
+		error = xfs_qm_quotip_zero(mp, pip, XFS_QMOPT_PQUOTA,
 					 &buffer_list);
 		if (error)
 			goto error_return;

[PATCH 13/13] xfs: refactor dquot iteration

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Create a helper function to iterate all the dquots of a given type in
the system, and refactor the dquot scrub to use it.  This will get more
use in the quota repair code.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/scrub/quota.c |   45 +++++++++++++++++++++------------------------
 fs/xfs/xfs_dquot.c   |   32 ++++++++++++++++++++++++++++++++
 fs/xfs/xfs_dquot.h   |    5 +++++
 3 files changed, 58 insertions(+), 24 deletions(-)


diff --git a/fs/xfs/scrub/quota.c b/fs/xfs/scrub/quota.c
index 50415e8..0ba3ab4 100644
--- a/fs/xfs/scrub/quota.c
+++ b/fs/xfs/scrub/quota.c
@@ -77,14 +77,21 @@ xfs_scrub_setup_quota(
 
 /* Quotas. */
 
+struct xfs_scrub_quota_info {
+	struct xfs_scrub_context	*sc;
+	xfs_dqid_t			last_id;
+};
+
 /* Scrub the fields in an individual quota item. */
-STATIC void
+STATIC int
 xfs_scrub_quota_item(
-	struct xfs_scrub_context	*sc,
-	uint				dqtype,
 	struct xfs_dquot		*dq,
-	xfs_dqid_t			id)
+	uint				dqtype,
+	xfs_dqid_t			id,
+	void				*priv)
 {
+	struct xfs_scrub_quota_info	*sqi = priv;
+	struct xfs_scrub_context	*sc = sqi->sc;
 	struct xfs_mount		*mp = sc->mp;
 	struct xfs_disk_dquot		*d = &dq->q_core;
 	struct xfs_quotainfo		*qi = mp->m_quotainfo;
@@ -100,6 +107,7 @@ xfs_scrub_quota_item(
 	unsigned long long		rcount;
 	xfs_ino_t			fs_icount;
 
+	sqi->last_id = id;
 	offset = id / qi->qi_dqperchunk;
 
 	/*
@@ -183,6 +191,8 @@ xfs_scrub_quota_item(
 		xfs_scrub_fblock_set_warning(sc, XFS_DATA_FORK, offset);
 	if (id != 0 && rhard != 0 && rcount > rhard)
 		xfs_scrub_fblock_set_warning(sc, XFS_DATA_FORK, offset);
+
+	return 0;
 }
 
 /* Scrub all of a quota type's items. */
@@ -191,13 +201,12 @@ xfs_scrub_quota(
 	struct xfs_scrub_context	*sc)
 {
 	struct xfs_bmbt_irec		irec = { 0 };
+	struct xfs_scrub_quota_info	sqi;
 	struct xfs_mount		*mp = sc->mp;
 	struct xfs_inode		*ip;
 	struct xfs_quotainfo		*qi = mp->m_quotainfo;
-	struct xfs_dquot		*dq;
 	xfs_fileoff_t			max_dqid_off;
 	xfs_fileoff_t			off = 0;
-	xfs_dqid_t			id = 0;
 	uint				dqtype;
 	int				nimaps;
 	int				error = 0;
@@ -264,24 +273,12 @@ xfs_scrub_quota(
 		goto out;
 
 	/* Check all the quota items. */
-	while (id < ((xfs_dqid_t)-1ULL)) {
-		if (xfs_scrub_should_terminate(sc, &error))
-			break;
-
-		error = xfs_qm_dqget_next(mp, id, dqtype, &dq);
-		if (error == -ENOENT)
-			break;
-		if (!xfs_scrub_fblock_process_error(sc, XFS_DATA_FORK,
-				id * qi->qi_dqperchunk, &error))
-			break;
-
-		xfs_scrub_quota_item(sc, dqtype, dq, id);
-
-		id = be32_to_cpu(dq->q_core.d_id) + 1;
-		xfs_qm_dqput(dq);
-		if (!id)
-			break;
-	}
+	sqi.sc = sc;
+	sqi.last_id = 0;
+	error = xfs_qm_dqiterate(mp, dqtype, xfs_scrub_quota_item, &sqi);
+	if (!xfs_scrub_fblock_process_error(sc, XFS_DATA_FORK,
+			sqi.last_id * qi->qi_dqperchunk, &error))
+		goto out;
 
 out:
 	/* We set sc->ip earlier, so make sure we clear it now. */
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 7ce3dbd..1f1139d 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -1213,3 +1213,35 @@ xfs_qm_exit(void)
 	kmem_zone_destroy(xfs_qm_dqtrxzone);
 	kmem_zone_destroy(xfs_qm_dqzone);
 }
+
+/*
+ * Iterate every dquot of a particular type.  The caller must ensure that the
+ * particular quota type is active.  iter_fn can return negative error codes,
+ * or XFS_BTREE_QUERY_RANGE_ABORT to indicate that it wants to stop iterating.
+ */
+int
+xfs_qm_dqiterate(
+	struct xfs_mount	*mp,
+	uint			dqtype,
+	xfs_qm_dqiterate_fn	iter_fn,
+	void			*priv)
+{
+	struct xfs_dquot	*dq;
+	xfs_dqid_t		id = 0;
+	int			error;
+
+	do {
+		error = xfs_qm_dqget_next(mp, id, dqtype, &dq);
+		if (error == -ENOENT)
+			return 0;
+		if (error)
+			return error;
+
+		id = be32_to_cpu(dq->q_core.d_id);
+		error = iter_fn(dq, dqtype, id, priv);
+		xfs_qm_dqput(dq);
+		id++;
+	} while (error == 0 && id != 0);
+
+	return error;
+}
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index fe99e43..5427355 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -194,4 +194,9 @@ static inline struct xfs_dquot *xfs_qm_dqhold(struct xfs_dquot *dqp)
 	return dqp;
 }
 
+typedef int (*xfs_qm_dqiterate_fn)(struct xfs_dquot *dq, uint dqtype,
+		xfs_dqid_t id, void *priv);
+int xfs_qm_dqiterate(struct xfs_mount *mp, uint dqtype,
+		xfs_qm_dqiterate_fn iter_fn, void *priv);
+
 #endif /* __XFS_DQUOT_H__ */

Re: [PATCH 01/13] xfs: refactor XFS_QMOPT_DQNEXT out of existence

[Brian Foster]

On Sun, Apr 22, 2018 at 08:05:58AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> There's only one caller of DQNEXT and its semantics can be moved into a
> separate function, so create the function and get rid of the flag.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_quota_defs.h |    1 
>  fs/xfs/scrub/quota.c           |    3 -
>  fs/xfs/xfs_dquot.c             |   63 ++++++++++++++------------
>  fs/xfs/xfs_dquot.h             |    2 +
>  fs/xfs/xfs_qm.h                |    6 ++-
>  fs/xfs/xfs_qm_syscalls.c       |   96 +++++++++++++++++++++++++++-------------
>  fs/xfs/xfs_quotaops.c          |    8 +--
>  7 files changed, 108 insertions(+), 71 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
> index bb1b13a..c80b226 100644
> --- a/fs/xfs/libxfs/xfs_quota_defs.h
> +++ b/fs/xfs/libxfs/xfs_quota_defs.h
> @@ -114,7 +114,6 @@ typedef uint16_t	xfs_qwarncnt_t;
>  #define XFS_QMOPT_SBVERSION	0x0000040 /* change superblock version num */
>  #define XFS_QMOPT_GQUOTA	0x0002000 /* group dquot requested */
>  #define XFS_QMOPT_ENOSPC	0x0004000 /* enospc instead of edquot (prj) */
> -#define XFS_QMOPT_DQNEXT	0x0008000 /* return next dquot >= this ID */
>  
>  /*
>   * flags to xfs_trans_mod_dquot to indicate which field needs to be
> diff --git a/fs/xfs/scrub/quota.c b/fs/xfs/scrub/quota.c
> index 6ba465e..50415e8 100644
> --- a/fs/xfs/scrub/quota.c
> +++ b/fs/xfs/scrub/quota.c
> @@ -268,8 +268,7 @@ xfs_scrub_quota(
>  		if (xfs_scrub_should_terminate(sc, &error))
>  			break;
>  
> -		error = xfs_qm_dqget(mp, NULL, id, dqtype, XFS_QMOPT_DQNEXT,
> -				&dq);
> +		error = xfs_qm_dqget_next(mp, id, dqtype, &dq);
>  		if (error == -ENOENT)
>  			break;
>  		if (!xfs_scrub_fblock_process_error(sc, XFS_DATA_FORK,
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index a7daef9..79a9df6 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -728,18 +728,6 @@ xfs_qm_dqget(
>  			goto restart;
>  		}
>  
> -		/* uninit / unused quota found in radix tree, keep looking  */
> -		if (flags & XFS_QMOPT_DQNEXT) {
> -			if (XFS_IS_DQUOT_UNINITIALIZED(dqp)) {
> -				xfs_dqunlock(dqp);
> -				mutex_unlock(&qi->qi_tree_lock);
> -				error = xfs_dq_get_next_id(mp, type, &id);
> -				if (error)
> -					return error;
> -				goto restart;
> -			}
> -		}
> -
>  		dqp->q_nrefs++;
>  		mutex_unlock(&qi->qi_tree_lock);
>  
> @@ -766,13 +754,6 @@ xfs_qm_dqget(
>  	if (ip)
>  		xfs_ilock(ip, XFS_ILOCK_EXCL);
>  
> -	/* If we are asked to find next active id, keep looking */
> -	if (error == -ENOENT && (flags & XFS_QMOPT_DQNEXT)) {
> -		error = xfs_dq_get_next_id(mp, type, &id);
> -		if (!error)
> -			goto restart;
> -	}
> -
>  	if (error)
>  		return error;
>  
> @@ -823,17 +804,6 @@ xfs_qm_dqget(
>  	qi->qi_dquots++;
>  	mutex_unlock(&qi->qi_tree_lock);
>  
> -	/* If we are asked to find next active id, keep looking */
> -	if (flags & XFS_QMOPT_DQNEXT) {
> -		if (XFS_IS_DQUOT_UNINITIALIZED(dqp)) {
> -			xfs_qm_dqput(dqp);
> -			error = xfs_dq_get_next_id(mp, type, &id);
> -			if (error)
> -				return error;
> -			goto restart;
> -		}
> -	}
> -
>   dqret:
>  	ASSERT((ip == NULL) || xfs_isilocked(ip, XFS_ILOCK_EXCL));
>  	trace_xfs_dqget_miss(dqp);
> @@ -842,6 +812,39 @@ xfs_qm_dqget(
>  }
>  
>  /*
> + * Starting at @id and progressing upwards, look for an initialized incore
> + * dquot, lock it, and return it.
> + */
> +int
> +xfs_qm_dqget_next(
> +	struct xfs_mount	*mp,
> +	xfs_dqid_t		id,
> +	uint			type,
> +	struct xfs_dquot	**dqpp)
> +{
> +	struct xfs_dquot	*dqp;
> +	int			error = 0;
> +
> +	*dqpp = NULL;
> +	for (; !error; error = xfs_dq_get_next_id(mp, type, &id)) {
> +		error = xfs_qm_dqget(mp, NULL, id, type, 0, &dqp);
> +		if (error == -ENOENT)
> +			continue;
> +		else if (error != 0)
> +			break;
> +
> +		if (!XFS_IS_DQUOT_UNINITIALIZED(dqp)) {
> +			*dqpp = dqp;
> +			return 0;
> +		}
> +
> +		xfs_qm_dqput(dqp);
> +	}
> +
> +	return error;
> +}
> +
> +/*
>   * Release a reference to the dquot (decrement ref-count) and unlock it.
>   *
>   * If there is a group quota attached to this dquot, carefully release that
> diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
> index 2f536f3..303e71d 100644
> --- a/fs/xfs/xfs_dquot.h
> +++ b/fs/xfs/xfs_dquot.h
> @@ -171,6 +171,8 @@ extern void		xfs_qm_adjust_dqlimits(struct xfs_mount *,
>  					       struct xfs_dquot *);
>  extern int		xfs_qm_dqget(xfs_mount_t *, xfs_inode_t *,
>  					xfs_dqid_t, uint, uint, xfs_dquot_t **);
> +extern int		xfs_qm_dqget_next(struct xfs_mount *mp, xfs_dqid_t id,
> +					uint type, struct xfs_dquot **dqpp);
>  extern void		xfs_qm_dqput(xfs_dquot_t *);
>  
>  extern void		xfs_dqlock2(struct xfs_dquot *, struct xfs_dquot *);
> diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
> index 2975a82..e3129b2 100644
> --- a/fs/xfs/xfs_qm.h
> +++ b/fs/xfs/xfs_qm.h
> @@ -170,8 +170,10 @@ extern void		xfs_qm_dqrele_all_inodes(struct xfs_mount *, uint);
>  
>  /* quota ops */
>  extern int		xfs_qm_scall_trunc_qfiles(struct xfs_mount *, uint);
> -extern int		xfs_qm_scall_getquota(struct xfs_mount *, xfs_dqid_t *,
> -					uint, struct qc_dqblk *, uint);
> +extern int		xfs_qm_scall_getquota(struct xfs_mount *, xfs_dqid_t,
> +					uint, struct qc_dqblk *);
> +extern int		xfs_qm_scall_getquota_next(struct xfs_mount *,
> +					xfs_dqid_t *, uint, struct qc_dqblk *);
>  extern int		xfs_qm_scall_setqlim(struct xfs_mount *, xfs_dqid_t, uint,
>  					struct qc_dqblk *);
>  extern int		xfs_qm_scall_quotaon(struct xfs_mount *, uint);
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index 9cb5c38..0234cfc 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -622,39 +622,14 @@ xfs_qm_log_quotaoff(
>  	return error;
>  }
>  
> -
> -int
> -xfs_qm_scall_getquota(
> +/* Fill out the quota context. */
> +static void
> +xfs_qm_scall_getquota_fill_qc(
>  	struct xfs_mount	*mp,
> -	xfs_dqid_t		*id,
>  	uint			type,
> -	struct qc_dqblk		*dst,
> -	uint			dqget_flags)
> +	const struct xfs_dquot	*dqp,
> +	struct qc_dqblk		*dst)
>  {
> -	struct xfs_dquot	*dqp;
> -	int			error;
> -
> -	/*
> -	 * Try to get the dquot. We don't want it allocated on disk, so
> -	 * we aren't passing the XFS_QMOPT_DOALLOC flag. If it doesn't
> -	 * exist, we'll get ENOENT back.
> -	 */
> -	error = xfs_qm_dqget(mp, NULL, *id, type, dqget_flags, &dqp);
> -	if (error)
> -		return error;
> -
> -	/*
> -	 * If everything's NULL, this dquot doesn't quite exist as far as
> -	 * our utility programs are concerned.
> -	 */
> -	if (XFS_IS_DQUOT_UNINITIALIZED(dqp)) {
> -		error = -ENOENT;
> -		goto out_put;
> -	}
> -
> -	/* Fill in the ID we actually read from disk */
> -	*id = be32_to_cpu(dqp->q_core.d_id);
> -
>  	memset(dst, 0, sizeof(*dst));
>  	dst->d_spc_hardlimit =
>  		XFS_FSB_TO_B(mp, be64_to_cpu(dqp->q_core.d_blk_hardlimit));
> @@ -696,7 +671,7 @@ xfs_qm_scall_getquota(
>  	if (((XFS_IS_UQUOTA_ENFORCED(mp) && type == XFS_DQ_USER) ||
>  	     (XFS_IS_GQUOTA_ENFORCED(mp) && type == XFS_DQ_GROUP) ||
>  	     (XFS_IS_PQUOTA_ENFORCED(mp) && type == XFS_DQ_PROJ)) &&
> -	    *id != 0) {
> +	    dqp->q_core.d_id != 0) {
>  		if ((dst->d_space > dst->d_spc_softlimit) &&
>  		    (dst->d_spc_softlimit > 0)) {
>  			ASSERT(dst->d_spc_timer != 0);
> @@ -707,11 +682,70 @@ xfs_qm_scall_getquota(
>  		}
>  	}
>  #endif
> +}
> +
> +/* Return the quota information for the dquot matching id. */
> +int
> +xfs_qm_scall_getquota(
> +	struct xfs_mount	*mp,
> +	xfs_dqid_t		id,
> +	uint			type,
> +	struct qc_dqblk		*dst)
> +{
> +	struct xfs_dquot	*dqp;
> +	int			error;
> +
> +	/*
> +	 * Try to get the dquot. We don't want it allocated on disk, so
> +	 * we aren't passing the XFS_QMOPT_DOALLOC flag. If it doesn't
> +	 * exist, we'll get ENOENT back.
> +	 */
> +	error = xfs_qm_dqget(mp, NULL, id, type, 0, &dqp);
> +	if (error)
> +		return error;
> +
> +	/*
> +	 * If everything's NULL, this dquot doesn't quite exist as far as
> +	 * our utility programs are concerned.
> +	 */
> +	if (XFS_IS_DQUOT_UNINITIALIZED(dqp)) {
> +		error = -ENOENT;
> +		goto out_put;
> +	}
> +
> +	xfs_qm_scall_getquota_fill_qc(mp, type, dqp, dst);
> +
>  out_put:
>  	xfs_qm_dqput(dqp);
>  	return error;
>  }
>  
> +/*
> + * Return the quota information for the first initialized dquot whose id
> + * is at least as high as id.
> + */
> +int
> +xfs_qm_scall_getquota_next(
> +	struct xfs_mount	*mp,
> +	xfs_dqid_t		*id,
> +	uint			type,
> +	struct qc_dqblk		*dst)
> +{
> +	struct xfs_dquot	*dqp;
> +	int			error;
> +
> +	error = xfs_qm_dqget_next(mp, *id, type, &dqp);
> +	if (error)
> +		return error;
> +
> +	/* Fill in the ID we actually read from disk */
> +	*id = be32_to_cpu(dqp->q_core.d_id);
> +
> +	xfs_qm_scall_getquota_fill_qc(mp, type, dqp, dst);
> +
> +	xfs_qm_dqput(dqp);
> +	return error;
> +}
>  
>  STATIC int
>  xfs_dqrele_inode(
> diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c
> index a651085..c93fc91 100644
> --- a/fs/xfs/xfs_quotaops.c
> +++ b/fs/xfs/xfs_quotaops.c
> @@ -239,8 +239,7 @@ xfs_fs_get_dqblk(
>  		return -ESRCH;
>  
>  	id = from_kqid(&init_user_ns, qid);
> -	return xfs_qm_scall_getquota(mp, &id,
> -				      xfs_quota_type(qid.type), qdq, 0);
> +	return xfs_qm_scall_getquota(mp, id, xfs_quota_type(qid.type), qdq);
>  }
>  
>  /* Return quota info for active quota >= this qid */
> @@ -260,9 +259,8 @@ xfs_fs_get_nextdqblk(
>  		return -ESRCH;
>  
>  	id = from_kqid(&init_user_ns, *qid);
> -	ret = xfs_qm_scall_getquota(mp, &id,
> -				    xfs_quota_type(qid->type), qdq,
> -				    XFS_QMOPT_DQNEXT);
> +	ret = xfs_qm_scall_getquota_next(mp, &id, xfs_quota_type(qid->type),
> +			qdq);
>  	if (ret)
>  		return ret;
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/13] xfs: refactor dquot cache handling

[Brian Foster]

On Sun, Apr 22, 2018 at 08:06:05AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Delegate the dquot cache handling (radix tree lookup and insertion) to
> separate helper functions so that we can continue to simplify the body
> of dqget.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_dquot.c |  112 ++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 78 insertions(+), 34 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 79a9df6..43b0b32 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -679,6 +679,81 @@ xfs_dq_get_next_id(
>  }
>  
>  /*
> + * Look up the dquot in the in-core cache.  If found, the dquot is returned
> + * locked and ready to go.
> + */
> +static struct xfs_dquot *
> +xfs_qm_dqget_cache_lookup(
> +	struct xfs_mount	*mp,
> +	struct xfs_quotainfo	*qi,
> +	struct radix_tree_root	*tree,
> +	xfs_dqid_t		id)
> +{
> +	struct xfs_dquot	*dqp;
> +
> +restart:
> +	mutex_lock(&qi->qi_tree_lock);
> +	dqp = radix_tree_lookup(tree, id);
> +	if (!dqp) {
> +		mutex_unlock(&qi->qi_tree_lock);
> +		XFS_STATS_INC(mp, xs_qm_dqcachemisses);
> +		return NULL;
> +	}
> +
> +	xfs_dqlock(dqp);
> +	if (dqp->dq_flags & XFS_DQ_FREEING) {
> +		xfs_dqunlock(dqp);
> +		mutex_unlock(&qi->qi_tree_lock);
> +		trace_xfs_dqget_freeing(dqp);
> +		delay(1);
> +		goto restart;
> +	}
> +
> +	dqp->q_nrefs++;
> +	mutex_unlock(&qi->qi_tree_lock);
> +
> +	trace_xfs_dqget_hit(dqp);
> +	XFS_STATS_INC(mp, xs_qm_dqcachehits);
> +	return dqp;
> +}
> +
> +/*
> + * Try to insert a new dquot into the in-core cache.  If an error occurs the
> + * caller should throw away the dquot and start over.  Otherwise, the dquot
> + * is returned locked (and held by the cache) as if there had been a cache
> + * hit.
> + */
> +static int
> +xfs_qm_dqget_cache_insert(
> +	struct xfs_mount	*mp,
> +	struct xfs_quotainfo	*qi,
> +	struct radix_tree_root	*tree,
> +	xfs_dqid_t		id,
> +	struct xfs_dquot	*dqp)
> +{
> +	int			error;
> +
> +	mutex_lock(&qi->qi_tree_lock);
> +	error = radix_tree_insert(tree, id, dqp);
> +	if (unlikely(error)) {
> +		/* Duplicate found!  Caller must try again. */
> +		WARN_ON(error != -EEXIST);
> +		mutex_unlock(&qi->qi_tree_lock);
> +		trace_xfs_dqget_dup(dqp);
> +		return error;
> +	}
> +
> +	/* Return a locked dquot to the caller, with a reference taken. */
> +	xfs_dqlock(dqp);
> +	dqp->q_nrefs = 1;
> +
> +	qi->qi_dquots++;
> +	mutex_unlock(&qi->qi_tree_lock);
> +
> +	return 0;
> +}
> +
> +/*
>   * Given the file system, inode OR id, and type (UDQUOT/GDQUOT), return a
>   * a locked dquot, doing an allocation (if requested) as needed.
>   * When both an inode and an id are given, the inode's id takes precedence.
> @@ -716,28 +791,11 @@ xfs_qm_dqget(
>  	}
>  
>  restart:
> -	mutex_lock(&qi->qi_tree_lock);
> -	dqp = radix_tree_lookup(tree, id);
> +	dqp = xfs_qm_dqget_cache_lookup(mp, qi, tree, id);
>  	if (dqp) {
> -		xfs_dqlock(dqp);
> -		if (dqp->dq_flags & XFS_DQ_FREEING) {
> -			xfs_dqunlock(dqp);
> -			mutex_unlock(&qi->qi_tree_lock);
> -			trace_xfs_dqget_freeing(dqp);
> -			delay(1);
> -			goto restart;
> -		}
> -
> -		dqp->q_nrefs++;
> -		mutex_unlock(&qi->qi_tree_lock);
> -
> -		trace_xfs_dqget_hit(dqp);
> -		XFS_STATS_INC(mp, xs_qm_dqcachehits);
>  		*O_dqpp = dqp;
>  		return 0;
>  	}
> -	mutex_unlock(&qi->qi_tree_lock);
> -	XFS_STATS_INC(mp, xs_qm_dqcachemisses);
>  
>  	/*
>  	 * Dquot cache miss. We don't want to keep the inode lock across
> @@ -779,31 +837,17 @@ xfs_qm_dqget(
>  		}
>  	}
>  
> -	mutex_lock(&qi->qi_tree_lock);
> -	error = radix_tree_insert(tree, id, dqp);
> -	if (unlikely(error)) {
> -		WARN_ON(error != -EEXIST);
> -
> +	error = xfs_qm_dqget_cache_insert(mp, qi, tree, id, dqp);
> +	if (error) {
>  		/*
>  		 * Duplicate found. Just throw away the new dquot and start
>  		 * over.
>  		 */
> -		mutex_unlock(&qi->qi_tree_lock);
> -		trace_xfs_dqget_dup(dqp);
>  		xfs_qm_dqdestroy(dqp);
>  		XFS_STATS_INC(mp, xs_qm_dquot_dups);
>  		goto restart;
>  	}
>  
> -	/*
> -	 * We return a locked dquot to the caller, with a reference taken
> -	 */
> -	xfs_dqlock(dqp);
> -	dqp->q_nrefs = 1;
> -
> -	qi->qi_dquots++;
> -	mutex_unlock(&qi->qi_tree_lock);
> -
>   dqret:
>  	ASSERT((ip == NULL) || xfs_isilocked(ip, XFS_ILOCK_EXCL));
>  	trace_xfs_dqget_miss(dqp);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/13] xfs: delegate dqget input checks to helper function

[Brian Foster]

On Sun, Apr 22, 2018 at 08:06:11AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Move the dqget input checks to a separate function in preparation for
> splitting up the dqget functionality.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_dquot.c |   39 ++++++++++++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 43b0b32..9a3a05e 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -753,6 +753,33 @@ xfs_qm_dqget_cache_insert(
>  	return 0;
>  }
>  
> +/* Check our input parameters. */
> +static int
> +xfs_qm_dqget_checks(
> +	struct xfs_mount	*mp,
> +	uint			type)
> +{
> +	if (!XFS_IS_QUOTA_RUNNING(mp)) {
> +		ASSERT(0);
> +		return -ESRCH;
> +	}
> +
> +	if ((!XFS_IS_UQUOTA_ON(mp) && type == XFS_DQ_USER) ||
> +	    (!XFS_IS_PQUOTA_ON(mp) && type == XFS_DQ_PROJ) ||
> +	    (!XFS_IS_GQUOTA_ON(mp) && type == XFS_DQ_GROUP)) {
> +		return -ESRCH;
> +	}
> +
> +	if (type != XFS_DQ_USER &&
> +	    type != XFS_DQ_PROJ &&
> +	    type != XFS_DQ_GROUP) {
> +		ASSERT(0);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * Given the file system, inode OR id, and type (UDQUOT/GDQUOT), return a
>   * a locked dquot, doing an allocation (if requested) as needed.
> @@ -775,16 +802,10 @@ xfs_qm_dqget(
>  	struct xfs_dquot	*dqp;
>  	int			error;
>  
> -	ASSERT(XFS_IS_QUOTA_RUNNING(mp));
> -	if ((! XFS_IS_UQUOTA_ON(mp) && type == XFS_DQ_USER) ||
> -	    (! XFS_IS_PQUOTA_ON(mp) && type == XFS_DQ_PROJ) ||
> -	    (! XFS_IS_GQUOTA_ON(mp) && type == XFS_DQ_GROUP)) {
> -		return -ESRCH;
> -	}
> +	error = xfs_qm_dqget_checks(mp, type);
> +	if (error)
> +		return error;
>  
> -	ASSERT(type == XFS_DQ_USER ||
> -	       type == XFS_DQ_PROJ ||
> -	       type == XFS_DQ_GROUP);
>  	if (ip) {
>  		ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
>  		ASSERT(xfs_inode_dquot(ip, type) == NULL);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/13] xfs: remove unnecessary xfs_qm_dqattach parameter

[Brian Foster]

On Sun, Apr 22, 2018 at 08:06:17AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The flags argument is always zero, get rid of it.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_attr.c |    4 ++--
>  fs/xfs/xfs_bmap_util.c   |    6 +++---
>  fs/xfs/xfs_inode.c       |   10 +++++-----
>  fs/xfs/xfs_iomap.c       |    4 ++--
>  fs/xfs/xfs_iops.c        |    2 +-
>  fs/xfs/xfs_qm.c          |    7 +++----
>  fs/xfs/xfs_quota.h       |    4 ++--
>  fs/xfs/xfs_reflink.c     |    2 +-
>  8 files changed, 19 insertions(+), 20 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 35a1244..c3d02a6 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -236,7 +236,7 @@ xfs_attr_set(
>  	args.op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
>  	args.total = xfs_attr_calc_size(&args, &local);
>  
> -	error = xfs_qm_dqattach(dp, 0);
> +	error = xfs_qm_dqattach(dp);
>  	if (error)
>  		return error;
>  
> @@ -427,7 +427,7 @@ xfs_attr_remove(
>  	 */
>  	args.op_flags = XFS_DA_OP_OKNOENT;
>  
> -	error = xfs_qm_dqattach(dp, 0);
> +	error = xfs_qm_dqattach(dp);
>  	if (error)
>  		return error;
>  
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 8cd8c41..65b2cc2 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -848,7 +848,7 @@ xfs_free_eofblocks(
>  		/*
>  		 * Attach the dquots to the inode up front.
>  		 */
> -		error = xfs_qm_dqattach(ip, 0);
> +		error = xfs_qm_dqattach(ip);
>  		if (error)
>  			return error;
>  
> @@ -918,7 +918,7 @@ xfs_alloc_file_space(
>  	if (XFS_FORCED_SHUTDOWN(mp))
>  		return -EIO;
>  
> -	error = xfs_qm_dqattach(ip, 0);
> +	error = xfs_qm_dqattach(ip);
>  	if (error)
>  		return error;
>  
> @@ -1169,7 +1169,7 @@ xfs_free_file_space(
>  
>  	trace_xfs_free_file_space(ip);
>  
> -	error = xfs_qm_dqattach(ip, 0);
> +	error = xfs_qm_dqattach(ip);
>  	if (error)
>  		return error;
>  
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 2b70c8b..1da9863 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1411,11 +1411,11 @@ xfs_link(
>  	if (XFS_FORCED_SHUTDOWN(mp))
>  		return -EIO;
>  
> -	error = xfs_qm_dqattach(sip, 0);
> +	error = xfs_qm_dqattach(sip);
>  	if (error)
>  		goto std_return;
>  
> -	error = xfs_qm_dqattach(tdp, 0);
> +	error = xfs_qm_dqattach(tdp);
>  	if (error)
>  		goto std_return;
>  
> @@ -1911,7 +1911,7 @@ xfs_inactive(
>  	     ip->i_d.di_nextents > 0 || ip->i_delayed_blks > 0))
>  		truncate = 1;
>  
> -	error = xfs_qm_dqattach(ip, 0);
> +	error = xfs_qm_dqattach(ip);
>  	if (error)
>  		return;
>  
> @@ -2574,11 +2574,11 @@ xfs_remove(
>  	if (XFS_FORCED_SHUTDOWN(mp))
>  		return -EIO;
>  
> -	error = xfs_qm_dqattach(dp, 0);
> +	error = xfs_qm_dqattach(dp);
>  	if (error)
>  		goto std_return;
>  
> -	error = xfs_qm_dqattach(ip, 0);
> +	error = xfs_qm_dqattach(ip);
>  	if (error)
>  		goto std_return;
>  
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 046469f..58b04ea 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -224,7 +224,7 @@ xfs_iomap_write_direct(
>  	 * necessary and move on to transaction setup.
>  	 */
>  	xfs_iunlock(ip, lockmode);
> -	error = xfs_qm_dqattach(ip, 0);
> +	error = xfs_qm_dqattach(ip);
>  	if (error)
>  		return error;
>  
> @@ -692,7 +692,7 @@ xfs_iomap_write_allocate(
>  	/*
>  	 * Make sure that the dquots are there.
>  	 */
> -	error = xfs_qm_dqattach(ip, 0);
> +	error = xfs_qm_dqattach(ip);
>  	if (error)
>  		return error;
>  
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index a3ed3c8..5afe3c2 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -855,7 +855,7 @@ xfs_setattr_size(
>  	/*
>  	 * Make sure that the dquots are attached to the inode.
>  	 */
> -	error = xfs_qm_dqattach(ip, 0);
> +	error = xfs_qm_dqattach(ip);
>  	if (error)
>  		return error;
>  
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index ec39ae2..d919b85 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -381,8 +381,7 @@ xfs_qm_dqattach_locked(
>  
>  int
>  xfs_qm_dqattach(
> -	struct xfs_inode	*ip,
> -	uint			flags)
> +	struct xfs_inode	*ip)
>  {
>  	int			error;
>  
> @@ -390,7 +389,7 @@ xfs_qm_dqattach(
>  		return 0;
>  
>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
> -	error = xfs_qm_dqattach_locked(ip, flags);
> +	error = xfs_qm_dqattach_locked(ip, 0);
>  	xfs_iunlock(ip, XFS_ILOCK_EXCL);
>  
>  	return error;
> @@ -1933,7 +1932,7 @@ xfs_qm_vop_rename_dqattach(
>  		 */
>  		if (i == 0 || ip != i_tab[i-1]) {
>  			if (XFS_NOT_DQATTACHED(mp, ip)) {
> -				error = xfs_qm_dqattach(ip, 0);
> +				error = xfs_qm_dqattach(ip);
>  				if (error)
>  					return error;
>  			}
> diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h
> index ce6506a..a4d3922 100644
> --- a/fs/xfs/xfs_quota.h
> +++ b/fs/xfs/xfs_quota.h
> @@ -90,7 +90,7 @@ extern struct xfs_dquot *xfs_qm_vop_chown(struct xfs_trans *,
>  extern int xfs_qm_vop_chown_reserve(struct xfs_trans *, struct xfs_inode *,
>  		struct xfs_dquot *, struct xfs_dquot *,
>  		struct xfs_dquot *, uint);
> -extern int xfs_qm_dqattach(struct xfs_inode *, uint);
> +extern int xfs_qm_dqattach(struct xfs_inode *);
>  extern int xfs_qm_dqattach_locked(struct xfs_inode *, uint);
>  extern void xfs_qm_dqdetach(struct xfs_inode *);
>  extern void xfs_qm_dqrele(struct xfs_dquot *);
> @@ -132,7 +132,7 @@ static inline int xfs_trans_reserve_quota_bydquots(struct xfs_trans *tp,
>  #define xfs_qm_vop_rename_dqattach(it)					(0)
>  #define xfs_qm_vop_chown(tp, ip, old, new)				(NULL)
>  #define xfs_qm_vop_chown_reserve(tp, ip, u, g, p, fl)			(0)
> -#define xfs_qm_dqattach(ip, fl)						(0)
> +#define xfs_qm_dqattach(ip)						(0)
>  #define xfs_qm_dqattach_locked(ip, fl)					(0)
>  #define xfs_qm_dqdetach(ip)
>  #define xfs_qm_dqrele(d)
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index cdbd342..fd8e6af 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -1359,7 +1359,7 @@ xfs_reflink_remap_range(
>  		goto out_unlock;
>  
>  	/* Attach dquots to dest inode before changing block map */
> -	ret = xfs_qm_dqattach(dest, 0);
> +	ret = xfs_qm_dqattach(dest);
>  	if (ret)
>  		goto out_unlock;
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/13] xfs: split out dqget for inodes from regular dqget

[Brian Foster]

On Sun, Apr 22, 2018 at 08:06:24AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> There are two uses of dqget here -- one is to return the dquot for a
> given type and id, and the other is to return the dquot for a given type
> and inode.  Those are two separate things, so split them into two
> smaller functions.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_dquot.c       |  148 ++++++++++++++++++++++++++++++++--------------
>  fs/xfs/xfs_dquot.h       |   11 +++
>  fs/xfs/xfs_qm.c          |   22 ++-----
>  fs/xfs/xfs_qm_bhv.c      |    2 -
>  fs/xfs/xfs_qm_syscalls.c |    4 +
>  5 files changed, 124 insertions(+), 63 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 9a3a05e..62415f1 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -781,24 +781,19 @@ xfs_qm_dqget_checks(
>  }
...
> +/*
> + * Return the dquot for a given inode and type.  If @can_alloc is true, then
> + * allocate blocks if needed.  The inode's ILOCK must be held and it must not
> + * have already had an inode attached.
> + */
> +int
> +xfs_qm_dqget_inode(
> +	struct xfs_mount	*mp,
> +	struct xfs_inode	*ip,

I guess we don't really need to pass mp if this requires ip. Otherwise
looks fine:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> +	uint			type,
> +	uint			can_alloc,
> +	struct xfs_dquot	**O_dqpp)
> +{
> +	struct xfs_quotainfo	*qi = mp->m_quotainfo;
> +	struct radix_tree_root	*tree = xfs_dquot_tree(qi, type);
> +	struct xfs_dquot	*dqp;
> +	xfs_dqid_t		id;
> +	uint			flags = 0;
> +	int			error;
> +
> +	error = xfs_qm_dqget_checks(mp, type);
> +	if (error)
> +		return error;
> +
> +	if (can_alloc)
> +		flags |= XFS_QMOPT_DQALLOC;
> +
> +	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> +	ASSERT(xfs_inode_dquot(ip, type) == NULL);
> +
> +	id = xfs_qm_id_for_quotatype(ip, type);
>  
>  restart:
>  	dqp = xfs_qm_dqget_cache_lookup(mp, qi, tree, id);
> @@ -825,37 +892,30 @@ xfs_qm_dqget(
>  	 * lock here means dealing with a chown that can happen before
>  	 * we re-acquire the lock.
>  	 */
> -	if (ip)
> -		xfs_iunlock(ip, XFS_ILOCK_EXCL);
> -
> +	xfs_iunlock(ip, XFS_ILOCK_EXCL);
>  	error = xfs_qm_dqread(mp, id, type, flags, &dqp);
> -
> -	if (ip)
> -		xfs_ilock(ip, XFS_ILOCK_EXCL);
> -
> +	xfs_ilock(ip, XFS_ILOCK_EXCL);
>  	if (error)
>  		return error;
>  
> -	if (ip) {
> -		/*
> -		 * A dquot could be attached to this inode by now, since
> -		 * we had dropped the ilock.
> -		 */
> -		if (xfs_this_quota_on(mp, type)) {
> -			struct xfs_dquot	*dqp1;
> -
> -			dqp1 = xfs_inode_dquot(ip, type);
> -			if (dqp1) {
> -				xfs_qm_dqdestroy(dqp);
> -				dqp = dqp1;
> -				xfs_dqlock(dqp);
> -				goto dqret;
> -			}
> -		} else {
> -			/* inode stays locked on return */
> +	/*
> +	 * A dquot could be attached to this inode by now, since we had
> +	 * dropped the ilock.
> +	 */
> +	if (xfs_this_quota_on(mp, type)) {
> +		struct xfs_dquot	*dqp1;
> +
> +		dqp1 = xfs_inode_dquot(ip, type);
> +		if (dqp1) {
>  			xfs_qm_dqdestroy(dqp);
> -			return -ESRCH;
> +			dqp = dqp1;
> +			xfs_dqlock(dqp);
> +			goto dqret;
>  		}
> +	} else {
> +		/* inode stays locked on return */
> +		xfs_qm_dqdestroy(dqp);
> +		return -ESRCH;
>  	}
>  
>  	error = xfs_qm_dqget_cache_insert(mp, qi, tree, id, dqp);
> @@ -869,8 +929,8 @@ xfs_qm_dqget(
>  		goto restart;
>  	}
>  
> - dqret:
> -	ASSERT((ip == NULL) || xfs_isilocked(ip, XFS_ILOCK_EXCL));
> +dqret:
> +	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
>  	trace_xfs_dqget_miss(dqp);
>  	*O_dqpp = dqp;
>  	return 0;
> @@ -892,7 +952,7 @@ xfs_qm_dqget_next(
>  
>  	*dqpp = NULL;
>  	for (; !error; error = xfs_dq_get_next_id(mp, type, &id)) {
> -		error = xfs_qm_dqget(mp, NULL, id, type, 0, &dqp);
> +		error = xfs_qm_dqget(mp, id, type, 0, &dqp);
>  		if (error == -ENOENT)
>  			continue;
>  		else if (error != 0)
> diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
> index 303e71d..1458de3 100644
> --- a/fs/xfs/xfs_dquot.h
> +++ b/fs/xfs/xfs_dquot.h
> @@ -169,8 +169,15 @@ extern void		xfs_qm_adjust_dqtimers(xfs_mount_t *,
>  					xfs_disk_dquot_t *);
>  extern void		xfs_qm_adjust_dqlimits(struct xfs_mount *,
>  					       struct xfs_dquot *);
> -extern int		xfs_qm_dqget(xfs_mount_t *, xfs_inode_t *,
> -					xfs_dqid_t, uint, uint, xfs_dquot_t **);
> +extern xfs_dqid_t	xfs_qm_id_for_quotatype(struct xfs_inode *ip,
> +					uint type);
> +extern int		xfs_qm_dqget(struct xfs_mount *mp, xfs_dqid_t id,
> +					uint type, uint flags,
> +					struct xfs_dquot **dqpp);
> +extern int		xfs_qm_dqget_inode(struct xfs_mount *mp,
> +					struct xfs_inode *ip, uint type,
> +					uint can_alloc,
> +					struct xfs_dquot **dqpp);
>  extern int		xfs_qm_dqget_next(struct xfs_mount *mp, xfs_dqid_t id,
>  					uint type, struct xfs_dquot **dqpp);
>  extern void		xfs_qm_dqput(xfs_dquot_t *);
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index d919b85..385d315 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -291,7 +291,7 @@ xfs_qm_dqattach_one(
>  	 * exist on disk and we didn't ask it to allocate; ESRCH if quotas got
>  	 * turned off suddenly.
>  	 */
> -	error = xfs_qm_dqget(ip->i_mount, ip, id, type, doalloc, &dqp);
> +	error = xfs_qm_dqget_inode(ip->i_mount, ip, type, doalloc, &dqp);
>  	if (error)
>  		return error;
>  
> @@ -1074,7 +1074,7 @@ xfs_qm_quotacheck_dqadjust(
>  	struct xfs_dquot	*dqp;
>  	int			error;
>  
> -	error = xfs_qm_dqget(mp, ip, id, type, XFS_QMOPT_DQALLOC, &dqp);
> +	error = xfs_qm_dqget_inode(mp, ip, type, XFS_QMOPT_DQALLOC, &dqp);
>  	if (error) {
>  		/*
>  		 * Shouldn't be able to turn off quotas here.
> @@ -1693,10 +1693,8 @@ xfs_qm_vop_dqalloc(
>  			 * holding ilock.
>  			 */
>  			xfs_iunlock(ip, lockflags);
> -			error = xfs_qm_dqget(mp, NULL, uid,
> -						 XFS_DQ_USER,
> -						 XFS_QMOPT_DQALLOC,
> -						 &uq);
> +			error = xfs_qm_dqget(mp, uid, XFS_DQ_USER,
> +					XFS_QMOPT_DQALLOC, &uq);
>  			if (error) {
>  				ASSERT(error != -ENOENT);
>  				return error;
> @@ -1719,10 +1717,8 @@ xfs_qm_vop_dqalloc(
>  	if ((flags & XFS_QMOPT_GQUOTA) && XFS_IS_GQUOTA_ON(mp)) {
>  		if (ip->i_d.di_gid != gid) {
>  			xfs_iunlock(ip, lockflags);
> -			error = xfs_qm_dqget(mp, NULL, gid,
> -						 XFS_DQ_GROUP,
> -						 XFS_QMOPT_DQALLOC,
> -						 &gq);
> +			error = xfs_qm_dqget(mp, gid, XFS_DQ_GROUP,
> +					XFS_QMOPT_DQALLOC, &gq);
>  			if (error) {
>  				ASSERT(error != -ENOENT);
>  				goto error_rele;
> @@ -1738,10 +1734,8 @@ xfs_qm_vop_dqalloc(
>  	if ((flags & XFS_QMOPT_PQUOTA) && XFS_IS_PQUOTA_ON(mp)) {
>  		if (xfs_get_projid(ip) != prid) {
>  			xfs_iunlock(ip, lockflags);
> -			error = xfs_qm_dqget(mp, NULL, (xfs_dqid_t)prid,
> -						 XFS_DQ_PROJ,
> -						 XFS_QMOPT_DQALLOC,
> -						 &pq);
> +			error = xfs_qm_dqget(mp, (xfs_dqid_t)prid, XFS_DQ_PROJ,
> +					XFS_QMOPT_DQALLOC, &pq);
>  			if (error) {
>  				ASSERT(error != -ENOENT);
>  				goto error_rele;
> diff --git a/fs/xfs/xfs_qm_bhv.c b/fs/xfs/xfs_qm_bhv.c
> index 2be6d27..531e822 100644
> --- a/fs/xfs/xfs_qm_bhv.c
> +++ b/fs/xfs/xfs_qm_bhv.c
> @@ -72,7 +72,7 @@ xfs_qm_statvfs(
>  	xfs_mount_t		*mp = ip->i_mount;
>  	xfs_dquot_t		*dqp;
>  
> -	if (!xfs_qm_dqget(mp, NULL, xfs_get_projid(ip), XFS_DQ_PROJ, 0, &dqp)) {
> +	if (!xfs_qm_dqget(mp, xfs_get_projid(ip), XFS_DQ_PROJ, 0, &dqp)) {
>  		xfs_fill_statvfs_from_dquot(statp, dqp);
>  		xfs_qm_dqput(dqp);
>  	}
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index 0234cfc..b9243f5 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -425,7 +425,7 @@ xfs_qm_scall_setqlim(
>  	 * a reference to the dquot, so it's safe to do this unlock/lock without
>  	 * it being reclaimed in the mean time.
>  	 */
> -	error = xfs_qm_dqget(mp, NULL, id, type, XFS_QMOPT_DQALLOC, &dqp);
> +	error = xfs_qm_dqget(mp, id, type, XFS_QMOPT_DQALLOC, &dqp);
>  	if (error) {
>  		ASSERT(error != -ENOENT);
>  		goto out_unlock;
> @@ -700,7 +700,7 @@ xfs_qm_scall_getquota(
>  	 * we aren't passing the XFS_QMOPT_DOALLOC flag. If it doesn't
>  	 * exist, we'll get ENOENT back.
>  	 */
> -	error = xfs_qm_dqget(mp, NULL, id, type, 0, &dqp);
> +	error = xfs_qm_dqget(mp, id, type, 0, &dqp);
>  	if (error)
>  		return error;
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/13] xfs: fetch dquots directly during quotacheck

[Brian Foster]

On Sun, Apr 22, 2018 at 08:06:30AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Quotacheck only runs during mount, which means that there are no other
> processes in the system that could be doing chown or chproj.  Therefore
> there's no potential for racing to attach dquots to the inode so we can
> drop all the ILOCK and race detection bits from quotacheck.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_qm.c |   28 ++++++++++++----------------
>  1 file changed, 12 insertions(+), 16 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 385d315..123ea5f 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -1065,16 +1065,17 @@ xfs_qm_dqiterate(
>  STATIC int
>  xfs_qm_quotacheck_dqadjust(
>  	struct xfs_inode	*ip,
> -	xfs_dqid_t		id,
>  	uint			type,
>  	xfs_qcnt_t		nblks,
>  	xfs_qcnt_t		rtblks)
>  {
>  	struct xfs_mount	*mp = ip->i_mount;
>  	struct xfs_dquot	*dqp;
> +	xfs_dqid_t		id;
>  	int			error;
>  
> -	error = xfs_qm_dqget_inode(mp, ip, type, XFS_QMOPT_DQALLOC, &dqp);
> +	id = xfs_qm_id_for_quotatype(ip, type);
> +	error = xfs_qm_dqget(mp, id, type, XFS_QMOPT_DQALLOC, &dqp);
>  	if (error) {
>  		/*
>  		 * Shouldn't be able to turn off quotas here.
> @@ -1147,13 +1148,10 @@ xfs_qm_dqusage_adjust(
>  	}
>  
>  	/*
> -	 * We don't _need_ to take the ilock EXCL. However, the xfs_qm_dqget
> -	 * interface expects the inode to be exclusively locked because that's
> -	 * the case in all other instances. It's OK that we do this because
> -	 * quotacheck is done only at mount time.
> +	 * 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, XFS_ILOCK_EXCL,
> -			 &ip);
> +	error = xfs_iget(mp, NULL, ino, XFS_IGET_DONTCACHE, 0, &ip);
>  	if (error) {
>  		*res = BULKSTAT_RV_NOTHING;
>  		return error;
> @@ -1188,33 +1186,31 @@ xfs_qm_dqusage_adjust(
>  	 * and quotaoffs don't race. (Quotachecks happen at mount time only).
>  	 */
>  	if (XFS_IS_UQUOTA_ON(mp)) {
> -		error = xfs_qm_quotacheck_dqadjust(ip, ip->i_d.di_uid,
> -						   XFS_DQ_USER, nblks, rtblks);
> +		error = xfs_qm_quotacheck_dqadjust(ip, XFS_DQ_USER, nblks,
> +				rtblks);
>  		if (error)
>  			goto error0;
>  	}
>  
>  	if (XFS_IS_GQUOTA_ON(mp)) {
> -		error = xfs_qm_quotacheck_dqadjust(ip, ip->i_d.di_gid,
> -						   XFS_DQ_GROUP, nblks, rtblks);
> +		error = xfs_qm_quotacheck_dqadjust(ip, XFS_DQ_GROUP, nblks,
> +				rtblks);
>  		if (error)
>  			goto error0;
>  	}
>  
>  	if (XFS_IS_PQUOTA_ON(mp)) {
> -		error = xfs_qm_quotacheck_dqadjust(ip, xfs_get_projid(ip),
> -						   XFS_DQ_PROJ, nblks, rtblks);
> +		error = xfs_qm_quotacheck_dqadjust(ip, XFS_DQ_PROJ, nblks,
> +				rtblks);
>  		if (error)
>  			goto error0;
>  	}
>  
> -	xfs_iunlock(ip, XFS_ILOCK_EXCL);
>  	IRELE(ip);
>  	*res = BULKSTAT_RV_DIDONE;
>  	return 0;
>  
>  error0:
> -	xfs_iunlock(ip, XFS_ILOCK_EXCL);
>  	IRELE(ip);
>  	*res = BULKSTAT_RV_GIVEUP;
>  	return error;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/13] xfs: refactor incore dquot initialization functions

[Brian Foster]

On Sun, Apr 22, 2018 at 08:06:37AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Create two incore dquot initialization functions that will help us to
> disentangle dqget and dqread.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_dquot.c |   80 +++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 50 insertions(+), 30 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 62415f1..7154909 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -491,26 +491,14 @@ xfs_qm_dqtobp(
>  	return 0;
>  }
>  
> -
> -/*
> - * Read in the ondisk dquot using dqtobp() then copy it to an incore version,
> - * and release the buffer immediately.
> - *
> - * If XFS_QMOPT_DQALLOC is set, allocate a dquot on disk if it needed.
> - */
> -int
> -xfs_qm_dqread(
> +/* Allocate and initialize everything we need for an incore dquot. */
> +STATIC struct xfs_dquot *
> +xfs_qm_dqinit_once(

I find the init_once() naming confusing. How about something more
straightforward like xfs_dquot_alloc()?

>  	struct xfs_mount	*mp,
>  	xfs_dqid_t		id,
> -	uint			type,
> -	uint			flags,
> -	struct xfs_dquot	**O_dqpp)
> +	uint			type)
>  {
>  	struct xfs_dquot	*dqp;
> -	struct xfs_disk_dquot	*ddqp;
> -	struct xfs_buf		*bp;
> -	struct xfs_trans	*tp = NULL;
> -	int			error;
>  
>  	dqp = kmem_zone_zalloc(xfs_qm_dqzone, KM_SLEEP);
>  
> @@ -549,7 +537,52 @@ xfs_qm_dqread(
>  	}
>  
>  	XFS_STATS_INC(mp, xs_qm_dquot);
> +	return dqp;
> +}
> +
> +/* Copy the in-core quota fields in from the on-disk buffer. */
> +STATIC void
> +xfs_qm_dqinit_from_buf(
> +	struct xfs_dquot	*dqp,
> +	struct xfs_disk_dquot	*ddqp)
> +{
> +	/* copy everything from disk dquot to the incore dquot */
> +	memcpy(&dqp->q_core, ddqp, sizeof(xfs_disk_dquot_t));
> +	xfs_qm_dquot_logitem_init(dqp);

I wonder if the logitem init should really be part of the "init from
buf" helper?

Otherwise looks fine:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> +
> +	/*
> +	 * Reservation counters are defined as reservation plus current usage
> +	 * to avoid having to add every time.
> +	 */
> +	dqp->q_res_bcount = be64_to_cpu(ddqp->d_bcount);
> +	dqp->q_res_icount = be64_to_cpu(ddqp->d_icount);
> +	dqp->q_res_rtbcount = be64_to_cpu(ddqp->d_rtbcount);
> +
> +	/* initialize the dquot speculative prealloc thresholds */
> +	xfs_dquot_set_prealloc_limits(dqp);
> +}
> +
> +/*
> + * Read in the ondisk dquot using dqtobp() then copy it to an incore version,
> + * and release the buffer immediately.
> + *
> + * If XFS_QMOPT_DQALLOC is set, allocate a dquot on disk if it needed.
> + */
> +int
> +xfs_qm_dqread(
> +	struct xfs_mount	*mp,
> +	xfs_dqid_t		id,
> +	uint			type,
> +	uint			flags,
> +	struct xfs_dquot	**O_dqpp)
> +{
> +	struct xfs_dquot	*dqp;
> +	struct xfs_disk_dquot	*ddqp;
> +	struct xfs_buf		*bp;
> +	struct xfs_trans	*tp = NULL;
> +	int			error;
>  
> +	dqp = xfs_qm_dqinit_once(mp, id, type);
>  	trace_xfs_dqread(dqp);
>  
>  	if (flags & XFS_QMOPT_DQALLOC) {
> @@ -574,20 +607,7 @@ xfs_qm_dqread(
>  		goto error1;
>  	}
>  
> -	/* copy everything from disk dquot to the incore dquot */
> -	memcpy(&dqp->q_core, ddqp, sizeof(xfs_disk_dquot_t));
> -	xfs_qm_dquot_logitem_init(dqp);
> -
> -	/*
> -	 * Reservation counters are defined as reservation plus current usage
> -	 * to avoid having to add every time.
> -	 */
> -	dqp->q_res_bcount = be64_to_cpu(ddqp->d_bcount);
> -	dqp->q_res_icount = be64_to_cpu(ddqp->d_icount);
> -	dqp->q_res_rtbcount = be64_to_cpu(ddqp->d_rtbcount);
> -
> -	/* initialize the dquot speculative prealloc thresholds */
> -	xfs_dquot_set_prealloc_limits(dqp);
> +	xfs_qm_dqinit_from_buf(dqp, ddqp);
>  
>  	/* Mark the buf so that this will stay incore a little longer */
>  	xfs_buf_set_ref(bp, XFS_DQUOT_REF);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/13] xfs: refactor XFS_QMOPT_DQNEXT out of existence

[Christoph Hellwig]

On Sun, Apr 22, 2018 at 08:05:58AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> There's only one caller of DQNEXT and its semantics can be moved into a
> separate function, so create the function and get rid of the flag.

Is there any good reason to keep the xfs_qm_scall_getquota{,next} helpers
instead of simplify factoring them into their only caller?

Otherwise this looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 02/13] xfs: refactor dquot cache handling

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 03/13] xfs: delegate dqget input checks to helper function

[Christoph Hellwig]

> +	if (!XFS_IS_QUOTA_RUNNING(mp)) {
> +		ASSERT(0);
> +		return -ESRCH;
> +	}

Maybe something like:

	if (WARN_ON_ONCE(!XFS_IS_QUOTA_RUNNING(mp)))
		return -ESRCH;

here and for the other ASSERT(0) case?

> +	if ((!XFS_IS_UQUOTA_ON(mp) && type == XFS_DQ_USER) ||
> +	    (!XFS_IS_PQUOTA_ON(mp) && type == XFS_DQ_PROJ) ||
> +	    (!XFS_IS_GQUOTA_ON(mp) && type == XFS_DQ_GROUP)) {
> +		return -ESRCH;
> +	}

No need for the curly braces.

> +
> +	if (type != XFS_DQ_USER &&
> +	    type != XFS_DQ_PROJ &&
> +	    type != XFS_DQ_GROUP) {
> +		ASSERT(0);
> +		return -EINVAL;

If we really care about these checks why not structure it as something
like:

	switch (type) {
	case XFS_DQ_USER:
		if (!XFS_IS_UQUOTA_ON(mp))
			return -ESRCH
		return 0;
	case XFS_DQ_PROJ:
		if (!XFS_IS_PQUOTA_ON(mp))
			return -ESRCH
		return 0;
	case XFS_DQ_GROUP:
		if (!XFS_IS_GQUOTA_ON(mp))
			return -ESRCH
		return 0;
	default:
		WARN_ON_ONCE(1);
		return -EINVAL;

Otherwise looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 04/13] xfs: remove unnecessary xfs_qm_dqattach parameter

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 05/13] xfs: split out dqget for inodes from regular dqget

[Christoph Hellwig]

> +int
> +xfs_qm_dqget_inode(
> +	struct xfs_mount	*mp,

As pointed out by Brian, we can skip passing this argument.

> +	struct xfs_inode	*ip,
> +	uint			type,
> +	uint			can_alloc,

can_alloc should probably be a bool, or passing XFS_QMOPT_DQALLOC
directly.  

Otherwise looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 06/13] xfs: fetch dquots directly during quotacheck

[Christoph Hellwig]

On Sun, Apr 22, 2018 at 08:06:30AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Quotacheck only runs during mount, which means that there are no other
> processes in the system that could be doing chown or chproj.  Therefore
> there's no potential for racing to attach dquots to the inode so we can
> drop all the ILOCK and race detection bits from quotacheck.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Btw, one thing I've wondered for a while is if there is any point in
keeping the dquot pointers in the inode at all.  With the radix tree
the dquots should be a few indirections and thus very few cache lines
away at any point in time, while saving us all the setup overhead, as
well as shrinking the inode.

Re: [PATCH 07/13] xfs: refactor incore dquot initialization functions

[Christoph Hellwig]

On Sun, Apr 22, 2018 at 08:06:37AM -0700, Darrick J. Wong wrote:
> -/*
> - * Read in the ondisk dquot using dqtobp() then copy it to an incore version,
> - * and release the buffer immediately.
> - *
> - * If XFS_QMOPT_DQALLOC is set, allocate a dquot on disk if it needed.
> - */
> -int
> -xfs_qm_dqread(
> +/* Allocate and initialize everything we need for an incore dquot. */
> +STATIC struct xfs_dquot *
> +xfs_qm_dqinit_once(
>  	struct xfs_mount	*mp,
>  	xfs_dqid_t		id,
> -	uint			type,
> -	uint			flags,
> -	struct xfs_dquot	**O_dqpp)
> +	uint			type)
>  {
>  	struct xfs_dquot	*dqp;
> -	struct xfs_disk_dquot	*ddqp;
> -	struct xfs_buf		*bp;
> -	struct xfs_trans	*tp = NULL;
> -	int			error;
>  
>  	dqp = kmem_zone_zalloc(xfs_qm_dqzone, KM_SLEEP);

Shouldn't this be called something like xfs_dquot_alloc given that
it allocates the xfs_dquot structure?

> +/* Copy the in-core quota fields in from the on-disk buffer. */
> +STATIC void
> +xfs_qm_dqinit_from_buf(
> +	struct xfs_dquot	*dqp,
> +	struct xfs_disk_dquot	*ddqp)
> +{

xfs_dquot_from_disk?

Also didn't we stop using STATIC for new code a while ago?

> +	/* copy everything from disk dquot to the incore dquot */
> +	memcpy(&dqp->q_core, ddqp, sizeof(xfs_disk_dquot_t));

	memcpy(&dqp->q_core, ddqp, sizeof(dqp->q_core));

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 08/13] xfs: refactor xfs_qm_dqtobp and xfs_qm_dqalloc

[Christoph Hellwig]

>  /*
> + * Ensure that the given in-core dquot has a buffer on disk backing it, and
> + * return the buffer. This is called when the bmapi finds a hole.
>   */
>  STATIC int
> +xfs_qm_dqalloc_ondisk(
> +	struct xfs_trans	**tpp,
> +	struct xfs_dquot	*dqp,
> +	struct xfs_buf		**bpp)
>  {
> +	struct xfs_bmbt_irec	map;
> +	struct xfs_defer_ops	dfops;
> +	struct xfs_mount	*mp = (*tpp)->t_mountp;
> +	struct xfs_buf		*bp;
> +	struct xfs_inode	*quotip;
> +	xfs_fsblock_t		firstblock;
> +	int			nmaps = 1;
> +	int			error;
>  
>  	trace_xfs_dqalloc(dqp);
>  
> +	quotip = xfs_quota_inode(mp, dqp->dq_flags);

I'd initialize quotip on the line where it is declarated.

> +xfs_qm_dqread_ondisk(
> +	struct xfs_mount	*mp,
> +	struct xfs_dquot	*dqp,
> +	struct xfs_buf		**bpp)
>  {
>  	struct xfs_bmbt_irec	map;
>  	struct xfs_buf		*bp;
>  	struct xfs_inode	*quotip;
>  	uint			lock_mode;
> +	int			nmaps = 1;
> +	int			error;
>  
> +	quotip = xfs_quota_inode(mp, dqp->dq_flags);

Same here?

Also shouldn't the function names be something like

xfs_dquot_disk_alloc and xfs_dquot_disk_read?

Re: [PATCH 09/13] xfs: remove xfs_qm_dqread flags argument

[Christoph Hellwig]

On Sun, Apr 22, 2018 at 08:06:58AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Move the _qm_dqread functionality to _qm_dqensure, then remove the flags
> argument from xfs_qm_dqread since the only DQALLOC users were internal
> to xfs_dquot.c anyway.

Unless this makes something much easier later in the series I'd rather
not change the names and calling conventions here.  dqensure just sounds
very weird, and having dqread is a trivial wrapper for it also doesn't
seem to be very intuitive.

Re: [PATCH 10/13] xfs: replace XFS_QMOPT_DQALLOC with XFS_DQGET_{ALLOC, EXISTS}

[Christoph Hellwig]

On Sun, Apr 22, 2018 at 08:07:05AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> DQALLOC is only ever used with xfs_qm_dqget*, and the only flag that the
> _dqget family of functions cares about is DQALLOC.  Therefore, change
> the name to make it clear that it's only a dqget flag.  Since these are
> modes and no longer bit flags, define a XFS_DQGET_EXISTS flag and
> convert all the callers.

I'm almost tempted to just have a 'bool alloc' argument instead.

Re: [PATCH 11/13] xfs: report failing address when dquot verifier fails

[Christoph Hellwig]

On Sun, Apr 22, 2018 at 08:07:11AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Pass the failing address through to the corruption report when dquot
> verifiers fail.

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 12/13] xfs: rename on-disk dquot counter zap functions

[Christoph Hellwig]

I agree with getting rid of the old names, but what about

xfs_qm_reset_dqcounts_buf and xfs_qm_reset_dqcounts_all to fit with
the naming for the existing xfs_qm_reset_dqcounts low-level function?

Re: [PATCH 05/13] xfs: split out dqget for inodes from regular dqget

[Darrick J. Wong]

On Mon, Apr 23, 2018 at 07:23:46PM +0200, Christoph Hellwig wrote:
> > +int
> > +xfs_qm_dqget_inode(
> > +	struct xfs_mount	*mp,
> 
> As pointed out by Brian, we can skip passing this argument.
> 
> > +	struct xfs_inode	*ip,
> > +	uint			type,
> > +	uint			can_alloc,
> 
> can_alloc should probably be a bool, or passing XFS_QMOPT_DQALLOC
> directly.  

Later I just turn it into a 'dqmode' argument where you can pass
XFS_DQGET_ALLOC (read or alloc on-disk dquot) or XFS_DQGET_EXISTS (read
from disk, error out if it's not there).

It's tempting to make those an enum xfs_dqget_mode to enforce that,
maybe I'll send that as a patch 14.

--D

> 
> Otherwise looks fine:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/13] xfs: refactor xfs_qm_dqtobp and xfs_qm_dqalloc

[Brian Foster]

On Sun, Apr 22, 2018 at 08:06:44AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Separate the disk dquot read and allocation functionality into
> two helper functions, then refactor dqread to call them directly.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_dquot.c |  261 ++++++++++++++++++++--------------------------------
>  1 file changed, 99 insertions(+), 162 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 7154909..ef4cd56 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
...
> @@ -574,74 +532,53 @@ xfs_qm_dqread(
>  	xfs_dqid_t		id,
>  	uint			type,
>  	uint			flags,
> -	struct xfs_dquot	**O_dqpp)
> +	struct xfs_dquot	**dqpp)
>  {
>  	struct xfs_dquot	*dqp;
> -	struct xfs_disk_dquot	*ddqp;
>  	struct xfs_buf		*bp;
> -	struct xfs_trans	*tp = NULL;
> +	struct xfs_trans	*tp;
>  	int			error;
>  
>  	dqp = xfs_qm_dqinit_once(mp, id, type);
>  	trace_xfs_dqread(dqp);
>  
> -	if (flags & XFS_QMOPT_DQALLOC) {
> +	/* Try to read the buffer... */
> +	error = xfs_qm_dqread_ondisk(mp, dqp, &bp);
> +	if (error == -ENOENT && (flags & XFS_QMOPT_DQALLOC)) {
> +		/* ...or allocate a new block and buffer. */
>  		error = xfs_trans_alloc(mp, &M_RES(mp)->tr_qm_dqalloc,
>  				XFS_QM_DQALLOC_SPACE_RES(mp), 0, 0, &tp);
>  		if (error)
> -			goto error0;
> -	}
> +			goto err;
>  
> -	/*
> -	 * get a pointer to the on-disk dquot and the buffer containing it
> -	 * dqp already knows its own type (GROUP/USER).
> -	 */
> -	error = xfs_qm_dqtobp(&tp, dqp, &ddqp, &bp, flags);
> -	if (error) {
> -		/*
> -		 * This can happen if quotas got turned off (ESRCH),
> -		 * or if the dquot didn't exist on disk and we ask to
> -		 * allocate (ENOENT).
> -		 */
> -		trace_xfs_dqread_fail(dqp);
> -		goto error1;
> -	}
> +		error = xfs_qm_dqalloc_ondisk(&tp, dqp, &bp);
> +		if (error)
> +			goto err_cancel;
>  
> -	xfs_qm_dqinit_from_buf(dqp, ddqp);
> +		error = xfs_trans_commit(tp);
> +	}
> +	ASSERT(xfs_buf_islocked(bp));

Doesn't this fail if the tx commit above fails? Granted we've probably
shut down...

Otherwise looks fine modulo Christoph's comments.

Brian

P.S., I see that assert was fixed up in the next patch, but then we have
a couple of the same calls separated by the init call. That probably
should be fixed up here in any event...

> +	if (error)
> +		goto err;
>  
> -	/* Mark the buf so that this will stay incore a little longer */
> -	xfs_buf_set_ref(bp, XFS_DQUOT_REF);
> +	xfs_qm_dqinit_from_buf(dqp, bp);
>  
>  	/*
> -	 * We got the buffer with a xfs_trans_read_buf() (in dqtobp())
> -	 * So we need to release with xfs_trans_brelse().
> -	 * The strategy here is identical to that of inodes; we lock
> -	 * the dquot in xfs_qm_dqget() before making it accessible to
> -	 * others. This is because dquots, like inodes, need a good level of
> -	 * concurrency, and we don't want to take locks on the entire buffers
> -	 * for dquot accesses.
> -	 * Note also that the dquot buffer may even be dirty at this point, if
> -	 * this particular dquot was repaired. We still aren't afraid to
> -	 * brelse it because we have the changes incore.
> +	 * At this point we should have a clean locked buffer.  Release the
> +	 * buffer since the incore dquot has its own copy and locking
> +	 * protocol so we needn't tie up the buffer any further.
>  	 */
>  	ASSERT(xfs_buf_islocked(bp));
> -	xfs_trans_brelse(tp, bp);
> -
> -	if (tp) {
> -		error = xfs_trans_commit(tp);
> -		if (error)
> -			goto error0;
> -	}
> -
> -	*O_dqpp = dqp;
> +	xfs_buf_relse(bp);
> +	*dqpp = dqp;
>  	return error;
>  
> -error1:
> -	if (tp)
> -		xfs_trans_cancel(tp);
> -error0:
> +err_cancel:
> +	xfs_trans_cancel(tp);
> +err:
> +	trace_xfs_dqread_fail(dqp);
>  	xfs_qm_dqdestroy(dqp);
> -	*O_dqpp = NULL;
> +	*dqpp = NULL;
>  	return error;
>  }
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/13] xfs: replace XFS_QMOPT_DQALLOC with XFS_DQGET_{ALLOC, EXISTS}

[Brian Foster]

On Mon, Apr 23, 2018 at 07:33:41PM +0200, Christoph Hellwig wrote:
> On Sun, Apr 22, 2018 at 08:07:05AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > DQALLOC is only ever used with xfs_qm_dqget*, and the only flag that the
> > _dqget family of functions cares about is DQALLOC.  Therefore, change
> > the name to make it clear that it's only a dqget flag.  Since these are
> > modes and no longer bit flags, define a XFS_DQGET_EXISTS flag and
> > convert all the callers.
> 
> I'm almost tempted to just have a 'bool alloc' argument instead.

I agree. I'm not a big fan of defining the 'exists' semantic for the
no-flags case. To me, it reads as a behavior modifier until/unless you
catch the fact that it's defined to zero. An alloc param should
self-document the behavior nicely, imo.

Brian

> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/13] xfs: refactor dquot iteration

[Brian Foster]

On Sun, Apr 22, 2018 at 08:07:24AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Create a helper function to iterate all the dquots of a given type in
> the system, and refactor the dquot scrub to use it.  This will get more
> use in the quota repair code.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/scrub/quota.c |   45 +++++++++++++++++++++------------------------
>  fs/xfs/xfs_dquot.c   |   32 ++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_dquot.h   |    5 +++++
>  3 files changed, 58 insertions(+), 24 deletions(-)
> 
> 
...
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 7ce3dbd..1f1139d 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -1213,3 +1213,35 @@ xfs_qm_exit(void)
>  	kmem_zone_destroy(xfs_qm_dqtrxzone);
>  	kmem_zone_destroy(xfs_qm_dqzone);
>  }
> +
> +/*
> + * Iterate every dquot of a particular type.  The caller must ensure that the
> + * particular quota type is active.  iter_fn can return negative error codes,
> + * or XFS_BTREE_QUERY_RANGE_ABORT to indicate that it wants to stop iterating.
> + */
> +int
> +xfs_qm_dqiterate(
> +	struct xfs_mount	*mp,
> +	uint			dqtype,
> +	xfs_qm_dqiterate_fn	iter_fn,
> +	void			*priv)
> +{
> +	struct xfs_dquot	*dq;
> +	xfs_dqid_t		id = 0;
> +	int			error;
> +
> +	do {
> +		error = xfs_qm_dqget_next(mp, id, dqtype, &dq);
> +		if (error == -ENOENT)
> +			return 0;
> +		if (error)
> +			return error;
> +
> +		id = be32_to_cpu(dq->q_core.d_id);
> +		error = iter_fn(dq, dqtype, id, priv);

This passes the id from the dquot into the scrub function, which goes
and does:

        if (id > be32_to_cpu(d->d_id))
                xfs_scrub_fblock_set_corrupt(sc, XFS_DATA_FORK, offset);

Is the intent to pass the search id into the iterator callback? If so,
perhaps a more descriptive name might be helpful (i.e., search_id, key,
whatever..). Otherwise looks Ok to me.

Brian

> +		xfs_qm_dqput(dq);
> +		id++;
> +	} while (error == 0 && id != 0);
> +
> +	return error;
> +}
> diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
> index fe99e43..5427355 100644
> --- a/fs/xfs/xfs_dquot.h
> +++ b/fs/xfs/xfs_dquot.h
> @@ -194,4 +194,9 @@ static inline struct xfs_dquot *xfs_qm_dqhold(struct xfs_dquot *dqp)
>  	return dqp;
>  }
>  
> +typedef int (*xfs_qm_dqiterate_fn)(struct xfs_dquot *dq, uint dqtype,
> +		xfs_dqid_t id, void *priv);
> +int xfs_qm_dqiterate(struct xfs_mount *mp, uint dqtype,
> +		xfs_qm_dqiterate_fn iter_fn, void *priv);
> +
>  #endif /* __XFS_DQUOT_H__ */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/13] xfs: refactor dquot iteration

[Darrick J. Wong]

On Tue, Apr 24, 2018 at 09:08:09AM -0400, Brian Foster wrote:
> On Sun, Apr 22, 2018 at 08:07:24AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Create a helper function to iterate all the dquots of a given type in
> > the system, and refactor the dquot scrub to use it.  This will get more
> > use in the quota repair code.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/scrub/quota.c |   45 +++++++++++++++++++++------------------------
> >  fs/xfs/xfs_dquot.c   |   32 ++++++++++++++++++++++++++++++++
> >  fs/xfs/xfs_dquot.h   |    5 +++++
> >  3 files changed, 58 insertions(+), 24 deletions(-)
> > 
> > 
> ...
> > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> > index 7ce3dbd..1f1139d 100644
> > --- a/fs/xfs/xfs_dquot.c
> > +++ b/fs/xfs/xfs_dquot.c
> > @@ -1213,3 +1213,35 @@ xfs_qm_exit(void)
> >  	kmem_zone_destroy(xfs_qm_dqtrxzone);
> >  	kmem_zone_destroy(xfs_qm_dqzone);
> >  }
> > +
> > +/*
> > + * Iterate every dquot of a particular type.  The caller must ensure that the
> > + * particular quota type is active.  iter_fn can return negative error codes,
> > + * or XFS_BTREE_QUERY_RANGE_ABORT to indicate that it wants to stop iterating.
> > + */
> > +int
> > +xfs_qm_dqiterate(
> > +	struct xfs_mount	*mp,
> > +	uint			dqtype,
> > +	xfs_qm_dqiterate_fn	iter_fn,
> > +	void			*priv)
> > +{
> > +	struct xfs_dquot	*dq;
> > +	xfs_dqid_t		id = 0;
> > +	int			error;
> > +
> > +	do {
> > +		error = xfs_qm_dqget_next(mp, id, dqtype, &dq);
> > +		if (error == -ENOENT)
> > +			return 0;
> > +		if (error)
> > +			return error;
> > +
> > +		id = be32_to_cpu(dq->q_core.d_id);
> > +		error = iter_fn(dq, dqtype, id, priv);
> 
> This passes the id from the dquot into the scrub function, which goes
> and does:
> 
>         if (id > be32_to_cpu(d->d_id))
>                 xfs_scrub_fblock_set_corrupt(sc, XFS_DATA_FORK, offset);
> 
> Is the intent to pass the search id into the iterator callback? If so,
> perhaps a more descriptive name might be helpful (i.e., search_id, key,
> whatever..). Otherwise looks Ok to me.

Yep, ihe id parameter ought to be the id we passed into dqget_next.
Thanks for catching that.

--D

> Brian
> 
> > +		xfs_qm_dqput(dq);
> > +		id++;
> > +	} while (error == 0 && id != 0);
> > +
> > +	return error;
> > +}
> > diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
> > index fe99e43..5427355 100644
> > --- a/fs/xfs/xfs_dquot.h
> > +++ b/fs/xfs/xfs_dquot.h
> > @@ -194,4 +194,9 @@ static inline struct xfs_dquot *xfs_qm_dqhold(struct xfs_dquot *dqp)
> >  	return dqp;
> >  }
> >  
> > +typedef int (*xfs_qm_dqiterate_fn)(struct xfs_dquot *dq, uint dqtype,
> > +		xfs_dqid_t id, void *priv);
> > +int xfs_qm_dqiterate(struct xfs_mount *mp, uint dqtype,
> > +		xfs_qm_dqiterate_fn iter_fn, void *priv);
> > +
> >  #endif /* __XFS_DQUOT_H__ */
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/13] xfs: refactor xfs_qm_dqtobp and xfs_qm_dqalloc

[Darrick J. Wong]

On Tue, Apr 24, 2018 at 09:07:31AM -0400, Brian Foster wrote:
> On Sun, Apr 22, 2018 at 08:06:44AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Separate the disk dquot read and allocation functionality into
> > two helper functions, then refactor dqread to call them directly.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/xfs_dquot.c |  261 ++++++++++++++++++++--------------------------------
> >  1 file changed, 99 insertions(+), 162 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> > index 7154909..ef4cd56 100644
> > --- a/fs/xfs/xfs_dquot.c
> > +++ b/fs/xfs/xfs_dquot.c
> ...
> > @@ -574,74 +532,53 @@ xfs_qm_dqread(
> >  	xfs_dqid_t		id,
> >  	uint			type,
> >  	uint			flags,
> > -	struct xfs_dquot	**O_dqpp)
> > +	struct xfs_dquot	**dqpp)
> >  {
> >  	struct xfs_dquot	*dqp;
> > -	struct xfs_disk_dquot	*ddqp;
> >  	struct xfs_buf		*bp;
> > -	struct xfs_trans	*tp = NULL;
> > +	struct xfs_trans	*tp;
> >  	int			error;
> >  
> >  	dqp = xfs_qm_dqinit_once(mp, id, type);
> >  	trace_xfs_dqread(dqp);
> >  
> > -	if (flags & XFS_QMOPT_DQALLOC) {
> > +	/* Try to read the buffer... */
> > +	error = xfs_qm_dqread_ondisk(mp, dqp, &bp);
> > +	if (error == -ENOENT && (flags & XFS_QMOPT_DQALLOC)) {
> > +		/* ...or allocate a new block and buffer. */
> >  		error = xfs_trans_alloc(mp, &M_RES(mp)->tr_qm_dqalloc,
> >  				XFS_QM_DQALLOC_SPACE_RES(mp), 0, 0, &tp);
> >  		if (error)
> > -			goto error0;
> > -	}
> > +			goto err;
> >  
> > -	/*
> > -	 * get a pointer to the on-disk dquot and the buffer containing it
> > -	 * dqp already knows its own type (GROUP/USER).
> > -	 */
> > -	error = xfs_qm_dqtobp(&tp, dqp, &ddqp, &bp, flags);
> > -	if (error) {
> > -		/*
> > -		 * This can happen if quotas got turned off (ESRCH),
> > -		 * or if the dquot didn't exist on disk and we ask to
> > -		 * allocate (ENOENT).
> > -		 */
> > -		trace_xfs_dqread_fail(dqp);
> > -		goto error1;
> > -	}
> > +		error = xfs_qm_dqalloc_ondisk(&tp, dqp, &bp);
> > +		if (error)
> > +			goto err_cancel;
> >  
> > -	xfs_qm_dqinit_from_buf(dqp, ddqp);
> > +		error = xfs_trans_commit(tp);
> > +	}
> > +	ASSERT(xfs_buf_islocked(bp));
> 
> Doesn't this fail if the tx commit above fails? Granted we've probably
> shut down...
> 
> Otherwise looks fine modulo Christoph's comments.
> 
> Brian
> 
> P.S., I see that assert was fixed up in the next patch, but then we have
> a couple of the same calls separated by the init call. That probably
> should be fixed up here in any event...

Aha, that's where that fix went, sorry. :(

--D

> 
> > +	if (error)
> > +		goto err;
> >  
> > -	/* Mark the buf so that this will stay incore a little longer */
> > -	xfs_buf_set_ref(bp, XFS_DQUOT_REF);
> > +	xfs_qm_dqinit_from_buf(dqp, bp);
> >  
> >  	/*
> > -	 * We got the buffer with a xfs_trans_read_buf() (in dqtobp())
> > -	 * So we need to release with xfs_trans_brelse().
> > -	 * The strategy here is identical to that of inodes; we lock
> > -	 * the dquot in xfs_qm_dqget() before making it accessible to
> > -	 * others. This is because dquots, like inodes, need a good level of
> > -	 * concurrency, and we don't want to take locks on the entire buffers
> > -	 * for dquot accesses.
> > -	 * Note also that the dquot buffer may even be dirty at this point, if
> > -	 * this particular dquot was repaired. We still aren't afraid to
> > -	 * brelse it because we have the changes incore.
> > +	 * At this point we should have a clean locked buffer.  Release the
> > +	 * buffer since the incore dquot has its own copy and locking
> > +	 * protocol so we needn't tie up the buffer any further.
> >  	 */
> >  	ASSERT(xfs_buf_islocked(bp));
> > -	xfs_trans_brelse(tp, bp);
> > -
> > -	if (tp) {
> > -		error = xfs_trans_commit(tp);
> > -		if (error)
> > -			goto error0;
> > -	}
> > -
> > -	*O_dqpp = dqp;
> > +	xfs_buf_relse(bp);
> > +	*dqpp = dqp;
> >  	return error;
> >  
> > -error1:
> > -	if (tp)
> > -		xfs_trans_cancel(tp);
> > -error0:
> > +err_cancel:
> > +	xfs_trans_cancel(tp);
> > +err:
> > +	trace_xfs_dqread_fail(dqp);
> >  	xfs_qm_dqdestroy(dqp);
> > -	*O_dqpp = NULL;
> > +	*dqpp = NULL;
> >  	return error;
> >  }
> >  
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/13] xfs: replace XFS_QMOPT_DQALLOC with XFS_DQGET_{ALLOC, EXISTS}

[Darrick J. Wong]

On Tue, Apr 24, 2018 at 09:07:49AM -0400, Brian Foster wrote:
> On Mon, Apr 23, 2018 at 07:33:41PM +0200, Christoph Hellwig wrote:
> > On Sun, Apr 22, 2018 at 08:07:05AM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > DQALLOC is only ever used with xfs_qm_dqget*, and the only flag that the
> > > _dqget family of functions cares about is DQALLOC.  Therefore, change
> > > the name to make it clear that it's only a dqget flag.  Since these are
> > > modes and no longer bit flags, define a XFS_DQGET_EXISTS flag and
> > > convert all the callers.
> > 
> > I'm almost tempted to just have a 'bool alloc' argument instead.
> 
> I agree. I'm not a big fan of defining the 'exists' semantic for the
> no-flags case. To me, it reads as a behavior modifier until/unless you
> catch the fact that it's defined to zero. An alloc param should
> self-document the behavior nicely, imo.

Ok, I'll change it to a bool.  FWIW Dave just said the same thing.

--D

> Brian
> 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/13] xfs: refactor incore dquot initialization functions

[Eric Sandeen]

On 4/23/18 11:27 AM, Christoph Hellwig wrote:
> On Sun, Apr 22, 2018 at 08:06:37AM -0700, Darrick J. Wong wrote:
>> -/*
>> - * Read in the ondisk dquot using dqtobp() then copy it to an incore version,
>> - * and release the buffer immediately.
>> - *
>> - * If XFS_QMOPT_DQALLOC is set, allocate a dquot on disk if it needed.
>> - */
>> -int
>> -xfs_qm_dqread(
>> +/* Allocate and initialize everything we need for an incore dquot. */
>> +STATIC struct xfs_dquot *
>> +xfs_qm_dqinit_once(
>>  	struct xfs_mount	*mp,
>>  	xfs_dqid_t		id,
>> -	uint			type,
>> -	uint			flags,
>> -	struct xfs_dquot	**O_dqpp)
>> +	uint			type)
>>  {
>>  	struct xfs_dquot	*dqp;
>> -	struct xfs_disk_dquot	*ddqp;
>> -	struct xfs_buf		*bp;
>> -	struct xfs_trans	*tp = NULL;
>> -	int			error;
>>  
>>  	dqp = kmem_zone_zalloc(xfs_qm_dqzone, KM_SLEEP);
> 
> Shouldn't this be called something like xfs_dquot_alloc given that
> it allocates the xfs_dquot structure?

Preaching to the choir here but I agree, "init_once" has a pretty specific
meaning in slab allocation paths, and this isn't it. ;)

-Eric

Re: [PATCH 09/13] xfs: remove xfs_qm_dqread flags argument

[Darrick J. Wong]

On Mon, Apr 23, 2018 at 07:32:57PM +0200, Christoph Hellwig wrote:
> On Sun, Apr 22, 2018 at 08:06:58AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Move the _qm_dqread functionality to _qm_dqensure, then remove the flags
> > argument from xfs_qm_dqread since the only DQALLOC users were internal
> > to xfs_dquot.c anyway.
> 
> Unless this makes something much easier later in the series I'd rather
> not change the names and calling conventions here.  dqensure just sounds
> very weird, and having dqread is a trivial wrapper for it also doesn't
> seem to be very intuitive.

How about xfs_dquot_setup for a name instead of dqensure?

The purpose of making dqread a trivial wrapper is so that we can drop
the flags parameter from dqread which is always zero.  Later on in the
quotacheck repair patch I'll have a repair-specific use of the _setup
function and friends.

--D

> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 12/13] xfs: rename on-disk dquot counter zap functions

[Darrick J. Wong]

On Mon, Apr 23, 2018 at 07:35:48PM +0200, Christoph Hellwig wrote:
> I agree with getting rid of the old names, but what about
> 
> xfs_qm_reset_dqcounts_buf and xfs_qm_reset_dqcounts_all to fit with
> the naming for the existing xfs_qm_reset_dqcounts low-level function?

Sounds good to me.

--D

> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 05/13] xfs: split out dqget for inodes from regular dqget

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

There are two uses of dqget here -- one is to return the dquot for a
given type and id, and the other is to return the dquot for a given type
and inode.  Those are two separate things, so split them into two
smaller functions.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_dquot.c       |  148 ++++++++++++++++++++++++++++++++--------------
 fs/xfs/xfs_dquot.h       |   10 ++-
 fs/xfs/xfs_iomap.c       |    2 -
 fs/xfs/xfs_qm.c          |   39 +++++-------
 fs/xfs/xfs_qm_bhv.c      |    2 -
 fs/xfs/xfs_qm_syscalls.c |    4 +
 fs/xfs/xfs_quota.h       |    2 -
 fs/xfs/xfs_reflink.c     |    4 +
 8 files changed, 134 insertions(+), 77 deletions(-)


diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 028d0d513cdf..e3e51f6e2880 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -782,24 +782,19 @@ xfs_qm_dqget_checks(
 }
 
 /*
- * Given the file system, inode OR id, and type (UDQUOT/GDQUOT), return a
- * a locked dquot, doing an allocation (if requested) as needed.
- * When both an inode and an id are given, the inode's id takes precedence.
- * That is, if the id changes while we don't hold the ilock inside this
- * function, the new dquot is returned, not necessarily the one requested
- * in the id argument.
+ * Given the file system, id, and type (UDQUOT/GDQUOT), return a a locked
+ * dquot, doing an allocation (if requested) as needed.
  */
 int
 xfs_qm_dqget(
-	xfs_mount_t	*mp,
-	xfs_inode_t	*ip,	  /* locked inode (optional) */
-	xfs_dqid_t	id,	  /* uid/projid/gid depending on type */
-	uint		type,	  /* XFS_DQ_USER/XFS_DQ_PROJ/XFS_DQ_GROUP */
-	uint		flags,	  /* DQALLOC, DQSUSER, DQREPAIR, DOWARN */
-	xfs_dquot_t	**O_dqpp) /* OUT : locked incore dquot */
+	struct xfs_mount	*mp,
+	xfs_dqid_t		id,
+	uint			type,
+	uint			flags,	  /* DQALLOC, DQSUSER, DQREPAIR, DOWARN */
+	struct xfs_dquot	**O_dqpp)
 {
 	struct xfs_quotainfo	*qi = mp->m_quotainfo;
-	struct radix_tree_root *tree = xfs_dquot_tree(qi, type);
+	struct radix_tree_root	*tree = xfs_dquot_tree(qi, type);
 	struct xfs_dquot	*dqp;
 	int			error;
 
@@ -807,10 +802,82 @@ xfs_qm_dqget(
 	if (error)
 		return error;
 
-	if (ip) {
-		ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
-		ASSERT(xfs_inode_dquot(ip, type) == NULL);
+restart:
+	dqp = xfs_qm_dqget_cache_lookup(mp, qi, tree, id);
+	if (dqp) {
+		*O_dqpp = dqp;
+		return 0;
+	}
+
+	error = xfs_qm_dqread(mp, id, type, flags, &dqp);
+	if (error)
+		return error;
+
+	error = xfs_qm_dqget_cache_insert(mp, qi, tree, id, dqp);
+	if (error) {
+		/*
+		 * Duplicate found. Just throw away the new dquot and start
+		 * over.
+		 */
+		xfs_qm_dqdestroy(dqp);
+		XFS_STATS_INC(mp, xs_qm_dquot_dups);
+		goto restart;
+	}
+
+	trace_xfs_dqget_miss(dqp);
+	*O_dqpp = dqp;
+	return 0;
+}
+
+/* Return the quota id for a given inode and type. */
+xfs_dqid_t
+xfs_qm_id_for_quotatype(
+	struct xfs_inode	*ip,
+	uint			type)
+{
+	switch (type) {
+	case XFS_DQ_USER:
+		return ip->i_d.di_uid;
+	case XFS_DQ_GROUP:
+		return ip->i_d.di_gid;
+	case XFS_DQ_PROJ:
+		return xfs_get_projid(ip);
 	}
+	ASSERT(0);
+	return 0;
+}
+
+/*
+ * Return the dquot for a given inode and type.  If @can_alloc is true, then
+ * allocate blocks if needed.  The inode's ILOCK must be held and it must not
+ * have already had an inode attached.
+ */
+int
+xfs_qm_dqget_inode(
+	struct xfs_inode	*ip,
+	uint			type,
+	bool			can_alloc,
+	struct xfs_dquot	**O_dqpp)
+{
+	struct xfs_mount	*mp = ip->i_mount;
+	struct xfs_quotainfo	*qi = mp->m_quotainfo;
+	struct radix_tree_root	*tree = xfs_dquot_tree(qi, type);
+	struct xfs_dquot	*dqp;
+	xfs_dqid_t		id;
+	uint			flags = 0;
+	int			error;
+
+	error = xfs_qm_dqget_checks(mp, type);
+	if (error)
+		return error;
+
+	if (can_alloc)
+		flags |= XFS_QMOPT_DQALLOC;
+
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+	ASSERT(xfs_inode_dquot(ip, type) == NULL);
+
+	id = xfs_qm_id_for_quotatype(ip, type);
 
 restart:
 	dqp = xfs_qm_dqget_cache_lookup(mp, qi, tree, id);
@@ -826,37 +893,30 @@ xfs_qm_dqget(
 	 * lock here means dealing with a chown that can happen before
 	 * we re-acquire the lock.
 	 */
-	if (ip)
-		xfs_iunlock(ip, XFS_ILOCK_EXCL);
-
+	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 	error = xfs_qm_dqread(mp, id, type, flags, &dqp);
-
-	if (ip)
-		xfs_ilock(ip, XFS_ILOCK_EXCL);
-
+	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	if (error)
 		return error;
 
-	if (ip) {
-		/*
-		 * A dquot could be attached to this inode by now, since
-		 * we had dropped the ilock.
-		 */
-		if (xfs_this_quota_on(mp, type)) {
-			struct xfs_dquot	*dqp1;
-
-			dqp1 = xfs_inode_dquot(ip, type);
-			if (dqp1) {
-				xfs_qm_dqdestroy(dqp);
-				dqp = dqp1;
-				xfs_dqlock(dqp);
-				goto dqret;
-			}
-		} else {
-			/* inode stays locked on return */
+	/*
+	 * A dquot could be attached to this inode by now, since we had
+	 * dropped the ilock.
+	 */
+	if (xfs_this_quota_on(mp, type)) {
+		struct xfs_dquot	*dqp1;
+
+		dqp1 = xfs_inode_dquot(ip, type);
+		if (dqp1) {
 			xfs_qm_dqdestroy(dqp);
-			return -ESRCH;
+			dqp = dqp1;
+			xfs_dqlock(dqp);
+			goto dqret;
 		}
+	} else {
+		/* inode stays locked on return */
+		xfs_qm_dqdestroy(dqp);
+		return -ESRCH;
 	}
 
 	error = xfs_qm_dqget_cache_insert(mp, qi, tree, id, dqp);
@@ -870,8 +930,8 @@ xfs_qm_dqget(
 		goto restart;
 	}
 
- dqret:
-	ASSERT((ip == NULL) || xfs_isilocked(ip, XFS_ILOCK_EXCL));
+dqret:
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
 	trace_xfs_dqget_miss(dqp);
 	*O_dqpp = dqp;
 	return 0;
@@ -893,7 +953,7 @@ xfs_qm_dqget_next(
 
 	*dqpp = NULL;
 	for (; !error; error = xfs_dq_get_next_id(mp, type, &id)) {
-		error = xfs_qm_dqget(mp, NULL, id, type, 0, &dqp);
+		error = xfs_qm_dqget(mp, id, type, 0, &dqp);
 		if (error == -ENOENT)
 			continue;
 		else if (error != 0)
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 303e71dfcf70..8a5b30e952b0 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -169,8 +169,14 @@ extern void		xfs_qm_adjust_dqtimers(xfs_mount_t *,
 					xfs_disk_dquot_t *);
 extern void		xfs_qm_adjust_dqlimits(struct xfs_mount *,
 					       struct xfs_dquot *);
-extern int		xfs_qm_dqget(xfs_mount_t *, xfs_inode_t *,
-					xfs_dqid_t, uint, uint, xfs_dquot_t **);
+extern xfs_dqid_t	xfs_qm_id_for_quotatype(struct xfs_inode *ip,
+					uint type);
+extern int		xfs_qm_dqget(struct xfs_mount *mp, xfs_dqid_t id,
+					uint type, uint flags,
+					struct xfs_dquot **dqpp);
+extern int		xfs_qm_dqget_inode(struct xfs_inode *ip, uint type,
+					bool can_alloc,
+					struct xfs_dquot **dqpp);
 extern int		xfs_qm_dqget_next(struct xfs_mount *mp, xfs_dqid_t id,
 					uint type, struct xfs_dquot **dqpp);
 extern void		xfs_qm_dqput(xfs_dquot_t *);
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 58b04eace2bc..7f8fc83c1555 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -576,7 +576,7 @@ xfs_file_iomap_begin_delay(
 		goto done;
 	}
 
-	error = xfs_qm_dqattach_locked(ip, 0);
+	error = xfs_qm_dqattach_locked(ip, false);
 	if (error)
 		goto out_unlock;
 
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index d919b8554bf0..25c0718e1993 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -265,7 +265,7 @@ xfs_qm_dqattach_one(
 	xfs_inode_t	*ip,
 	xfs_dqid_t	id,
 	uint		type,
-	uint		doalloc,
+	bool		doalloc,
 	xfs_dquot_t	**IO_idqpp)
 {
 	xfs_dquot_t	*dqp;
@@ -291,7 +291,7 @@ xfs_qm_dqattach_one(
 	 * exist on disk and we didn't ask it to allocate; ESRCH if quotas got
 	 * turned off suddenly.
 	 */
-	error = xfs_qm_dqget(ip->i_mount, ip, id, type, doalloc, &dqp);
+	error = xfs_qm_dqget_inode(ip, type, doalloc, &dqp);
 	if (error)
 		return error;
 
@@ -333,7 +333,7 @@ xfs_qm_need_dqattach(
 int
 xfs_qm_dqattach_locked(
 	xfs_inode_t	*ip,
-	uint		flags)
+	bool		doalloc)
 {
 	xfs_mount_t	*mp = ip->i_mount;
 	int		error = 0;
@@ -345,8 +345,7 @@ xfs_qm_dqattach_locked(
 
 	if (XFS_IS_UQUOTA_ON(mp) && !ip->i_udquot) {
 		error = xfs_qm_dqattach_one(ip, ip->i_d.di_uid, XFS_DQ_USER,
-						flags & XFS_QMOPT_DQALLOC,
-						&ip->i_udquot);
+				doalloc, &ip->i_udquot);
 		if (error)
 			goto done;
 		ASSERT(ip->i_udquot);
@@ -354,8 +353,7 @@ xfs_qm_dqattach_locked(
 
 	if (XFS_IS_GQUOTA_ON(mp) && !ip->i_gdquot) {
 		error = xfs_qm_dqattach_one(ip, ip->i_d.di_gid, XFS_DQ_GROUP,
-						flags & XFS_QMOPT_DQALLOC,
-						&ip->i_gdquot);
+				doalloc, &ip->i_gdquot);
 		if (error)
 			goto done;
 		ASSERT(ip->i_gdquot);
@@ -363,8 +361,7 @@ xfs_qm_dqattach_locked(
 
 	if (XFS_IS_PQUOTA_ON(mp) && !ip->i_pdquot) {
 		error = xfs_qm_dqattach_one(ip, xfs_get_projid(ip), XFS_DQ_PROJ,
-						flags & XFS_QMOPT_DQALLOC,
-						&ip->i_pdquot);
+				doalloc, &ip->i_pdquot);
 		if (error)
 			goto done;
 		ASSERT(ip->i_pdquot);
@@ -389,7 +386,7 @@ xfs_qm_dqattach(
 		return 0;
 
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
-	error = xfs_qm_dqattach_locked(ip, 0);
+	error = xfs_qm_dqattach_locked(ip, false);
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 
 	return error;
@@ -1074,7 +1071,7 @@ xfs_qm_quotacheck_dqadjust(
 	struct xfs_dquot	*dqp;
 	int			error;
 
-	error = xfs_qm_dqget(mp, ip, id, type, XFS_QMOPT_DQALLOC, &dqp);
+	error = xfs_qm_dqget_inode(ip, type, true, &dqp);
 	if (error) {
 		/*
 		 * Shouldn't be able to turn off quotas here.
@@ -1674,7 +1671,7 @@ xfs_qm_vop_dqalloc(
 	 * if necessary. The dquot(s) will not be locked.
 	 */
 	if (XFS_NOT_DQATTACHED(mp, ip)) {
-		error = xfs_qm_dqattach_locked(ip, XFS_QMOPT_DQALLOC);
+		error = xfs_qm_dqattach_locked(ip, true);
 		if (error) {
 			xfs_iunlock(ip, lockflags);
 			return error;
@@ -1693,10 +1690,8 @@ xfs_qm_vop_dqalloc(
 			 * holding ilock.
 			 */
 			xfs_iunlock(ip, lockflags);
-			error = xfs_qm_dqget(mp, NULL, uid,
-						 XFS_DQ_USER,
-						 XFS_QMOPT_DQALLOC,
-						 &uq);
+			error = xfs_qm_dqget(mp, uid, XFS_DQ_USER,
+					XFS_QMOPT_DQALLOC, &uq);
 			if (error) {
 				ASSERT(error != -ENOENT);
 				return error;
@@ -1719,10 +1714,8 @@ xfs_qm_vop_dqalloc(
 	if ((flags & XFS_QMOPT_GQUOTA) && XFS_IS_GQUOTA_ON(mp)) {
 		if (ip->i_d.di_gid != gid) {
 			xfs_iunlock(ip, lockflags);
-			error = xfs_qm_dqget(mp, NULL, gid,
-						 XFS_DQ_GROUP,
-						 XFS_QMOPT_DQALLOC,
-						 &gq);
+			error = xfs_qm_dqget(mp, gid, XFS_DQ_GROUP,
+					XFS_QMOPT_DQALLOC, &gq);
 			if (error) {
 				ASSERT(error != -ENOENT);
 				goto error_rele;
@@ -1738,10 +1731,8 @@ xfs_qm_vop_dqalloc(
 	if ((flags & XFS_QMOPT_PQUOTA) && XFS_IS_PQUOTA_ON(mp)) {
 		if (xfs_get_projid(ip) != prid) {
 			xfs_iunlock(ip, lockflags);
-			error = xfs_qm_dqget(mp, NULL, (xfs_dqid_t)prid,
-						 XFS_DQ_PROJ,
-						 XFS_QMOPT_DQALLOC,
-						 &pq);
+			error = xfs_qm_dqget(mp, (xfs_dqid_t)prid, XFS_DQ_PROJ,
+					XFS_QMOPT_DQALLOC, &pq);
 			if (error) {
 				ASSERT(error != -ENOENT);
 				goto error_rele;
diff --git a/fs/xfs/xfs_qm_bhv.c b/fs/xfs/xfs_qm_bhv.c
index 2be6d2735ca9..531e8224dcb6 100644
--- a/fs/xfs/xfs_qm_bhv.c
+++ b/fs/xfs/xfs_qm_bhv.c
@@ -72,7 +72,7 @@ xfs_qm_statvfs(
 	xfs_mount_t		*mp = ip->i_mount;
 	xfs_dquot_t		*dqp;
 
-	if (!xfs_qm_dqget(mp, NULL, xfs_get_projid(ip), XFS_DQ_PROJ, 0, &dqp)) {
+	if (!xfs_qm_dqget(mp, xfs_get_projid(ip), XFS_DQ_PROJ, 0, &dqp)) {
 		xfs_fill_statvfs_from_dquot(statp, dqp);
 		xfs_qm_dqput(dqp);
 	}
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 0234cfc4d445..b9243f554697 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -425,7 +425,7 @@ xfs_qm_scall_setqlim(
 	 * a reference to the dquot, so it's safe to do this unlock/lock without
 	 * it being reclaimed in the mean time.
 	 */
-	error = xfs_qm_dqget(mp, NULL, id, type, XFS_QMOPT_DQALLOC, &dqp);
+	error = xfs_qm_dqget(mp, id, type, XFS_QMOPT_DQALLOC, &dqp);
 	if (error) {
 		ASSERT(error != -ENOENT);
 		goto out_unlock;
@@ -700,7 +700,7 @@ xfs_qm_scall_getquota(
 	 * we aren't passing the XFS_QMOPT_DOALLOC flag. If it doesn't
 	 * exist, we'll get ENOENT back.
 	 */
-	error = xfs_qm_dqget(mp, NULL, id, type, 0, &dqp);
+	error = xfs_qm_dqget(mp, id, type, 0, &dqp);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h
index a4d392240cf4..1c79ebbe5236 100644
--- a/fs/xfs/xfs_quota.h
+++ b/fs/xfs/xfs_quota.h
@@ -91,7 +91,7 @@ extern int xfs_qm_vop_chown_reserve(struct xfs_trans *, struct xfs_inode *,
 		struct xfs_dquot *, struct xfs_dquot *,
 		struct xfs_dquot *, uint);
 extern int xfs_qm_dqattach(struct xfs_inode *);
-extern int xfs_qm_dqattach_locked(struct xfs_inode *, uint);
+extern int xfs_qm_dqattach_locked(struct xfs_inode *ip, bool doalloc);
 extern void xfs_qm_dqdetach(struct xfs_inode *);
 extern void xfs_qm_dqrele(struct xfs_dquot *);
 extern void xfs_qm_statvfs(struct xfs_inode *, struct kstatfs *);
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index fd8e6afb8e1c..d7f251bd7acc 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -305,7 +305,7 @@ xfs_reflink_reserve_cow(
 	 * Fork all the shared blocks from our write offset until the end of
 	 * the extent.
 	 */
-	error = xfs_qm_dqattach_locked(ip, 0);
+	error = xfs_qm_dqattach_locked(ip, false);
 	if (error)
 		return error;
 
@@ -431,7 +431,7 @@ xfs_reflink_allocate_cow(
 		if (error)
 			return error;
 
-		error = xfs_qm_dqattach_locked(ip, 0);
+		error = xfs_qm_dqattach_locked(ip, false);
 		if (error)
 			goto out;
 		goto retry;