Re: [RFC] virtio: Support releasing lock during kick

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Jun 25, 2010 at 12:39:21PM +0930, Rusty Russell wrote:
> On Thu, 24 Jun 2010 03:00:30 pm Stefan Hajnoczi wrote:
> > On Wed, Jun 23, 2010 at 11:12 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> > > Shouldn't it be possible to just drop the lock before invoking
> > > virtqueue_kick() and reacquire it afterwards?  There's nothing in that
> > > virtqueue_kick() path that the lock is protecting AFAICT.
> > 
> > No, that would lead to a race condition because vq->num_added is
> > modified by both virtqueue_add_buf_gfp() and virtqueue_kick().
> > Without a lock held during virtqueue_kick() another vcpu could add
> > bufs while vq->num_added is used and cleared by virtqueue_kick():
> 
> Right, this dovetails with another proposed change (was it Michael?)
> where we would update the avail idx inside add_buf, rather than waiting
> until kick.  This means a barrier inside add_buf, but that's probably
> fine.
> 
> If we do that, then we don't need a lock on virtqueue_kick.
> 
> Michael, thoughts?

Maybe not even that: I think we could just do virtio_wmb()
in add, and keep the mb() in kick.

What I'm a bit worried about is contention on the cacheline
including index and flags: the more we write to that line,
the worse it gets.

So need to test performance impact of this change:
I didn't find time to do this yet, as I am trying
to finalize the used index publishing patches.
Any takers?

Do we see performance improvement after making kick lockless?

> Thanks,
> Rusty.
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html