| Message ID | 159797590052.965217.10856208107922013686.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: widen timestamps to deal with y2038 | expand |
On Thu, Aug 20, 2020 at 07:11:40PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Define explicit limits on the range of quota grace period expiration > timeouts and refactor the code that modifies the timeouts into helpers > that clamp the values appropriately. Note that we'll deal with the > grace period timer separately. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > Reviewed-by: Amir Goldstein <amir73il@gmail.com> > --- > fs/xfs/libxfs/xfs_format.h | 22 ++++++++++++++++++++++ > fs/xfs/xfs_dquot.c | 13 ++++++++++++- > fs/xfs/xfs_dquot.h | 2 ++ > fs/xfs/xfs_ondisk.h | 2 ++ > fs/xfs/xfs_qm_syscalls.c | 9 +++++++-- > 5 files changed, 45 insertions(+), 3 deletions(-) > > > diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h > index b1b8a5c05cea..ef36978239ac 100644 > --- a/fs/xfs/libxfs/xfs_format.h > +++ b/fs/xfs/libxfs/xfs_format.h > @@ -1197,6 +1197,28 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev) > > #define XFS_DQTYPE_ANY (XFS_DQTYPE_REC_MASK) > > +/* > + * XFS Quota Timers > + * ================ > + * > + * Quota grace period expiration timers are an unsigned 32-bit seconds counter; > + * 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. > + */ > + > +/* > + * Smallest possible quota expiration with traditional timestamps, which is > + * Jan 1 00:00:01 UTC 1970. > + */ > +#define XFS_DQ_TIMEOUT_MIN ((int64_t)1) > + > +/* > + * Largest possible quota expiration with traditional timestamps, which is > + * Feb 7 06:28:15 UTC 2106. > + */ > +#define XFS_DQ_TIMEOUT_MAX ((int64_t)U32_MAX) > + > /* > * This is the main portion of the on-disk representation of quota information > * for a user. We pad this with some more expansion room to construct the on > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c > index bcd73b9c2994..2425b1c30d11 100644 > --- a/fs/xfs/xfs_dquot.c > +++ b/fs/xfs/xfs_dquot.c > @@ -98,6 +98,16 @@ xfs_qm_adjust_dqlimits( > xfs_dquot_set_prealloc_limits(dq); > } > > +/* Set the expiration time of a quota's grace period. */ > +void > +xfs_dquot_set_timeout( > + time64_t *timer, > + time64_t value) > +{ > + *timer = clamp_t(time64_t, value, XFS_DQ_TIMEOUT_MIN, > + XFS_DQ_TIMEOUT_MAX); > +} Why doesn't this just return the value? That would seem like a much more natural calling convention to me.
On Thu, Aug 20, 2020 at 07:11:40PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Define explicit limits on the range of quota grace period expiration > timeouts and refactor the code that modifies the timeouts into helpers > that clamp the values appropriately. Note that we'll deal with the > grace period timer separately. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > Reviewed-by: Amir Goldstein <amir73il@gmail.com> > --- > fs/xfs/libxfs/xfs_format.h | 22 ++++++++++++++++++++++ > fs/xfs/xfs_dquot.c | 13 ++++++++++++- > fs/xfs/xfs_dquot.h | 2 ++ > fs/xfs/xfs_ondisk.h | 2 ++ > fs/xfs/xfs_qm_syscalls.c | 9 +++++++-- > 5 files changed, 45 insertions(+), 3 deletions(-) > > > diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h > index b1b8a5c05cea..ef36978239ac 100644 > --- a/fs/xfs/libxfs/xfs_format.h > +++ b/fs/xfs/libxfs/xfs_format.h > @@ -1197,6 +1197,28 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev) > > #define XFS_DQTYPE_ANY (XFS_DQTYPE_REC_MASK) > > +/* > + * XFS Quota Timers > + * ================ > + * > + * Quota grace period expiration timers are an unsigned 32-bit seconds counter; > + * 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. > + */ > + > +/* > + * Smallest possible quota expiration with traditional timestamps, which is > + * Jan 1 00:00:01 UTC 1970. > + */ > +#define XFS_DQ_TIMEOUT_MIN ((int64_t)1) > + > +/* > + * Largest possible quota expiration with traditional timestamps, which is > + * Feb 7 06:28:15 UTC 2106. > + */ > +#define XFS_DQ_TIMEOUT_MAX ((int64_t)U32_MAX) Same as last patch - these reference the unix epoch, so perhaps they should be named that way.... > * This is the main portion of the on-disk representation of quota information > * for a user. We pad this with some more expansion room to construct the on > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c > index bcd73b9c2994..2425b1c30d11 100644 > --- a/fs/xfs/xfs_dquot.c > +++ b/fs/xfs/xfs_dquot.c > @@ -98,6 +98,16 @@ xfs_qm_adjust_dqlimits( > xfs_dquot_set_prealloc_limits(dq); > } > > +/* Set the expiration time of a quota's grace period. */ > +void > +xfs_dquot_set_timeout( > + time64_t *timer, > + time64_t value) > +{ > + *timer = clamp_t(time64_t, value, XFS_DQ_TIMEOUT_MIN, > + XFS_DQ_TIMEOUT_MAX); > +} > + Not sure I see any benefit in passing *timer as a parameter over just returning a time64_t value... Cheers, Dave.
On Mon, Aug 24, 2020 at 09:57:37AM +1000, Dave Chinner wrote: > On Thu, Aug 20, 2020 at 07:11:40PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > Define explicit limits on the range of quota grace period expiration > > timeouts and refactor the code that modifies the timeouts into helpers > > that clamp the values appropriately. Note that we'll deal with the > > grace period timer separately. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > Reviewed-by: Amir Goldstein <amir73il@gmail.com> > > --- > > fs/xfs/libxfs/xfs_format.h | 22 ++++++++++++++++++++++ > > fs/xfs/xfs_dquot.c | 13 ++++++++++++- > > fs/xfs/xfs_dquot.h | 2 ++ > > fs/xfs/xfs_ondisk.h | 2 ++ > > fs/xfs/xfs_qm_syscalls.c | 9 +++++++-- > > 5 files changed, 45 insertions(+), 3 deletions(-) > > > > > > diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h > > index b1b8a5c05cea..ef36978239ac 100644 > > --- a/fs/xfs/libxfs/xfs_format.h > > +++ b/fs/xfs/libxfs/xfs_format.h > > @@ -1197,6 +1197,28 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev) > > > > #define XFS_DQTYPE_ANY (XFS_DQTYPE_REC_MASK) > > > > +/* > > + * XFS Quota Timers > > + * ================ > > + * > > + * Quota grace period expiration timers are an unsigned 32-bit seconds counter; > > + * 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. > > + */ > > + > > +/* > > + * Smallest possible quota expiration with traditional timestamps, which is > > + * Jan 1 00:00:01 UTC 1970. > > + */ > > +#define XFS_DQ_TIMEOUT_MIN ((int64_t)1) > > + > > +/* > > + * Largest possible quota expiration with traditional timestamps, which is > > + * Feb 7 06:28:15 UTC 2106. > > + */ > > +#define XFS_DQ_TIMEOUT_MAX ((int64_t)U32_MAX) > > Same as last patch - these reference the unix epoch, so perhaps > they should be named that way.... > > > * This is the main portion of the on-disk representation of quota information > > * for a user. We pad this with some more expansion room to construct the on > > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c > > index bcd73b9c2994..2425b1c30d11 100644 > > --- a/fs/xfs/xfs_dquot.c > > +++ b/fs/xfs/xfs_dquot.c > > @@ -98,6 +98,16 @@ xfs_qm_adjust_dqlimits( > > xfs_dquot_set_prealloc_limits(dq); > > } > > > > +/* Set the expiration time of a quota's grace period. */ > > +void > > +xfs_dquot_set_timeout( > > + time64_t *timer, > > + time64_t value) > > +{ > > + *timer = clamp_t(time64_t, value, XFS_DQ_TIMEOUT_MIN, > > + XFS_DQ_TIMEOUT_MAX); > > +} > > + > > Not sure I see any benefit in passing *timer as a parameter over > just returning a time64_t value... I'll look into changing them, both for this and the next patch. I /think/ it should be fairly straightforward. --D > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h index b1b8a5c05cea..ef36978239ac 100644 --- a/fs/xfs/libxfs/xfs_format.h +++ b/fs/xfs/libxfs/xfs_format.h @@ -1197,6 +1197,28 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev) #define XFS_DQTYPE_ANY (XFS_DQTYPE_REC_MASK) +/* + * XFS Quota Timers + * ================ + * + * Quota grace period expiration timers are an unsigned 32-bit seconds counter; + * 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. + */ + +/* + * Smallest possible quota expiration with traditional timestamps, which is + * Jan 1 00:00:01 UTC 1970. + */ +#define XFS_DQ_TIMEOUT_MIN ((int64_t)1) + +/* + * Largest possible quota expiration with traditional timestamps, which is + * Feb 7 06:28:15 UTC 2106. + */ +#define XFS_DQ_TIMEOUT_MAX ((int64_t)U32_MAX) + /* * This is the main portion of the on-disk representation of quota information * for a user. We pad this with some more expansion room to construct the on diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c index bcd73b9c2994..2425b1c30d11 100644 --- a/fs/xfs/xfs_dquot.c +++ b/fs/xfs/xfs_dquot.c @@ -98,6 +98,16 @@ xfs_qm_adjust_dqlimits( xfs_dquot_set_prealloc_limits(dq); } +/* Set the expiration time of a quota's grace period. */ +void +xfs_dquot_set_timeout( + time64_t *timer, + time64_t value) +{ + *timer = clamp_t(time64_t, value, XFS_DQ_TIMEOUT_MIN, + XFS_DQ_TIMEOUT_MAX); +} + /* * Determine if this quota counter is over either limit and set the quota * timers as appropriate. @@ -112,7 +122,8 @@ xfs_qm_adjust_res_timer( if ((res->softlimit && res->count > res->softlimit) || (res->hardlimit && res->count > res->hardlimit)) { if (res->timer == 0) - res->timer = ktime_get_real_seconds() + qlim->time; + xfs_dquot_set_timeout(&res->timer, + ktime_get_real_seconds() + qlim->time); } else { if (res->timer == 0) res->warnings = 0; diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h index 282a65da93c7..11bd0ee9b0fa 100644 --- a/fs/xfs/xfs_dquot.h +++ b/fs/xfs/xfs_dquot.h @@ -237,4 +237,6 @@ typedef int (*xfs_qm_dqiterate_fn)(struct xfs_dquot *dq, int xfs_qm_dqiterate(struct xfs_mount *mp, xfs_dqtype_t type, xfs_qm_dqiterate_fn iter_fn, void *priv); +void xfs_dquot_set_timeout(time64_t *timer, time64_t limit); + #endif /* __XFS_DQUOT_H__ */ diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h index 48a64fa49f91..38ccffcf3336 100644 --- a/fs/xfs/xfs_ondisk.h +++ b/fs/xfs/xfs_ondisk.h @@ -25,6 +25,8 @@ xfs_check_limits(void) /* make sure timestamp limits are correct */ XFS_CHECK_VALUE(XFS_INO_TIME_MIN, -2147483648LL); XFS_CHECK_VALUE(XFS_INO_TIME_MAX, 2147483647LL); + XFS_CHECK_VALUE(XFS_DQ_TIMEOUT_MIN, 1LL); + XFS_CHECK_VALUE(XFS_DQ_TIMEOUT_MAX, 4294967295LL); } static inline void __init diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c index 1c542b4a5220..b16d533a6feb 100644 --- a/fs/xfs/xfs_qm_syscalls.c +++ b/fs/xfs/xfs_qm_syscalls.c @@ -483,9 +483,14 @@ xfs_setqlim_timer( struct xfs_quota_limits *qlim, s64 timer) { - res->timer = timer; - if (qlim) + if (qlim) { + /* Set the length of the default grace period. */ + res->timer = timer; qlim->time = timer; + } else { + /* Set the grace period expiration on a quota. */ + xfs_dquot_set_timeout(&res->timer, timer); + } } /*