Re: [PATCH] ratelimit: restrict the delay time to a non-negative value

[Markus Armbruster]

Wang Liang <wangliangzz@126.com> writes:

> On Tue, 2022-09-20 at 13:18 +0000, Alberto Garcia wrote:
>> On Tue 20 Sep 2022 08:33:50 PM +08, wangliangzz@126.com wrote:
>> > From: Wang Liang <wangliangzz@inspur.com>
>> > 
>> > The delay time should never be a negative value.
>> > 
>> > -    return limit->slice_end_time - now;
>> > +    return MAX(limit->slice_end_time - now, 0);
>> 
>> How can this be negative? slice_end_time is guaranteed to be larger
>> than
>> now:
>> 
>>     if (limit->slice_end_time < now) {
>>         /* Previous, possibly extended, time slice finished; reset
>> the
>>          * accounting. */
>>         limit->slice_start_time = now;
>>         limit->slice_end_time = now + limit->slice_ns;
>>         limit->dispatched = 0;
>>     }
>> 
> This is just a guarantee. 

Smells like an invariant to me.

> If slice_end_time is assigned later by
>     limit->slice_end_time = limit->slice_start_time +
>         (uint64_t)(delay_slices * limit->slice_ns);
> There may be precision issues at that time.

What are the issues exactly?  What misbehavior are you observing?

Your commit message should show how delay time can become negative, and
why that's bad.