[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/2] add a header file for atomic operations
From: |
Torvald Riegel |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/2] add a header file for atomic operations |
Date: |
Wed, 19 Jun 2013 22:39:54 +0200 |
On Thu, 2013-06-20 at 04:59 +0800, Liu Ping Fan wrote:
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index fbabf99..28abe1e 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -16,6 +16,7 @@
> #include <sys/ioctl.h>
> #include "hw/virtio/vhost.h"
> #include "hw/hw.h"
> +#include "qemu/atomic.h"
> #include "qemu/range.h"
> #include <linux/vhost.h>
> #include "exec/address-spaces.h"
> @@ -47,11 +48,9 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
> addr += VHOST_LOG_CHUNK;
> continue;
> }
> - /* Data must be read atomically. We don't really
> - * need the barrier semantics of __sync
> - * builtins, but it's easier to use them than
> - * roll our own. */
> - log = __sync_fetch_and_and(from, 0);
> + /* Data must be read atomically. We don't really need barrier
> semantics
> + * but it's easier to use atomic_* than roll our own. */
> + log = atomic_xchg(from, 0);
If you really don't need any ordering guarantees / barriers here, then
using a relaxed load should be fine. But my gut feeling tells me you
probably do need some barriers; either you are "re-using" another
barrier (and then the comment should probably point out which), or it
must be a case where it's either fine to read any value someone (else)
wrote or there's no concurrent store after all.