Dear Max,
Max Reitz <mre...@redhat.com> writes:
> [ Good signature by key: 0x58B381CE2DC89CF99730EE643BB14202E838ACAD ]
Feel free to drop by if you happen to be in the Stuttgart area some
time. PGP key signing, a beverage of your choice and optionally some
chatting about qemu and related topics. :)
> On 04.07.2016 16:30, Sascha Silbe wrote:
>> Max Reitz <mre...@redhat.com> writes:
[...]
[include/qemu/ratelimit.h]
>>>> static inline int64_t ratelimit_calculate_delay(RateLimit *limit,
>>>> uint64_t n)
>>>> {
>>>> int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
>>>> + uint64_t delay_slices;
>>>>
>>>> - if (limit->next_slice_time < now) {
>>>> - limit->next_slice_time = now + limit->slice_ns;
>>>> + assert(limit->slice_quota && limit->slice_ns);
>>>> +
>>>> + 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;
>>>> }
>>>> - if (limit->dispatched == 0 || limit->dispatched + n <=
>>>> limit->slice_quota) {
>>>> - limit->dispatched += n;
>>>> +
>>>> + limit->dispatched += n;
>>>> + if (limit->dispatched < limit->slice_quota) {
>>>
>>> Nitpick: This should probably stay <=.
>>
>> This is a subtle edge case. Previously, when limit->dispatched ==
>> limit->slice_quota, we returned 0 so that the _current_ request (which
>> is still within quota) wouldn't be delayed. Now, we return a delay so
>> that the _next_ request (which would be over quota) will be delayed.
>
> Hm, but that depends on the size of the next request. Of course, if we
> get limit->dispatched == limit->slice_quota we know for sure that we
> need to delay the next request. But if we get limit->dispatched ==
> limit->slice_quota - 1... Then we probably also have to delay it, but we
> don't know for sure.
No matter where exactly we draw the line, due to the way the block job
rate limiting works (fixed size time slices, fixed size requests) there
will always be cases where we're off the target rate quite a bit, in one
or the other direction.
For rate limits where we can send an integer number of chunks per time
slice (i.e. some MiB/s sized value), the "<" condition is probably
better. We'll send out a couple of chunks that exactly match the quota,
then sleep for the rest of the time slice. If we'd use "<=", we'd send
out one extra chunk before we start sleeping.
But I don't care much either way, "<=" is fine with me, too.
Sascha
--
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641
