Re: [Qemu-devel] [PATCH v1 2/2] block/archipelago: Use QEMU atomic builtins

[Paolo Bonzini <pbonzini@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);
>          }
>      }
>