[PATCH 0/4] xfs: enable per-type quota timers and warn limits

[PATCH 0/4] xfs: enable per-type quota timers and warn limits

[Eric Sandeen]

Quota timers are currently a mess.  Right now, at mount time,
we pick up the first enabled type and use that for the single
timer in mp->m_quotainfo.

Interestingly, if we set a timer on a different type, /that/
gets set into mp->m_quotainfo where it stays in effect until
the next mount, when we pick the first enabled type again.

We actually write the timer values to each type of quota inode,
but only one is ever in force, according to the interesting behavior
described above.

This series allows quota timers & warn limits to be independently
set and enforced for each quota type.

All the action is in the last patch, the first 3 are cleanups to
help.

-Eric

[PATCH 1/4] xfs: fix up some whitespace in quota code

[Eric Sandeen]

There is a fair bit of whitespace damage in the quota code, so
fix up enough of it that subsequent patches are restricted to
functional change to aid review.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/xfs_dquot.c    | 16 ++++++++--------
 fs/xfs/xfs_qm.h       | 44 +++++++++++++++++++++----------------------
 fs/xfs/xfs_quotaops.c |  8 ++++----
 3 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index d223e1ae90a6..02f433d1f13a 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -205,16 +205,16 @@ xfs_qm_adjust_dqtimers(
  */
 STATIC void
 xfs_qm_init_dquot_blk(
-	xfs_trans_t	*tp,
-	xfs_mount_t	*mp,
-	xfs_dqid_t	id,
-	uint		type,
-	xfs_buf_t	*bp)
+	struct xfs_trans	*tp,
+	struct xfs_mount	*mp,
+	xfs_dqid_t		id,
+	uint			type,
+	struct xfs_buf		*bp)
 {
 	struct xfs_quotainfo	*q = mp->m_quotainfo;
-	xfs_dqblk_t	*d;
-	xfs_dqid_t	curid;
-	int		i;
+	xfs_dqblk_t		*d;
+	xfs_dqid_t		curid;
+	int			i;
 
 	ASSERT(tp);
 	ASSERT(xfs_buf_islocked(bp));
diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
index 4e57edca8bce..3a850401b102 100644
--- a/fs/xfs/xfs_qm.h
+++ b/fs/xfs/xfs_qm.h
@@ -42,12 +42,12 @@ extern struct kmem_zone	*xfs_qm_dqtrxzone;
 #define XFS_DQUOT_CLUSTER_SIZE_FSB	(xfs_filblks_t)1
 
 struct xfs_def_quota {
-	xfs_qcnt_t       bhardlimit;     /* default data blk hard limit */
-	xfs_qcnt_t       bsoftlimit;	 /* default data blk soft limit */
-	xfs_qcnt_t       ihardlimit;	 /* default inode count hard limit */
-	xfs_qcnt_t       isoftlimit;	 /* default inode count soft limit */
-	xfs_qcnt_t	 rtbhardlimit;   /* default realtime blk hard limit */
-	xfs_qcnt_t	 rtbsoftlimit;   /* default realtime blk soft limit */
+	xfs_qcnt_t	bhardlimit;	/* default data blk hard limit */
+	xfs_qcnt_t	bsoftlimit;	/* default data blk soft limit */
+	xfs_qcnt_t	ihardlimit;	/* default inode count hard limit */
+	xfs_qcnt_t	isoftlimit;	/* default inode count soft limit */
+	xfs_qcnt_t	rtbhardlimit;	/* default realtime blk hard limit */
+	xfs_qcnt_t	rtbsoftlimit;	/* default realtime blk soft limit */
 };
 
 /*
@@ -55,28 +55,28 @@ struct xfs_def_quota {
  * The mount structure keeps a pointer to this.
  */
 struct xfs_quotainfo {
-	struct radix_tree_root qi_uquota_tree;
-	struct radix_tree_root qi_gquota_tree;
-	struct radix_tree_root qi_pquota_tree;
-	struct mutex qi_tree_lock;
+	struct radix_tree_root	qi_uquota_tree;
+	struct radix_tree_root	qi_gquota_tree;
+	struct radix_tree_root	qi_pquota_tree;
+	struct mutex		qi_tree_lock;
 	struct xfs_inode	*qi_uquotaip;	/* user quota inode */
 	struct xfs_inode	*qi_gquotaip;	/* group quota inode */
 	struct xfs_inode	*qi_pquotaip;	/* project quota inode */
-	struct list_lru	 qi_lru;
-	int		 qi_dquots;
-	time64_t	 qi_btimelimit;	 /* limit for blks timer */
-	time64_t	 qi_itimelimit;	 /* limit for inodes timer */
-	time64_t	 qi_rtbtimelimit;/* limit for rt blks timer */
-	xfs_qwarncnt_t	 qi_bwarnlimit;	 /* limit for blks warnings */
-	xfs_qwarncnt_t	 qi_iwarnlimit;	 /* limit for inodes warnings */
-	xfs_qwarncnt_t	 qi_rtbwarnlimit;/* limit for rt blks warnings */
-	struct mutex	 qi_quotaofflock;/* to serialize quotaoff */
-	xfs_filblks_t	 qi_dqchunklen;	 /* # BBs in a chunk of dqs */
-	uint		 qi_dqperchunk;	 /* # ondisk dqs in above chunk */
+	struct list_lru		qi_lru;
+	int			qi_dquots;
+	time64_t		qi_btimelimit;	/* limit for blks timer */
+	time64_t		qi_itimelimit;	/* limit for inodes timer */
+	time64_t		qi_rtbtimelimit;/* limit for rt blks timer */
+	xfs_qwarncnt_t		qi_bwarnlimit;	/* limit for blks warnings */
+	xfs_qwarncnt_t		qi_iwarnlimit;	/* limit for inodes warnings */
+	xfs_qwarncnt_t		qi_rtbwarnlimit;/* limit for rt blks warnings */
+	struct mutex		qi_quotaofflock;/* to serialize quotaoff */
+	xfs_filblks_t		qi_dqchunklen;	/* # BBs in a chunk of dqs */
+	uint			qi_dqperchunk;	/* # ondisk dq in above chunk */
 	struct xfs_def_quota	qi_usr_default;
 	struct xfs_def_quota	qi_grp_default;
 	struct xfs_def_quota	qi_prj_default;
-	struct shrinker	qi_shrinker;
+	struct shrinker		qi_shrinker;
 };
 
 static inline struct radix_tree_root *
diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c
index 38669e827206..cb16a91dd1d4 100644
--- a/fs/xfs/xfs_quotaops.c
+++ b/fs/xfs/xfs_quotaops.c
@@ -23,8 +23,8 @@ xfs_qm_fill_state(
 	struct xfs_inode	*ip,
 	xfs_ino_t		ino)
 {
-	struct xfs_quotainfo *q = mp->m_quotainfo;
-	bool tempqip = false;
+	struct xfs_quotainfo	*q = mp->m_quotainfo;
+	bool			tempqip = false;
 
 	tstate->ino = ino;
 	if (!ip && ino == NULLFSINO)
@@ -109,8 +109,8 @@ xfs_fs_set_info(
 	int			type,
 	struct qc_info		*info)
 {
-	struct xfs_mount *mp = XFS_M(sb);
-	struct qc_dqblk newlim;
+	struct xfs_mount	*mp = XFS_M(sb);
+	struct qc_dqblk		newlim;
 
 	if (sb_rdonly(sb))
 		return -EROFS;
-- 
2.17.0

[PATCH 2/4] xfs: simplify args to xfs_get_defquota

[Eric Sandeen]

There's no real reason to pass both xfs_dquot and xfs_quotainfo to
xfs_get_defquota, because the latter can be obtained from the former.
This simplifies a bit more of the argument passing.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/xfs_dquot.c       |  3 +--
 fs/xfs/xfs_qm.c          | 17 ++++++++---------
 fs/xfs/xfs_qm.h          |  3 ++-
 fs/xfs/xfs_qm_syscalls.c |  2 +-
 fs/xfs/xfs_trans_dquot.c |  3 +--
 5 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 02f433d1f13a..ddf41c24efcd 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -69,13 +69,12 @@ xfs_qm_adjust_dqlimits(
 	struct xfs_mount	*mp,
 	struct xfs_dquot	*dq)
 {
-	struct xfs_quotainfo	*q = mp->m_quotainfo;
 	struct xfs_disk_dquot	*d = &dq->q_core;
 	struct xfs_def_quota	*defq;
 	int			prealloc = 0;
 
 	ASSERT(d->d_id);
-	defq = xfs_get_defquota(dq, q);
+	defq = xfs_get_defquota(dq);
 
 	if (defq->bsoftlimit && !d->d_blk_softlimit) {
 		d->d_blk_softlimit = cpu_to_be64(defq->bsoftlimit);
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 0b0909657bad..b3cd87d0bccb 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -541,8 +541,7 @@ xfs_qm_shrink_count(
 STATIC void
 xfs_qm_set_defquota(
 	struct xfs_mount	*mp,
-	uint			type,
-	struct xfs_quotainfo	*qinf)
+	uint			type)
 {
 	struct xfs_dquot	*dqp;
 	struct xfs_def_quota	*defq;
@@ -554,7 +553,7 @@ xfs_qm_set_defquota(
 		return;
 
 	ddqp = &dqp->q_core;
-	defq = xfs_get_defquota(dqp, qinf);
+	defq = xfs_get_defquota(dqp);
 
 	/*
 	 * Timers and warnings have been already set, let's just set the
@@ -572,9 +571,9 @@ xfs_qm_set_defquota(
 /* Initialize quota time limits from the root dquot. */
 static void
 xfs_qm_init_timelimits(
-	struct xfs_mount	*mp,
-	struct xfs_quotainfo	*qinf)
+	struct xfs_mount	*mp)
 {
+	struct xfs_quotainfo	*qinf = mp->m_quotainfo;
 	struct xfs_disk_dquot	*ddqp;
 	struct xfs_dquot	*dqp;
 	uint			type;
@@ -671,14 +670,14 @@ xfs_qm_init_quotainfo(
 
 	mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_ALL_QUOTA_CHKD);
 
-	xfs_qm_init_timelimits(mp, qinf);
+	xfs_qm_init_timelimits(mp);
 
 	if (XFS_IS_UQUOTA_RUNNING(mp))
-		xfs_qm_set_defquota(mp, XFS_DQ_USER, qinf);
+		xfs_qm_set_defquota(mp, XFS_DQ_USER);
 	if (XFS_IS_GQUOTA_RUNNING(mp))
-		xfs_qm_set_defquota(mp, XFS_DQ_GROUP, qinf);
+		xfs_qm_set_defquota(mp, XFS_DQ_GROUP);
 	if (XFS_IS_PQUOTA_RUNNING(mp))
-		xfs_qm_set_defquota(mp, XFS_DQ_PROJ, qinf);
+		xfs_qm_set_defquota(mp, XFS_DQ_PROJ);
 
 	qinf->qi_shrinker.count_objects = xfs_qm_shrink_count;
 	qinf->qi_shrinker.scan_objects = xfs_qm_shrink_scan;
diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
index 3a850401b102..4cefe1abb1d4 100644
--- a/fs/xfs/xfs_qm.h
+++ b/fs/xfs/xfs_qm.h
@@ -164,9 +164,10 @@ extern int		xfs_qm_scall_quotaon(struct xfs_mount *, uint);
 extern int		xfs_qm_scall_quotaoff(struct xfs_mount *, uint);
 
 static inline struct xfs_def_quota *
-xfs_get_defquota(struct xfs_dquot *dqp, struct xfs_quotainfo *qi)
+xfs_get_defquota(struct xfs_dquot *dqp)
 {
 	struct xfs_def_quota *defq;
+	struct xfs_quotainfo *qi = dqp->q_mount->m_quotainfo;
 
 	if (XFS_QM_ISUDQ(dqp))
 		defq = &qi->qi_usr_default;
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 1ea82764bf89..e08c2f04f3ab 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -478,7 +478,7 @@ xfs_qm_scall_setqlim(
 		goto out_unlock;
 	}
 
-	defq = xfs_get_defquota(dqp, q);
+	defq = xfs_get_defquota(dqp);
 	xfs_dqunlock(dqp);
 
 	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_qm_setqlim, 0, 0, 0, &tp);
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index d1b9869bc5fa..7470b02c5198 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -585,13 +585,12 @@ xfs_trans_dqresv(
 	xfs_qwarncnt_t		warnlimit;
 	xfs_qcnt_t		total_count;
 	xfs_qcnt_t		*resbcountp;
-	struct xfs_quotainfo	*q = mp->m_quotainfo;
 	struct xfs_def_quota	*defq;
 
 
 	xfs_dqlock(dqp);
 
-	defq = xfs_get_defquota(dqp, q);
+	defq = xfs_get_defquota(dqp);
 
 	if (flags & XFS_TRANS_DQ_RES_BLKS) {
 		hardlimit = be64_to_cpu(dqp->q_core.d_blk_hardlimit);
-- 
2.17.0

[PATCH 3/4] xfs: pass xfs_dquot to xfs_qm_adjust_dqtimers

[Eric Sandeen]

Pass xfs_dquot rather than xfs_disk_dquot to xfs_qm_adjust_dqtimers;
this makes it symmetric with xfs_qm_adjust_dqlimits and will help
the next patch.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/xfs_dquot.c       | 3 ++-
 fs/xfs/xfs_dquot.h       | 2 +-
 fs/xfs/xfs_qm.c          | 2 +-
 fs/xfs/xfs_qm_syscalls.c | 2 +-
 fs/xfs/xfs_trans_dquot.c | 2 +-
 5 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index ddf41c24efcd..5c5fdb62f69c 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -113,8 +113,9 @@ xfs_qm_adjust_dqlimits(
 void
 xfs_qm_adjust_dqtimers(
 	struct xfs_mount	*mp,
-	struct xfs_disk_dquot	*d)
+	struct xfs_dquot	*dq)
 {
+	struct xfs_disk_dquot	*d = &dq->q_core;
 	ASSERT(d->d_id);
 
 #ifdef DEBUG
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index fe3e46df604b..71e36c85e20b 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -154,7 +154,7 @@ void		xfs_qm_dqdestroy(struct xfs_dquot *dqp);
 int		xfs_qm_dqflush(struct xfs_dquot *dqp, struct xfs_buf **bpp);
 void		xfs_qm_dqunpin_wait(struct xfs_dquot *dqp);
 void		xfs_qm_adjust_dqtimers(struct xfs_mount *mp,
-						struct xfs_disk_dquot *d);
+						struct xfs_dquot *d);
 void		xfs_qm_adjust_dqlimits(struct xfs_mount *mp,
 						struct xfs_dquot *d);
 xfs_dqid_t	xfs_qm_id_for_quotatype(struct xfs_inode *ip, uint type);
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index b3cd87d0bccb..4e543e2bc290 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1103,7 +1103,7 @@ xfs_qm_quotacheck_dqadjust(
 	 */
 	if (dqp->q_core.d_id) {
 		xfs_qm_adjust_dqlimits(mp, dqp);
-		xfs_qm_adjust_dqtimers(mp, &dqp->q_core);
+		xfs_qm_adjust_dqtimers(mp, dqp);
 	}
 
 	dqp->dq_flags |= XFS_DQ_DIRTY;
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index e08c2f04f3ab..ba79f355a14e 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -587,7 +587,7 @@ xfs_qm_scall_setqlim(
 		 * is on or off. We don't really want to bother with iterating
 		 * over all ondisk dquots and turning the timers on/off.
 		 */
-		xfs_qm_adjust_dqtimers(mp, ddq);
+		xfs_qm_adjust_dqtimers(mp, dqp);
 	}
 	dqp->dq_flags |= XFS_DQ_DIRTY;
 	xfs_trans_log_dquot(tp, dqp);
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index 7470b02c5198..7ae907ec7d47 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -388,7 +388,7 @@ xfs_trans_apply_dquot_deltas(
 			 */
 			if (d->d_id) {
 				xfs_qm_adjust_dqlimits(tp->t_mountp, dqp);
-				xfs_qm_adjust_dqtimers(tp->t_mountp, d);
+				xfs_qm_adjust_dqtimers(tp->t_mountp, dqp);
 			}
 
 			dqp->dq_flags |= XFS_DQ_DIRTY;
-- 
2.17.0

[PATCH 4/4] xfs: per-type quota timers and warn limits

[Eric Sandeen]

Move timers and warnings out of xfs_quotainfo and into xfs_def_quota
so that we can utilize them on a per-type basis, rather than enforcing
them based on the values found in the first enabled quota type.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/xfs_dquot.c       |  9 ++++--
 fs/xfs/xfs_qm.c          | 61 ++++++++++++++++++++++++----------------
 fs/xfs/xfs_qm.h          | 13 +++++----
 fs/xfs/xfs_qm_syscalls.c | 12 ++++----
 fs/xfs/xfs_quotaops.c    | 22 +++++++--------
 fs/xfs/xfs_trans_dquot.c |  6 ++--
 6 files changed, 69 insertions(+), 54 deletions(-)

diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 5c5fdb62f69c..4aff555cfab0 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -116,7 +116,10 @@ xfs_qm_adjust_dqtimers(
 	struct xfs_dquot	*dq)
 {
 	struct xfs_disk_dquot	*d = &dq->q_core;
+	struct xfs_def_quota	*defq;
+
 	ASSERT(d->d_id);
+	defq = xfs_get_defquota(dq);
 
 #ifdef DEBUG
 	if (d->d_blk_hardlimit)
@@ -138,7 +141,7 @@ xfs_qm_adjust_dqtimers(
 		     (be64_to_cpu(d->d_bcount) >
 		      be64_to_cpu(d->d_blk_hardlimit)))) {
 			d->d_btimer = cpu_to_be32(ktime_get_real_seconds() +
-					mp->m_quotainfo->qi_btimelimit);
+					defq->btimelimit);
 		} else {
 			d->d_bwarns = 0;
 		}
@@ -161,7 +164,7 @@ xfs_qm_adjust_dqtimers(
 		     (be64_to_cpu(d->d_icount) >
 		      be64_to_cpu(d->d_ino_hardlimit)))) {
 			d->d_itimer = cpu_to_be32(ktime_get_real_seconds() +
-					mp->m_quotainfo->qi_itimelimit);
+					defq->itimelimit);
 		} else {
 			d->d_iwarns = 0;
 		}
@@ -184,7 +187,7 @@ xfs_qm_adjust_dqtimers(
 		     (be64_to_cpu(d->d_rtbcount) >
 		      be64_to_cpu(d->d_rtb_hardlimit)))) {
 			d->d_rtbtimer = cpu_to_be32(ktime_get_real_seconds() +
-					mp->m_quotainfo->qi_rtbtimelimit);
+					defq->rtbtimelimit);
 		} else {
 			d->d_rtbwarns = 0;
 		}
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 4e543e2bc290..25003591ecb4 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -571,20 +571,37 @@ xfs_qm_set_defquota(
 /* Initialize quota time limits from the root dquot. */
 static void
 xfs_qm_init_timelimits(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	uint			type)
 {
 	struct xfs_quotainfo	*qinf = mp->m_quotainfo;
+	struct xfs_def_quota	*defq;
 	struct xfs_disk_dquot	*ddqp;
 	struct xfs_dquot	*dqp;
-	uint			type;
 	int			error;
 
-	qinf->qi_btimelimit = XFS_QM_BTIMELIMIT;
-	qinf->qi_itimelimit = XFS_QM_ITIMELIMIT;
-	qinf->qi_rtbtimelimit = XFS_QM_RTBTIMELIMIT;
-	qinf->qi_bwarnlimit = XFS_QM_BWARNLIMIT;
-	qinf->qi_iwarnlimit = XFS_QM_IWARNLIMIT;
-	qinf->qi_rtbwarnlimit = XFS_QM_RTBWARNLIMIT;
+
+	switch (type) {
+	case XFS_DQ_USER:
+		defq = &qinf->qi_usr_default;
+		break;
+	case XFS_DQ_GROUP:
+		defq = &qinf->qi_grp_default;
+		break;
+	case XFS_DQ_PROJ:
+		defq = &qinf->qi_prj_default;
+		break;
+	default:
+		ASSERT(0);
+		/* fall through */
+	}
+
+	defq->btimelimit = XFS_QM_BTIMELIMIT;
+	defq->itimelimit = XFS_QM_ITIMELIMIT;
+	defq->rtbtimelimit = XFS_QM_RTBTIMELIMIT;
+	defq->bwarnlimit = XFS_QM_BWARNLIMIT;
+	defq->iwarnlimit = XFS_QM_IWARNLIMIT;
+	defq->rtbwarnlimit = XFS_QM_RTBWARNLIMIT;
 
 	/*
 	 * We try to get the limits from the superuser's limits fields.
@@ -592,39 +609,31 @@ xfs_qm_init_timelimits(
 	 *
 	 * Since we may not have done a quotacheck by this point, just read
 	 * the dquot without attaching it to any hashtables or lists.
-	 *
-	 * Timers and warnings are globally set by the first timer found in
-	 * user/group/proj quota types, otherwise a default value is used.
-	 * This should be split into different fields per quota type.
 	 */
-	if (XFS_IS_UQUOTA_RUNNING(mp))
-		type = XFS_DQ_USER;
-	else if (XFS_IS_GQUOTA_RUNNING(mp))
-		type = XFS_DQ_GROUP;
-	else
-		type = XFS_DQ_PROJ;
 	error = xfs_qm_dqget_uncached(mp, 0, type, &dqp);
 	if (error)
 		return;
 
 	ddqp = &dqp->q_core;
+	defq = xfs_get_defquota(dqp);
+
 	/*
 	 * The warnings and timers set the grace period given to
 	 * a user or group before he or she can not perform any
 	 * more writing. If it is zero, a default is used.
 	 */
 	if (ddqp->d_btimer)
-		qinf->qi_btimelimit = be32_to_cpu(ddqp->d_btimer);
+		defq->btimelimit = be32_to_cpu(ddqp->d_btimer);
 	if (ddqp->d_itimer)
-		qinf->qi_itimelimit = be32_to_cpu(ddqp->d_itimer);
+		defq->itimelimit = be32_to_cpu(ddqp->d_itimer);
 	if (ddqp->d_rtbtimer)
-		qinf->qi_rtbtimelimit = be32_to_cpu(ddqp->d_rtbtimer);
+		defq->rtbtimelimit = be32_to_cpu(ddqp->d_rtbtimer);
 	if (ddqp->d_bwarns)
-		qinf->qi_bwarnlimit = be16_to_cpu(ddqp->d_bwarns);
+		defq->bwarnlimit = be16_to_cpu(ddqp->d_bwarns);
 	if (ddqp->d_iwarns)
-		qinf->qi_iwarnlimit = be16_to_cpu(ddqp->d_iwarns);
+		defq->iwarnlimit = be16_to_cpu(ddqp->d_iwarns);
 	if (ddqp->d_rtbwarns)
-		qinf->qi_rtbwarnlimit = be16_to_cpu(ddqp->d_rtbwarns);
+		defq->rtbwarnlimit = be16_to_cpu(ddqp->d_rtbwarns);
 
 	xfs_qm_dqdestroy(dqp);
 }
@@ -670,7 +679,9 @@ xfs_qm_init_quotainfo(
 
 	mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_ALL_QUOTA_CHKD);
 
-	xfs_qm_init_timelimits(mp);
+	xfs_qm_init_timelimits(mp, XFS_DQ_USER);
+	xfs_qm_init_timelimits(mp, XFS_DQ_GROUP);
+	xfs_qm_init_timelimits(mp, XFS_DQ_PROJ);
 
 	if (XFS_IS_UQUOTA_RUNNING(mp))
 		xfs_qm_set_defquota(mp, XFS_DQ_USER);
diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
index 4cefe1abb1d4..9866e9f4edfd 100644
--- a/fs/xfs/xfs_qm.h
+++ b/fs/xfs/xfs_qm.h
@@ -41,7 +41,14 @@ extern struct kmem_zone	*xfs_qm_dqtrxzone;
  */
 #define XFS_DQUOT_CLUSTER_SIZE_FSB	(xfs_filblks_t)1
 
+/* Defaults for each quota type: time limits, warn limits, usage limits */
 struct xfs_def_quota {
+	time64_t	btimelimit;	/* limit for blks timer */
+	time64_t	itimelimit;	/* limit for inodes timer */
+	time64_t	rtbtimelimit;	/* limit for rt blks timer */
+	xfs_qwarncnt_t	bwarnlimit;	/* limit for blks warnings */
+	xfs_qwarncnt_t	iwarnlimit;	/* limit for inodes warnings */
+	xfs_qwarncnt_t	rtbwarnlimit;	/* limit for rt blks warnings */
 	xfs_qcnt_t	bhardlimit;	/* default data blk hard limit */
 	xfs_qcnt_t	bsoftlimit;	/* default data blk soft limit */
 	xfs_qcnt_t	ihardlimit;	/* default inode count hard limit */
@@ -64,12 +71,6 @@ struct xfs_quotainfo {
 	struct xfs_inode	*qi_pquotaip;	/* project quota inode */
 	struct list_lru		qi_lru;
 	int			qi_dquots;
-	time64_t		qi_btimelimit;	/* limit for blks timer */
-	time64_t		qi_itimelimit;	/* limit for inodes timer */
-	time64_t		qi_rtbtimelimit;/* limit for rt blks timer */
-	xfs_qwarncnt_t		qi_bwarnlimit;	/* limit for blks warnings */
-	xfs_qwarncnt_t		qi_iwarnlimit;	/* limit for inodes warnings */
-	xfs_qwarncnt_t		qi_rtbwarnlimit;/* limit for rt blks warnings */
 	struct mutex		qi_quotaofflock;/* to serialize quotaoff */
 	xfs_filblks_t		qi_dqchunklen;	/* # BBs in a chunk of dqs */
 	uint			qi_dqperchunk;	/* # ondisk dq in above chunk */
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index ba79f355a14e..063850003c97 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -562,23 +562,23 @@ xfs_qm_scall_setqlim(
 		 * for warnings.
 		 */
 		if (newlim->d_fieldmask & QC_SPC_TIMER) {
-			q->qi_btimelimit = newlim->d_spc_timer;
+			defq->btimelimit = newlim->d_spc_timer;
 			ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer);
 		}
 		if (newlim->d_fieldmask & QC_INO_TIMER) {
-			q->qi_itimelimit = newlim->d_ino_timer;
+			defq->itimelimit = newlim->d_ino_timer;
 			ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer);
 		}
 		if (newlim->d_fieldmask & QC_RT_SPC_TIMER) {
-			q->qi_rtbtimelimit = newlim->d_rt_spc_timer;
+			defq->rtbtimelimit = newlim->d_rt_spc_timer;
 			ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer);
 		}
 		if (newlim->d_fieldmask & QC_SPC_WARNS)
-			q->qi_bwarnlimit = newlim->d_spc_warns;
+			defq->bwarnlimit = newlim->d_spc_warns;
 		if (newlim->d_fieldmask & QC_INO_WARNS)
-			q->qi_iwarnlimit = newlim->d_ino_warns;
+			defq->iwarnlimit = newlim->d_ino_warns;
 		if (newlim->d_fieldmask & QC_RT_SPC_WARNS)
-			q->qi_rtbwarnlimit = newlim->d_rt_spc_warns;
+			defq->rtbwarnlimit = newlim->d_rt_spc_warns;
 	} else {
 		/*
 		 * If the user is now over quota, start the timelimit.
diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c
index cb16a91dd1d4..51be282d28b3 100644
--- a/fs/xfs/xfs_quotaops.c
+++ b/fs/xfs/xfs_quotaops.c
@@ -21,9 +21,9 @@ xfs_qm_fill_state(
 	struct qc_type_state	*tstate,
 	struct xfs_mount	*mp,
 	struct xfs_inode	*ip,
-	xfs_ino_t		ino)
+	xfs_ino_t		ino,
+	struct xfs_def_quota	*defq)
 {
-	struct xfs_quotainfo	*q = mp->m_quotainfo;
 	bool			tempqip = false;
 
 	tstate->ino = ino;
@@ -37,12 +37,12 @@ xfs_qm_fill_state(
 	tstate->flags |= QCI_SYSFILE;
 	tstate->blocks = ip->i_d.di_nblocks;
 	tstate->nextents = ip->i_d.di_nextents;
-	tstate->spc_timelimit = (u32)q->qi_btimelimit;
-	tstate->ino_timelimit = (u32)q->qi_itimelimit;
-	tstate->rt_spc_timelimit = (u32)q->qi_rtbtimelimit;
-	tstate->spc_warnlimit = q->qi_bwarnlimit;
-	tstate->ino_warnlimit = q->qi_iwarnlimit;
-	tstate->rt_spc_warnlimit = q->qi_rtbwarnlimit;
+	tstate->spc_timelimit = (u32)defq->btimelimit;
+	tstate->ino_timelimit = (u32)defq->itimelimit;
+	tstate->rt_spc_timelimit = (u32)defq->rtbtimelimit;
+	tstate->spc_warnlimit = defq->bwarnlimit;
+	tstate->ino_warnlimit = defq->iwarnlimit;
+	tstate->rt_spc_warnlimit = defq->rtbwarnlimit;
 	if (tempqip)
 		xfs_irele(ip);
 }
@@ -77,11 +77,11 @@ xfs_fs_get_quota_state(
 		state->s_state[PRJQUOTA].flags |= QCI_LIMITS_ENFORCED;
 
 	xfs_qm_fill_state(&state->s_state[USRQUOTA], mp, q->qi_uquotaip,
-			  mp->m_sb.sb_uquotino);
+			  mp->m_sb.sb_uquotino, &q->qi_usr_default);
 	xfs_qm_fill_state(&state->s_state[GRPQUOTA], mp, q->qi_gquotaip,
-			  mp->m_sb.sb_gquotino);
+			  mp->m_sb.sb_gquotino, &q->qi_grp_default);
 	xfs_qm_fill_state(&state->s_state[PRJQUOTA], mp, q->qi_pquotaip,
-			  mp->m_sb.sb_pquotino);
+			  mp->m_sb.sb_pquotino, &q->qi_prj_default);
 	return 0;
 }
 
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index 7ae907ec7d47..e4eca607e512 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -601,7 +601,7 @@ xfs_trans_dqresv(
 			softlimit = defq->bsoftlimit;
 		timer = be32_to_cpu(dqp->q_core.d_btimer);
 		warns = be16_to_cpu(dqp->q_core.d_bwarns);
-		warnlimit = dqp->q_mount->m_quotainfo->qi_bwarnlimit;
+		warnlimit = defq->bwarnlimit;
 		resbcountp = &dqp->q_res_bcount;
 	} else {
 		ASSERT(flags & XFS_TRANS_DQ_RES_RTBLKS);
@@ -613,7 +613,7 @@ xfs_trans_dqresv(
 			softlimit = defq->rtbsoftlimit;
 		timer = be32_to_cpu(dqp->q_core.d_rtbtimer);
 		warns = be16_to_cpu(dqp->q_core.d_rtbwarns);
-		warnlimit = dqp->q_mount->m_quotainfo->qi_rtbwarnlimit;
+		warnlimit = defq->rtbwarnlimit;
 		resbcountp = &dqp->q_res_rtbcount;
 	}
 
@@ -649,7 +649,7 @@ xfs_trans_dqresv(
 			total_count = be64_to_cpu(dqp->q_core.d_icount) + ninos;
 			timer = be32_to_cpu(dqp->q_core.d_itimer);
 			warns = be16_to_cpu(dqp->q_core.d_iwarns);
-			warnlimit = dqp->q_mount->m_quotainfo->qi_iwarnlimit;
+			warnlimit = defq->iwarnlimit;
 			hardlimit = be64_to_cpu(dqp->q_core.d_ino_hardlimit);
 			if (!hardlimit)
 				hardlimit = defq->ihardlimit;
-- 
2.17.0

Re: [PATCH 0/4] xfs: enable per-type quota timers and warn limits

[Eric Sandeen]

<it would be fair to ask me for an xfstest for this> ;)

I'll try to get to that next week, but figured I'd float the series for review.

Thanks,
-Eric

Re: [PATCH 1/4] xfs: fix up some whitespace in quota code

[Allison Collins]

On 2/8/20 2:10 PM, Eric Sandeen wrote:
> There is a fair bit of whitespace damage in the quota code, so
> fix up enough of it that subsequent patches are restricted to
> functional change to aid review.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Ok, looks like a good clean up
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   fs/xfs/xfs_dquot.c    | 16 ++++++++--------
>   fs/xfs/xfs_qm.h       | 44 +++++++++++++++++++++----------------------
>   fs/xfs/xfs_quotaops.c |  8 ++++----
>   3 files changed, 34 insertions(+), 34 deletions(-)
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index d223e1ae90a6..02f433d1f13a 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -205,16 +205,16 @@ xfs_qm_adjust_dqtimers(
>    */
>   STATIC void
>   xfs_qm_init_dquot_blk(
> -	xfs_trans_t	*tp,
> -	xfs_mount_t	*mp,
> -	xfs_dqid_t	id,
> -	uint		type,
> -	xfs_buf_t	*bp)
> +	struct xfs_trans	*tp,
> +	struct xfs_mount	*mp,
> +	xfs_dqid_t		id,
> +	uint			type,
> +	struct xfs_buf		*bp)
>   {
>   	struct xfs_quotainfo	*q = mp->m_quotainfo;
> -	xfs_dqblk_t	*d;
> -	xfs_dqid_t	curid;
> -	int		i;
> +	xfs_dqblk_t		*d;
> +	xfs_dqid_t		curid;
> +	int			i;
>   
>   	ASSERT(tp);
>   	ASSERT(xfs_buf_islocked(bp));
> diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
> index 4e57edca8bce..3a850401b102 100644
> --- a/fs/xfs/xfs_qm.h
> +++ b/fs/xfs/xfs_qm.h
> @@ -42,12 +42,12 @@ extern struct kmem_zone	*xfs_qm_dqtrxzone;
>   #define XFS_DQUOT_CLUSTER_SIZE_FSB	(xfs_filblks_t)1
>   
>   struct xfs_def_quota {
> -	xfs_qcnt_t       bhardlimit;     /* default data blk hard limit */
> -	xfs_qcnt_t       bsoftlimit;	 /* default data blk soft limit */
> -	xfs_qcnt_t       ihardlimit;	 /* default inode count hard limit */
> -	xfs_qcnt_t       isoftlimit;	 /* default inode count soft limit */
> -	xfs_qcnt_t	 rtbhardlimit;   /* default realtime blk hard limit */
> -	xfs_qcnt_t	 rtbsoftlimit;   /* default realtime blk soft limit */
> +	xfs_qcnt_t	bhardlimit;	/* default data blk hard limit */
> +	xfs_qcnt_t	bsoftlimit;	/* default data blk soft limit */
> +	xfs_qcnt_t	ihardlimit;	/* default inode count hard limit */
> +	xfs_qcnt_t	isoftlimit;	/* default inode count soft limit */
> +	xfs_qcnt_t	rtbhardlimit;	/* default realtime blk hard limit */
> +	xfs_qcnt_t	rtbsoftlimit;	/* default realtime blk soft limit */
>   };
>   
>   /*
> @@ -55,28 +55,28 @@ struct xfs_def_quota {
>    * The mount structure keeps a pointer to this.
>    */
>   struct xfs_quotainfo {
> -	struct radix_tree_root qi_uquota_tree;
> -	struct radix_tree_root qi_gquota_tree;
> -	struct radix_tree_root qi_pquota_tree;
> -	struct mutex qi_tree_lock;
> +	struct radix_tree_root	qi_uquota_tree;
> +	struct radix_tree_root	qi_gquota_tree;
> +	struct radix_tree_root	qi_pquota_tree;
> +	struct mutex		qi_tree_lock;
>   	struct xfs_inode	*qi_uquotaip;	/* user quota inode */
>   	struct xfs_inode	*qi_gquotaip;	/* group quota inode */
>   	struct xfs_inode	*qi_pquotaip;	/* project quota inode */
> -	struct list_lru	 qi_lru;
> -	int		 qi_dquots;
> -	time64_t	 qi_btimelimit;	 /* limit for blks timer */
> -	time64_t	 qi_itimelimit;	 /* limit for inodes timer */
> -	time64_t	 qi_rtbtimelimit;/* limit for rt blks timer */
> -	xfs_qwarncnt_t	 qi_bwarnlimit;	 /* limit for blks warnings */
> -	xfs_qwarncnt_t	 qi_iwarnlimit;	 /* limit for inodes warnings */
> -	xfs_qwarncnt_t	 qi_rtbwarnlimit;/* limit for rt blks warnings */
> -	struct mutex	 qi_quotaofflock;/* to serialize quotaoff */
> -	xfs_filblks_t	 qi_dqchunklen;	 /* # BBs in a chunk of dqs */
> -	uint		 qi_dqperchunk;	 /* # ondisk dqs in above chunk */
> +	struct list_lru		qi_lru;
> +	int			qi_dquots;
> +	time64_t		qi_btimelimit;	/* limit for blks timer */
> +	time64_t		qi_itimelimit;	/* limit for inodes timer */
> +	time64_t		qi_rtbtimelimit;/* limit for rt blks timer */
> +	xfs_qwarncnt_t		qi_bwarnlimit;	/* limit for blks warnings */
> +	xfs_qwarncnt_t		qi_iwarnlimit;	/* limit for inodes warnings */
> +	xfs_qwarncnt_t		qi_rtbwarnlimit;/* limit for rt blks warnings */
> +	struct mutex		qi_quotaofflock;/* to serialize quotaoff */
> +	xfs_filblks_t		qi_dqchunklen;	/* # BBs in a chunk of dqs */
> +	uint			qi_dqperchunk;	/* # ondisk dq in above chunk */
>   	struct xfs_def_quota	qi_usr_default;
>   	struct xfs_def_quota	qi_grp_default;
>   	struct xfs_def_quota	qi_prj_default;
> -	struct shrinker	qi_shrinker;
> +	struct shrinker		qi_shrinker;
>   };
>   
>   static inline struct radix_tree_root *
> diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c
> index 38669e827206..cb16a91dd1d4 100644
> --- a/fs/xfs/xfs_quotaops.c
> +++ b/fs/xfs/xfs_quotaops.c
> @@ -23,8 +23,8 @@ xfs_qm_fill_state(
>   	struct xfs_inode	*ip,
>   	xfs_ino_t		ino)
>   {
> -	struct xfs_quotainfo *q = mp->m_quotainfo;
> -	bool tempqip = false;
> +	struct xfs_quotainfo	*q = mp->m_quotainfo;
> +	bool			tempqip = false;
>   
>   	tstate->ino = ino;
>   	if (!ip && ino == NULLFSINO)
> @@ -109,8 +109,8 @@ xfs_fs_set_info(
>   	int			type,
>   	struct qc_info		*info)
>   {
> -	struct xfs_mount *mp = XFS_M(sb);
> -	struct qc_dqblk newlim;
> +	struct xfs_mount	*mp = XFS_M(sb);
> +	struct qc_dqblk		newlim;
>   
>   	if (sb_rdonly(sb))
>   		return -EROFS;
> 

Re: [PATCH 2/4] xfs: simplify args to xfs_get_defquota

[Allison Collins]

On 2/8/20 2:11 PM, Eric Sandeen wrote:
> There's no real reason to pass both xfs_dquot and xfs_quotainfo to
> xfs_get_defquota, because the latter can be obtained from the former.
> This simplifies a bit more of the argument passing.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks ok to me
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   fs/xfs/xfs_dquot.c       |  3 +--
>   fs/xfs/xfs_qm.c          | 17 ++++++++---------
>   fs/xfs/xfs_qm.h          |  3 ++-
>   fs/xfs/xfs_qm_syscalls.c |  2 +-
>   fs/xfs/xfs_trans_dquot.c |  3 +--
>   5 files changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 02f433d1f13a..ddf41c24efcd 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -69,13 +69,12 @@ xfs_qm_adjust_dqlimits(
>   	struct xfs_mount	*mp,
>   	struct xfs_dquot	*dq)
>   {
> -	struct xfs_quotainfo	*q = mp->m_quotainfo;
>   	struct xfs_disk_dquot	*d = &dq->q_core;
>   	struct xfs_def_quota	*defq;
>   	int			prealloc = 0;
>   
>   	ASSERT(d->d_id);
> -	defq = xfs_get_defquota(dq, q);
> +	defq = xfs_get_defquota(dq);
>   
>   	if (defq->bsoftlimit && !d->d_blk_softlimit) {
>   		d->d_blk_softlimit = cpu_to_be64(defq->bsoftlimit);
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 0b0909657bad..b3cd87d0bccb 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -541,8 +541,7 @@ xfs_qm_shrink_count(
>   STATIC void
>   xfs_qm_set_defquota(
>   	struct xfs_mount	*mp,
> -	uint			type,
> -	struct xfs_quotainfo	*qinf)
> +	uint			type)
>   {
>   	struct xfs_dquot	*dqp;
>   	struct xfs_def_quota	*defq;
> @@ -554,7 +553,7 @@ xfs_qm_set_defquota(
>   		return;
>   
>   	ddqp = &dqp->q_core;
> -	defq = xfs_get_defquota(dqp, qinf);
> +	defq = xfs_get_defquota(dqp);
>   
>   	/*
>   	 * Timers and warnings have been already set, let's just set the
> @@ -572,9 +571,9 @@ xfs_qm_set_defquota(
>   /* Initialize quota time limits from the root dquot. */
>   static void
>   xfs_qm_init_timelimits(
> -	struct xfs_mount	*mp,
> -	struct xfs_quotainfo	*qinf)
> +	struct xfs_mount	*mp)
>   {
> +	struct xfs_quotainfo	*qinf = mp->m_quotainfo;
>   	struct xfs_disk_dquot	*ddqp;
>   	struct xfs_dquot	*dqp;
>   	uint			type;
> @@ -671,14 +670,14 @@ xfs_qm_init_quotainfo(
>   
>   	mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_ALL_QUOTA_CHKD);
>   
> -	xfs_qm_init_timelimits(mp, qinf);
> +	xfs_qm_init_timelimits(mp);
>   
>   	if (XFS_IS_UQUOTA_RUNNING(mp))
> -		xfs_qm_set_defquota(mp, XFS_DQ_USER, qinf);
> +		xfs_qm_set_defquota(mp, XFS_DQ_USER);
>   	if (XFS_IS_GQUOTA_RUNNING(mp))
> -		xfs_qm_set_defquota(mp, XFS_DQ_GROUP, qinf);
> +		xfs_qm_set_defquota(mp, XFS_DQ_GROUP);
>   	if (XFS_IS_PQUOTA_RUNNING(mp))
> -		xfs_qm_set_defquota(mp, XFS_DQ_PROJ, qinf);
> +		xfs_qm_set_defquota(mp, XFS_DQ_PROJ);
>   
>   	qinf->qi_shrinker.count_objects = xfs_qm_shrink_count;
>   	qinf->qi_shrinker.scan_objects = xfs_qm_shrink_scan;
> diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
> index 3a850401b102..4cefe1abb1d4 100644
> --- a/fs/xfs/xfs_qm.h
> +++ b/fs/xfs/xfs_qm.h
> @@ -164,9 +164,10 @@ extern int		xfs_qm_scall_quotaon(struct xfs_mount *, uint);
>   extern int		xfs_qm_scall_quotaoff(struct xfs_mount *, uint);
>   
>   static inline struct xfs_def_quota *
> -xfs_get_defquota(struct xfs_dquot *dqp, struct xfs_quotainfo *qi)
> +xfs_get_defquota(struct xfs_dquot *dqp)
>   {
>   	struct xfs_def_quota *defq;
> +	struct xfs_quotainfo *qi = dqp->q_mount->m_quotainfo;
>   
>   	if (XFS_QM_ISUDQ(dqp))
>   		defq = &qi->qi_usr_default;
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index 1ea82764bf89..e08c2f04f3ab 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -478,7 +478,7 @@ xfs_qm_scall_setqlim(
>   		goto out_unlock;
>   	}
>   
> -	defq = xfs_get_defquota(dqp, q);
> +	defq = xfs_get_defquota(dqp);
>   	xfs_dqunlock(dqp);
>   
>   	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_qm_setqlim, 0, 0, 0, &tp);
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index d1b9869bc5fa..7470b02c5198 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -585,13 +585,12 @@ xfs_trans_dqresv(
>   	xfs_qwarncnt_t		warnlimit;
>   	xfs_qcnt_t		total_count;
>   	xfs_qcnt_t		*resbcountp;
> -	struct xfs_quotainfo	*q = mp->m_quotainfo;
>   	struct xfs_def_quota	*defq;
>   
>   
>   	xfs_dqlock(dqp);
>   
> -	defq = xfs_get_defquota(dqp, q);
> +	defq = xfs_get_defquota(dqp);
>   
>   	if (flags & XFS_TRANS_DQ_RES_BLKS) {
>   		hardlimit = be64_to_cpu(dqp->q_core.d_blk_hardlimit);
> 

Re: [PATCH 3/4] xfs: pass xfs_dquot to xfs_qm_adjust_dqtimers

[Allison Collins]

On 2/8/20 2:11 PM, Eric Sandeen wrote:
> Pass xfs_dquot rather than xfs_disk_dquot to xfs_qm_adjust_dqtimers;
> this makes it symmetric with xfs_qm_adjust_dqlimits and will help
> the next patch.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks fine
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   fs/xfs/xfs_dquot.c       | 3 ++-
>   fs/xfs/xfs_dquot.h       | 2 +-
>   fs/xfs/xfs_qm.c          | 2 +-
>   fs/xfs/xfs_qm_syscalls.c | 2 +-
>   fs/xfs/xfs_trans_dquot.c | 2 +-
>   5 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index ddf41c24efcd..5c5fdb62f69c 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -113,8 +113,9 @@ xfs_qm_adjust_dqlimits(
>   void
>   xfs_qm_adjust_dqtimers(
>   	struct xfs_mount	*mp,
> -	struct xfs_disk_dquot	*d)
> +	struct xfs_dquot	*dq)
>   {
> +	struct xfs_disk_dquot	*d = &dq->q_core;
>   	ASSERT(d->d_id);
>   
>   #ifdef DEBUG
> diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
> index fe3e46df604b..71e36c85e20b 100644
> --- a/fs/xfs/xfs_dquot.h
> +++ b/fs/xfs/xfs_dquot.h
> @@ -154,7 +154,7 @@ void		xfs_qm_dqdestroy(struct xfs_dquot *dqp);
>   int		xfs_qm_dqflush(struct xfs_dquot *dqp, struct xfs_buf **bpp);
>   void		xfs_qm_dqunpin_wait(struct xfs_dquot *dqp);
>   void		xfs_qm_adjust_dqtimers(struct xfs_mount *mp,
> -						struct xfs_disk_dquot *d);
> +						struct xfs_dquot *d);
>   void		xfs_qm_adjust_dqlimits(struct xfs_mount *mp,
>   						struct xfs_dquot *d);
>   xfs_dqid_t	xfs_qm_id_for_quotatype(struct xfs_inode *ip, uint type);
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index b3cd87d0bccb..4e543e2bc290 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -1103,7 +1103,7 @@ xfs_qm_quotacheck_dqadjust(
>   	 */
>   	if (dqp->q_core.d_id) {
>   		xfs_qm_adjust_dqlimits(mp, dqp);
> -		xfs_qm_adjust_dqtimers(mp, &dqp->q_core);
> +		xfs_qm_adjust_dqtimers(mp, dqp);
>   	}
>   
>   	dqp->dq_flags |= XFS_DQ_DIRTY;
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index e08c2f04f3ab..ba79f355a14e 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -587,7 +587,7 @@ xfs_qm_scall_setqlim(
>   		 * is on or off. We don't really want to bother with iterating
>   		 * over all ondisk dquots and turning the timers on/off.
>   		 */
> -		xfs_qm_adjust_dqtimers(mp, ddq);
> +		xfs_qm_adjust_dqtimers(mp, dqp);
>   	}
>   	dqp->dq_flags |= XFS_DQ_DIRTY;
>   	xfs_trans_log_dquot(tp, dqp);
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index 7470b02c5198..7ae907ec7d47 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -388,7 +388,7 @@ xfs_trans_apply_dquot_deltas(
>   			 */
>   			if (d->d_id) {
>   				xfs_qm_adjust_dqlimits(tp->t_mountp, dqp);
> -				xfs_qm_adjust_dqtimers(tp->t_mountp, d);
> +				xfs_qm_adjust_dqtimers(tp->t_mountp, dqp);
>   			}
>   
>   			dqp->dq_flags |= XFS_DQ_DIRTY;
> 

Re: [PATCH 4/4] xfs: per-type quota timers and warn limits

[Allison Collins]

On 2/8/20 2:12 PM, Eric Sandeen wrote:
> Move timers and warnings out of xfs_quotainfo and into xfs_def_quota
> so that we can utilize them on a per-type basis, rather than enforcing
> them based on the values found in the first enabled quota type.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Ok I think it makes sense
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   fs/xfs/xfs_dquot.c       |  9 ++++--
>   fs/xfs/xfs_qm.c          | 61 ++++++++++++++++++++++++----------------
>   fs/xfs/xfs_qm.h          | 13 +++++----
>   fs/xfs/xfs_qm_syscalls.c | 12 ++++----
>   fs/xfs/xfs_quotaops.c    | 22 +++++++--------
>   fs/xfs/xfs_trans_dquot.c |  6 ++--
>   6 files changed, 69 insertions(+), 54 deletions(-)
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 5c5fdb62f69c..4aff555cfab0 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -116,7 +116,10 @@ xfs_qm_adjust_dqtimers(
>   	struct xfs_dquot	*dq)
>   {
>   	struct xfs_disk_dquot	*d = &dq->q_core;
> +	struct xfs_def_quota	*defq;
> +
>   	ASSERT(d->d_id);
> +	defq = xfs_get_defquota(dq);
>   
>   #ifdef DEBUG
>   	if (d->d_blk_hardlimit)
> @@ -138,7 +141,7 @@ xfs_qm_adjust_dqtimers(
>   		     (be64_to_cpu(d->d_bcount) >
>   		      be64_to_cpu(d->d_blk_hardlimit)))) {
>   			d->d_btimer = cpu_to_be32(ktime_get_real_seconds() +
> -					mp->m_quotainfo->qi_btimelimit);
> +					defq->btimelimit);
>   		} else {
>   			d->d_bwarns = 0;
>   		}
> @@ -161,7 +164,7 @@ xfs_qm_adjust_dqtimers(
>   		     (be64_to_cpu(d->d_icount) >
>   		      be64_to_cpu(d->d_ino_hardlimit)))) {
>   			d->d_itimer = cpu_to_be32(ktime_get_real_seconds() +
> -					mp->m_quotainfo->qi_itimelimit);
> +					defq->itimelimit);
>   		} else {
>   			d->d_iwarns = 0;
>   		}
> @@ -184,7 +187,7 @@ xfs_qm_adjust_dqtimers(
>   		     (be64_to_cpu(d->d_rtbcount) >
>   		      be64_to_cpu(d->d_rtb_hardlimit)))) {
>   			d->d_rtbtimer = cpu_to_be32(ktime_get_real_seconds() +
> -					mp->m_quotainfo->qi_rtbtimelimit);
> +					defq->rtbtimelimit);
>   		} else {
>   			d->d_rtbwarns = 0;
>   		}
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 4e543e2bc290..25003591ecb4 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -571,20 +571,37 @@ xfs_qm_set_defquota(
>   /* Initialize quota time limits from the root dquot. */
>   static void
>   xfs_qm_init_timelimits(
> -	struct xfs_mount	*mp)
> +	struct xfs_mount	*mp,
> +	uint			type)
>   {
>   	struct xfs_quotainfo	*qinf = mp->m_quotainfo;
> +	struct xfs_def_quota	*defq;
>   	struct xfs_disk_dquot	*ddqp;
>   	struct xfs_dquot	*dqp;
> -	uint			type;
>   	int			error;
>   
> -	qinf->qi_btimelimit = XFS_QM_BTIMELIMIT;
> -	qinf->qi_itimelimit = XFS_QM_ITIMELIMIT;
> -	qinf->qi_rtbtimelimit = XFS_QM_RTBTIMELIMIT;
> -	qinf->qi_bwarnlimit = XFS_QM_BWARNLIMIT;
> -	qinf->qi_iwarnlimit = XFS_QM_IWARNLIMIT;
> -	qinf->qi_rtbwarnlimit = XFS_QM_RTBWARNLIMIT;
> +
> +	switch (type) {
> +	case XFS_DQ_USER:
> +		defq = &qinf->qi_usr_default;
> +		break;
> +	case XFS_DQ_GROUP:
> +		defq = &qinf->qi_grp_default;
> +		break;
> +	case XFS_DQ_PROJ:
> +		defq = &qinf->qi_prj_default;
> +		break;
> +	default:
> +		ASSERT(0);
> +		/* fall through */
> +	}
> +
> +	defq->btimelimit = XFS_QM_BTIMELIMIT;
> +	defq->itimelimit = XFS_QM_ITIMELIMIT;
> +	defq->rtbtimelimit = XFS_QM_RTBTIMELIMIT;
> +	defq->bwarnlimit = XFS_QM_BWARNLIMIT;
> +	defq->iwarnlimit = XFS_QM_IWARNLIMIT;
> +	defq->rtbwarnlimit = XFS_QM_RTBWARNLIMIT;
>   
>   	/*
>   	 * We try to get the limits from the superuser's limits fields.
> @@ -592,39 +609,31 @@ xfs_qm_init_timelimits(
>   	 *
>   	 * Since we may not have done a quotacheck by this point, just read
>   	 * the dquot without attaching it to any hashtables or lists.
> -	 *
> -	 * Timers and warnings are globally set by the first timer found in
> -	 * user/group/proj quota types, otherwise a default value is used.
> -	 * This should be split into different fields per quota type.
>   	 */
> -	if (XFS_IS_UQUOTA_RUNNING(mp))
> -		type = XFS_DQ_USER;
> -	else if (XFS_IS_GQUOTA_RUNNING(mp))
> -		type = XFS_DQ_GROUP;
> -	else
> -		type = XFS_DQ_PROJ;
>   	error = xfs_qm_dqget_uncached(mp, 0, type, &dqp);
>   	if (error)
>   		return;
>   
>   	ddqp = &dqp->q_core;
> +	defq = xfs_get_defquota(dqp);
> +
>   	/*
>   	 * The warnings and timers set the grace period given to
>   	 * a user or group before he or she can not perform any
>   	 * more writing. If it is zero, a default is used.
>   	 */
>   	if (ddqp->d_btimer)
> -		qinf->qi_btimelimit = be32_to_cpu(ddqp->d_btimer);
> +		defq->btimelimit = be32_to_cpu(ddqp->d_btimer);
>   	if (ddqp->d_itimer)
> -		qinf->qi_itimelimit = be32_to_cpu(ddqp->d_itimer);
> +		defq->itimelimit = be32_to_cpu(ddqp->d_itimer);
>   	if (ddqp->d_rtbtimer)
> -		qinf->qi_rtbtimelimit = be32_to_cpu(ddqp->d_rtbtimer);
> +		defq->rtbtimelimit = be32_to_cpu(ddqp->d_rtbtimer);
>   	if (ddqp->d_bwarns)
> -		qinf->qi_bwarnlimit = be16_to_cpu(ddqp->d_bwarns);
> +		defq->bwarnlimit = be16_to_cpu(ddqp->d_bwarns);
>   	if (ddqp->d_iwarns)
> -		qinf->qi_iwarnlimit = be16_to_cpu(ddqp->d_iwarns);
> +		defq->iwarnlimit = be16_to_cpu(ddqp->d_iwarns);
>   	if (ddqp->d_rtbwarns)
> -		qinf->qi_rtbwarnlimit = be16_to_cpu(ddqp->d_rtbwarns);
> +		defq->rtbwarnlimit = be16_to_cpu(ddqp->d_rtbwarns);
>   
>   	xfs_qm_dqdestroy(dqp);
>   }
> @@ -670,7 +679,9 @@ xfs_qm_init_quotainfo(
>   
>   	mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_ALL_QUOTA_CHKD);
>   
> -	xfs_qm_init_timelimits(mp);
> +	xfs_qm_init_timelimits(mp, XFS_DQ_USER);
> +	xfs_qm_init_timelimits(mp, XFS_DQ_GROUP);
> +	xfs_qm_init_timelimits(mp, XFS_DQ_PROJ);
>   
>   	if (XFS_IS_UQUOTA_RUNNING(mp))
>   		xfs_qm_set_defquota(mp, XFS_DQ_USER);
> diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
> index 4cefe1abb1d4..9866e9f4edfd 100644
> --- a/fs/xfs/xfs_qm.h
> +++ b/fs/xfs/xfs_qm.h
> @@ -41,7 +41,14 @@ extern struct kmem_zone	*xfs_qm_dqtrxzone;
>    */
>   #define XFS_DQUOT_CLUSTER_SIZE_FSB	(xfs_filblks_t)1
>   
> +/* Defaults for each quota type: time limits, warn limits, usage limits */
>   struct xfs_def_quota {
> +	time64_t	btimelimit;	/* limit for blks timer */
> +	time64_t	itimelimit;	/* limit for inodes timer */
> +	time64_t	rtbtimelimit;	/* limit for rt blks timer */
> +	xfs_qwarncnt_t	bwarnlimit;	/* limit for blks warnings */
> +	xfs_qwarncnt_t	iwarnlimit;	/* limit for inodes warnings */
> +	xfs_qwarncnt_t	rtbwarnlimit;	/* limit for rt blks warnings */
>   	xfs_qcnt_t	bhardlimit;	/* default data blk hard limit */
>   	xfs_qcnt_t	bsoftlimit;	/* default data blk soft limit */
>   	xfs_qcnt_t	ihardlimit;	/* default inode count hard limit */
> @@ -64,12 +71,6 @@ struct xfs_quotainfo {
>   	struct xfs_inode	*qi_pquotaip;	/* project quota inode */
>   	struct list_lru		qi_lru;
>   	int			qi_dquots;
> -	time64_t		qi_btimelimit;	/* limit for blks timer */
> -	time64_t		qi_itimelimit;	/* limit for inodes timer */
> -	time64_t		qi_rtbtimelimit;/* limit for rt blks timer */
> -	xfs_qwarncnt_t		qi_bwarnlimit;	/* limit for blks warnings */
> -	xfs_qwarncnt_t		qi_iwarnlimit;	/* limit for inodes warnings */
> -	xfs_qwarncnt_t		qi_rtbwarnlimit;/* limit for rt blks warnings */
>   	struct mutex		qi_quotaofflock;/* to serialize quotaoff */
>   	xfs_filblks_t		qi_dqchunklen;	/* # BBs in a chunk of dqs */
>   	uint			qi_dqperchunk;	/* # ondisk dq in above chunk */
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index ba79f355a14e..063850003c97 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -562,23 +562,23 @@ xfs_qm_scall_setqlim(
>   		 * for warnings.
>   		 */
>   		if (newlim->d_fieldmask & QC_SPC_TIMER) {
> -			q->qi_btimelimit = newlim->d_spc_timer;
> +			defq->btimelimit = newlim->d_spc_timer;
>   			ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer);
>   		}
>   		if (newlim->d_fieldmask & QC_INO_TIMER) {
> -			q->qi_itimelimit = newlim->d_ino_timer;
> +			defq->itimelimit = newlim->d_ino_timer;
>   			ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer);
>   		}
>   		if (newlim->d_fieldmask & QC_RT_SPC_TIMER) {
> -			q->qi_rtbtimelimit = newlim->d_rt_spc_timer;
> +			defq->rtbtimelimit = newlim->d_rt_spc_timer;
>   			ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer);
>   		}
>   		if (newlim->d_fieldmask & QC_SPC_WARNS)
> -			q->qi_bwarnlimit = newlim->d_spc_warns;
> +			defq->bwarnlimit = newlim->d_spc_warns;
>   		if (newlim->d_fieldmask & QC_INO_WARNS)
> -			q->qi_iwarnlimit = newlim->d_ino_warns;
> +			defq->iwarnlimit = newlim->d_ino_warns;
>   		if (newlim->d_fieldmask & QC_RT_SPC_WARNS)
> -			q->qi_rtbwarnlimit = newlim->d_rt_spc_warns;
> +			defq->rtbwarnlimit = newlim->d_rt_spc_warns;
>   	} else {
>   		/*
>   		 * If the user is now over quota, start the timelimit.
> diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c
> index cb16a91dd1d4..51be282d28b3 100644
> --- a/fs/xfs/xfs_quotaops.c
> +++ b/fs/xfs/xfs_quotaops.c
> @@ -21,9 +21,9 @@ xfs_qm_fill_state(
>   	struct qc_type_state	*tstate,
>   	struct xfs_mount	*mp,
>   	struct xfs_inode	*ip,
> -	xfs_ino_t		ino)
> +	xfs_ino_t		ino,
> +	struct xfs_def_quota	*defq)
>   {
> -	struct xfs_quotainfo	*q = mp->m_quotainfo;
>   	bool			tempqip = false;
>   
>   	tstate->ino = ino;
> @@ -37,12 +37,12 @@ xfs_qm_fill_state(
>   	tstate->flags |= QCI_SYSFILE;
>   	tstate->blocks = ip->i_d.di_nblocks;
>   	tstate->nextents = ip->i_d.di_nextents;
> -	tstate->spc_timelimit = (u32)q->qi_btimelimit;
> -	tstate->ino_timelimit = (u32)q->qi_itimelimit;
> -	tstate->rt_spc_timelimit = (u32)q->qi_rtbtimelimit;
> -	tstate->spc_warnlimit = q->qi_bwarnlimit;
> -	tstate->ino_warnlimit = q->qi_iwarnlimit;
> -	tstate->rt_spc_warnlimit = q->qi_rtbwarnlimit;
> +	tstate->spc_timelimit = (u32)defq->btimelimit;
> +	tstate->ino_timelimit = (u32)defq->itimelimit;
> +	tstate->rt_spc_timelimit = (u32)defq->rtbtimelimit;
> +	tstate->spc_warnlimit = defq->bwarnlimit;
> +	tstate->ino_warnlimit = defq->iwarnlimit;
> +	tstate->rt_spc_warnlimit = defq->rtbwarnlimit;
>   	if (tempqip)
>   		xfs_irele(ip);
>   }
> @@ -77,11 +77,11 @@ xfs_fs_get_quota_state(
>   		state->s_state[PRJQUOTA].flags |= QCI_LIMITS_ENFORCED;
>   
>   	xfs_qm_fill_state(&state->s_state[USRQUOTA], mp, q->qi_uquotaip,
> -			  mp->m_sb.sb_uquotino);
> +			  mp->m_sb.sb_uquotino, &q->qi_usr_default);
>   	xfs_qm_fill_state(&state->s_state[GRPQUOTA], mp, q->qi_gquotaip,
> -			  mp->m_sb.sb_gquotino);
> +			  mp->m_sb.sb_gquotino, &q->qi_grp_default);
>   	xfs_qm_fill_state(&state->s_state[PRJQUOTA], mp, q->qi_pquotaip,
> -			  mp->m_sb.sb_pquotino);
> +			  mp->m_sb.sb_pquotino, &q->qi_prj_default);
>   	return 0;
>   }
>   
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index 7ae907ec7d47..e4eca607e512 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -601,7 +601,7 @@ xfs_trans_dqresv(
>   			softlimit = defq->bsoftlimit;
>   		timer = be32_to_cpu(dqp->q_core.d_btimer);
>   		warns = be16_to_cpu(dqp->q_core.d_bwarns);
> -		warnlimit = dqp->q_mount->m_quotainfo->qi_bwarnlimit;
> +		warnlimit = defq->bwarnlimit;
>   		resbcountp = &dqp->q_res_bcount;
>   	} else {
>   		ASSERT(flags & XFS_TRANS_DQ_RES_RTBLKS);
> @@ -613,7 +613,7 @@ xfs_trans_dqresv(
>   			softlimit = defq->rtbsoftlimit;
>   		timer = be32_to_cpu(dqp->q_core.d_rtbtimer);
>   		warns = be16_to_cpu(dqp->q_core.d_rtbwarns);
> -		warnlimit = dqp->q_mount->m_quotainfo->qi_rtbwarnlimit;
> +		warnlimit = defq->rtbwarnlimit;
>   		resbcountp = &dqp->q_res_rtbcount;
>   	}
>   
> @@ -649,7 +649,7 @@ xfs_trans_dqresv(
>   			total_count = be64_to_cpu(dqp->q_core.d_icount) + ninos;
>   			timer = be32_to_cpu(dqp->q_core.d_itimer);
>   			warns = be16_to_cpu(dqp->q_core.d_iwarns);
> -			warnlimit = dqp->q_mount->m_quotainfo->qi_iwarnlimit;
> +			warnlimit = defq->iwarnlimit;
>   			hardlimit = be64_to_cpu(dqp->q_core.d_ino_hardlimit);
>   			if (!hardlimit)
>   				hardlimit = defq->ihardlimit;
> 

Re: [PATCH 0/4] xfs: enable per-type quota timers and warn limits

[Darrick J. Wong]

On Sat, Feb 08, 2020 at 03:09:19PM -0600, Eric Sandeen wrote:
> Quota timers are currently a mess.  Right now, at mount time,
> we pick up the first enabled type and use that for the single
> timer in mp->m_quotainfo.
> 
> Interestingly, if we set a timer on a different type, /that/
> gets set into mp->m_quotainfo where it stays in effect until
> the next mount, when we pick the first enabled type again.
> 
> We actually write the timer values to each type of quota inode,
> but only one is ever in force, according to the interesting behavior
> described above.
> 
> This series allows quota timers & warn limits to be independently
> set and enforced for each quota type.

Is there a test case demonstrating this behavior?

Also, what do the other filesystems (well ok ext4) do?

--D

> All the action is in the last patch, the first 3 are cleanups to
> help.
> 
> -Eric
> 

Re: [PATCH 0/4] xfs: enable per-type quota timers and warn limits

[Eric Sandeen]

On 2/11/20 9:43 AM, Darrick J. Wong wrote:
> On Sat, Feb 08, 2020 at 03:09:19PM -0600, Eric Sandeen wrote:
>> Quota timers are currently a mess.  Right now, at mount time,
>> we pick up the first enabled type and use that for the single
>> timer in mp->m_quotainfo.
>>
>> Interestingly, if we set a timer on a different type, /that/
>> gets set into mp->m_quotainfo where it stays in effect until
>> the next mount, when we pick the first enabled type again.
>>
>> We actually write the timer values to each type of quota inode,
>> but only one is ever in force, according to the interesting behavior
>> described above.
>>
>> This series allows quota timers & warn limits to be independently
>> set and enforced for each quota type.
> 
> Is there a test case demonstrating this behavior?

I do still owe this a testcase.
Planned to do it yesterday and then life happened, as it does.

> Also, what do the other filesystems (well ok ext4) do?

I'll let you know after I write the testcase ;)

-Eric

Re: [PATCH 0/4] xfs: enable per-type quota timers and warn limits

[Eric Sandeen]

On 2/11/20 9:52 AM, Eric Sandeen wrote:
> 
> 
> On 2/11/20 9:43 AM, Darrick J. Wong wrote:
>> On Sat, Feb 08, 2020 at 03:09:19PM -0600, Eric Sandeen wrote:
>>> Quota timers are currently a mess.  Right now, at mount time,
>>> we pick up the first enabled type and use that for the single
>>> timer in mp->m_quotainfo.
>>>
>>> Interestingly, if we set a timer on a different type, /that/
>>> gets set into mp->m_quotainfo where it stays in effect until
>>> the next mount, when we pick the first enabled type again.
>>>
>>> We actually write the timer values to each type of quota inode,
>>> but only one is ever in force, according to the interesting behavior
>>> described above.
>>>
>>> This series allows quota timers & warn limits to be independently
>>> set and enforced for each quota type.
>>
>> Is there a test case demonstrating this behavior?
> 
> I do still owe this a testcase.
> Planned to do it yesterday and then life happened, as it does.
> 
>> Also, what do the other filesystems (well ok ext4) do?
> 
> I'll let you know after I write the testcase ;)

Spoiler: it works as expected on ext4.

set user  block & inode grace to 2 & 4 minutes;
set group block & inode grace to 4 & 8 minutes:

# setquota -t -u 120 240  mnt-ext4/
# setquota -t -g 360 480  mnt-ext4/
# setquota -t -u 120 240  mnt-xfs/
# setquota -t -g 360 480  mnt-xfs/

report user & group grace limits:

# repquota -ug mnt-ext4/ | grep "Report\|^Block"
*** Report for user quotas on device /dev/loop1
Block grace time: 00:02; Inode grace time: 00:04
*** Report for group quotas on device /dev/loop1
Block grace time: 00:06; Inode grace time: 00:08

ext4 shows all four different grace periods

# repquota -ug mnt-xfs/ | grep "Report\|^Block"
*** Report for user quotas on device /dev/loop2
Block grace time: 00:02; Inode grace time: 00:04
*** Report for group quotas on device /dev/loop2
Block grace time: 00:02; Inode grace time: 00:04

xfs shows the same grace periods for user & group, despite setting different
values for each.

-Eric

Re: [PATCH 1/4] xfs: fix up some whitespace in quota code

[Christoph Hellwig]

On Sat, Feb 08, 2020 at 03:10:37PM -0600, Eric Sandeen wrote:
> There is a fair bit of whitespace damage in the quota code, so
> fix up enough of it that subsequent patches are restricted to
> functional change to aid review.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

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

Re: [PATCH 2/4] xfs: simplify args to xfs_get_defquota

[Christoph Hellwig]

> -	struct xfs_quotainfo	*q = mp->m_quotainfo;
>  	struct xfs_disk_dquot	*d = &dq->q_core;
>  	struct xfs_def_quota	*defq;
>  	int			prealloc = 0;
>  
>  	ASSERT(d->d_id);
> -	defq = xfs_get_defquota(dq, q);
> +	defq = xfs_get_defquota(dq);

Move this up to the declaration line while you're at it?

>  {
>  	struct xfs_dquot	*dqp;
>  	struct xfs_def_quota	*defq;
> @@ -554,7 +553,7 @@ xfs_qm_set_defquota(
>  		return;
>  
>  	ddqp = &dqp->q_core;
> -	defq = xfs_get_defquota(dqp, qinf);
> +	defq = xfs_get_defquota(dqp);

Same here.

> @@ -585,13 +585,12 @@ xfs_trans_dqresv(
>  	xfs_qwarncnt_t		warnlimit;
>  	xfs_qcnt_t		total_count;
>  	xfs_qcnt_t		*resbcountp;
> -	struct xfs_quotainfo	*q = mp->m_quotainfo;
>  	struct xfs_def_quota	*defq;
>  
>  
>  	xfs_dqlock(dqp);
>  
> -	defq = xfs_get_defquota(dqp, q);
> +	defq = xfs_get_defquota(dqp);

And here.

Otherwise looks good:

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

Re: [PATCH 3/4] xfs: pass xfs_dquot to xfs_qm_adjust_dqtimers

[Christoph Hellwig]

On Sat, Feb 08, 2020 at 03:11:38PM -0600, Eric Sandeen wrote:
> Pass xfs_dquot rather than xfs_disk_dquot to xfs_qm_adjust_dqtimers;
> this makes it symmetric with xfs_qm_adjust_dqlimits and will help
> the next patch.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

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

Re: [PATCH 4/4] xfs: per-type quota timers and warn limits

[Christoph Hellwig]

>  	struct xfs_disk_dquot	*d = &dq->q_core;
> +	struct xfs_def_quota	*defq;
> +
>  	ASSERT(d->d_id);
> +	defq = xfs_get_defquota(dq);

Move up to the declaration line?

> +	switch (type) {
> +	case XFS_DQ_USER:
> +		defq = &qinf->qi_usr_default;
> +		break;
> +	case XFS_DQ_GROUP:
> +		defq = &qinf->qi_grp_default;
> +		break;
> +	case XFS_DQ_PROJ:
> +		defq = &qinf->qi_prj_default;
> +		break;
> +	default:
> +		ASSERT(0);
> +		/* fall through */
> +	}

Should this go into a helper?  Or even better replace the
qi_*default members with an array that the type can index into?

> @@ -592,39 +609,31 @@ xfs_qm_init_timelimits(
>  	 *
>  	 * Since we may not have done a quotacheck by this point, just read
>  	 * the dquot without attaching it to any hashtables or lists.
> -	 *
> -	 * Timers and warnings are globally set by the first timer found in
> -	 * user/group/proj quota types, otherwise a default value is used.
> -	 * This should be split into different fields per quota type.
>  	 */
> -	if (XFS_IS_UQUOTA_RUNNING(mp))
> -		type = XFS_DQ_USER;
> -	else if (XFS_IS_GQUOTA_RUNNING(mp))
> -		type = XFS_DQ_GROUP;
> -	else
> -		type = XFS_DQ_PROJ;
>  	error = xfs_qm_dqget_uncached(mp, 0, type, &dqp);
>  	if (error)
>  		return;
>  
>  	ddqp = &dqp->q_core;
> +	defq = xfs_get_defquota(dqp);

Isn't the defq variable already initialized earlier in the function?

Re: [PATCH 4/4] xfs: per-type quota timers and warn limits

[Eric Sandeen]

On 2/17/20 7:43 AM, Christoph Hellwig wrote:
>>  	struct xfs_disk_dquot	*d = &dq->q_core;
>> +	struct xfs_def_quota	*defq;
>> +
>>  	ASSERT(d->d_id);
>> +	defq = xfs_get_defquota(dq);
> 
> Move up to the declaration line?

*shrug* ok

>> +	switch (type) {
>> +	case XFS_DQ_USER:
>> +		defq = &qinf->qi_usr_default;
>> +		break;
>> +	case XFS_DQ_GROUP:
>> +		defq = &qinf->qi_grp_default;
>> +		break;
>> +	case XFS_DQ_PROJ:
>> +		defq = &qinf->qi_prj_default;
>> +		break;
>> +	default:
>> +		ASSERT(0);
>> +		/* fall through */
>> +	}
> 
> Should this go into a helper?  Or even better replace the
> qi_*default members with an array that the type can index into?

Like maybe xfs_get_defquota() ;)

Not sure what I was thinking here, whoops.

>> @@ -592,39 +609,31 @@ xfs_qm_init_timelimits(
>>  	 *
>>  	 * Since we may not have done a quotacheck by this point, just read
>>  	 * the dquot without attaching it to any hashtables or lists.
>> -	 *
>> -	 * Timers and warnings are globally set by the first timer found in
>> -	 * user/group/proj quota types, otherwise a default value is used.
>> -	 * This should be split into different fields per quota type.
>>  	 */
>> -	if (XFS_IS_UQUOTA_RUNNING(mp))
>> -		type = XFS_DQ_USER;
>> -	else if (XFS_IS_GQUOTA_RUNNING(mp))
>> -		type = XFS_DQ_GROUP;
>> -	else
>> -		type = XFS_DQ_PROJ;
>>  	error = xfs_qm_dqget_uncached(mp, 0, type, &dqp);
>>  	if (error)
>>  		return;
>>  
>>  	ddqp = &dqp->q_core;
>> +	defq = xfs_get_defquota(dqp);
> 
> Isn't the defq variable already initialized earlier in the function?

Oh, uh ... whoops.  Not sure how I messed that up, thanks.

-Eric

Re: [PATCH 0/4] xfs: enable per-type quota timers and warn limits

[Zorro Lang]

On Sat, Feb 08, 2020 at 03:09:19PM -0600, Eric Sandeen wrote:
> Quota timers are currently a mess.  Right now, at mount time,
> we pick up the first enabled type and use that for the single
> timer in mp->m_quotainfo.
> 
> Interestingly, if we set a timer on a different type, /that/
> gets set into mp->m_quotainfo where it stays in effect until
> the next mount, when we pick the first enabled type again.
> 
> We actually write the timer values to each type of quota inode,
> but only one is ever in force, according to the interesting behavior
> described above.
> 
> This series allows quota timers & warn limits to be independently
> set and enforced for each quota type.
> 
> All the action is in the last patch, the first 3 are cleanups to
> help.

This patchset looks good, but the testing for xfs quota timers looks
not so well. Please check the emails(test case) I sent to fstests@:
  [PATCH 1/2] generic: per-type quota timers set/get test
  [PATCH 2/2] generic: test per-type quota softlimit enforcement timeout

Why xfs has such different test results? Please feel free to tell me,
if the case is wrong.

Thanks,
Zorro

> 
> -Eric
> 

Re: [PATCH 2/4] xfs: simplify args to xfs_get_defquota

[Eric Sandeen]

On 2/17/20 7:38 AM, Christoph Hellwig wrote:
>> -	struct xfs_quotainfo	*q = mp->m_quotainfo;
>>  	struct xfs_disk_dquot	*d = &dq->q_core;
>>  	struct xfs_def_quota	*defq;
>>  	int			prealloc = 0;
>>  
>>  	ASSERT(d->d_id);
>> -	defq = xfs_get_defquota(dq, q);
>> +	defq = xfs_get_defquota(dq);
> 
> Move this up to the declaration line while you're at it?

ok

>>  {
>>  	struct xfs_dquot	*dqp;
>>  	struct xfs_def_quota	*defq;
>> @@ -554,7 +553,7 @@ xfs_qm_set_defquota(
>>  		return;
>>  
>>  	ddqp = &dqp->q_core;
>> -	defq = xfs_get_defquota(dqp, qinf);
>> +	defq = xfs_get_defquota(dqp);

can't do that here.  have to call xfs_qm_dqget_uncached(&dqp) first.

> Same here.
> 
>> @@ -585,13 +585,12 @@ xfs_trans_dqresv(
>>  	xfs_qwarncnt_t		warnlimit;
>>  	xfs_qcnt_t		total_count;
>>  	xfs_qcnt_t		*resbcountp;
>> -	struct xfs_quotainfo	*q = mp->m_quotainfo;
>>  	struct xfs_def_quota	*defq;
>>  
>>  
>>  	xfs_dqlock(dqp);
>>  
>> -	defq = xfs_get_defquota(dqp, q);
>> +	defq = xfs_get_defquota(dqp);
> 
> And here.

I ... guess this doesn't have to be under the lock?  I guess I'd rather not make
anything approaching a functional change in this otherwise cosmetic patch by moving
it tho.

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

Re: [PATCH 0/4] xfs: enable per-type quota timers and warn limits

[Eric Sandeen]

On 2/17/20 10:49 PM, Zorro Lang wrote:
> On Sat, Feb 08, 2020 at 03:09:19PM -0600, Eric Sandeen wrote:
>> Quota timers are currently a mess.  Right now, at mount time,
>> we pick up the first enabled type and use that for the single
>> timer in mp->m_quotainfo.
>>
>> Interestingly, if we set a timer on a different type, /that/
>> gets set into mp->m_quotainfo where it stays in effect until
>> the next mount, when we pick the first enabled type again.
>>
>> We actually write the timer values to each type of quota inode,
>> but only one is ever in force, according to the interesting behavior
>> described above.
>>
>> This series allows quota timers & warn limits to be independently
>> set and enforced for each quota type.
>>
>> All the action is in the last patch, the first 3 are cleanups to
>> help.
> 
> This patchset looks good, but the testing for xfs quota timers looks
> not so well. Please check the emails(test case) I sent to fstests@:
>   [PATCH 1/2] generic: per-type quota timers set/get test
>   [PATCH 2/2] generic: test per-type quota softlimit enforcement timeout
> 
> Why xfs has such different test results? Please feel free to tell me,
> if the case is wrong.

Thanks for this, Zorro, I'll take a look.

-Eric

> Thanks,
> Zorro
> 
>>
>> -Eric
>>
>