Patchwork [RFC,10/16] qcow2: Delay the COW

login
register
mail settings
Submitter Kevin Wolf
Date Sept. 18, 2012, 11:40 a.m.
Message ID <1347968442-8860-11-git-send-email-kwolf@redhat.com>
Download mbox | patch
Permalink /patch/184684/
State New
Headers show

Comments

Kevin Wolf - Sept. 18, 2012, 11:40 a.m.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cluster.c |   29 +++++++++++++++++++++++++++++
 block/qcow2.c         |   31 ++++++++++++++++++++++++++++---
 block/qcow2.h         |   10 ++++++++++
 3 files changed, 67 insertions(+), 3 deletions(-)
Paolo Bonzini - Sept. 18, 2012, 2:27 p.m.
Il 18/09/2012 13:40, Kevin Wolf ha scritto:
> +again:
> +    QLIST_FOREACH(m, &s->cluster_allocs, next_in_flight) {
> +        if (m->sleeping) {
> +            qemu_coroutine_enter(m->co, NULL);
> +            /* next_in_flight link could have become invalid */
> +            goto again;
> +        }
> +    }
> +
>      qemu_co_rwlock_wrlock(&s->l2meta_flush);
>  }
>  
>  static inline coroutine_fn void resume_l2meta(BDRVQcowState *s)
>  {
> +    s->in_l2meta_flush = false;
>      qemu_co_rwlock_unlock(&s->l2meta_flush);
>  }
>  
>  static bool qcow2_drain(BlockDriverState *bs)
>  {
>      BDRVQcowState *s = bs->opaque;
> +    QCowL2Meta *m;
> +
> +    QLIST_FOREACH(m, &s->cluster_allocs, next_in_flight) {
> +        if (m->sleeping) {
> +            qemu_coroutine_enter(m->co, NULL);
> +        }
> +    }
>  

Why are the goto and in_l2meta_flush not needed here?  If they are,
perhaps stop_l2meta can just use qcow2_drain?

Paolo
Kevin Wolf - Sept. 18, 2012, 2:49 p.m.
Am 18.09.2012 16:27, schrieb Paolo Bonzini:
> Il 18/09/2012 13:40, Kevin Wolf ha scritto:
>> +again:
>> +    QLIST_FOREACH(m, &s->cluster_allocs, next_in_flight) {
>> +        if (m->sleeping) {
>> +            qemu_coroutine_enter(m->co, NULL);
>> +            /* next_in_flight link could have become invalid */
>> +            goto again;
>> +        }
>> +    }
>> +
>>      qemu_co_rwlock_wrlock(&s->l2meta_flush);
>>  }
>>  
>>  static inline coroutine_fn void resume_l2meta(BDRVQcowState *s)
>>  {
>> +    s->in_l2meta_flush = false;
>>      qemu_co_rwlock_unlock(&s->l2meta_flush);
>>  }
>>  
>>  static bool qcow2_drain(BlockDriverState *bs)
>>  {
>>      BDRVQcowState *s = bs->opaque;
>> +    QCowL2Meta *m;
>> +
>> +    QLIST_FOREACH(m, &s->cluster_allocs, next_in_flight) {
>> +        if (m->sleeping) {
>> +            qemu_coroutine_enter(m->co, NULL);
>> +        }
>> +    }
>>  
> 
> Why are the goto and in_l2meta_flush not needed here?  If they are,
> perhaps stop_l2meta can just use qcow2_drain?

I think you're right, thanks.

Kevin
Blue Swirl - Sept. 19, 2012, 6:47 p.m.
On Tue, Sep 18, 2012 at 11:40 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2-cluster.c |   29 +++++++++++++++++++++++++++++
>  block/qcow2.c         |   31 ++++++++++++++++++++++++++++---
>  block/qcow2.h         |   10 ++++++++++
>  3 files changed, 67 insertions(+), 3 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index a89d68d..0f50888 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -791,6 +791,34 @@ out:
>      return i;
>  }
>
> +struct KickL2Meta {
> +    QEMUBH *bh;
> +    QCowL2Meta *m;
> +};
> +
> +static void kick_l2meta_bh(void *opaque)
> +{
> +    struct KickL2Meta *k = opaque;
> +    QCowL2Meta *m = k->m;
> +
> +    qemu_bh_delete(k->bh);
> +    free(k);

You use g_malloc() below.

> +
> +    if (m->sleeping) {
> +        qemu_coroutine_enter(m->co, NULL);
> +    }
> +}
> +
> +static void kick_l2meta(QCowL2Meta *m)
> +{
> +    struct KickL2Meta *k = g_malloc(sizeof(*k));
> +
> +    k->bh = qemu_bh_new(kick_l2meta_bh, k);
> +    k->m = m;
> +
> +    qemu_bh_schedule(k->bh);
> +}
> +
>  /*
>   * Check if there already is an AIO write request in flight which allocates
>   * the same cluster. In this case we need to wait until the previous
> @@ -823,6 +851,7 @@ static int handle_dependencies(BlockDriverState *bs, uint64_t guest_offset,
>                  /* Wait for the dependency to complete. We need to recheck
>                   * the free/allocated clusters when we continue. */
>                  qemu_co_mutex_unlock(&s->lock);
> +                kick_l2meta(old_alloc);
>                  qemu_co_queue_wait(&old_alloc->dependent_requests);
>                  qemu_co_mutex_lock(&s->lock);
>                  return -EAGAIN;
> diff --git a/block/qcow2.c b/block/qcow2.c
> index f9881d0..2e220c7 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -765,6 +765,12 @@ static void coroutine_fn process_l2meta(void *opaque)
>      BDRVQcowState *s = bs->opaque;
>      int ret;
>
> +    if (!s->in_l2meta_flush) {
> +        m->sleeping = true;
> +        co_sleep_ns(rt_clock, 1000000);
> +        m->sleeping = false;
> +    }
> +
>      qemu_co_mutex_lock(&s->lock);
>
>      ret = qcow2_alloc_cluster_link_l2(bs, m);
> @@ -781,17 +787,37 @@ static void coroutine_fn process_l2meta(void *opaque)
>
>  static inline coroutine_fn void stop_l2meta(BDRVQcowState *s)
>  {
> +    QCowL2Meta *m;
> +
> +    s->in_l2meta_flush = true;
> +again:
> +    QLIST_FOREACH(m, &s->cluster_allocs, next_in_flight) {
> +        if (m->sleeping) {
> +            qemu_coroutine_enter(m->co, NULL);
> +            /* next_in_flight link could have become invalid */
> +            goto again;
> +        }
> +    }
> +
>      qemu_co_rwlock_wrlock(&s->l2meta_flush);
>  }
>
>  static inline coroutine_fn void resume_l2meta(BDRVQcowState *s)
>  {
> +    s->in_l2meta_flush = false;
>      qemu_co_rwlock_unlock(&s->l2meta_flush);
>  }
>
>  static bool qcow2_drain(BlockDriverState *bs)
>  {
>      BDRVQcowState *s = bs->opaque;
> +    QCowL2Meta *m;
> +
> +    QLIST_FOREACH(m, &s->cluster_allocs, next_in_flight) {
> +        if (m->sleeping) {
> +            qemu_coroutine_enter(m->co, NULL);
> +        }
> +    }
>
>      return !QLIST_EMPTY(&s->cluster_allocs);
>  }
> @@ -876,7 +902,6 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs,
>          }
>
>          if (l2meta != NULL) {
> -            Coroutine *co;
>              ProcessL2Meta p = {
>                  .bs = bs,
>                  .m  = l2meta,
> @@ -886,8 +911,8 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs,
>              qemu_co_rwlock_rdlock(&s->l2meta_flush);
>
>              l2meta->is_written = true;
> -            co = qemu_coroutine_create(process_l2meta);
> -            qemu_coroutine_enter(co, &p);
> +            l2meta->co = qemu_coroutine_create(process_l2meta);
> +            qemu_coroutine_enter(l2meta->co, &p);
>
>              l2meta = NULL;
>              qemu_co_mutex_lock(&s->lock);
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 73dac17..8bf145c 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -169,6 +169,7 @@ typedef struct BDRVQcowState {
>       * Writers: Anyone who requires l2meta to be flushed
>       */
>      CoRwlock l2meta_flush;
> +    bool in_l2meta_flush;
>
>      uint32_t crypt_method; /* current crypt method, 0 if no key yet */
>      uint32_t crypt_method_header;
> @@ -245,6 +246,15 @@ typedef struct QCowL2Meta
>      bool is_written;
>
>      /**
> +     * true if the request is sleeping in the COW delay and the coroutine may
> +     * be reentered in order to cancel the timer.
> +     */
> +    bool sleeping;
> +
> +    /** Coroutine that handles delayed COW and updates L2 entry */
> +    Coroutine *co;
> +
> +    /**
>       * Requests that overlap with this allocation and wait to be restarted
>       * when the allocating request has completed.
>       */
> --
> 1.7.6.5
>
>
Kevin Wolf - Sept. 20, 2012, 6:58 a.m.
Am 19.09.2012 20:47, schrieb Blue Swirl:
> On Tue, Sep 18, 2012 at 11:40 AM, Kevin Wolf <kwolf@redhat.com> wrote:
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>  block/qcow2-cluster.c |   29 +++++++++++++++++++++++++++++
>>  block/qcow2.c         |   31 ++++++++++++++++++++++++++++---
>>  block/qcow2.h         |   10 ++++++++++
>>  3 files changed, 67 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
>> index a89d68d..0f50888 100644
>> --- a/block/qcow2-cluster.c
>> +++ b/block/qcow2-cluster.c
>> @@ -791,6 +791,34 @@ out:
>>      return i;
>>  }
>>
>> +struct KickL2Meta {
>> +    QEMUBH *bh;
>> +    QCowL2Meta *m;
>> +};
>> +
>> +static void kick_l2meta_bh(void *opaque)
>> +{
>> +    struct KickL2Meta *k = opaque;
>> +    QCowL2Meta *m = k->m;
>> +
>> +    qemu_bh_delete(k->bh);
>> +    free(k);
> 
> You use g_malloc() below.

Oops! Thanks, will fix.

Kevin

Patch

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index a89d68d..0f50888 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -791,6 +791,34 @@  out:
     return i;
 }
 
+struct KickL2Meta {
+    QEMUBH *bh;
+    QCowL2Meta *m;
+};
+
+static void kick_l2meta_bh(void *opaque)
+{
+    struct KickL2Meta *k = opaque;
+    QCowL2Meta *m = k->m;
+
+    qemu_bh_delete(k->bh);
+    free(k);
+
+    if (m->sleeping) {
+        qemu_coroutine_enter(m->co, NULL);
+    }
+}
+
+static void kick_l2meta(QCowL2Meta *m)
+{
+    struct KickL2Meta *k = g_malloc(sizeof(*k));
+
+    k->bh = qemu_bh_new(kick_l2meta_bh, k);
+    k->m = m;
+
+    qemu_bh_schedule(k->bh);
+}
+
 /*
  * Check if there already is an AIO write request in flight which allocates
  * the same cluster. In this case we need to wait until the previous
@@ -823,6 +851,7 @@  static int handle_dependencies(BlockDriverState *bs, uint64_t guest_offset,
                 /* Wait for the dependency to complete. We need to recheck
                  * the free/allocated clusters when we continue. */
                 qemu_co_mutex_unlock(&s->lock);
+                kick_l2meta(old_alloc);
                 qemu_co_queue_wait(&old_alloc->dependent_requests);
                 qemu_co_mutex_lock(&s->lock);
                 return -EAGAIN;
diff --git a/block/qcow2.c b/block/qcow2.c
index f9881d0..2e220c7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -765,6 +765,12 @@  static void coroutine_fn process_l2meta(void *opaque)
     BDRVQcowState *s = bs->opaque;
     int ret;
 
+    if (!s->in_l2meta_flush) {
+        m->sleeping = true;
+        co_sleep_ns(rt_clock, 1000000);
+        m->sleeping = false;
+    }
+
     qemu_co_mutex_lock(&s->lock);
 
     ret = qcow2_alloc_cluster_link_l2(bs, m);
@@ -781,17 +787,37 @@  static void coroutine_fn process_l2meta(void *opaque)
 
 static inline coroutine_fn void stop_l2meta(BDRVQcowState *s)
 {
+    QCowL2Meta *m;
+
+    s->in_l2meta_flush = true;
+again:
+    QLIST_FOREACH(m, &s->cluster_allocs, next_in_flight) {
+        if (m->sleeping) {
+            qemu_coroutine_enter(m->co, NULL);
+            /* next_in_flight link could have become invalid */
+            goto again;
+        }
+    }
+
     qemu_co_rwlock_wrlock(&s->l2meta_flush);
 }
 
 static inline coroutine_fn void resume_l2meta(BDRVQcowState *s)
 {
+    s->in_l2meta_flush = false;
     qemu_co_rwlock_unlock(&s->l2meta_flush);
 }
 
 static bool qcow2_drain(BlockDriverState *bs)
 {
     BDRVQcowState *s = bs->opaque;
+    QCowL2Meta *m;
+
+    QLIST_FOREACH(m, &s->cluster_allocs, next_in_flight) {
+        if (m->sleeping) {
+            qemu_coroutine_enter(m->co, NULL);
+        }
+    }
 
     return !QLIST_EMPTY(&s->cluster_allocs);
 }
@@ -876,7 +902,6 @@  static coroutine_fn int qcow2_co_writev(BlockDriverState *bs,
         }
 
         if (l2meta != NULL) {
-            Coroutine *co;
             ProcessL2Meta p = {
                 .bs = bs,
                 .m  = l2meta,
@@ -886,8 +911,8 @@  static coroutine_fn int qcow2_co_writev(BlockDriverState *bs,
             qemu_co_rwlock_rdlock(&s->l2meta_flush);
 
             l2meta->is_written = true;
-            co = qemu_coroutine_create(process_l2meta);
-            qemu_coroutine_enter(co, &p);
+            l2meta->co = qemu_coroutine_create(process_l2meta);
+            qemu_coroutine_enter(l2meta->co, &p);
 
             l2meta = NULL;
             qemu_co_mutex_lock(&s->lock);
diff --git a/block/qcow2.h b/block/qcow2.h
index 73dac17..8bf145c 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -169,6 +169,7 @@  typedef struct BDRVQcowState {
      * Writers: Anyone who requires l2meta to be flushed
      */
     CoRwlock l2meta_flush;
+    bool in_l2meta_flush;
 
     uint32_t crypt_method; /* current crypt method, 0 if no key yet */
     uint32_t crypt_method_header;
@@ -245,6 +246,15 @@  typedef struct QCowL2Meta
     bool is_written;
 
     /**
+     * true if the request is sleeping in the COW delay and the coroutine may
+     * be reentered in order to cancel the timer.
+     */
+    bool sleeping;
+
+    /** Coroutine that handles delayed COW and updates L2 entry */
+    Coroutine *co;
+
+    /**
      * Requests that overlap with this allocation and wait to be restarted
      * when the allocating request has completed.
      */