On 12/31/19 7:11 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Refactor the code that sets the default quota grace period into a helper
> function so that we can override the ondisk behavior later.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_format.h | 8 ++++++++
> fs/xfs/xfs_ondisk.h | 2 ++
> fs/xfs/xfs_qm_syscalls.c | 35 +++++++++++++++++++++++------------
> fs/xfs/xfs_trans_dquot.c | 16 ++++++++++++----
> 4 files changed, 45 insertions(+), 16 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index 95761b38fe86..557db5e51eec 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -1188,6 +1188,10 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
> * time zero is the Unix epoch, Jan 1 00:00:01 UTC 1970. An expiration value
> * of zero means that the quota limit has not been reached, and therefore no
> * expiration has been set.
> + *
> + * The length of quota grace periods are unsigned 32-bit quantities in units of
> + * seconds (which are stored in the root dquot). A value of zero means to use
> + * the default period.
Doesn't a value of zero mean that the soft limit has not been exceeded, and no
timer is in force? And when soft limit is exceeded, the timer starts ticking
based on the value in the root dquot?
i.e. you can't set a custom per-user grace period, can you?
Perhaps:
* The length of quota grace periods are unsigned 32-bit quantities in units of
* seconds. The grace period for each quota type is stored in the root dquot
* and is applied/transferred to a user quota when it exceeds a soft limit.
> */
>
> /*
> @@ -1202,6 +1206,10 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
> */
> #define XFS_DQ_TIMEOUT_MAX ((int64_t)U32_MAX)
>
> +/* Quota grace periods, ranging from zero (use the defaults) to ~136 years. */
same thing. The default can be set between 0 and ~136 years, that gets transferred
to any user who exceeds soft quota, and it counts down from there.
> +#define XFS_DQ_GRACE_MIN ((int64_t)0)
> +#define XFS_DQ_GRACE_MAX ((int64_t)U32_MAX)
> +
> /*
> * This is the main portion of the on-disk representation of quota
> * information for a user. This is the q_core of the struct xfs_dquot that
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index 52dc5326b7bf..b8811f927a3c 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -27,6 +27,8 @@ xfs_check_ondisk_structs(void)
> XFS_CHECK_VALUE(XFS_INO_TIME_MAX, 2147483647LL);
> XFS_CHECK_VALUE(XFS_DQ_TIMEOUT_MIN, 1LL);
> XFS_CHECK_VALUE(XFS_DQ_TIMEOUT_MAX, 4294967295LL);
> + XFS_CHECK_VALUE(XFS_DQ_GRACE_MIN, 0LL);
> + XFS_CHECK_VALUE(XFS_DQ_GRACE_MAX, 4294967295LL);
*cough* notondisk *cough*
>
> /* ag/file structures */
> XFS_CHECK_STRUCT_SIZE(struct xfs_acl, 4);
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index 74220948a360..20a6d304d1be 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -438,6 +438,20 @@ xfs_qm_scall_quotaon(
> return 0;
> }
>
> +/* Set a new quota grace period. */
> +static inline void
> +xfs_qm_set_grace(
> + time_t *qi_limit,
^ doesn't get used?
> + __be32 *dtimer,
> + const s64 grace)
> +{
> + time64_t new_grace;
> +
> + new_grace = clamp_t(time64_t, grace, XFS_DQ_GRACE_MIN,
> + XFS_DQ_GRACE_MAX);
> + *dtimer = cpu_to_be32(new_grace);
You've lost setting the qi_limit here (q->qi_btimelimit etc)
> +}
> +
> #define XFS_QC_MASK \
> (QC_LIMIT_MASK | QC_TIMER_MASK | QC_WARNS_MASK)
>
> @@ -567,18 +581,15 @@ xfs_qm_scall_setqlim(
> * soft and hard limit values (already done, above), and
> * for warnings.
> */
> - if (newlim->d_fieldmask & QC_SPC_TIMER) {
> - q->qi_btimelimit = newlim->d_spc_timer;
i.e. qi_btimelimit never gets set now, which is what actually controls
the timers when a uid/gid/pid goes over softlimit.
> - ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer);
> - }
> - if (newlim->d_fieldmask & QC_INO_TIMER) {
> - q->qi_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;
> - ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer);
> - }
> + if (newlim->d_fieldmask & QC_SPC_TIMER)
> + xfs_qm_set_grace(&q->qi_btimelimit, &ddq->d_btimer,
> + newlim->d_spc_timer);
> + if (newlim->d_fieldmask & QC_INO_TIMER)
> + xfs_qm_set_grace(&q->qi_itimelimit, &ddq->d_itimer,
> + newlim->d_ino_timer);
> + if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
> + xfs_qm_set_grace(&q->qi_rtbtimelimit, &ddq->d_rtbtimer,
> + newlim->d_rt_spc_timer);
> if (newlim->d_fieldmask & QC_SPC_WARNS)
> q->qi_bwarnlimit = newlim->d_spc_warns;
> if (newlim->d_fieldmask & QC_INO_WARNS)
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index 248cfc369efc..7a2a3bd11db9 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -563,6 +563,14 @@ xfs_quota_warn(
> mp->m_super->s_dev, type);
> }
>
> +/* Has a quota grace period expired? */
seems like this is not part of "quota grace period setting code"
- needs to be in a separate patch?
> +static inline bool
> +xfs_quota_timer_exceeded(
> + time64_t timer)
> +{
> + return timer != 0 && get_seconds() > timer;
> +}
> +
> /*
> * This reserves disk blocks and inodes against a dquot.
> * Flags indicate if the dquot is to be locked here and also
> @@ -580,7 +588,7 @@ xfs_trans_dqresv(
> {
> xfs_qcnt_t hardlimit;
> xfs_qcnt_t softlimit;
> - time_t timer;
> + time64_t timer;
<this needs rebasing I guess, after b8a0880a37e2f43aa3bcd147182e95a4ebd82279>
> xfs_qwarncnt_t warns;
> xfs_qwarncnt_t warnlimit;
> xfs_qcnt_t total_count;
> @@ -635,7 +643,7 @@ xfs_trans_dqresv(
> goto error_return;
> }
> if (softlimit && total_count > softlimit) {
> - if ((timer != 0 && get_seconds() > timer) ||
> + if (xfs_quota_timer_exceeded(timer) ||
> (warns != 0 && warns >= warnlimit)) {
> xfs_quota_warn(mp, dqp,
> QUOTA_NL_BSOFTLONGWARN);
> @@ -662,8 +670,8 @@ xfs_trans_dqresv(
> goto error_return;
> }
> if (softlimit && total_count > softlimit) {
> - if ((timer != 0 && get_seconds() > timer) ||
> - (warns != 0 && warns >= warnlimit)) {
> + if (xfs_quota_timer_exceeded(timer) ||
> + (warns != 0 && warns >= warnlimit)) {
TBH don't really see the point of this refactoring/helper, especially if not
done for warns. I think open coding is fine.
> xfs_quota_warn(mp, dqp,
> QUOTA_NL_ISOFTLONGWARN);
> goto error_return;
>
