Re: [PATCH 1/2] qemu-kvm/rbd: use new librados callback for ack/complete

[Yehuda Sadeh Weinraub <yehudasa@xxxxxxxxx>]

This one should fix and remove the nasty usleep that handles the acb
initialization race. I didn't verify that it actually works, so please
let me know if there are any issues with it.

Thanks,
Yehuda

diff --git a/block/rbd.c b/block/rbd.c
index 3ebf285..7128e23 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -56,7 +56,6 @@ typedef struct RBDAIOCB {
     int write;
     int64_t sector_num;
     int aiocnt;
-    int rccomplete;
 } RBDAIOCB;

 typedef struct RADOSCB {
@@ -391,11 +390,7 @@ static void rbd_finish_aiocb(rados_completion_t
c, RADOSCB * rcb)
     }
     qemu_free(rcb);
     i = 0;
-    while ((acb->aiocnt == 0) && !acb->rccomplete && i < 5) {
-        usleep(100);
-        i++;
-    }
-    if ((acb->aiocnt == 0) && acb->rccomplete && acb->bh) {
+    if ((acb->aiocnt == 0) && acb->bh) {
         qemu_bh_schedule(acb->bh);
     }
 }
@@ -425,7 +420,7 @@ static BlockDriverAIOCB
*rbd_aio_rw_vector(BlockDriverState * bs,
     RADOSCB *rcb;
     rados_completion_t c;
     char n[RBD_MAX_SEG_NAME_SIZE];
-    int64_t segnr, segoffs, segsize;
+    int64_t segnr, segoffs, segsize, last_segnr;
     int64_t off, size;
     char *buf;

@@ -437,7 +432,6 @@ static BlockDriverAIOCB
*rbd_aio_rw_vector(BlockDriverState * bs,
     acb->bounce = qemu_blockalign(bs, qiov->size);
     acb->aiocnt = 0;
     acb->ret = 0;
-    acb->rccomplete = 0;

     if (!acb->bh) {
         acb->bh = qemu_bh_new(rbd_aio_bh_cb, acb);
@@ -455,6 +449,9 @@ static BlockDriverAIOCB
*rbd_aio_rw_vector(BlockDriverState * bs,
     segoffs = (int64_t) (off % s->objsize);
     segsize = (int64_t) (s->objsize - segoffs);

+    last_segnr = ((off + size - 1) / s->objsize);
+    acb->aiocnt = (last_segnr - segnr) + 1;
+
     while (size > 0) {
         if (size < segsize) {
             segsize = size;
@@ -469,8 +466,6 @@ static BlockDriverAIOCB
*rbd_aio_rw_vector(BlockDriverState * bs,
         rcb->segsize = segsize;
         rcb->buf = buf;

-        acb->aiocnt++;
-
         rados_aio_create_completion(rcb, (rados_callback_t) rbd_finish_aiocb,
                                     NULL, &c);
         if (write) {
@@ -486,8 +481,6 @@ static BlockDriverAIOCB
*rbd_aio_rw_vector(BlockDriverState * bs,
         segnr++;
     }

-    acb->rccomplete = 1;
-
     return &acb->common;
 }

On Thu, May 6, 2010 at 1:54 PM, Christian Brunner <chb@xxxxxx> wrote:
>
> This patch uses the new librados aio callback function (from the unstable
> branch of ceph).
>
> Christian
> ---
>  block/rbd.c |   51 ++++++++++++++++++++++++---------------------------
>  1 files changed, 24 insertions(+), 27 deletions(-)
>
> diff --git a/block/rbd.c b/block/rbd.c
> index 12a85dc..dfd2eca 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -393,34 +393,31 @@ static void rbd_finish_aiocb(rados_completion_t c, RADOSCB * rcb)
>     int64_t r;
>     int i;
>
> -    if (rados_aio_is_complete(c)) {
> -        acb->aiocnt--;
> -        r = rados_aio_get_return_value(c);
> -        rados_aio_set_callback(c, NULL, NULL);
> -        rados_aio_release(c);
> -        if (acb->write) {
> -            acb->ret += r;
> +    acb->aiocnt--;
> +    r = rados_aio_get_return_value(c);
> +    rados_aio_release(c);
> +    if (acb->write) {
> +        acb->ret += r;
> +    } else {
> +        if (r < 0) {
> +            memset(rcb->buf, 0, rcb->segsize);
> +            acb->ret += rcb->segsize;
> +        } else if (r < rcb->segsize) {
> +            memset(rcb->buf + r, 0, rcb->segsize - r);
> +            acb->ret += rcb->segsize;
>         } else {
> -            if (r < 0) {
> -                memset(rcb->buf, 0, rcb->segsize);
> -                acb->ret += rcb->segsize;
> -            } else if (r < rcb->segsize) {
> -                memset(rcb->buf + r, 0, rcb->segsize - r);
> -                acb->ret += rcb->segsize;
> -            } else {
> -                acb->ret += r;
> -            }
> -        }
> -        qemu_free(rcb);
> -        i = 0;
> -        while ((acb->aiocnt == 0) && !acb->rccomplete && i < 5) {
> -            usleep(100);
> -            i++;
> -        }
> -        if ((acb->aiocnt == 0) && acb->rccomplete && acb->bh) {
> -            qemu_bh_schedule(acb->bh);
> +            acb->ret += r;
>         }
>     }
> +    qemu_free(rcb);
> +    i = 0;
> +    while ((acb->aiocnt == 0) && !acb->rccomplete && i < 5) {
> +        usleep(100);
> +        i++;
> +    }
> +    if ((acb->aiocnt == 0) && acb->rccomplete && acb->bh) {
> +        qemu_bh_schedule(acb->bh);
> +    }
>  }
>
>  static void rbd_aio_bh_cb(void *opaque)
> @@ -494,8 +491,8 @@ static BlockDriverAIOCB *rbd_aio_rw_vector(BlockDriverState * bs,
>
>         acb->aiocnt++;
>
> -        rados_aio_create_completion((rados_callback_t) rbd_finish_aiocb,
> -                                    rcb, &c);
> +        rados_aio_create_completion(rcb, (rados_callback_t) rbd_finish_aiocb,
> +                                    NULL, &c);
>         if (write) {
>             rados_aio_write(s->pool, n, segoffs, buf, segsize, c);
>         } else {
> --
> 1.6.6.1
>
>
> --
> Christian Brunner                              MUC.DE e.V.
>                                               Joseph-Dollinger-Bogen 14
>                                               D-80807 Muenchen
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html