From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755015Ab1KNAte (ORCPT ); Sun, 13 Nov 2011 19:49:34 -0500 Received: from ozlabs.org ([203.10.76.45]:57148 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752481Ab1KNAta (ORCPT ); Sun, 13 Nov 2011 19:49:30 -0500 From: Rusty Russell To: "Michael S. Tsirkin" Cc: mst@redhat.com, Christoph Hellwig , linux-kernel@vger.kernel.org, kvm@vger.kernel.org, virtualization Subject: Re: [PATCH 5 of 5] virtio: expose added descriptors immediately In-Reply-To: <20111113210256.GA31621@redhat.com> References: <20111113210256.GA31621@redhat.com> User-Agent: Notmuch/0.6.1-1 (http://notmuchmail.org) Emacs/23.3.1 (i686-pc-linux-gnu) Date: Mon, 14 Nov 2011 11:13:57 +1030 Message-ID: <87lirjpm7m.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, 13 Nov 2011 23:03:14 +0200, "Michael S. Tsirkin" wrote: > On Thu, Nov 03, 2011 at 06:12:53PM +1030, Rusty Russell wrote: > > A virtio driver does virtqueue_add_buf() multiple times before finally > > calling virtqueue_kick(); previously we only exposed the added buffers > > in the virtqueue_kick() call. This means we don't need a memory > > barrier in virtqueue_add_buf(), but it reduces concurrency as the > > device (ie. host) can't see the buffers until the kick. > > > > Signed-off-by: Rusty Russell > > In the past I played with a patch like this, but I didn't see a > performance gain either way. Do you see any gain? No, but I haven't run it on real hardware. lguest may see a win with this in theory, since the virtqueue processing is fully async, so I'll run some tests. > I'm a bit concerned that with this patch, a buggy driver that > adds more than 2^16 descriptors without a kick > would seem to work sometimes. Let's add WARN_ON(vq->num_added > (1 << 16))? Hmm, I guess it could wait for the add to fail before doing a kick, but noone does that at the moment, so I've added a slight variant: WARN_ON(vq->num_added > vq->vring.num); Thanks, Rusty.