From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:55463)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1TtFnG-0005OU-5i
	for qemu-devel@nongnu.org; Thu, 10 Jan 2013 05:55:31 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1TtFnE-0001GP-QB
	for qemu-devel@nongnu.org; Thu, 10 Jan 2013 05:55:29 -0500
Received: from mx1.redhat.com ([209.132.183.28]:32235)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1TtFnE-0001GI-JO
	for qemu-devel@nongnu.org; Thu, 10 Jan 2013 05:55:28 -0500
Message-ID: <50EE9E1B.2010805@redhat.com>
Date: Thu, 10 Jan 2013 11:55:23 +0100
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <50EE8810.7080507@dlhnet.de> <8738y9l3ka.fsf@blackfin.pond.sub.org>
In-Reply-To: <8738y9l3ka.fsf@blackfin.pond.sub.org>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid
 locking
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Markus Armbruster <armbru@redhat.com>
Cc: kwolf@redhat.com, Peter Lieven <pl@dlhnet.de>, "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>

Il 10/01/2013 11:45, Markus Armbruster ha scritto:
> 
>> > If io_limits are specified during runtime that exceed the number of operations in flight
>> > bs->io_base is not initialized in the else statement in bdrv_exceed_io_limits().
> I'm confused.
> 
>     if ((bs->slice_start < now)
>         && (bs->slice_end > now)) {
>         bs->slice_end = now + bs->slice_time;
>     } else {
>         bs->slice_time  =  5 * BLOCK_IO_SLICE_TIME;
>         bs->slice_start = now;
>         bs->slice_end   = now + bs->slice_time;
> 
>         bs->io_base.bytes[is_write]  = bs->nr_bytes[is_write];
>         bs->io_base.bytes[!is_write] = bs->nr_bytes[!is_write];
> 
>         bs->io_base.ios[is_write]    = bs->nr_ops[is_write];
>         bs->io_base.ios[!is_write]   = bs->nr_ops[!is_write];
>     }

bdrv_io_limits_enable correctly starts a new slice (the first three
lines of the else), but does not set io_base correctly for that slice.
Here is how io_base is used:

    bytes_base  = bs->nr_bytes[is_write] - bs->io_base.bytes[is_write];
    bytes_res   = (unsigned) nb_sectors * BDRV_SECTOR_SIZE;

    if (bytes_base + bytes_res <= bytes_limit) {
        /* no wait */
    } else {
        /* operation needs to be throttled */
    }

As a result, any I/O operations that are triggered between now and
bs->slice_end are incorrectly limited.  If 10 MB of data has been
written since the VM was started, QEMU thinks that 10 MB of data has
been written in this slice.

Paolo