From: Eric Sandeen <sandeen@sandeen.net>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 06/14] xfs: refactor default quota grace period setting code
Date: Wed, 12 Feb 2020 18:15:18 -0600 [thread overview]
Message-ID: <2fde1e65-7ede-3c47-81bd-d39906a8dc77@sandeen.net> (raw)
In-Reply-To: <157784110016.1364230.5024129406313355261.stgit@magnolia>
On 12/31/19 7:11 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> 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@oracle.com>
> ---
> 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;
>
next prev parent reply other threads:[~2020-02-13 0:15 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-01 1:11 [PATCH 00/14] xfs: widen timestamps to deal with y2038 Darrick J. Wong
2020-01-01 1:11 ` [PATCH 01/14] xfs: explicitly define inode timestamp range Darrick J. Wong
2020-02-12 23:00 ` Eric Sandeen
2020-02-13 1:26 ` Darrick J. Wong
2020-02-13 1:50 ` Eric Sandeen
2020-02-13 1:53 ` Darrick J. Wong
2020-01-01 1:11 ` [PATCH 02/14] xfs: preserve default grace interval during quotacheck Darrick J. Wong
2020-02-12 23:35 ` Eric Sandeen
2020-02-19 4:55 ` Eric Sandeen
2020-03-03 3:03 ` Eric Sandeen
2020-03-03 15:48 ` Darrick J. Wong
2020-03-03 15:52 ` Eric Sandeen
2020-01-01 1:11 ` [PATCH 03/14] xfs: refactor quota exceeded test Darrick J. Wong
2020-02-12 23:51 ` Eric Sandeen
2020-02-13 1:41 ` Darrick J. Wong
2020-02-13 1:52 ` Eric Sandeen
2020-02-13 1:59 ` Darrick J. Wong
2020-05-31 14:04 ` Amir Goldstein
2020-01-01 1:11 ` [PATCH 04/14] xfs: fix quota timer inactivation Darrick J. Wong
2020-05-31 15:04 ` Amir Goldstein
2020-06-01 23:56 ` Darrick J. Wong
2020-01-01 1:11 ` [PATCH 05/14] xfs: refactor quota expiration timer modification Darrick J. Wong
2020-02-12 23:57 ` Eric Sandeen
2020-02-13 1:46 ` Darrick J. Wong
2020-02-13 3:27 ` Eric Sandeen
2020-02-13 3:32 ` Eric Sandeen
2020-02-13 5:33 ` Darrick J. Wong
2020-01-01 1:11 ` [PATCH 06/14] xfs: refactor default quota grace period setting code Darrick J. Wong
2020-02-13 0:15 ` Eric Sandeen [this message]
2020-02-13 1:53 ` Darrick J. Wong
2020-02-13 2:03 ` Darrick J. Wong
2020-01-01 1:11 ` [PATCH 07/14] xfs: remove xfs_timestamp_t Darrick J. Wong
2020-01-01 1:11 ` [PATCH 08/14] xfs: move xfs_log_dinode_to_disk to the log code Darrick J. Wong
2020-01-01 1:11 ` [PATCH 09/14] xfs: refactor timestamp coding Darrick J. Wong
2020-01-01 1:12 ` [PATCH 10/14] xfs: convert struct xfs_timestamp to union Darrick J. Wong
2020-01-01 1:12 ` [PATCH 11/14] xfs: widen ondisk timestamps to deal with y2038 problem Darrick J. Wong
2020-05-31 12:30 ` Amir Goldstein
2020-06-01 23:17 ` Darrick J. Wong
2020-06-02 4:26 ` Amir Goldstein
2020-01-01 1:12 ` [PATCH 12/14] xfs: cache quota grace period expiration times incore Darrick J. Wong
2020-01-01 1:12 ` [PATCH 13/14] xfs: enable bigtime for quota timers Darrick J. Wong
2020-05-31 17:07 ` Amir Goldstein
2020-06-02 0:09 ` Darrick J. Wong
2020-06-02 4:04 ` Amir Goldstein
2020-01-01 1:12 ` [PATCH 14/14] xfs: enable big timestamps Darrick J. Wong
2020-05-26 9:20 ` [PATCH 00/14] xfs: widen timestamps to deal with y2038 Amir Goldstein
2020-05-26 15:57 ` Darrick J. Wong
2020-05-26 16:42 ` Amir Goldstein
2020-05-31 17:31 ` Amir Goldstein
2020-06-02 0:09 ` Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2fde1e65-7ede-3c47-81bd-d39906a8dc77@sandeen.net \
--to=sandeen@sandeen.net \
--cc=darrick.wong@oracle.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).