From: Paolo Bonzini <pbonzini@redhat.com>
To: Chrysostomos Nanakos <cnanakos@grnet.gr>, qemu-devel@nongnu.org
Cc: kwolf@redhat.com, stefanha@redhat.com
Subject: Re: [Qemu-devel] [PATCH] block/archipelago: Use QEMU atomic builtins
Date: Tue, 02 Sep 2014 15:09:35 +0200 [thread overview]
Message-ID: <5405C18F.8080503@redhat.com> (raw)
In-Reply-To: <1409661695-32038-1-git-send-email-cnanakos@grnet.gr>
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
next prev parent reply other threads:[~2014-09-02 13:09 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-02 12:41 [Qemu-devel] [PATCH] block/archipelago: Use QEMU atomic builtins Chrysostomos Nanakos
2014-09-02 13:09 ` Paolo Bonzini [this message]
2014-09-03 10:37 ` Stefan Hajnoczi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5405C18F.8080503@redhat.com \
--to=pbonzini@redhat.com \
--cc=cnanakos@grnet.gr \
--cc=kwolf@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).