Message ID | 159770502083.3956827.8660123941779980742.stgit@magnolia (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | xfs: widen timestamps to deal with y2038 | expand |
On Tue, Aug 18, 2020 at 1:59 AM Darrick J. Wong <darrick.wong@oracle.com> 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. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Amir Goldstein <amir73il@gmail.com> Question below... ... > 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); > + } > } I understand why you did this. This is mid series craft, but it looks very odd to your average reviewer. Maybe leave a TODO comment above res->timer = timer which will be removed later in the series? Not critical. Thanks, Amir.
On Tue, Aug 18, 2020 at 09:48:14AM +0300, Amir Goldstein wrote: > On Tue, Aug 18, 2020 at 1:59 AM Darrick J. Wong <darrick.wong@oracle.com> 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. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > Reviewed-by: Amir Goldstein <amir73il@gmail.com> > > Question below... > > ... > > > 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); > > + } > > } > > I understand why you did this. This is mid series craft, but it looks very odd > to your average reviewer. > > Maybe leave a TODO comment above res->timer = timer which will be > removed later in the series? > > Not critical. <shrug> The grace period case gets changed in the very next patch, but I suppose it wouldn't hurt to mention that in the commit log. --D > > Thanks, > Amir.
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); + } } /*