Re: [Qemu-block] [PATCH v3 09/21] block: Add average I/O queue depth to BlockDeviceTimedStats

[Stefan Hajnoczi]

On Thu, Oct 22, 2015 at 11:11:19AM +0300, Alberto Garcia wrote:
> +/* Get the sum of all accounted values
> + * @ta:      the TimedAverage structure
> + * @elapsed: if non-NULL, the elapsed time (in ns) will be stored here
> + * @ret:     the sum of all accounted values
> + */
> +uint64_t timed_average_sum(TimedAverage *ta, uint64_t *elapsed)
> +{
> +    TimedAverageWindow *w;
> +    check_expirations(ta);
> +    w = current_window(ta);
> +    if (elapsed != NULL) {
> +        int64_t remaining = w->expiration - 
> qemu_clock_get_ns(ta->clock_type);
> +        *elapsed = ta->period - remaining;

There is no guarantee that qemu_clock_get_ns(ta->clock_type) executes
quickly after check_expirations(ta).  If the system is under heavy load
and swapping, maybe significant amounts of time have passed.  This race
condition could result in bad elapsed values being calculated.

We need to be careful about elapsed <= 0 values, especially if the
caller divides by elapsed and is exposed to Divide By 0 exceptions.

Either check_expirations() needs to integrate qemu_clock_get_ns() or we
simply need to deal with bogus values here.