|
From: | Chrysostomos Nanakos |
Subject: | Re: [Qemu-devel] [PATCH v1 2/2] block/archipelago: Use QEMU atomic builtins |
Date: | Mon, 01 Sep 2014 13:13:07 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.7.0 |
On 09/01/2014 12:43 PM, Paolo Bonzini wrote:
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 <address@hidden> --- 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).
Nice catch!
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.
Yes atomic_inc seems to be a lot better. Thanks!
Paoloif (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) {
What about this one? It seems easier to me to read the above and understand what is going on.
By any means, it's up to you to accept patch 1 :) Regards, Chrysostomos.
g_free(segreq); } }
[Prev in Thread] | Current Thread | [Next in Thread] |