On Wed, Feb 12, 2020 at 09:32:33PM -0600, Eric Sandeen wrote:
>
>
> On 2/12/20 9:27 PM, Eric Sandeen wrote:
> > On 2/12/20 7:46 PM, Darrick J. Wong wrote:
> >> 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>
> >
> > ...
> >
> >>>> } 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.
> >
> > +void
> > +xfs_dquot_to_disk_timestamp(
> > + __be32 *dtimer,
> > + const struct timespec64 *tv)
> > +{
> > + *dtimer = cpu_to_be32(tv->tv_sec);
> > +}
> >
> > static inline void
> > xfs_quota_clear_timer(
> > + time64_t *itimer,
> > __be32 *dtimer)
> > {
> > - *dtimer = cpu_to_be32(0);
> > + struct timespec64 tv = { 0 };
> > +
> > + *itimer = tv.tv_sec;
> > + xfs_dquot_to_disk_timestamp(dtimer, &tv);
> > }
> >
> > xfs_quota_clear_timer(&dqp->q_btimer, &d->d_btimer);
> >
> > That's still a very fancy way of saying:
> >
> > dqp->q_btimer = d->d_btimer = 0;
> >
> > I think? Can't really see what value the timespec64 adds here.
> >
> > -Eric
> >
>
> Actually,
>
> xfs_quota_set_timer(
> + time64_t *itimer,
> __be32 *dtimer,
> time_t limit)
> {
> - time64_t new_timeout;
> + struct timespec64 tv = { 0 };
>
> - new_timeout = xfs_dquot_clamp_timer(get_seconds() + limit);
> - *dtimer = cpu_to_be32(new_timeout);
> + tv.tv_sec = xfs_dquot_clamp_timer(ktime_get_real_seconds() + limit);
> + *itimer = tv.tv_sec;
> + xfs_dquot_to_disk_timestamp(dtimer, &tv);
> }
>
> I'm not sure why there's a timespec64 here either. Isn't everything
> we're dealing with on timers in seconds, using only tv_sec, and time64_t would
> suffice instead of using a timespec64 just to carry around a seconds value?
Yes, the grace periods recorded in the timer fields in dquot 0 are
intervals measured in seconds.
However, for dquot != 0, the timer fields store the time of the
expiration, so I settled on timespec64 as the incore structure so that
XFS consistently uses struct timespec64 to represent specific points in
time.
(That and time64_t doesn't exist in userspace.)
--D
> -Eric
