* [PATCH v2 0/3] xfs: remove quota warnings
@ 2022-05-05 1:18 Catherine Hoang
2022-05-05 1:18 ` [PATCH v2 1/3] xfs: remove quota warning limit from struct xfs_quota_limits Catherine Hoang
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Catherine Hoang @ 2022-05-05 1:18 UTC (permalink / raw
To: linux-xfs
Hi all,
Based on recent discussion, it seems like there is a consensus that quota
warnings should be removed from xfs.
Warnings in xfs quota is an unused feature that is currently documented as
unimplemented. These patches remove the quota warnings and cleans up any
related code.
v1->v2:
- Added patch 2 to remove warning counters
- Prevent warning fields from being set on all dquots
Future patches will also remove warnings from the VFS code, since that
seems like the logical next step.
Comments and feedback are appreciated!
Catherine
Catherine Hoang (3):
xfs: remove quota warning limit from struct xfs_quota_limits
xfs: remove warning counters from struct xfs_dquot_res
xfs: don't set warns on dquots
fs/xfs/libxfs/xfs_quota_defs.h | 1 -
fs/xfs/xfs_dquot.c | 15 ++++-----------
fs/xfs/xfs_dquot.h | 8 --------
fs/xfs/xfs_qm.c | 9 ---------
fs/xfs/xfs_qm.h | 5 -----
fs/xfs/xfs_qm_syscalls.c | 25 +++++--------------------
fs/xfs/xfs_quotaops.c | 6 +++---
fs/xfs/xfs_trans_dquot.c | 3 +--
8 files changed, 13 insertions(+), 59 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] xfs: remove quota warning limit from struct xfs_quota_limits
2022-05-05 1:18 [PATCH v2 0/3] xfs: remove quota warnings Catherine Hoang
@ 2022-05-05 1:18 ` Catherine Hoang
2022-05-05 1:18 ` [PATCH v2 2/3] xfs: remove warning counters from struct xfs_dquot_res Catherine Hoang
2022-05-05 1:18 ` [PATCH v2 3/3] xfs: don't set warns on dquots Catherine Hoang
2 siblings, 0 replies; 9+ messages in thread
From: Catherine Hoang @ 2022-05-05 1:18 UTC (permalink / raw
To: linux-xfs
Warning limits in xfs quota is an unused feature that is currently
documented as unimplemented, and it is unclear what the intended behavior
of these limits are. Remove the ‘warn’ field from struct xfs_quota_limits
and any other related code.
Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/xfs_qm.c | 9 ---------
fs/xfs/xfs_qm.h | 5 -----
fs/xfs/xfs_qm_syscalls.c | 17 +++--------------
fs/xfs/xfs_quotaops.c | 6 +++---
fs/xfs/xfs_trans_dquot.c | 3 +--
5 files changed, 7 insertions(+), 33 deletions(-)
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index f165d1a3de1d..8fc813cb6011 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -582,9 +582,6 @@ xfs_qm_init_timelimits(
defq->blk.time = XFS_QM_BTIMELIMIT;
defq->ino.time = XFS_QM_ITIMELIMIT;
defq->rtb.time = XFS_QM_RTBTIMELIMIT;
- defq->blk.warn = XFS_QM_BWARNLIMIT;
- defq->ino.warn = XFS_QM_IWARNLIMIT;
- defq->rtb.warn = XFS_QM_RTBWARNLIMIT;
/*
* We try to get the limits from the superuser's limits fields.
@@ -608,12 +605,6 @@ xfs_qm_init_timelimits(
defq->ino.time = dqp->q_ino.timer;
if (dqp->q_rtb.timer)
defq->rtb.time = dqp->q_rtb.timer;
- if (dqp->q_blk.warnings)
- defq->blk.warn = dqp->q_blk.warnings;
- if (dqp->q_ino.warnings)
- defq->ino.warn = dqp->q_ino.warnings;
- if (dqp->q_rtb.warnings)
- defq->rtb.warn = dqp->q_rtb.warnings;
xfs_qm_dqdestroy(dqp);
}
diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
index 5bb12717ea28..9683f0457d19 100644
--- a/fs/xfs/xfs_qm.h
+++ b/fs/xfs/xfs_qm.h
@@ -34,7 +34,6 @@ struct xfs_quota_limits {
xfs_qcnt_t hard; /* default hard limit */
xfs_qcnt_t soft; /* default soft limit */
time64_t time; /* limit for timers */
- xfs_qwarncnt_t warn; /* limit for warnings */
};
/* Defaults for each quota type: time limits, warn limits, usage limits */
@@ -134,10 +133,6 @@ struct xfs_dquot_acct {
#define XFS_QM_RTBTIMELIMIT (7 * 24*60*60) /* 1 week */
#define XFS_QM_ITIMELIMIT (7 * 24*60*60) /* 1 week */
-#define XFS_QM_BWARNLIMIT 5
-#define XFS_QM_IWARNLIMIT 5
-#define XFS_QM_RTBWARNLIMIT 5
-
extern void xfs_qm_destroy_quotainfo(struct xfs_mount *);
/* quota ops */
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 7d5a31827681..e7f3ac60ebd9 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -250,17 +250,6 @@ xfs_setqlim_limits(
return true;
}
-static inline void
-xfs_setqlim_warns(
- struct xfs_dquot_res *res,
- struct xfs_quota_limits *qlim,
- int warns)
-{
- res->warnings = warns;
- if (qlim)
- qlim->warn = warns;
-}
-
static inline void
xfs_setqlim_timer(
struct xfs_mount *mp,
@@ -355,7 +344,7 @@ xfs_qm_scall_setqlim(
if (xfs_setqlim_limits(mp, res, qlim, hard, soft, "blk"))
xfs_dquot_set_prealloc_limits(dqp);
if (newlim->d_fieldmask & QC_SPC_WARNS)
- xfs_setqlim_warns(res, qlim, newlim->d_spc_warns);
+ res->warnings = newlim->d_spc_warns;
if (newlim->d_fieldmask & QC_SPC_TIMER)
xfs_setqlim_timer(mp, res, qlim, newlim->d_spc_timer);
@@ -371,7 +360,7 @@ xfs_qm_scall_setqlim(
xfs_setqlim_limits(mp, res, qlim, hard, soft, "rtb");
if (newlim->d_fieldmask & QC_RT_SPC_WARNS)
- xfs_setqlim_warns(res, qlim, newlim->d_rt_spc_warns);
+ res->warnings = newlim->d_rt_spc_warns;
if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
xfs_setqlim_timer(mp, res, qlim, newlim->d_rt_spc_timer);
@@ -387,7 +376,7 @@ xfs_qm_scall_setqlim(
xfs_setqlim_limits(mp, res, qlim, hard, soft, "ino");
if (newlim->d_fieldmask & QC_INO_WARNS)
- xfs_setqlim_warns(res, qlim, newlim->d_ino_warns);
+ res->warnings = newlim->d_ino_warns;
if (newlim->d_fieldmask & QC_INO_TIMER)
xfs_setqlim_timer(mp, res, qlim, newlim->d_ino_timer);
diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c
index 07989bd67728..50391730241f 100644
--- a/fs/xfs/xfs_quotaops.c
+++ b/fs/xfs/xfs_quotaops.c
@@ -40,9 +40,9 @@ xfs_qm_fill_state(
tstate->spc_timelimit = (u32)defq->blk.time;
tstate->ino_timelimit = (u32)defq->ino.time;
tstate->rt_spc_timelimit = (u32)defq->rtb.time;
- tstate->spc_warnlimit = defq->blk.warn;
- tstate->ino_warnlimit = defq->ino.warn;
- tstate->rt_spc_warnlimit = defq->rtb.warn;
+ tstate->spc_warnlimit = 0;
+ tstate->ino_warnlimit = 0;
+ tstate->rt_spc_warnlimit = 0;
if (tempqip)
xfs_irele(ip);
}
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index ebe2c227eb2f..aa00cf67ad72 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -597,8 +597,7 @@ xfs_dqresv_check(
if (softlimit && total_count > softlimit) {
time64_t now = ktime_get_real_seconds();
- if ((res->timer != 0 && now > res->timer) ||
- (res->warnings != 0 && res->warnings >= qlim->warn)) {
+ if (res->timer != 0 && now > res->timer) {
*fatal = true;
return QUOTA_NL_ISOFTLONGWARN;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] xfs: remove warning counters from struct xfs_dquot_res
2022-05-05 1:18 [PATCH v2 0/3] xfs: remove quota warnings Catherine Hoang
2022-05-05 1:18 ` [PATCH v2 1/3] xfs: remove quota warning limit from struct xfs_quota_limits Catherine Hoang
@ 2022-05-05 1:18 ` Catherine Hoang
2022-05-05 16:19 ` Darrick J. Wong
2022-05-05 17:45 ` Alli
2022-05-05 1:18 ` [PATCH v2 3/3] xfs: don't set warns on dquots Catherine Hoang
2 siblings, 2 replies; 9+ messages in thread
From: Catherine Hoang @ 2022-05-05 1:18 UTC (permalink / raw
To: linux-xfs
Warning counts are not used anywhere in the kernel. In addition, there
are no use cases, test coverage, or documentation for this
functionality. Remove the 'warnings' field from struct xfs_dquot_res and
any other related code.
Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
---
fs/xfs/libxfs/xfs_quota_defs.h | 1 -
fs/xfs/xfs_dquot.c | 15 ++++-----------
fs/xfs/xfs_dquot.h | 8 --------
fs/xfs/xfs_qm_syscalls.c | 12 +++---------
4 files changed, 7 insertions(+), 29 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index a02c5062f9b2..c1e96abefed2 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -16,7 +16,6 @@
* and quota-limits. This is a waste in the common case, but hey ...
*/
typedef uint64_t xfs_qcnt_t;
-typedef uint16_t xfs_qwarncnt_t;
typedef uint8_t xfs_dqtype_t;
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 5afedcbc78c7..aff727ba603f 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -136,10 +136,7 @@ xfs_qm_adjust_res_timer(
res->timer = xfs_dquot_set_timeout(mp,
ktime_get_real_seconds() + qlim->time);
} else {
- if (res->timer == 0)
- res->warnings = 0;
- else
- res->timer = 0;
+ res->timer = 0;
}
}
@@ -589,10 +586,6 @@ xfs_dquot_from_disk(
dqp->q_ino.count = be64_to_cpu(ddqp->d_icount);
dqp->q_rtb.count = be64_to_cpu(ddqp->d_rtbcount);
- dqp->q_blk.warnings = be16_to_cpu(ddqp->d_bwarns);
- dqp->q_ino.warnings = be16_to_cpu(ddqp->d_iwarns);
- dqp->q_rtb.warnings = be16_to_cpu(ddqp->d_rtbwarns);
-
dqp->q_blk.timer = xfs_dquot_from_disk_ts(ddqp, ddqp->d_btimer);
dqp->q_ino.timer = xfs_dquot_from_disk_ts(ddqp, ddqp->d_itimer);
dqp->q_rtb.timer = xfs_dquot_from_disk_ts(ddqp, ddqp->d_rtbtimer);
@@ -634,9 +627,9 @@ xfs_dquot_to_disk(
ddqp->d_icount = cpu_to_be64(dqp->q_ino.count);
ddqp->d_rtbcount = cpu_to_be64(dqp->q_rtb.count);
- ddqp->d_bwarns = cpu_to_be16(dqp->q_blk.warnings);
- ddqp->d_iwarns = cpu_to_be16(dqp->q_ino.warnings);
- ddqp->d_rtbwarns = cpu_to_be16(dqp->q_rtb.warnings);
+ ddqp->d_bwarns = 0;
+ ddqp->d_iwarns = 0;
+ ddqp->d_rtbwarns = 0;
ddqp->d_btimer = xfs_dquot_to_disk_ts(dqp, dqp->q_blk.timer);
ddqp->d_itimer = xfs_dquot_to_disk_ts(dqp, dqp->q_ino.timer);
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 6b5e3cf40c8b..80c8f851a2f3 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -44,14 +44,6 @@ struct xfs_dquot_res {
* in seconds since the Unix epoch.
*/
time64_t timer;
-
- /*
- * For root dquots, this is the maximum number of warnings that will
- * be issued for this quota type. Otherwise, this is the number of
- * warnings issued against this quota. Note that none of this is
- * implemented.
- */
- xfs_qwarncnt_t warnings;
};
static inline bool
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index e7f3ac60ebd9..2149c203b1d0 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -343,8 +343,6 @@ xfs_qm_scall_setqlim(
if (xfs_setqlim_limits(mp, res, qlim, hard, soft, "blk"))
xfs_dquot_set_prealloc_limits(dqp);
- if (newlim->d_fieldmask & QC_SPC_WARNS)
- res->warnings = newlim->d_spc_warns;
if (newlim->d_fieldmask & QC_SPC_TIMER)
xfs_setqlim_timer(mp, res, qlim, newlim->d_spc_timer);
@@ -359,8 +357,6 @@ xfs_qm_scall_setqlim(
qlim = id == 0 ? &defq->rtb : NULL;
xfs_setqlim_limits(mp, res, qlim, hard, soft, "rtb");
- if (newlim->d_fieldmask & QC_RT_SPC_WARNS)
- res->warnings = newlim->d_rt_spc_warns;
if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
xfs_setqlim_timer(mp, res, qlim, newlim->d_rt_spc_timer);
@@ -375,8 +371,6 @@ xfs_qm_scall_setqlim(
qlim = id == 0 ? &defq->ino : NULL;
xfs_setqlim_limits(mp, res, qlim, hard, soft, "ino");
- if (newlim->d_fieldmask & QC_INO_WARNS)
- res->warnings = newlim->d_ino_warns;
if (newlim->d_fieldmask & QC_INO_TIMER)
xfs_setqlim_timer(mp, res, qlim, newlim->d_ino_timer);
@@ -417,13 +411,13 @@ xfs_qm_scall_getquota_fill_qc(
dst->d_ino_count = dqp->q_ino.reserved;
dst->d_spc_timer = dqp->q_blk.timer;
dst->d_ino_timer = dqp->q_ino.timer;
- dst->d_ino_warns = dqp->q_ino.warnings;
- dst->d_spc_warns = dqp->q_blk.warnings;
+ dst->d_ino_warns = 0;
+ dst->d_spc_warns = 0;
dst->d_rt_spc_hardlimit = XFS_FSB_TO_B(mp, dqp->q_rtb.hardlimit);
dst->d_rt_spc_softlimit = XFS_FSB_TO_B(mp, dqp->q_rtb.softlimit);
dst->d_rt_space = XFS_FSB_TO_B(mp, dqp->q_rtb.reserved);
dst->d_rt_spc_timer = dqp->q_rtb.timer;
- dst->d_rt_spc_warns = dqp->q_rtb.warnings;
+ dst->d_rt_spc_warns = 0;
/*
* Internally, we don't reset all the timers when quota enforcement
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] xfs: don't set warns on dquots
2022-05-05 1:18 [PATCH v2 0/3] xfs: remove quota warnings Catherine Hoang
2022-05-05 1:18 ` [PATCH v2 1/3] xfs: remove quota warning limit from struct xfs_quota_limits Catherine Hoang
2022-05-05 1:18 ` [PATCH v2 2/3] xfs: remove warning counters from struct xfs_dquot_res Catherine Hoang
@ 2022-05-05 1:18 ` Catherine Hoang
2022-05-05 16:17 ` Darrick J. Wong
2022-05-05 17:44 ` Alli
2 siblings, 2 replies; 9+ messages in thread
From: Catherine Hoang @ 2022-05-05 1:18 UTC (permalink / raw
To: linux-xfs
Having just dropped support for quota warning limits and warning counters,
the warning fields no longer have any meaning. Return -EINVAL if the
fieldmask has any of the QC_*_WARNS set.
Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/xfs_qm_syscalls.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 2149c203b1d0..188e1fed2eba 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -290,6 +290,8 @@ xfs_qm_scall_setqlim(
return -EINVAL;
if ((newlim->d_fieldmask & XFS_QC_MASK) == 0)
return 0;
+ if (newlim->d_fieldmask & QC_WARNS_MASK)
+ return -EINVAL;
/*
* Get the dquot (locked) before we start, as we need to do a
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] xfs: don't set warns on dquots
2022-05-05 1:18 ` [PATCH v2 3/3] xfs: don't set warns on dquots Catherine Hoang
@ 2022-05-05 16:17 ` Darrick J. Wong
2022-05-05 20:48 ` Catherine Hoang
2022-05-05 17:44 ` Alli
1 sibling, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2022-05-05 16:17 UTC (permalink / raw
To: Catherine Hoang; +Cc: linux-xfs
On Wed, May 04, 2022 at 06:18:15PM -0700, Catherine Hoang wrote:
> Having just dropped support for quota warning limits and warning counters,
> the warning fields no longer have any meaning. Return -EINVAL if the
> fieldmask has any of the QC_*_WARNS set.
>
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Slight nit I noticed in this patch: I think you should remove
QC_WARNS_MASK from XFS_QC_SETINFO_MASK and XFS_QC_MASK. The first will
block the setting of warning counters from quotactl_ops.set_info, and
the second blocks it from quotactl_ops.set_dqblk. With both those
#defins updated, I think you don't need the change below.
--D
> ---
> fs/xfs/xfs_qm_syscalls.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index 2149c203b1d0..188e1fed2eba 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -290,6 +290,8 @@ xfs_qm_scall_setqlim(
> return -EINVAL;
> if ((newlim->d_fieldmask & XFS_QC_MASK) == 0)
> return 0;
> + if (newlim->d_fieldmask & QC_WARNS_MASK)
> + return -EINVAL;
>
> /*
> * Get the dquot (locked) before we start, as we need to do a
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] xfs: remove warning counters from struct xfs_dquot_res
2022-05-05 1:18 ` [PATCH v2 2/3] xfs: remove warning counters from struct xfs_dquot_res Catherine Hoang
@ 2022-05-05 16:19 ` Darrick J. Wong
2022-05-05 17:45 ` Alli
1 sibling, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2022-05-05 16:19 UTC (permalink / raw
To: Catherine Hoang; +Cc: linux-xfs
On Wed, May 04, 2022 at 06:18:14PM -0700, Catherine Hoang wrote:
> Warning counts are not used anywhere in the kernel. In addition, there
> are no use cases, test coverage, or documentation for this
> functionality. Remove the 'warnings' field from struct xfs_dquot_res and
> any other related code.
>
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
> ---
> fs/xfs/libxfs/xfs_quota_defs.h | 1 -
> fs/xfs/xfs_dquot.c | 15 ++++-----------
> fs/xfs/xfs_dquot.h | 8 --------
> fs/xfs/xfs_qm_syscalls.c | 12 +++---------
> 4 files changed, 7 insertions(+), 29 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
> index a02c5062f9b2..c1e96abefed2 100644
> --- a/fs/xfs/libxfs/xfs_quota_defs.h
> +++ b/fs/xfs/libxfs/xfs_quota_defs.h
> @@ -16,7 +16,6 @@
> * and quota-limits. This is a waste in the common case, but hey ...
> */
> typedef uint64_t xfs_qcnt_t;
> -typedef uint16_t xfs_qwarncnt_t;
>
> typedef uint8_t xfs_dqtype_t;
>
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 5afedcbc78c7..aff727ba603f 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -136,10 +136,7 @@ xfs_qm_adjust_res_timer(
> res->timer = xfs_dquot_set_timeout(mp,
> ktime_get_real_seconds() + qlim->time);
> } else {
> - if (res->timer == 0)
> - res->warnings = 0;
> - else
> - res->timer = 0;
> + res->timer = 0;
> }
> }
>
> @@ -589,10 +586,6 @@ xfs_dquot_from_disk(
> dqp->q_ino.count = be64_to_cpu(ddqp->d_icount);
> dqp->q_rtb.count = be64_to_cpu(ddqp->d_rtbcount);
>
> - dqp->q_blk.warnings = be16_to_cpu(ddqp->d_bwarns);
> - dqp->q_ino.warnings = be16_to_cpu(ddqp->d_iwarns);
> - dqp->q_rtb.warnings = be16_to_cpu(ddqp->d_rtbwarns);
> -
> dqp->q_blk.timer = xfs_dquot_from_disk_ts(ddqp, ddqp->d_btimer);
> dqp->q_ino.timer = xfs_dquot_from_disk_ts(ddqp, ddqp->d_itimer);
> dqp->q_rtb.timer = xfs_dquot_from_disk_ts(ddqp, ddqp->d_rtbtimer);
> @@ -634,9 +627,9 @@ xfs_dquot_to_disk(
> ddqp->d_icount = cpu_to_be64(dqp->q_ino.count);
> ddqp->d_rtbcount = cpu_to_be64(dqp->q_rtb.count);
>
> - ddqp->d_bwarns = cpu_to_be16(dqp->q_blk.warnings);
> - ddqp->d_iwarns = cpu_to_be16(dqp->q_ino.warnings);
> - ddqp->d_rtbwarns = cpu_to_be16(dqp->q_rtb.warnings);
> + ddqp->d_bwarns = 0;
> + ddqp->d_iwarns = 0;
> + ddqp->d_rtbwarns = 0;
Indenting damage here.
>
> ddqp->d_btimer = xfs_dquot_to_disk_ts(dqp, dqp->q_blk.timer);
> ddqp->d_itimer = xfs_dquot_to_disk_ts(dqp, dqp->q_ino.timer);
> diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
> index 6b5e3cf40c8b..80c8f851a2f3 100644
> --- a/fs/xfs/xfs_dquot.h
> +++ b/fs/xfs/xfs_dquot.h
> @@ -44,14 +44,6 @@ struct xfs_dquot_res {
> * in seconds since the Unix epoch.
> */
> time64_t timer;
> -
> - /*
> - * For root dquots, this is the maximum number of warnings that will
> - * be issued for this quota type. Otherwise, this is the number of
> - * warnings issued against this quota. Note that none of this is
> - * implemented.
> - */
> - xfs_qwarncnt_t warnings;
> };
>
> static inline bool
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index e7f3ac60ebd9..2149c203b1d0 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -343,8 +343,6 @@ xfs_qm_scall_setqlim(
>
> if (xfs_setqlim_limits(mp, res, qlim, hard, soft, "blk"))
> xfs_dquot_set_prealloc_limits(dqp);
> - if (newlim->d_fieldmask & QC_SPC_WARNS)
> - res->warnings = newlim->d_spc_warns;
Ok, so in this patch I guess setting warning counters is a soft fail, in
that you tell the kernel to store '5' and an immediate re-read returns
'0'. I think that's not an ABI break since two programs racing to set
'5' and '0' could see the same behavior.
(So it's the next patch where we actually introduce the hard failure, if
I'm reading this all correctly.)
With the indentation corrected,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> if (newlim->d_fieldmask & QC_SPC_TIMER)
> xfs_setqlim_timer(mp, res, qlim, newlim->d_spc_timer);
>
> @@ -359,8 +357,6 @@ xfs_qm_scall_setqlim(
> qlim = id == 0 ? &defq->rtb : NULL;
>
> xfs_setqlim_limits(mp, res, qlim, hard, soft, "rtb");
> - if (newlim->d_fieldmask & QC_RT_SPC_WARNS)
> - res->warnings = newlim->d_rt_spc_warns;
> if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
> xfs_setqlim_timer(mp, res, qlim, newlim->d_rt_spc_timer);
>
> @@ -375,8 +371,6 @@ xfs_qm_scall_setqlim(
> qlim = id == 0 ? &defq->ino : NULL;
>
> xfs_setqlim_limits(mp, res, qlim, hard, soft, "ino");
> - if (newlim->d_fieldmask & QC_INO_WARNS)
> - res->warnings = newlim->d_ino_warns;
> if (newlim->d_fieldmask & QC_INO_TIMER)
> xfs_setqlim_timer(mp, res, qlim, newlim->d_ino_timer);
>
> @@ -417,13 +411,13 @@ xfs_qm_scall_getquota_fill_qc(
> dst->d_ino_count = dqp->q_ino.reserved;
> dst->d_spc_timer = dqp->q_blk.timer;
> dst->d_ino_timer = dqp->q_ino.timer;
> - dst->d_ino_warns = dqp->q_ino.warnings;
> - dst->d_spc_warns = dqp->q_blk.warnings;
> + dst->d_ino_warns = 0;
> + dst->d_spc_warns = 0;
> dst->d_rt_spc_hardlimit = XFS_FSB_TO_B(mp, dqp->q_rtb.hardlimit);
> dst->d_rt_spc_softlimit = XFS_FSB_TO_B(mp, dqp->q_rtb.softlimit);
> dst->d_rt_space = XFS_FSB_TO_B(mp, dqp->q_rtb.reserved);
> dst->d_rt_spc_timer = dqp->q_rtb.timer;
> - dst->d_rt_spc_warns = dqp->q_rtb.warnings;
> + dst->d_rt_spc_warns = 0;
>
> /*
> * Internally, we don't reset all the timers when quota enforcement
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] xfs: don't set warns on dquots
2022-05-05 1:18 ` [PATCH v2 3/3] xfs: don't set warns on dquots Catherine Hoang
2022-05-05 16:17 ` Darrick J. Wong
@ 2022-05-05 17:44 ` Alli
1 sibling, 0 replies; 9+ messages in thread
From: Alli @ 2022-05-05 17:44 UTC (permalink / raw
To: Catherine Hoang, linux-xfs
On Wed, 2022-05-04 at 18:18 -0700, Catherine Hoang wrote:
> Having just dropped support for quota warning limits and warning
> counters,
> the warning fields no longer have any meaning. Return -EINVAL if the
> fieldmask has any of the QC_*_WARNS set.
>
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
I think it's a move in the right direction. Thanks Catherine!
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
> ---
> fs/xfs/xfs_qm_syscalls.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index 2149c203b1d0..188e1fed2eba 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -290,6 +290,8 @@ xfs_qm_scall_setqlim(
> return -EINVAL;
> if ((newlim->d_fieldmask & XFS_QC_MASK) == 0)
> return 0;
> + if (newlim->d_fieldmask & QC_WARNS_MASK)
> + return -EINVAL;
>
> /*
> * Get the dquot (locked) before we start, as we need to do a
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] xfs: remove warning counters from struct xfs_dquot_res
2022-05-05 1:18 ` [PATCH v2 2/3] xfs: remove warning counters from struct xfs_dquot_res Catherine Hoang
2022-05-05 16:19 ` Darrick J. Wong
@ 2022-05-05 17:45 ` Alli
1 sibling, 0 replies; 9+ messages in thread
From: Alli @ 2022-05-05 17:45 UTC (permalink / raw
To: Catherine Hoang, linux-xfs
On Wed, 2022-05-04 at 18:18 -0700, Catherine Hoang wrote:
> Warning counts are not used anywhere in the kernel. In addition,
> there
> are no use cases, test coverage, or documentation for this
> functionality. Remove the 'warnings' field from struct xfs_dquot_res
> and
> any other related code.
>
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
White space nits aside, I think this looks good
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
> ---
> fs/xfs/libxfs/xfs_quota_defs.h | 1 -
> fs/xfs/xfs_dquot.c | 15 ++++-----------
> fs/xfs/xfs_dquot.h | 8 --------
> fs/xfs/xfs_qm_syscalls.c | 12 +++---------
> 4 files changed, 7 insertions(+), 29 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_quota_defs.h
> b/fs/xfs/libxfs/xfs_quota_defs.h
> index a02c5062f9b2..c1e96abefed2 100644
> --- a/fs/xfs/libxfs/xfs_quota_defs.h
> +++ b/fs/xfs/libxfs/xfs_quota_defs.h
> @@ -16,7 +16,6 @@
> * and quota-limits. This is a waste in the common case, but hey ...
> */
> typedef uint64_t xfs_qcnt_t;
> -typedef uint16_t xfs_qwarncnt_t;
>
> typedef uint8_t xfs_dqtype_t;
>
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 5afedcbc78c7..aff727ba603f 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -136,10 +136,7 @@ xfs_qm_adjust_res_timer(
> res->timer = xfs_dquot_set_timeout(mp,
> ktime_get_real_seconds() +
> qlim->time);
> } else {
> - if (res->timer == 0)
> - res->warnings = 0;
> - else
> - res->timer = 0;
> + res->timer = 0;
> }
> }
>
> @@ -589,10 +586,6 @@ xfs_dquot_from_disk(
> dqp->q_ino.count = be64_to_cpu(ddqp->d_icount);
> dqp->q_rtb.count = be64_to_cpu(ddqp->d_rtbcount);
>
> - dqp->q_blk.warnings = be16_to_cpu(ddqp->d_bwarns);
> - dqp->q_ino.warnings = be16_to_cpu(ddqp->d_iwarns);
> - dqp->q_rtb.warnings = be16_to_cpu(ddqp->d_rtbwarns);
> -
> dqp->q_blk.timer = xfs_dquot_from_disk_ts(ddqp, ddqp-
> >d_btimer);
> dqp->q_ino.timer = xfs_dquot_from_disk_ts(ddqp, ddqp-
> >d_itimer);
> dqp->q_rtb.timer = xfs_dquot_from_disk_ts(ddqp, ddqp-
> >d_rtbtimer);
> @@ -634,9 +627,9 @@ xfs_dquot_to_disk(
> ddqp->d_icount = cpu_to_be64(dqp->q_ino.count);
> ddqp->d_rtbcount = cpu_to_be64(dqp->q_rtb.count);
>
> - ddqp->d_bwarns = cpu_to_be16(dqp->q_blk.warnings);
> - ddqp->d_iwarns = cpu_to_be16(dqp->q_ino.warnings);
> - ddqp->d_rtbwarns = cpu_to_be16(dqp->q_rtb.warnings);
> + ddqp->d_bwarns = 0;
> + ddqp->d_iwarns = 0;
> + ddqp->d_rtbwarns = 0;
>
> ddqp->d_btimer = xfs_dquot_to_disk_ts(dqp, dqp->q_blk.timer);
> ddqp->d_itimer = xfs_dquot_to_disk_ts(dqp, dqp->q_ino.timer);
> diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
> index 6b5e3cf40c8b..80c8f851a2f3 100644
> --- a/fs/xfs/xfs_dquot.h
> +++ b/fs/xfs/xfs_dquot.h
> @@ -44,14 +44,6 @@ struct xfs_dquot_res {
> * in seconds since the Unix epoch.
> */
> time64_t timer;
> -
> - /*
> - * For root dquots, this is the maximum number of warnings that
> will
> - * be issued for this quota type. Otherwise, this is the
> number of
> - * warnings issued against this quota. Note that none of this
> is
> - * implemented.
> - */
> - xfs_qwarncnt_t warnings;
> };
>
> static inline bool
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index e7f3ac60ebd9..2149c203b1d0 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -343,8 +343,6 @@ xfs_qm_scall_setqlim(
>
> if (xfs_setqlim_limits(mp, res, qlim, hard, soft, "blk"))
> xfs_dquot_set_prealloc_limits(dqp);
> - if (newlim->d_fieldmask & QC_SPC_WARNS)
> - res->warnings = newlim->d_spc_warns;
> if (newlim->d_fieldmask & QC_SPC_TIMER)
> xfs_setqlim_timer(mp, res, qlim, newlim->d_spc_timer);
>
> @@ -359,8 +357,6 @@ xfs_qm_scall_setqlim(
> qlim = id == 0 ? &defq->rtb : NULL;
>
> xfs_setqlim_limits(mp, res, qlim, hard, soft, "rtb");
> - if (newlim->d_fieldmask & QC_RT_SPC_WARNS)
> - res->warnings = newlim->d_rt_spc_warns;
> if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
> xfs_setqlim_timer(mp, res, qlim, newlim-
> >d_rt_spc_timer);
>
> @@ -375,8 +371,6 @@ xfs_qm_scall_setqlim(
> qlim = id == 0 ? &defq->ino : NULL;
>
> xfs_setqlim_limits(mp, res, qlim, hard, soft, "ino");
> - if (newlim->d_fieldmask & QC_INO_WARNS)
> - res->warnings = newlim->d_ino_warns;
> if (newlim->d_fieldmask & QC_INO_TIMER)
> xfs_setqlim_timer(mp, res, qlim, newlim->d_ino_timer);
>
> @@ -417,13 +411,13 @@ xfs_qm_scall_getquota_fill_qc(
> dst->d_ino_count = dqp->q_ino.reserved;
> dst->d_spc_timer = dqp->q_blk.timer;
> dst->d_ino_timer = dqp->q_ino.timer;
> - dst->d_ino_warns = dqp->q_ino.warnings;
> - dst->d_spc_warns = dqp->q_blk.warnings;
> + dst->d_ino_warns = 0;
> + dst->d_spc_warns = 0;
> dst->d_rt_spc_hardlimit = XFS_FSB_TO_B(mp, dqp-
> >q_rtb.hardlimit);
> dst->d_rt_spc_softlimit = XFS_FSB_TO_B(mp, dqp-
> >q_rtb.softlimit);
> dst->d_rt_space = XFS_FSB_TO_B(mp, dqp->q_rtb.reserved);
> dst->d_rt_spc_timer = dqp->q_rtb.timer;
> - dst->d_rt_spc_warns = dqp->q_rtb.warnings;
> + dst->d_rt_spc_warns = 0;
>
> /*
> * Internally, we don't reset all the timers when quota
> enforcement
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] xfs: don't set warns on dquots
2022-05-05 16:17 ` Darrick J. Wong
@ 2022-05-05 20:48 ` Catherine Hoang
0 siblings, 0 replies; 9+ messages in thread
From: Catherine Hoang @ 2022-05-05 20:48 UTC (permalink / raw
To: Darrick J. Wong; +Cc: linux-xfs@vger.kernel.org
> On May 5, 2022, at 9:17 AM, Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Wed, May 04, 2022 at 06:18:15PM -0700, Catherine Hoang wrote:
>> Having just dropped support for quota warning limits and warning counters,
>> the warning fields no longer have any meaning. Return -EINVAL if the
>> fieldmask has any of the QC_*_WARNS set.
>>
>> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
>> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
>
> Slight nit I noticed in this patch: I think you should remove
> QC_WARNS_MASK from XFS_QC_SETINFO_MASK and XFS_QC_MASK. The first will
> block the setting of warning counters from quotactl_ops.set_info, and
> the second blocks it from quotactl_ops.set_dqblk. With both those
> #defins updated, I think you don't need the change below.
>
> --D
Ok, that makes sense to me. I’ll add those changes, thanks!
Catherine
>
>> ---
>> fs/xfs/xfs_qm_syscalls.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
>> index 2149c203b1d0..188e1fed2eba 100644
>> --- a/fs/xfs/xfs_qm_syscalls.c
>> +++ b/fs/xfs/xfs_qm_syscalls.c
>> @@ -290,6 +290,8 @@ xfs_qm_scall_setqlim(
>> return -EINVAL;
>> if ((newlim->d_fieldmask & XFS_QC_MASK) == 0)
>> return 0;
>> + if (newlim->d_fieldmask & QC_WARNS_MASK)
>> + return -EINVAL;
>>
>> /*
>> * Get the dquot (locked) before we start, as we need to do a
>> --
>> 2.27.0
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-05-05 20:48 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-05 1:18 [PATCH v2 0/3] xfs: remove quota warnings Catherine Hoang
2022-05-05 1:18 ` [PATCH v2 1/3] xfs: remove quota warning limit from struct xfs_quota_limits Catherine Hoang
2022-05-05 1:18 ` [PATCH v2 2/3] xfs: remove warning counters from struct xfs_dquot_res Catherine Hoang
2022-05-05 16:19 ` Darrick J. Wong
2022-05-05 17:45 ` Alli
2022-05-05 1:18 ` [PATCH v2 3/3] xfs: don't set warns on dquots Catherine Hoang
2022-05-05 16:17 ` Darrick J. Wong
2022-05-05 20:48 ` Catherine Hoang
2022-05-05 17:44 ` Alli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).