From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ingo Molnar Subject: Re: [PATCH 1/2] kvm tools: Fix virt_queue__set_used_elem Date: Tue, 3 May 2011 21:44:37 +0200 Message-ID: <20110503194437.GA23144@elte.hu> References: <1304437058-15651-1-git-send-email-levinsasha928@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: penberg@kernel.org, asias.hejun@gmail.com, gorcunov@gmail.com, prasadjoshi124@gmail.com, kvm@vger.kernel.org To: Sasha Levin Return-path: Received: from mx2.mail.elte.hu ([157.181.151.9]:49450 "EHLO mx2.mail.elte.hu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754596Ab1ECTor (ORCPT ); Tue, 3 May 2011 15:44:47 -0400 Content-Disposition: inline In-Reply-To: <1304437058-15651-1-git-send-email-levinsasha928@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: * Sasha Levin wrote: > Increase idx only after updating the used element. > Not doing so may mark a buffer as used without having > it's head and length updated. > > Signed-off-by: Sasha Levin > --- > tools/kvm/virtio.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/tools/kvm/virtio.c b/tools/kvm/virtio.c > index 6249521..b2d2407 100644 > --- a/tools/kvm/virtio.c > +++ b/tools/kvm/virtio.c > @@ -7,9 +7,10 @@ > struct vring_used_elem *virt_queue__set_used_elem(struct virt_queue *queue, uint32_t head, uint32_t len) > { > struct vring_used_elem *used_elem; > - used_elem = &queue->vring.used->ring[queue->vring.used->idx++ % queue->vring.num]; > + used_elem = &queue->vring.used->ring[queue->vring.used->idx % queue->vring.num]; > used_elem->id = head; > used_elem->len = len; > + queue->vring.used->idx++; > return used_elem; > } Note that both the compiler and the CPU can reorder this code arbitrarily, so your patch does not address the problem. As mentioned in earlier discussions, you need memory barriers (which also act as compiler barriers) to express this dependency in the order of updates to these data structures. Thanks, Ingo