On Wed, Feb 12, 2020 at 05:57:24PM -0600, Eric Sandeen wrote:
> On 12/31/19 7:11 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
>
> ...
>
> > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> > index ae7bb6361a99..44bae5f16b55 100644
> > --- a/fs/xfs/xfs_dquot.c
> > +++ b/fs/xfs/xfs_dquot.c
> > @@ -113,6 +113,36 @@ xfs_quota_exceeded(
> > return *hardlimit && count > be64_to_cpup(hardlimit);
> > }
> >
> > +/*
> > + * Clamp a quota grace period expiration timer to the range that we support.
> > + */
> > +static inline time64_t
> > +xfs_dquot_clamp_timer(
> > + time64_t timer)
> > +{
> > + return clamp_t(time64_t, timer, XFS_DQ_TIMEOUT_MIN, XFS_DQ_TIMEOUT_MAX);
> > +}
> > +
> > +/* Set a quota grace period expiration timer. */
> > +static inline void
> > +xfs_quota_set_timer(
> > + __be32 *dtimer,
> > + time_t limit)
> > +{
> > + time64_t new_timeout;
> > +
> > + new_timeout = xfs_dquot_clamp_timer(get_seconds() + limit);
> > + *dtimer = cpu_to_be32(new_timeout);
> > +}
> > +
> > +/* Clear a quota grace period expiration timer. */
> > +static inline void
> > +xfs_quota_clear_timer(
> > + __be32 *dtimer)
> > +{
> > + *dtimer = cpu_to_be32(0);
>
> do we need to endian convert 0 to make sparse happy? I don't see us doing
> that anywhere else. TBH not really sure I see the reason for the function
> at all unless you really, really like the symmetry.
>
> > +}
> > +
> > /*
> > * Check the limits and timers of a dquot and start or reset timers
> > * if necessary.
> > @@ -152,14 +182,14 @@ xfs_qm_adjust_dqtimers(
> > &d->d_blk_softlimit, &d->d_blk_hardlimit);
> > if (!d->d_btimer) {
> > if (over) {
> > - d->d_btimer = cpu_to_be32(get_seconds() +
> > + xfs_quota_set_timer(&d->d_btimer,
> > mp->m_quotainfo->qi_btimelimit);
> > } else {
> > d->d_bwarns = 0;
> > }
> > } else {
> > if (!over) {
> > - d->d_btimer = 0;
> > + xfs_quota_clear_timer(&d->d_btimer);
>
> yeah that's a very fancy way to say "= 0" ;)
>
> ...
Yes, that's a fancy way to assign zero. However, consider that for
bigtime support, I had to add an incore quota timer so that timers more
or less fire when they're supposed to, and now there's a function to
convert the incore timespec64 value to whatever is ondisk:
/* Clear a quota grace period expiration timer. */
static inline void
xfs_quota_clear_timer(
struct xfs_disk_dquot *ddq,
time64_t *itimer,
__be32 *dtimer)
{
struct timespec64 tv = { 0 };
*itimer = tv.tv_sec;
xfs_dquot_to_disk_timestamp(ddq, dtimer, &tv);
}
It was at *that* point in the patchset that it seemed easier to call a
small function three times than to open-code this three times.
> > diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> > index f67f3645efcd..52dc5326b7bf 100644
> > --- a/fs/xfs/xfs_ondisk.h
> > +++ b/fs/xfs/xfs_ondisk.h
> > @@ -25,6 +25,8 @@ xfs_check_ondisk_structs(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);
>
> again grumble grumble really not checking an ondisk structure.
Same answer as before. :)
--D
> > /* ag/file structures */
> > XFS_CHECK_STRUCT_SIZE(struct xfs_acl, 4);
> >
