From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NULD2-0003Bd-La for qemu-devel@nongnu.org; Mon, 11 Jan 2010 09:25:32 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NULCz-0003AI-2G for qemu-devel@nongnu.org; Mon, 11 Jan 2010 09:25:32 -0500 Received: from [199.232.76.173] (port=33497 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NULCy-0003AE-RH for qemu-devel@nongnu.org; Mon, 11 Jan 2010 09:25:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:10635) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NULCy-0001bm-2Q for qemu-devel@nongnu.org; Mon, 11 Jan 2010 09:25:28 -0500 Message-ID: <4B4B34CB.2020308@redhat.com> Date: Mon, 11 Jan 2010 16:25:15 +0200 From: Avi Kivity MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [RFC][PATCH] performance improvement for windows guests, running on top of virtio block device References: <1263195647.2005.44.camel@localhost> <4B4AE1BD.4000400@redhat.com> <20100111134248.GA25622@lst.de> In-Reply-To: <20100111134248.GA25622@lst.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christoph Hellwig Cc: Dor Laor , Vadim Rozenfeld , qemu-devel On 01/11/2010 03:42 PM, Christoph Hellwig wrote: > On Mon, Jan 11, 2010 at 10:30:53AM +0200, Avi Kivity wrote: > >> The patch has potential to reduce performance on volumes with multiple >> spindles. Consider two processes issuing sequential reads into a RAID >> array. With this patch, the reads will be executed sequentially rather >> than in parallel, so I think a follow-on patch to make the minimum depth >> a parameter (set by the guest? the host?) would be helpful. >> > Let's think about the life cycle of I/O requests a bit. > > We have an idle virtqueue (aka one virtio-blk device). The first (read) > request comes in, we get the virtio notify from the guest, which calls > into virtio_blk_handle_output. With the new code we now disable the > notify once we start processing the first request. If the second > request hits the queue before we call into virtio_blk_get_request > the second time we're fine even with the new code as we keep picking it > up. If however it hits after we leave virtio_blk_handle_output, but > before we complete the first request we do indeed introduce additional > latency. > > So instead of disabling notify while requests are active we might want > to only disable it while we are inside virtio_blk_handle_output. > Something like the following minimally tested patch: > > > Index: qemu/hw/virtio-blk.c > =================================================================== > --- qemu.orig/hw/virtio-blk.c 2010-01-11 14:28:42.896010503 +0100 > +++ qemu/hw/virtio-blk.c 2010-01-11 14:40:13.535256353 +0100 > @@ -328,7 +328,15 @@ static void virtio_blk_handle_output(Vir > int num_writes = 0; > BlockDriverState *old_bs = NULL; > > + /* > + * While we are processing requests there is no need to get further > + * notifications from the guest - it'll just burn cpu cycles doing > + * useless context switches into the host. > + */ > + virtio_queue_set_notification(s->vq, 0); > + > while ((req = virtio_blk_get_request(s))) { > +handle_request: > if (req->elem.out_num< 1 || req->elem.in_num< 1) { > fprintf(stderr, "virtio-blk missing headers\n"); > exit(1); > @@ -358,6 +366,18 @@ static void virtio_blk_handle_output(Vir > } > } > > + /* > + * Once we're done processing all pending requests re-enable the queue > + * notification. If there's an entry pending after we enabled > + * notification again we hit a race and need to process it before > + * returning. > + */ > + virtio_queue_set_notification(s->vq, 1); > + req = virtio_blk_get_request(s); > + if (req) { > + goto handle_request; > + } > + > I don't think this will have much effect. First, the time spent in virtio_blk_handle_output() is a small fraction of total guest time, so the probability of the guest hitting the notifications closed window is low. Second, while we're in that function, the vcpu that kicked us is stalled, and other vcpus are likely to be locked out of the queue by the guest. -- error compiling committee.c: too many arguments to function