public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs: Split default quota limits by quota type V3
@ 2016-01-28 16:57 Carlos Maiolino
  2016-01-28 17:36 ` Eric Sandeen
  0 siblings, 1 reply; 6+ messages in thread
From: Carlos Maiolino @ 2016-01-28 16:57 UTC (permalink / raw)
  To: xfs

Default quotas are globally set due historical reasons. IRIX only supported user
and project quotas, and default quota was only applied to user quotas.

In Linux, when a default quota is set, all different quota types inherits the
same default value.

An user with a quota limit larger than the default quota value, will still be
limited to the default value because the group quotas also inherits the default
quotas. Unless the group which the user belongs to have a custom quota limit
set.

This patch aims to split the default quota value by quota type. Allowing each
quota type having different default values.

Default time limits are still set globally. XFS does not set a per-user/group
timer, but a single global timer. For changing this behavior, some changes
should be made in user-space tools another bugs being fixed.

Changelog:

	V2 - Remove comment about old behavior
	V3 - Keep time limit configuration inside xfs_qm_init_quotainfo to keep
	     the current behavior of quota timers

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_dquot.c       | 26 +++++++++++---------
 fs/xfs/xfs_qm.c          | 64 +++++++++++++++++++++++++++++++++++-------------
 fs/xfs/xfs_qm.h          | 34 ++++++++++++++++++++-----
 fs/xfs/xfs_qm_syscalls.c | 15 +++++++-----
 fs/xfs/xfs_trans_dquot.c | 15 +++++++-----
 5 files changed, 107 insertions(+), 47 deletions(-)

diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 9c44d38..23f551b 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -92,26 +92,28 @@ xfs_qm_adjust_dqlimits(
 {
 	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);
 
-	if (q->qi_bsoftlimit && !d->d_blk_softlimit) {
-		d->d_blk_softlimit = cpu_to_be64(q->qi_bsoftlimit);
+	if (defq->bsoftlimit && !d->d_blk_softlimit) {
+		d->d_blk_softlimit = cpu_to_be64(defq->bsoftlimit);
 		prealloc = 1;
 	}
-	if (q->qi_bhardlimit && !d->d_blk_hardlimit) {
-		d->d_blk_hardlimit = cpu_to_be64(q->qi_bhardlimit);
+	if (defq->bhardlimit && !d->d_blk_hardlimit) {
+		d->d_blk_hardlimit = cpu_to_be64(defq->bhardlimit);
 		prealloc = 1;
 	}
-	if (q->qi_isoftlimit && !d->d_ino_softlimit)
-		d->d_ino_softlimit = cpu_to_be64(q->qi_isoftlimit);
-	if (q->qi_ihardlimit && !d->d_ino_hardlimit)
-		d->d_ino_hardlimit = cpu_to_be64(q->qi_ihardlimit);
-	if (q->qi_rtbsoftlimit && !d->d_rtb_softlimit)
-		d->d_rtb_softlimit = cpu_to_be64(q->qi_rtbsoftlimit);
-	if (q->qi_rtbhardlimit && !d->d_rtb_hardlimit)
-		d->d_rtb_hardlimit = cpu_to_be64(q->qi_rtbhardlimit);
+	if (defq->isoftlimit && !d->d_ino_softlimit)
+		d->d_ino_softlimit = cpu_to_be64(defq->isoftlimit);
+	if (defq->ihardlimit && !d->d_ino_hardlimit)
+		d->d_ino_hardlimit = cpu_to_be64(defq->ihardlimit);
+	if (defq->rtbsoftlimit && !d->d_rtb_softlimit)
+		d->d_rtb_softlimit = cpu_to_be64(defq->rtbsoftlimit);
+	if (defq->rtbhardlimit && !d->d_rtb_hardlimit)
+		d->d_rtb_hardlimit = cpu_to_be64(defq->rtbhardlimit);
 
 	if (prealloc)
 		xfs_dquot_set_prealloc_limits(dq);
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 532ab79..4cbb22b 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -560,6 +560,37 @@ xfs_qm_shrink_count(
 	return list_lru_shrink_count(&qi->qi_lru, sc);
 }
 
+STATIC void
+xfs_qm_set_defquota(
+	xfs_mount_t	*mp,
+	uint		type,
+	xfs_quotainfo_t	*qinf)
+{
+	xfs_dquot_t		*dqp;
+	struct xfs_def_quota    *defq;
+	int			error;
+
+	error = xfs_qm_dqread(mp, 0, type, XFS_QMOPT_DOWARN, &dqp);
+
+	if (!error) {
+		xfs_disk_dquot_t        *ddqp = &dqp->q_core;
+
+		defq = xfs_get_defquota(dqp, qinf);
+
+		/*
+		 * Timers and warnings have been already set, let's just set the
+		 * default limits for this quota type
+		 */
+		defq->bhardlimit = be64_to_cpu(ddqp->d_blk_hardlimit);
+		defq->bsoftlimit = be64_to_cpu(ddqp->d_blk_softlimit);
+		defq->ihardlimit = be64_to_cpu(ddqp->d_ino_hardlimit);
+		defq->isoftlimit = be64_to_cpu(ddqp->d_ino_softlimit);
+		defq->rtbhardlimit = be64_to_cpu(ddqp->d_rtb_hardlimit);
+		defq->rtbsoftlimit = be64_to_cpu(ddqp->d_rtb_softlimit);
+		xfs_qm_dqdestroy(dqp);
+	}
+}
+
 /*
  * This initializes all the quota information that's kept in the
  * mount structure
@@ -606,27 +637,22 @@ xfs_qm_init_quotainfo(
 	 * We try to get the limits from the superuser's limits fields.
 	 * This is quite hacky, but it is standard quota practice.
 	 *
-	 * We look at the USR dquot with id == 0 first, but if user quotas
-	 * are not enabled we goto the GRP dquot with id == 0.
-	 * We don't really care to keep separate default limits for user
-	 * and group quotas, at least not at this point.
-	 *
 	 * 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 splitted into different fields per quota type.
 	 */
 	error = xfs_qm_dqread(mp, 0,
 			XFS_IS_UQUOTA_RUNNING(mp) ? XFS_DQ_USER :
 			 (XFS_IS_GQUOTA_RUNNING(mp) ? XFS_DQ_GROUP :
 			  XFS_DQ_PROJ),
 			XFS_QMOPT_DOWARN, &dqp);
+
 	if (!error) {
 		xfs_disk_dquot_t	*ddqp = &dqp->q_core;
 
-		/*
-		 * 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.
-		 */
 		qinf->qi_btimelimit = ddqp->d_btimer ?
 			be32_to_cpu(ddqp->d_btimer) : XFS_QM_BTIMELIMIT;
 		qinf->qi_itimelimit = ddqp->d_itimer ?
@@ -639,13 +665,6 @@ xfs_qm_init_quotainfo(
 			be16_to_cpu(ddqp->d_iwarns) : XFS_QM_IWARNLIMIT;
 		qinf->qi_rtbwarnlimit = ddqp->d_rtbwarns ?
 			be16_to_cpu(ddqp->d_rtbwarns) : XFS_QM_RTBWARNLIMIT;
-		qinf->qi_bhardlimit = be64_to_cpu(ddqp->d_blk_hardlimit);
-		qinf->qi_bsoftlimit = be64_to_cpu(ddqp->d_blk_softlimit);
-		qinf->qi_ihardlimit = be64_to_cpu(ddqp->d_ino_hardlimit);
-		qinf->qi_isoftlimit = be64_to_cpu(ddqp->d_ino_softlimit);
-		qinf->qi_rtbhardlimit = be64_to_cpu(ddqp->d_rtb_hardlimit);
-		qinf->qi_rtbsoftlimit = be64_to_cpu(ddqp->d_rtb_softlimit);
-
 		xfs_qm_dqdestroy(dqp);
 	} else {
 		qinf->qi_btimelimit = XFS_QM_BTIMELIMIT;
@@ -656,6 +675,17 @@ xfs_qm_init_quotainfo(
 		qinf->qi_rtbwarnlimit = XFS_QM_RTBWARNLIMIT;
 	}
 
+	/*
+	 * Default quota values are set by quota type, timer and warn limits
+	 * have already been setpreviously, no need for error check here.
+	 */
+	if (XFS_IS_UQUOTA_RUNNING(mp))
+		xfs_qm_set_defquota(mp, XFS_DQ_USER, qinf);
+	if (XFS_IS_GQUOTA_RUNNING(mp))
+		xfs_qm_set_defquota(mp, XFS_DQ_GROUP, qinf);
+	if (XFS_IS_PQUOTA_RUNNING(mp))
+		xfs_qm_set_defquota(mp, XFS_DQ_PROJ, qinf);
+
 	qinf->qi_shrinker.count_objects = xfs_qm_shrink_count;
 	qinf->qi_shrinker.scan_objects = xfs_qm_shrink_scan;
 	qinf->qi_shrinker.seeks = DEFAULT_SEEKS;
diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
index 996a040..45e2c36 100644
--- a/fs/xfs/xfs_qm.h
+++ b/fs/xfs/xfs_qm.h
@@ -53,6 +53,15 @@ 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 */
+};
+
 /*
  * Various quota information for individual filesystems.
  * The mount structure keeps a pointer to this.
@@ -76,12 +85,9 @@ typedef struct xfs_quotainfo {
 	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 */
-	xfs_qcnt_t	 qi_bhardlimit;	 /* default data blk hard limit */
-	xfs_qcnt_t	 qi_bsoftlimit;	 /* default data blk soft limit */
-	xfs_qcnt_t	 qi_ihardlimit;	 /* default inode count hard limit */
-	xfs_qcnt_t	 qi_isoftlimit;	 /* default inode count soft limit */
-	xfs_qcnt_t	 qi_rtbhardlimit;/* default realtime blk hard limit */
-	xfs_qcnt_t	 qi_rtbsoftlimit;/* default realtime blk soft limit */
+	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;
 } xfs_quotainfo_t;
 
@@ -171,4 +177,20 @@ extern int		xfs_qm_scall_setqlim(struct xfs_mount *, xfs_dqid_t, uint,
 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)
+{
+	struct xfs_def_quota *defq;
+
+	if (XFS_QM_ISUDQ(dqp))
+		defq = &qi->qi_usr_default;
+	else if (XFS_QM_ISGDQ(dqp))
+		defq = &qi->qi_grp_default;
+	else {
+		ASSERT(XFS_QM_ISPDQ(dqp));
+		defq = &qi->qi_prj_default;
+	}
+	return defq;
+}
+
 #endif /* __XFS_QM_H__ */
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 3640c6e..31830f0 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -404,6 +404,7 @@ xfs_qm_scall_setqlim(
 	struct xfs_disk_dquot	*ddq;
 	struct xfs_dquot	*dqp;
 	struct xfs_trans	*tp;
+	struct xfs_def_quota	*defq;
 	int			error;
 	xfs_qcnt_t		hard, soft;
 
@@ -431,6 +432,8 @@ xfs_qm_scall_setqlim(
 		ASSERT(error != -ENOENT);
 		goto out_unlock;
 	}
+
+	defq = xfs_get_defquota(dqp, q);
 	xfs_dqunlock(dqp);
 
 	tp = xfs_trans_alloc(mp, XFS_TRANS_QM_SETQLIM);
@@ -458,8 +461,8 @@ xfs_qm_scall_setqlim(
 		ddq->d_blk_softlimit = cpu_to_be64(soft);
 		xfs_dquot_set_prealloc_limits(dqp);
 		if (id == 0) {
-			q->qi_bhardlimit = hard;
-			q->qi_bsoftlimit = soft;
+			defq->bhardlimit = hard;
+			defq->bsoftlimit = soft;
 		}
 	} else {
 		xfs_debug(mp, "blkhard %Ld < blksoft %Ld", hard, soft);
@@ -474,8 +477,8 @@ xfs_qm_scall_setqlim(
 		ddq->d_rtb_hardlimit = cpu_to_be64(hard);
 		ddq->d_rtb_softlimit = cpu_to_be64(soft);
 		if (id == 0) {
-			q->qi_rtbhardlimit = hard;
-			q->qi_rtbsoftlimit = soft;
+			defq->rtbhardlimit = hard;
+			defq->rtbsoftlimit = soft;
 		}
 	} else {
 		xfs_debug(mp, "rtbhard %Ld < rtbsoft %Ld", hard, soft);
@@ -491,8 +494,8 @@ xfs_qm_scall_setqlim(
 		ddq->d_ino_hardlimit = cpu_to_be64(hard);
 		ddq->d_ino_softlimit = cpu_to_be64(soft);
 		if (id == 0) {
-			q->qi_ihardlimit = hard;
-			q->qi_isoftlimit = soft;
+			defq->ihardlimit = hard;
+			defq->isoftlimit = soft;
 		}
 	} else {
 		xfs_debug(mp, "ihard %Ld < isoft %Ld", hard, soft);
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index 9951701..c3d5472 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -609,17 +609,20 @@ xfs_trans_dqresv(
 	xfs_qcnt_t	total_count;
 	xfs_qcnt_t	*resbcountp;
 	xfs_quotainfo_t	*q = mp->m_quotainfo;
+	struct xfs_def_quota	*defq;
 
 
 	xfs_dqlock(dqp);
 
+	defq = xfs_get_defquota(dqp, q);
+
 	if (flags & XFS_TRANS_DQ_RES_BLKS) {
 		hardlimit = be64_to_cpu(dqp->q_core.d_blk_hardlimit);
 		if (!hardlimit)
-			hardlimit = q->qi_bhardlimit;
+			hardlimit = defq->bhardlimit;
 		softlimit = be64_to_cpu(dqp->q_core.d_blk_softlimit);
 		if (!softlimit)
-			softlimit = q->qi_bsoftlimit;
+			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;
@@ -628,10 +631,10 @@ xfs_trans_dqresv(
 		ASSERT(flags & XFS_TRANS_DQ_RES_RTBLKS);
 		hardlimit = be64_to_cpu(dqp->q_core.d_rtb_hardlimit);
 		if (!hardlimit)
-			hardlimit = q->qi_rtbhardlimit;
+			hardlimit = defq->rtbhardlimit;
 		softlimit = be64_to_cpu(dqp->q_core.d_rtb_softlimit);
 		if (!softlimit)
-			softlimit = q->qi_rtbsoftlimit;
+			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;
@@ -672,10 +675,10 @@ xfs_trans_dqresv(
 			warnlimit = dqp->q_mount->m_quotainfo->qi_iwarnlimit;
 			hardlimit = be64_to_cpu(dqp->q_core.d_ino_hardlimit);
 			if (!hardlimit)
-				hardlimit = q->qi_ihardlimit;
+				hardlimit = defq->ihardlimit;
 			softlimit = be64_to_cpu(dqp->q_core.d_ino_softlimit);
 			if (!softlimit)
-				softlimit = q->qi_isoftlimit;
+				softlimit = defq->isoftlimit;
 
 			if (hardlimit && total_count > hardlimit) {
 				xfs_quota_warn(mp, dqp, QUOTA_NL_IHARDWARN);
-- 
2.4.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs: Split default quota limits by quota type V3
  2016-01-28 16:57 [PATCH] xfs: Split default quota limits by quota type V3 Carlos Maiolino
@ 2016-01-28 17:36 ` Eric Sandeen
  2016-02-01  9:42   ` Carlos Maiolino
  2016-02-01  9:57   ` Carlos Maiolino
  0 siblings, 2 replies; 6+ messages in thread
From: Eric Sandeen @ 2016-01-28 17:36 UTC (permalink / raw)
  To: xfs

On 1/28/16 10:57 AM, Carlos Maiolino wrote:
> Default quotas are globally set due historical reasons. IRIX only supported user
> and project quotas, and default quota was only applied to user quotas.
> 
> In Linux, when a default quota is set, all different quota types inherits the
> same default value.
> 
> An user with a quota limit larger than the default quota value, will still be
> limited to the default value because the group quotas also inherits the default
> quotas. Unless the group which the user belongs to have a custom quota limit
> set.
> 
> This patch aims to split the default quota value by quota type. Allowing each
> quota type having different default values.
> 
> Default time limits are still set globally. XFS does not set a per-user/group
> timer, but a single global timer. For changing this behavior, some changes
> should be made in user-space tools another bugs being fixed.

Some minor comments about comments below.  But the code looks fine to me,
so:

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

and if you want to fix the comments & resend w/ my review tag I think that'd
be ok.

Care to write an xfstests testcase for the default quota limit while you're
at it?

Thanks,
-Eric

> Changelog:
> 
> 	V2 - Remove comment about old behavior
> 	V3 - Keep time limit configuration inside xfs_qm_init_quotainfo to keep
> 	     the current behavior of quota timers
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  fs/xfs/xfs_dquot.c       | 26 +++++++++++---------
>  fs/xfs/xfs_qm.c          | 64 +++++++++++++++++++++++++++++++++++-------------
>  fs/xfs/xfs_qm.h          | 34 ++++++++++++++++++++-----
>  fs/xfs/xfs_qm_syscalls.c | 15 +++++++-----
>  fs/xfs/xfs_trans_dquot.c | 15 +++++++-----
>  5 files changed, 107 insertions(+), 47 deletions(-)
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 9c44d38..23f551b 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -92,26 +92,28 @@ xfs_qm_adjust_dqlimits(
>  {
>  	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);
>  
> -	if (q->qi_bsoftlimit && !d->d_blk_softlimit) {
> -		d->d_blk_softlimit = cpu_to_be64(q->qi_bsoftlimit);
> +	if (defq->bsoftlimit && !d->d_blk_softlimit) {
> +		d->d_blk_softlimit = cpu_to_be64(defq->bsoftlimit);
>  		prealloc = 1;
>  	}
> -	if (q->qi_bhardlimit && !d->d_blk_hardlimit) {
> -		d->d_blk_hardlimit = cpu_to_be64(q->qi_bhardlimit);
> +	if (defq->bhardlimit && !d->d_blk_hardlimit) {
> +		d->d_blk_hardlimit = cpu_to_be64(defq->bhardlimit);
>  		prealloc = 1;
>  	}
> -	if (q->qi_isoftlimit && !d->d_ino_softlimit)
> -		d->d_ino_softlimit = cpu_to_be64(q->qi_isoftlimit);
> -	if (q->qi_ihardlimit && !d->d_ino_hardlimit)
> -		d->d_ino_hardlimit = cpu_to_be64(q->qi_ihardlimit);
> -	if (q->qi_rtbsoftlimit && !d->d_rtb_softlimit)
> -		d->d_rtb_softlimit = cpu_to_be64(q->qi_rtbsoftlimit);
> -	if (q->qi_rtbhardlimit && !d->d_rtb_hardlimit)
> -		d->d_rtb_hardlimit = cpu_to_be64(q->qi_rtbhardlimit);
> +	if (defq->isoftlimit && !d->d_ino_softlimit)
> +		d->d_ino_softlimit = cpu_to_be64(defq->isoftlimit);
> +	if (defq->ihardlimit && !d->d_ino_hardlimit)
> +		d->d_ino_hardlimit = cpu_to_be64(defq->ihardlimit);
> +	if (defq->rtbsoftlimit && !d->d_rtb_softlimit)
> +		d->d_rtb_softlimit = cpu_to_be64(defq->rtbsoftlimit);
> +	if (defq->rtbhardlimit && !d->d_rtb_hardlimit)
> +		d->d_rtb_hardlimit = cpu_to_be64(defq->rtbhardlimit);
>  
>  	if (prealloc)
>  		xfs_dquot_set_prealloc_limits(dq);
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 532ab79..4cbb22b 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -560,6 +560,37 @@ xfs_qm_shrink_count(
>  	return list_lru_shrink_count(&qi->qi_lru, sc);
>  }
>  
> +STATIC void
> +xfs_qm_set_defquota(
> +	xfs_mount_t	*mp,
> +	uint		type,
> +	xfs_quotainfo_t	*qinf)
> +{
> +	xfs_dquot_t		*dqp;
> +	struct xfs_def_quota    *defq;
> +	int			error;
> +
> +	error = xfs_qm_dqread(mp, 0, type, XFS_QMOPT_DOWARN, &dqp);
> +
> +	if (!error) {
> +		xfs_disk_dquot_t        *ddqp = &dqp->q_core;
> +
> +		defq = xfs_get_defquota(dqp, qinf);
> +
> +		/*
> +		 * Timers and warnings have been already set, let's just set the
> +		 * default limits for this quota type
> +		 */
> +		defq->bhardlimit = be64_to_cpu(ddqp->d_blk_hardlimit);
> +		defq->bsoftlimit = be64_to_cpu(ddqp->d_blk_softlimit);
> +		defq->ihardlimit = be64_to_cpu(ddqp->d_ino_hardlimit);
> +		defq->isoftlimit = be64_to_cpu(ddqp->d_ino_softlimit);
> +		defq->rtbhardlimit = be64_to_cpu(ddqp->d_rtb_hardlimit);
> +		defq->rtbsoftlimit = be64_to_cpu(ddqp->d_rtb_softlimit);
> +		xfs_qm_dqdestroy(dqp);
> +	}
> +}
> +
>  /*
>   * This initializes all the quota information that's kept in the
>   * mount structure
> @@ -606,27 +637,22 @@ xfs_qm_init_quotainfo(
>  	 * We try to get the limits from the superuser's limits fields.
>  	 * This is quite hacky, but it is standard quota practice.
>  	 *
> -	 * We look at the USR dquot with id == 0 first, but if user quotas
> -	 * are not enabled we goto the GRP dquot with id == 0.
> -	 * We don't really care to keep separate default limits for user
> -	 * and group quotas, at least not at this point.
> -	 *
>  	 * 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 splitted into different fields per quota type.

s/splitted/split/

>  	 */
>  	error = xfs_qm_dqread(mp, 0,
>  			XFS_IS_UQUOTA_RUNNING(mp) ? XFS_DQ_USER :
>  			 (XFS_IS_GQUOTA_RUNNING(mp) ? XFS_DQ_GROUP :
>  			  XFS_DQ_PROJ),
>  			XFS_QMOPT_DOWARN, &dqp);
> +
>  	if (!error) {
>  		xfs_disk_dquot_t	*ddqp = &dqp->q_core;
>  
> -		/*
> -		 * 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.
> -		 */

Not sure why you removed this comment?

>  		qinf->qi_btimelimit = ddqp->d_btimer ?
>  			be32_to_cpu(ddqp->d_btimer) : XFS_QM_BTIMELIMIT;
>  		qinf->qi_itimelimit = ddqp->d_itimer ?
> @@ -639,13 +665,6 @@ xfs_qm_init_quotainfo(
>  			be16_to_cpu(ddqp->d_iwarns) : XFS_QM_IWARNLIMIT;
>  		qinf->qi_rtbwarnlimit = ddqp->d_rtbwarns ?
>  			be16_to_cpu(ddqp->d_rtbwarns) : XFS_QM_RTBWARNLIMIT;
> -		qinf->qi_bhardlimit = be64_to_cpu(ddqp->d_blk_hardlimit);
> -		qinf->qi_bsoftlimit = be64_to_cpu(ddqp->d_blk_softlimit);
> -		qinf->qi_ihardlimit = be64_to_cpu(ddqp->d_ino_hardlimit);
> -		qinf->qi_isoftlimit = be64_to_cpu(ddqp->d_ino_softlimit);
> -		qinf->qi_rtbhardlimit = be64_to_cpu(ddqp->d_rtb_hardlimit);
> -		qinf->qi_rtbsoftlimit = be64_to_cpu(ddqp->d_rtb_softlimit);
> -
>  		xfs_qm_dqdestroy(dqp);
>  	} else {
>  		qinf->qi_btimelimit = XFS_QM_BTIMELIMIT;
> @@ -656,6 +675,17 @@ xfs_qm_init_quotainfo(
>  		qinf->qi_rtbwarnlimit = XFS_QM_RTBWARNLIMIT;
>  	}
>  
> +	/*
> +	 * Default quota values are set by quota type, timer and warn limits
> +	 * have already been setpreviously, no need for error check here.

"set previously" - and, not sure the comment about error checking is needed.
Previously, if we couldn't read the inode, we set timers but no limits.
And the limits were (and still are) zalloc'd so it's safe to ignore
the errors, I guess.


> +	 */
> +	if (XFS_IS_UQUOTA_RUNNING(mp))
> +		xfs_qm_set_defquota(mp, XFS_DQ_USER, qinf);
> +	if (XFS_IS_GQUOTA_RUNNING(mp))
> +		xfs_qm_set_defquota(mp, XFS_DQ_GROUP, qinf);
> +	if (XFS_IS_PQUOTA_RUNNING(mp))
> +		xfs_qm_set_defquota(mp, XFS_DQ_PROJ, qinf);
> +
>  	qinf->qi_shrinker.count_objects = xfs_qm_shrink_count;
>  	qinf->qi_shrinker.scan_objects = xfs_qm_shrink_scan;
>  	qinf->qi_shrinker.seeks = DEFAULT_SEEKS;
> diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
> index 996a040..45e2c36 100644
> --- a/fs/xfs/xfs_qm.h
> +++ b/fs/xfs/xfs_qm.h
> @@ -53,6 +53,15 @@ 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 */
> +};
> +
>  /*
>   * Various quota information for individual filesystems.
>   * The mount structure keeps a pointer to this.
> @@ -76,12 +85,9 @@ typedef struct xfs_quotainfo {
>  	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 */
> -	xfs_qcnt_t	 qi_bhardlimit;	 /* default data blk hard limit */
> -	xfs_qcnt_t	 qi_bsoftlimit;	 /* default data blk soft limit */
> -	xfs_qcnt_t	 qi_ihardlimit;	 /* default inode count hard limit */
> -	xfs_qcnt_t	 qi_isoftlimit;	 /* default inode count soft limit */
> -	xfs_qcnt_t	 qi_rtbhardlimit;/* default realtime blk hard limit */
> -	xfs_qcnt_t	 qi_rtbsoftlimit;/* default realtime blk soft limit */
> +	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;
>  } xfs_quotainfo_t;
>  
> @@ -171,4 +177,20 @@ extern int		xfs_qm_scall_setqlim(struct xfs_mount *, xfs_dqid_t, uint,
>  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)
> +{
> +	struct xfs_def_quota *defq;
> +
> +	if (XFS_QM_ISUDQ(dqp))
> +		defq = &qi->qi_usr_default;
> +	else if (XFS_QM_ISGDQ(dqp))
> +		defq = &qi->qi_grp_default;
> +	else {
> +		ASSERT(XFS_QM_ISPDQ(dqp));
> +		defq = &qi->qi_prj_default;
> +	}
> +	return defq;
> +}
> +
>  #endif /* __XFS_QM_H__ */
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index 3640c6e..31830f0 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -404,6 +404,7 @@ xfs_qm_scall_setqlim(
>  	struct xfs_disk_dquot	*ddq;
>  	struct xfs_dquot	*dqp;
>  	struct xfs_trans	*tp;
> +	struct xfs_def_quota	*defq;
>  	int			error;
>  	xfs_qcnt_t		hard, soft;
>  
> @@ -431,6 +432,8 @@ xfs_qm_scall_setqlim(
>  		ASSERT(error != -ENOENT);
>  		goto out_unlock;
>  	}
> +
> +	defq = xfs_get_defquota(dqp, q);
>  	xfs_dqunlock(dqp);
>  
>  	tp = xfs_trans_alloc(mp, XFS_TRANS_QM_SETQLIM);
> @@ -458,8 +461,8 @@ xfs_qm_scall_setqlim(
>  		ddq->d_blk_softlimit = cpu_to_be64(soft);
>  		xfs_dquot_set_prealloc_limits(dqp);
>  		if (id == 0) {
> -			q->qi_bhardlimit = hard;
> -			q->qi_bsoftlimit = soft;
> +			defq->bhardlimit = hard;
> +			defq->bsoftlimit = soft;
>  		}
>  	} else {
>  		xfs_debug(mp, "blkhard %Ld < blksoft %Ld", hard, soft);
> @@ -474,8 +477,8 @@ xfs_qm_scall_setqlim(
>  		ddq->d_rtb_hardlimit = cpu_to_be64(hard);
>  		ddq->d_rtb_softlimit = cpu_to_be64(soft);
>  		if (id == 0) {
> -			q->qi_rtbhardlimit = hard;
> -			q->qi_rtbsoftlimit = soft;
> +			defq->rtbhardlimit = hard;
> +			defq->rtbsoftlimit = soft;
>  		}
>  	} else {
>  		xfs_debug(mp, "rtbhard %Ld < rtbsoft %Ld", hard, soft);
> @@ -491,8 +494,8 @@ xfs_qm_scall_setqlim(
>  		ddq->d_ino_hardlimit = cpu_to_be64(hard);
>  		ddq->d_ino_softlimit = cpu_to_be64(soft);
>  		if (id == 0) {
> -			q->qi_ihardlimit = hard;
> -			q->qi_isoftlimit = soft;
> +			defq->ihardlimit = hard;
> +			defq->isoftlimit = soft;
>  		}
>  	} else {
>  		xfs_debug(mp, "ihard %Ld < isoft %Ld", hard, soft);
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index 9951701..c3d5472 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -609,17 +609,20 @@ xfs_trans_dqresv(
>  	xfs_qcnt_t	total_count;
>  	xfs_qcnt_t	*resbcountp;
>  	xfs_quotainfo_t	*q = mp->m_quotainfo;
> +	struct xfs_def_quota	*defq;
>  
>  
>  	xfs_dqlock(dqp);
>  
> +	defq = xfs_get_defquota(dqp, q);
> +
>  	if (flags & XFS_TRANS_DQ_RES_BLKS) {
>  		hardlimit = be64_to_cpu(dqp->q_core.d_blk_hardlimit);
>  		if (!hardlimit)
> -			hardlimit = q->qi_bhardlimit;
> +			hardlimit = defq->bhardlimit;
>  		softlimit = be64_to_cpu(dqp->q_core.d_blk_softlimit);
>  		if (!softlimit)
> -			softlimit = q->qi_bsoftlimit;
> +			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;
> @@ -628,10 +631,10 @@ xfs_trans_dqresv(
>  		ASSERT(flags & XFS_TRANS_DQ_RES_RTBLKS);
>  		hardlimit = be64_to_cpu(dqp->q_core.d_rtb_hardlimit);
>  		if (!hardlimit)
> -			hardlimit = q->qi_rtbhardlimit;
> +			hardlimit = defq->rtbhardlimit;
>  		softlimit = be64_to_cpu(dqp->q_core.d_rtb_softlimit);
>  		if (!softlimit)
> -			softlimit = q->qi_rtbsoftlimit;
> +			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;
> @@ -672,10 +675,10 @@ xfs_trans_dqresv(
>  			warnlimit = dqp->q_mount->m_quotainfo->qi_iwarnlimit;
>  			hardlimit = be64_to_cpu(dqp->q_core.d_ino_hardlimit);
>  			if (!hardlimit)
> -				hardlimit = q->qi_ihardlimit;
> +				hardlimit = defq->ihardlimit;
>  			softlimit = be64_to_cpu(dqp->q_core.d_ino_softlimit);
>  			if (!softlimit)
> -				softlimit = q->qi_isoftlimit;
> +				softlimit = defq->isoftlimit;
>  
>  			if (hardlimit && total_count > hardlimit) {
>  				xfs_quota_warn(mp, dqp, QUOTA_NL_IHARDWARN);
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs: Split default quota limits by quota type V3
  2016-01-28 17:36 ` Eric Sandeen
@ 2016-02-01  9:42   ` Carlos Maiolino
  2016-02-01  9:57   ` Carlos Maiolino
  1 sibling, 0 replies; 6+ messages in thread
From: Carlos Maiolino @ 2016-02-01  9:42 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: xfs

On Thu, Jan 28, 2016 at 11:36:55AM -0600, Eric Sandeen wrote:
> On 1/28/16 10:57 AM, Carlos Maiolino wrote:
> > Default quotas are globally set due historical reasons. IRIX only supported user
> > and project quotas, and default quota was only applied to user quotas.
> > 
> > In Linux, when a default quota is set, all different quota types inherits the
> > same default value.
> > 
> > An user with a quota limit larger than the default quota value, will still be
> > limited to the default value because the group quotas also inherits the default
> > quotas. Unless the group which the user belongs to have a custom quota limit
> > set.
> > 
> > This patch aims to split the default quota value by quota type. Allowing each
> > quota type having different default values.
> > 
> > Default time limits are still set globally. XFS does not set a per-user/group
> > timer, but a single global timer. For changing this behavior, some changes
> > should be made in user-space tools another bugs being fixed.
> 
> Some minor comments about comments below.  But the code looks fine to me,
> so:
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> 
> and if you want to fix the comments & resend w/ my review tag I think that'd
> be ok.
> 
> Care to write an xfstests testcase for the default quota limit while you're
> at it?
> 

Thanks, I'll fix the comments and resend it. I sometimes overcomment (if that
word even exists:) code, I'll fix that up.

Regarding xfstests, indeed, I'll write a xfstest for it.

Cheers.

> Thanks,
> -Eric
> 
> > Changelog:
> > 
> > 	V2 - Remove comment about old behavior
> > 	V3 - Keep time limit configuration inside xfs_qm_init_quotainfo to keep
> > 	     the current behavior of quota timers
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> >  fs/xfs/xfs_dquot.c       | 26 +++++++++++---------
> >  fs/xfs/xfs_qm.c          | 64 +++++++++++++++++++++++++++++++++++-------------
> >  fs/xfs/xfs_qm.h          | 34 ++++++++++++++++++++-----
> >  fs/xfs/xfs_qm_syscalls.c | 15 +++++++-----
> >  fs/xfs/xfs_trans_dquot.c | 15 +++++++-----
> >  5 files changed, 107 insertions(+), 47 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> > index 9c44d38..23f551b 100644
> > --- a/fs/xfs/xfs_dquot.c
> > +++ b/fs/xfs/xfs_dquot.c
> > @@ -92,26 +92,28 @@ xfs_qm_adjust_dqlimits(
> >  {
> >  	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);
> >  
> > -	if (q->qi_bsoftlimit && !d->d_blk_softlimit) {
> > -		d->d_blk_softlimit = cpu_to_be64(q->qi_bsoftlimit);
> > +	if (defq->bsoftlimit && !d->d_blk_softlimit) {
> > +		d->d_blk_softlimit = cpu_to_be64(defq->bsoftlimit);
> >  		prealloc = 1;
> >  	}
> > -	if (q->qi_bhardlimit && !d->d_blk_hardlimit) {
> > -		d->d_blk_hardlimit = cpu_to_be64(q->qi_bhardlimit);
> > +	if (defq->bhardlimit && !d->d_blk_hardlimit) {
> > +		d->d_blk_hardlimit = cpu_to_be64(defq->bhardlimit);
> >  		prealloc = 1;
> >  	}
> > -	if (q->qi_isoftlimit && !d->d_ino_softlimit)
> > -		d->d_ino_softlimit = cpu_to_be64(q->qi_isoftlimit);
> > -	if (q->qi_ihardlimit && !d->d_ino_hardlimit)
> > -		d->d_ino_hardlimit = cpu_to_be64(q->qi_ihardlimit);
> > -	if (q->qi_rtbsoftlimit && !d->d_rtb_softlimit)
> > -		d->d_rtb_softlimit = cpu_to_be64(q->qi_rtbsoftlimit);
> > -	if (q->qi_rtbhardlimit && !d->d_rtb_hardlimit)
> > -		d->d_rtb_hardlimit = cpu_to_be64(q->qi_rtbhardlimit);
> > +	if (defq->isoftlimit && !d->d_ino_softlimit)
> > +		d->d_ino_softlimit = cpu_to_be64(defq->isoftlimit);
> > +	if (defq->ihardlimit && !d->d_ino_hardlimit)
> > +		d->d_ino_hardlimit = cpu_to_be64(defq->ihardlimit);
> > +	if (defq->rtbsoftlimit && !d->d_rtb_softlimit)
> > +		d->d_rtb_softlimit = cpu_to_be64(defq->rtbsoftlimit);
> > +	if (defq->rtbhardlimit && !d->d_rtb_hardlimit)
> > +		d->d_rtb_hardlimit = cpu_to_be64(defq->rtbhardlimit);
> >  
> >  	if (prealloc)
> >  		xfs_dquot_set_prealloc_limits(dq);
> > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> > index 532ab79..4cbb22b 100644
> > --- a/fs/xfs/xfs_qm.c
> > +++ b/fs/xfs/xfs_qm.c
> > @@ -560,6 +560,37 @@ xfs_qm_shrink_count(
> >  	return list_lru_shrink_count(&qi->qi_lru, sc);
> >  }
> >  
> > +STATIC void
> > +xfs_qm_set_defquota(
> > +	xfs_mount_t	*mp,
> > +	uint		type,
> > +	xfs_quotainfo_t	*qinf)
> > +{
> > +	xfs_dquot_t		*dqp;
> > +	struct xfs_def_quota    *defq;
> > +	int			error;
> > +
> > +	error = xfs_qm_dqread(mp, 0, type, XFS_QMOPT_DOWARN, &dqp);
> > +
> > +	if (!error) {
> > +		xfs_disk_dquot_t        *ddqp = &dqp->q_core;
> > +
> > +		defq = xfs_get_defquota(dqp, qinf);
> > +
> > +		/*
> > +		 * Timers and warnings have been already set, let's just set the
> > +		 * default limits for this quota type
> > +		 */
> > +		defq->bhardlimit = be64_to_cpu(ddqp->d_blk_hardlimit);
> > +		defq->bsoftlimit = be64_to_cpu(ddqp->d_blk_softlimit);
> > +		defq->ihardlimit = be64_to_cpu(ddqp->d_ino_hardlimit);
> > +		defq->isoftlimit = be64_to_cpu(ddqp->d_ino_softlimit);
> > +		defq->rtbhardlimit = be64_to_cpu(ddqp->d_rtb_hardlimit);
> > +		defq->rtbsoftlimit = be64_to_cpu(ddqp->d_rtb_softlimit);
> > +		xfs_qm_dqdestroy(dqp);
> > +	}
> > +}
> > +
> >  /*
> >   * This initializes all the quota information that's kept in the
> >   * mount structure
> > @@ -606,27 +637,22 @@ xfs_qm_init_quotainfo(
> >  	 * We try to get the limits from the superuser's limits fields.
> >  	 * This is quite hacky, but it is standard quota practice.
> >  	 *
> > -	 * We look at the USR dquot with id == 0 first, but if user quotas
> > -	 * are not enabled we goto the GRP dquot with id == 0.
> > -	 * We don't really care to keep separate default limits for user
> > -	 * and group quotas, at least not at this point.
> > -	 *
> >  	 * 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 splitted into different fields per quota type.
> 
> s/splitted/split/
> 
> >  	 */
> >  	error = xfs_qm_dqread(mp, 0,
> >  			XFS_IS_UQUOTA_RUNNING(mp) ? XFS_DQ_USER :
> >  			 (XFS_IS_GQUOTA_RUNNING(mp) ? XFS_DQ_GROUP :
> >  			  XFS_DQ_PROJ),
> >  			XFS_QMOPT_DOWARN, &dqp);
> > +
> >  	if (!error) {
> >  		xfs_disk_dquot_t	*ddqp = &dqp->q_core;
> >  
> > -		/*
> > -		 * 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.
> > -		 */
> 
> Not sure why you removed this comment?
> 
> >  		qinf->qi_btimelimit = ddqp->d_btimer ?
> >  			be32_to_cpu(ddqp->d_btimer) : XFS_QM_BTIMELIMIT;
> >  		qinf->qi_itimelimit = ddqp->d_itimer ?
> > @@ -639,13 +665,6 @@ xfs_qm_init_quotainfo(
> >  			be16_to_cpu(ddqp->d_iwarns) : XFS_QM_IWARNLIMIT;
> >  		qinf->qi_rtbwarnlimit = ddqp->d_rtbwarns ?
> >  			be16_to_cpu(ddqp->d_rtbwarns) : XFS_QM_RTBWARNLIMIT;
> > -		qinf->qi_bhardlimit = be64_to_cpu(ddqp->d_blk_hardlimit);
> > -		qinf->qi_bsoftlimit = be64_to_cpu(ddqp->d_blk_softlimit);
> > -		qinf->qi_ihardlimit = be64_to_cpu(ddqp->d_ino_hardlimit);
> > -		qinf->qi_isoftlimit = be64_to_cpu(ddqp->d_ino_softlimit);
> > -		qinf->qi_rtbhardlimit = be64_to_cpu(ddqp->d_rtb_hardlimit);
> > -		qinf->qi_rtbsoftlimit = be64_to_cpu(ddqp->d_rtb_softlimit);
> > -
> >  		xfs_qm_dqdestroy(dqp);
> >  	} else {
> >  		qinf->qi_btimelimit = XFS_QM_BTIMELIMIT;
> > @@ -656,6 +675,17 @@ xfs_qm_init_quotainfo(
> >  		qinf->qi_rtbwarnlimit = XFS_QM_RTBWARNLIMIT;
> >  	}
> >  
> > +	/*
> > +	 * Default quota values are set by quota type, timer and warn limits
> > +	 * have already been setpreviously, no need for error check here.
> 
> "set previously" - and, not sure the comment about error checking is needed.
> Previously, if we couldn't read the inode, we set timers but no limits.
> And the limits were (and still are) zalloc'd so it's safe to ignore
> the errors, I guess.
> 
> 
> > +	 */
> > +	if (XFS_IS_UQUOTA_RUNNING(mp))
> > +		xfs_qm_set_defquota(mp, XFS_DQ_USER, qinf);
> > +	if (XFS_IS_GQUOTA_RUNNING(mp))
> > +		xfs_qm_set_defquota(mp, XFS_DQ_GROUP, qinf);
> > +	if (XFS_IS_PQUOTA_RUNNING(mp))
> > +		xfs_qm_set_defquota(mp, XFS_DQ_PROJ, qinf);
> > +
> >  	qinf->qi_shrinker.count_objects = xfs_qm_shrink_count;
> >  	qinf->qi_shrinker.scan_objects = xfs_qm_shrink_scan;
> >  	qinf->qi_shrinker.seeks = DEFAULT_SEEKS;
> > diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
> > index 996a040..45e2c36 100644
> > --- a/fs/xfs/xfs_qm.h
> > +++ b/fs/xfs/xfs_qm.h
> > @@ -53,6 +53,15 @@ 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 */
> > +};
> > +
> >  /*
> >   * Various quota information for individual filesystems.
> >   * The mount structure keeps a pointer to this.
> > @@ -76,12 +85,9 @@ typedef struct xfs_quotainfo {
> >  	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 */
> > -	xfs_qcnt_t	 qi_bhardlimit;	 /* default data blk hard limit */
> > -	xfs_qcnt_t	 qi_bsoftlimit;	 /* default data blk soft limit */
> > -	xfs_qcnt_t	 qi_ihardlimit;	 /* default inode count hard limit */
> > -	xfs_qcnt_t	 qi_isoftlimit;	 /* default inode count soft limit */
> > -	xfs_qcnt_t	 qi_rtbhardlimit;/* default realtime blk hard limit */
> > -	xfs_qcnt_t	 qi_rtbsoftlimit;/* default realtime blk soft limit */
> > +	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;
> >  } xfs_quotainfo_t;
> >  
> > @@ -171,4 +177,20 @@ extern int		xfs_qm_scall_setqlim(struct xfs_mount *, xfs_dqid_t, uint,
> >  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)
> > +{
> > +	struct xfs_def_quota *defq;
> > +
> > +	if (XFS_QM_ISUDQ(dqp))
> > +		defq = &qi->qi_usr_default;
> > +	else if (XFS_QM_ISGDQ(dqp))
> > +		defq = &qi->qi_grp_default;
> > +	else {
> > +		ASSERT(XFS_QM_ISPDQ(dqp));
> > +		defq = &qi->qi_prj_default;
> > +	}
> > +	return defq;
> > +}
> > +
> >  #endif /* __XFS_QM_H__ */
> > diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> > index 3640c6e..31830f0 100644
> > --- a/fs/xfs/xfs_qm_syscalls.c
> > +++ b/fs/xfs/xfs_qm_syscalls.c
> > @@ -404,6 +404,7 @@ xfs_qm_scall_setqlim(
> >  	struct xfs_disk_dquot	*ddq;
> >  	struct xfs_dquot	*dqp;
> >  	struct xfs_trans	*tp;
> > +	struct xfs_def_quota	*defq;
> >  	int			error;
> >  	xfs_qcnt_t		hard, soft;
> >  
> > @@ -431,6 +432,8 @@ xfs_qm_scall_setqlim(
> >  		ASSERT(error != -ENOENT);
> >  		goto out_unlock;
> >  	}
> > +
> > +	defq = xfs_get_defquota(dqp, q);
> >  	xfs_dqunlock(dqp);
> >  
> >  	tp = xfs_trans_alloc(mp, XFS_TRANS_QM_SETQLIM);
> > @@ -458,8 +461,8 @@ xfs_qm_scall_setqlim(
> >  		ddq->d_blk_softlimit = cpu_to_be64(soft);
> >  		xfs_dquot_set_prealloc_limits(dqp);
> >  		if (id == 0) {
> > -			q->qi_bhardlimit = hard;
> > -			q->qi_bsoftlimit = soft;
> > +			defq->bhardlimit = hard;
> > +			defq->bsoftlimit = soft;
> >  		}
> >  	} else {
> >  		xfs_debug(mp, "blkhard %Ld < blksoft %Ld", hard, soft);
> > @@ -474,8 +477,8 @@ xfs_qm_scall_setqlim(
> >  		ddq->d_rtb_hardlimit = cpu_to_be64(hard);
> >  		ddq->d_rtb_softlimit = cpu_to_be64(soft);
> >  		if (id == 0) {
> > -			q->qi_rtbhardlimit = hard;
> > -			q->qi_rtbsoftlimit = soft;
> > +			defq->rtbhardlimit = hard;
> > +			defq->rtbsoftlimit = soft;
> >  		}
> >  	} else {
> >  		xfs_debug(mp, "rtbhard %Ld < rtbsoft %Ld", hard, soft);
> > @@ -491,8 +494,8 @@ xfs_qm_scall_setqlim(
> >  		ddq->d_ino_hardlimit = cpu_to_be64(hard);
> >  		ddq->d_ino_softlimit = cpu_to_be64(soft);
> >  		if (id == 0) {
> > -			q->qi_ihardlimit = hard;
> > -			q->qi_isoftlimit = soft;
> > +			defq->ihardlimit = hard;
> > +			defq->isoftlimit = soft;
> >  		}
> >  	} else {
> >  		xfs_debug(mp, "ihard %Ld < isoft %Ld", hard, soft);
> > diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> > index 9951701..c3d5472 100644
> > --- a/fs/xfs/xfs_trans_dquot.c
> > +++ b/fs/xfs/xfs_trans_dquot.c
> > @@ -609,17 +609,20 @@ xfs_trans_dqresv(
> >  	xfs_qcnt_t	total_count;
> >  	xfs_qcnt_t	*resbcountp;
> >  	xfs_quotainfo_t	*q = mp->m_quotainfo;
> > +	struct xfs_def_quota	*defq;
> >  
> >  
> >  	xfs_dqlock(dqp);
> >  
> > +	defq = xfs_get_defquota(dqp, q);
> > +
> >  	if (flags & XFS_TRANS_DQ_RES_BLKS) {
> >  		hardlimit = be64_to_cpu(dqp->q_core.d_blk_hardlimit);
> >  		if (!hardlimit)
> > -			hardlimit = q->qi_bhardlimit;
> > +			hardlimit = defq->bhardlimit;
> >  		softlimit = be64_to_cpu(dqp->q_core.d_blk_softlimit);
> >  		if (!softlimit)
> > -			softlimit = q->qi_bsoftlimit;
> > +			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;
> > @@ -628,10 +631,10 @@ xfs_trans_dqresv(
> >  		ASSERT(flags & XFS_TRANS_DQ_RES_RTBLKS);
> >  		hardlimit = be64_to_cpu(dqp->q_core.d_rtb_hardlimit);
> >  		if (!hardlimit)
> > -			hardlimit = q->qi_rtbhardlimit;
> > +			hardlimit = defq->rtbhardlimit;
> >  		softlimit = be64_to_cpu(dqp->q_core.d_rtb_softlimit);
> >  		if (!softlimit)
> > -			softlimit = q->qi_rtbsoftlimit;
> > +			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;
> > @@ -672,10 +675,10 @@ xfs_trans_dqresv(
> >  			warnlimit = dqp->q_mount->m_quotainfo->qi_iwarnlimit;
> >  			hardlimit = be64_to_cpu(dqp->q_core.d_ino_hardlimit);
> >  			if (!hardlimit)
> > -				hardlimit = q->qi_ihardlimit;
> > +				hardlimit = defq->ihardlimit;
> >  			softlimit = be64_to_cpu(dqp->q_core.d_ino_softlimit);
> >  			if (!softlimit)
> > -				softlimit = q->qi_isoftlimit;
> > +				softlimit = defq->isoftlimit;
> >  
> >  			if (hardlimit && total_count > hardlimit) {
> >  				xfs_quota_warn(mp, dqp, QUOTA_NL_IHARDWARN);
> > 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs: Split default quota limits by quota type V3
  2016-01-28 17:36 ` Eric Sandeen
  2016-02-01  9:42   ` Carlos Maiolino
@ 2016-02-01  9:57   ` Carlos Maiolino
  2016-02-01 21:22     ` Dave Chinner
  1 sibling, 1 reply; 6+ messages in thread
From: Carlos Maiolino @ 2016-02-01  9:57 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: xfs

On Thu, Jan 28, 2016 at 11:36:55AM -0600, Eric Sandeen wrote:
> On 1/28/16 10:57 AM, Carlos Maiolino wrote:
> > Default quotas are globally set due historical reasons. IRIX only supported user
> > and project quotas, and default quota was only applied to user quotas.
> > 
> > In Linux, when a default quota is set, all different quota types inherits the
> > same default value.
> > 
> > An user with a quota limit larger than the default quota value, will still be
> > limited to the default value because the group quotas also inherits the default
> > quotas. Unless the group which the user belongs to have a custom quota limit
> > set.
> > 
> > This patch aims to split the default quota value by quota type. Allowing each
> > quota type having different default values.
> > 
> > Default time limits are still set globally. XFS does not set a per-user/group
> > timer, but a single global timer. For changing this behavior, some changes
> > should be made in user-space tools another bugs being fixed.
> 
> Some minor comments about comments below.  But the code looks fine to me,
> so:
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> 
> and if you want to fix the comments & resend w/ my review tag I think that'd
> be ok.
> 
> Care to write an xfstests testcase for the default quota limit while you're
> at it?
> 
> Thanks,
> -Eric
> 
> > Changelog:
> > 
> > 	V2 - Remove comment about old behavior
> > 	V3 - Keep time limit configuration inside xfs_qm_init_quotainfo to keep
> > 	     the current behavior of quota timers
> > 
> > +	 *
> > +	 * 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 splitted into different fields per quota type.
> 
> s/splitted/split/
> 
> >  	 */
> >  	error = xfs_qm_dqread(mp, 0,
> >  			XFS_IS_UQUOTA_RUNNING(mp) ? XFS_DQ_USER :
> >  			 (XFS_IS_GQUOTA_RUNNING(mp) ? XFS_DQ_GROUP :
> >  			  XFS_DQ_PROJ),
> >  			XFS_QMOPT_DOWARN, &dqp);
> > +
> >  	if (!error) {
> >  		xfs_disk_dquot_t	*ddqp = &dqp->q_core;
> >  
> > -		/*
> > -		 * 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.
> > -		 */
> 
> Not sure why you removed this comment?
> 
Regarding this comment, I think it's just redundant with the previous comment, I
left it in the code for the V4 though, I believe Dave can rip it off if it is
really redundant.

Cheers

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs: Split default quota limits by quota type V3
  2016-02-01  9:57   ` Carlos Maiolino
@ 2016-02-01 21:22     ` Dave Chinner
  2016-02-02 14:22       ` Carlos Maiolino
  0 siblings, 1 reply; 6+ messages in thread
From: Dave Chinner @ 2016-02-01 21:22 UTC (permalink / raw)
  To: Eric Sandeen, xfs

On Mon, Feb 01, 2016 at 10:57:07AM +0100, Carlos Maiolino wrote:
> On Thu, Jan 28, 2016 at 11:36:55AM -0600, Eric Sandeen wrote:
> > On 1/28/16 10:57 AM, Carlos Maiolino wrote:
> > >  		xfs_disk_dquot_t	*ddqp = &dqp->q_core;
> > >  
> > > -		/*
> > > -		 * 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.
> > > -		 */
> > 
> > Not sure why you removed this comment?
> > 
> Regarding this comment, I think it's just redundant with the previous comment, I
> left it in the code for the V4 though, I believe Dave can rip it off if it is
> really redundant.

No, please don't ask me to do make random patch edits on commit -
it's your job as a patch submitter to get it right. I will sometimes
do this for simple patches or patches that come from a
never-seen-before-and-never-seen-again patch submitter, but for a
regular developer submitting non-trivial patches I won't change the
patches because the issue hasn't been sorted out during peer review.

Work it out with Eric, get a reviewed-by tag, and I'll apply it as
reviewed.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs: Split default quota limits by quota type V3
  2016-02-01 21:22     ` Dave Chinner
@ 2016-02-02 14:22       ` Carlos Maiolino
  0 siblings, 0 replies; 6+ messages in thread
From: Carlos Maiolino @ 2016-02-02 14:22 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Eric Sandeen, xfs

On Tue, Feb 02, 2016 at 08:22:38AM +1100, Dave Chinner wrote:
> On Mon, Feb 01, 2016 at 10:57:07AM +0100, Carlos Maiolino wrote:
> > On Thu, Jan 28, 2016 at 11:36:55AM -0600, Eric Sandeen wrote:
> > > On 1/28/16 10:57 AM, Carlos Maiolino wrote:
> > > >  		xfs_disk_dquot_t	*ddqp = &dqp->q_core;
> > > >  
> > > > -		/*
> > > > -		 * 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.
> > > > -		 */
> > > 
> > > Not sure why you removed this comment?
> > > 
> > Regarding this comment, I think it's just redundant with the previous comment, I
> > left it in the code for the V4 though, I believe Dave can rip it off if it is
> > really redundant.
> 
> No, please don't ask me to do make random patch edits on commit -
> it's your job as a patch submitter to get it right. I will sometimes
> do this for simple patches or patches that come from a
> never-seen-before-and-never-seen-again patch submitter, but for a
> regular developer submitting non-trivial patches I won't change the
> patches because the issue hasn't been sorted out during peer review.
> 
Hey, I apologize, I had no intention to give you any extra job :) I meant that
in case you agreed or not with the comment, I thought you could decide by
ripping the comment off or not, I had no intention to actually give you some
unfinished job.

In any case, I sent a V4 of this patch with the comment's change Eric suggested,
adding his review-by too, although, a quick look from him again would be nice,
I'll figure it out with him.

cheers.


> Work it out with Eric, get a reviewed-by tag, and I'll apply it as
> reviewed.
> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-02-02 14:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-28 16:57 [PATCH] xfs: Split default quota limits by quota type V3 Carlos Maiolino
2016-01-28 17:36 ` Eric Sandeen
2016-02-01  9:42   ` Carlos Maiolino
2016-02-01  9:57   ` Carlos Maiolino
2016-02-01 21:22     ` Dave Chinner
2016-02-02 14:22       ` Carlos Maiolino

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox