From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34757) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UpPFS-0005rV-Nw for qemu-devel@nongnu.org; Wed, 19 Jun 2013 16:45:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UpPFL-0006EE-Pz for qemu-devel@nongnu.org; Wed, 19 Jun 2013 16:44:58 -0400 Received: from mail-vc0-f170.google.com ([209.85.220.170]:37039) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UpPFL-0006E9-M5 for qemu-devel@nongnu.org; Wed, 19 Jun 2013 16:44:51 -0400 Received: by mail-vc0-f170.google.com with SMTP id hf12so4179326vcb.15 for ; Wed, 19 Jun 2013 13:44:51 -0700 (PDT) Sender: Richard Henderson Message-ID: <51C2183D.7050605@twiddle.net> Date: Wed, 19 Jun 2013 13:44:45 -0700 From: Richard Henderson MIME-Version: 1.0 References: <1371675569-6516-1-git-send-email-pingfank@linux.vnet.ibm.com> <1371675569-6516-2-git-send-email-pingfank@linux.vnet.ibm.com> <1371674394.16968.26000.camel@triegel.csb> In-Reply-To: <1371674394.16968.26000.camel@triegel.csb> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 1/2] add a header file for atomic operations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Torvald Riegel Cc: Kevin Wolf , Anthony Liguori , Andrew Haley , Liu Ping Fan , qemu-devel@nongnu.org, Stefan Hajnoczi , Paolo Bonzini , "Paul E. McKenney" On 06/19/2013 01:39 PM, Torvald Riegel wrote: > 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 >> #include "hw/virtio/vhost.h" >> #include "hw/hw.h" >> +#include "qemu/atomic.h" >> #include "qemu/range.h" >> #include >> #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. > There is a store here, before and after. Read the value, store zero. I suppose what the comment is saying is that the atomic operation doesn't need to be ordered with respect to the rest of the surrounding code, as the object being synchronized is just that one integer. r~