From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40379) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TtGgo-0003Qf-9p for qemu-devel@nongnu.org; Thu, 10 Jan 2013 06:52:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TtGgn-0000aY-3D for qemu-devel@nongnu.org; Thu, 10 Jan 2013 06:52:54 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33006) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TtGgm-0000aT-RZ for qemu-devel@nongnu.org; Thu, 10 Jan 2013 06:52:53 -0500 From: Markus Armbruster References: <50EE8810.7080507@dlhnet.de> <8738y9l3ka.fsf@blackfin.pond.sub.org> <50EE9E1B.2010805@redhat.com> Date: Thu, 10 Jan 2013 12:52:48 +0100 In-Reply-To: <50EE9E1B.2010805@redhat.com> (Paolo Bonzini's message of "Thu, 10 Jan 2013 11:55:23 +0100") Message-ID: <87txqpgsq7.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid locking List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, Peter Lieven , "qemu-devel@nongnu.org" Paolo Bonzini writes: > 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. Work that into the commit message, and I'm happy :)