Re: [PATCH 1/2] kvm tools: Fix virt_queue__set_used_elem

[Ingo Molnar <mingo@elte.hu>]

* Sasha Levin <levinsasha928@gmail.com> 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 <levinsasha928@gmail.com>
> ---
>  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