From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:48827)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1XOO9Q-0004GF-4W
	for qemu-devel@nongnu.org; Mon, 01 Sep 2014 05:43:57 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1XOO9L-0002Xu-3U
	for qemu-devel@nongnu.org; Mon, 01 Sep 2014 05:43:52 -0400
Received: from mx1.redhat.com ([209.132.183.28]:34087)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1XOO9K-0002Ws-Q8
	for qemu-devel@nongnu.org; Mon, 01 Sep 2014 05:43:47 -0400
Message-ID: <54043FC6.9090206@redhat.com>
Date: Mon, 01 Sep 2014 11:43:34 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1409561921-4049-1-git-send-email-cnanakos@grnet.gr>
	<1409561921-4049-2-git-send-email-cnanakos@grnet.gr>
In-Reply-To: <1409561921-4049-2-git-send-email-cnanakos@grnet.gr>
Content-Type: text/plain; charset=iso-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v1 2/2] block/archipelago: Use QEMU atomic
	builtins
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Chrysostomos Nanakos <cnanakos@grnet.gr>, qemu-devel@nongnu.org
Cc: kwolf@redhat.com, stefanha@redhat.com

Il 01/09/2014 10:58, Chrysostomos Nanakos ha scritto:
> Replace __sync builtins with the ones provided by QEMU
> for atomic operations.
> 
> Signed-off-by: Chrysostomos Nanakos <cnanakos@grnet.gr>
> ---
>  block/archipelago.c |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/block/archipelago.c b/block/archipelago.c
> index 34f72dc..fa8cd29 100644
> --- a/block/archipelago.c
> +++ b/block/archipelago.c
> @@ -57,6 +57,7 @@
>  #include "qapi/qmp/qint.h"
>  #include "qapi/qmp/qstring.h"
>  #include "qapi/qmp/qjson.h"
> +#include "qemu/atomic.h"
>  
>  #include <inttypes.h>
>  #include <xseg/xseg.h>
> @@ -214,7 +215,7 @@ static void xseg_request_handler(void *state)
>  
>                  xseg_put_request(s->xseg, req, s->srcport);
>  
> -                if ((__sync_add_and_fetch(&segreq->ref, -1)) == 0) {
> +                if ((atomic_add_fetch(&segreq->ref, -1)) == 0) {

Why not just use "== 1" and avoid patch 1? :)

(Also, you could use atomic_fetch_dec).

>                      if (!segreq->failed) {
>                          reqdata->aio_cb->ret = segreq->count;
>                          archipelago_finish_aiocb(reqdata);
> @@ -233,7 +234,7 @@ static void xseg_request_handler(void *state)
>                  segreq->count += req->serviced;
>                  xseg_put_request(s->xseg, req, s->srcport);
>  
> -                if ((__sync_add_and_fetch(&segreq->ref, -1)) == 0) {
> +                if ((atomic_add_fetch(&segreq->ref, -1)) == 0) {
>                      if (!segreq->failed) {
>                          reqdata->aio_cb->ret = segreq->count;
>                          archipelago_finish_aiocb(reqdata);
> @@ -885,13 +886,13 @@ static int archipelago_aio_segmented_rw(BDRVArchipelagoState *s,
>      return 0;
>  
>  err_exit:
> -    __sync_add_and_fetch(&segreq->failed, 1);
> +    atomic_add_fetch(&segreq->failed, 1);

You can use atomic_inc here.

Paolo

>      if (segments_nr == 1) {
> -        if (__sync_add_and_fetch(&segreq->ref, -1) == 0) {
> +        if (atomic_add_fetch(&segreq->ref, -1) == 0) {
>              g_free(segreq);
>          }
>      } else {
> -        if ((__sync_add_and_fetch(&segreq->ref, -segments_nr + i)) == 0) {
> +        if ((atomic_add_fetch(&segreq->ref, -segments_nr + i)) == 0) {
>              g_free(segreq);
>          }
>      }
>