* [Qemu-devel] [PATCH] block/archipelago: Use QEMU atomic builtins
@ 2014-09-02 12:41 Chrysostomos Nanakos
2014-09-02 13:09 ` Paolo Bonzini
2014-09-03 10:37 ` Stefan Hajnoczi
0 siblings, 2 replies; 3+ messages in thread
From: Chrysostomos Nanakos @ 2014-09-02 12:41 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, Chrysostomos Nanakos, stefanha
Replace __sync builtins with ones provided by QEMU
for atomic operations.
Special thanks goes to Paolo Bonzini for his refactoring
suggestion in order to use the already existing atomic builtins
interface.
Signed-off-by: Chrysostomos Nanakos <cnanakos@grnet.gr>
---
block/archipelago.c | 76 ++++++++++++++++-----------------------------------
1 file changed, 23 insertions(+), 53 deletions(-)
diff --git a/block/archipelago.c b/block/archipelago.c
index 34f72dc..22a7daa 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_fetch_dec(&segreq->ref) == 1) {
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_fetch_dec(&segreq->ref) == 1) {
if (!segreq->failed) {
reqdata->aio_cb->ret = segreq->count;
archipelago_finish_aiocb(reqdata);
@@ -824,78 +825,47 @@ static int archipelago_aio_segmented_rw(BDRVArchipelagoState *s,
ArchipelagoAIOCB *aio_cb,
int op)
{
- int i, ret, segments_nr, last_segment_size;
+ int ret, segments_nr;
+ size_t pos = 0;
ArchipelagoSegmentedRequest *segreq;
- segreq = g_new(ArchipelagoSegmentedRequest, 1);
+ segreq = g_new0(ArchipelagoSegmentedRequest, 1);
if (op == ARCHIP_OP_FLUSH) {
segments_nr = 1;
- segreq->ref = segments_nr;
- segreq->total = count;
- segreq->count = 0;
- segreq->failed = 0;
- ret = archipelago_submit_request(s, 0, count, offset, aio_cb,
- segreq, ARCHIP_OP_FLUSH);
- if (ret < 0) {
- goto err_exit;
- }
- return 0;
+ } else {
+ segments_nr = (int)(count / MAX_REQUEST_SIZE) + \
+ ((count % MAX_REQUEST_SIZE) ? 1 : 0);
}
-
- segments_nr = (int)(count / MAX_REQUEST_SIZE) + \
- ((count % MAX_REQUEST_SIZE) ? 1 : 0);
- last_segment_size = (int)(count % MAX_REQUEST_SIZE);
-
- segreq->ref = segments_nr;
segreq->total = count;
- segreq->count = 0;
- segreq->failed = 0;
+ atomic_mb_set(&segreq->ref, segments_nr);
- for (i = 0; i < segments_nr - 1; i++) {
- ret = archipelago_submit_request(s, i * MAX_REQUEST_SIZE,
- MAX_REQUEST_SIZE,
- offset + i * MAX_REQUEST_SIZE,
- aio_cb, segreq, op);
+ while (segments_nr > 1) {
+ ret = archipelago_submit_request(s, pos,
+ MAX_REQUEST_SIZE,
+ offset + pos,
+ aio_cb, segreq, op);
if (ret < 0) {
goto err_exit;
}
+ count -= MAX_REQUEST_SIZE;
+ pos += MAX_REQUEST_SIZE;
+ segments_nr--;
}
-
- if ((segments_nr > 1) && last_segment_size) {
- ret = archipelago_submit_request(s, i * MAX_REQUEST_SIZE,
- last_segment_size,
- offset + i * MAX_REQUEST_SIZE,
- aio_cb, segreq, op);
- } else if ((segments_nr > 1) && !last_segment_size) {
- ret = archipelago_submit_request(s, i * MAX_REQUEST_SIZE,
- MAX_REQUEST_SIZE,
- offset + i * MAX_REQUEST_SIZE,
- aio_cb, segreq, op);
- } else if (segments_nr == 1) {
- ret = archipelago_submit_request(s, 0, count, offset, aio_cb,
- segreq, op);
- }
+ ret = archipelago_submit_request(s, pos, count, offset + pos,
+ aio_cb, segreq, op);
if (ret < 0) {
goto err_exit;
}
-
return 0;
err_exit:
- __sync_add_and_fetch(&segreq->failed, 1);
- if (segments_nr == 1) {
- if (__sync_add_and_fetch(&segreq->ref, -1) == 0) {
- g_free(segreq);
- }
- } else {
- if ((__sync_add_and_fetch(&segreq->ref, -segments_nr + i)) == 0) {
- g_free(segreq);
- }
+ segreq->failed = 1;
+ if (atomic_fetch_sub(&segreq->ref, segments_nr) == segments_nr) {
+ g_free(segreq);
}
-
return ret;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] block/archipelago: Use QEMU atomic builtins
2014-09-02 12:41 [Qemu-devel] [PATCH] block/archipelago: Use QEMU atomic builtins Chrysostomos Nanakos
@ 2014-09-02 13:09 ` Paolo Bonzini
2014-09-03 10:37 ` Stefan Hajnoczi
1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2014-09-02 13:09 UTC (permalink / raw)
To: Chrysostomos Nanakos, qemu-devel; +Cc: kwolf, stefanha
Il 02/09/2014 14:41, Chrysostomos Nanakos ha scritto:
> Replace __sync builtins with ones provided by QEMU
> for atomic operations.
>
> Special thanks goes to Paolo Bonzini for his refactoring
> suggestion in order to use the already existing atomic builtins
> interface.
>
> Signed-off-by: Chrysostomos Nanakos <cnanakos@grnet.gr>
> ---
> block/archipelago.c | 76 ++++++++++++++++-----------------------------------
> 1 file changed, 23 insertions(+), 53 deletions(-)
>
> diff --git a/block/archipelago.c b/block/archipelago.c
> index 34f72dc..22a7daa 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_fetch_dec(&segreq->ref) == 1) {
> 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_fetch_dec(&segreq->ref) == 1) {
> if (!segreq->failed) {
> reqdata->aio_cb->ret = segreq->count;
> archipelago_finish_aiocb(reqdata);
> @@ -824,78 +825,47 @@ static int archipelago_aio_segmented_rw(BDRVArchipelagoState *s,
> ArchipelagoAIOCB *aio_cb,
> int op)
> {
> - int i, ret, segments_nr, last_segment_size;
> + int ret, segments_nr;
> + size_t pos = 0;
> ArchipelagoSegmentedRequest *segreq;
>
> - segreq = g_new(ArchipelagoSegmentedRequest, 1);
> + segreq = g_new0(ArchipelagoSegmentedRequest, 1);
>
> if (op == ARCHIP_OP_FLUSH) {
> segments_nr = 1;
> - segreq->ref = segments_nr;
> - segreq->total = count;
> - segreq->count = 0;
> - segreq->failed = 0;
> - ret = archipelago_submit_request(s, 0, count, offset, aio_cb,
> - segreq, ARCHIP_OP_FLUSH);
> - if (ret < 0) {
> - goto err_exit;
> - }
> - return 0;
> + } else {
> + segments_nr = (int)(count / MAX_REQUEST_SIZE) + \
> + ((count % MAX_REQUEST_SIZE) ? 1 : 0);
> }
> -
> - segments_nr = (int)(count / MAX_REQUEST_SIZE) + \
> - ((count % MAX_REQUEST_SIZE) ? 1 : 0);
> - last_segment_size = (int)(count % MAX_REQUEST_SIZE);
> -
> - segreq->ref = segments_nr;
> segreq->total = count;
> - segreq->count = 0;
> - segreq->failed = 0;
> + atomic_mb_set(&segreq->ref, segments_nr);
>
> - for (i = 0; i < segments_nr - 1; i++) {
> - ret = archipelago_submit_request(s, i * MAX_REQUEST_SIZE,
> - MAX_REQUEST_SIZE,
> - offset + i * MAX_REQUEST_SIZE,
> - aio_cb, segreq, op);
> + while (segments_nr > 1) {
> + ret = archipelago_submit_request(s, pos,
> + MAX_REQUEST_SIZE,
> + offset + pos,
> + aio_cb, segreq, op);
>
> if (ret < 0) {
> goto err_exit;
> }
> + count -= MAX_REQUEST_SIZE;
> + pos += MAX_REQUEST_SIZE;
> + segments_nr--;
> }
> -
> - if ((segments_nr > 1) && last_segment_size) {
> - ret = archipelago_submit_request(s, i * MAX_REQUEST_SIZE,
> - last_segment_size,
> - offset + i * MAX_REQUEST_SIZE,
> - aio_cb, segreq, op);
> - } else if ((segments_nr > 1) && !last_segment_size) {
> - ret = archipelago_submit_request(s, i * MAX_REQUEST_SIZE,
> - MAX_REQUEST_SIZE,
> - offset + i * MAX_REQUEST_SIZE,
> - aio_cb, segreq, op);
> - } else if (segments_nr == 1) {
> - ret = archipelago_submit_request(s, 0, count, offset, aio_cb,
> - segreq, op);
> - }
> + ret = archipelago_submit_request(s, pos, count, offset + pos,
> + aio_cb, segreq, op);
>
> if (ret < 0) {
> goto err_exit;
> }
> -
> return 0;
>
> err_exit:
> - __sync_add_and_fetch(&segreq->failed, 1);
> - if (segments_nr == 1) {
> - if (__sync_add_and_fetch(&segreq->ref, -1) == 0) {
> - g_free(segreq);
> - }
> - } else {
> - if ((__sync_add_and_fetch(&segreq->ref, -segments_nr + i)) == 0) {
> - g_free(segreq);
> - }
> + segreq->failed = 1;
> + if (atomic_fetch_sub(&segreq->ref, segments_nr) == segments_nr) {
> + g_free(segreq);
> }
> -
> return ret;
> }
>
>
Thanks for picking up the patch!
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] block/archipelago: Use QEMU atomic builtins
2014-09-02 12:41 [Qemu-devel] [PATCH] block/archipelago: Use QEMU atomic builtins Chrysostomos Nanakos
2014-09-02 13:09 ` Paolo Bonzini
@ 2014-09-03 10:37 ` Stefan Hajnoczi
1 sibling, 0 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2014-09-03 10:37 UTC (permalink / raw)
To: Chrysostomos Nanakos; +Cc: kwolf, pbonzini, qemu-devel, stefanha
[-- Attachment #1: Type: text/plain, Size: 598 bytes --]
On Tue, Sep 02, 2014 at 03:41:34PM +0300, Chrysostomos Nanakos wrote:
> Replace __sync builtins with ones provided by QEMU
> for atomic operations.
>
> Special thanks goes to Paolo Bonzini for his refactoring
> suggestion in order to use the already existing atomic builtins
> interface.
>
> Signed-off-by: Chrysostomos Nanakos <cnanakos@grnet.gr>
> ---
> block/archipelago.c | 76 ++++++++++++++++-----------------------------------
> 1 file changed, 23 insertions(+), 53 deletions(-)
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-09-03 10:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-02 12:41 [Qemu-devel] [PATCH] block/archipelago: Use QEMU atomic builtins Chrysostomos Nanakos
2014-09-02 13:09 ` Paolo Bonzini
2014-09-03 10:37 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).