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?
-Eric
