Message ID | 1347968442-8860-15-git-send-email-kwolf@redhat.com |
---|---|
State | New |
Headers | show |
Il 18/09/2012 13:40, Kevin Wolf ha scritto: > static void run_dependent_requests(BDRVQcowState *s, QCowL2Meta *m) > { > - /* Take the request off the list of running requests */ > - if (m->nb_clusters != 0) { > - QLIST_REMOVE(m, next_in_flight); > - } > - > /* Restart all dependent requests */ > if (!qemu_co_queue_empty(&m->dependent_requests)) { > - qemu_co_mutex_unlock(&s->lock); > qemu_co_queue_restart_all(&m->dependent_requests); > - qemu_co_mutex_lock(&s->lock); > } The comment and if can go away. Perhaps this patch could be moved earlier in the series? (Just asking, in case the rebase is not too painful). Paolo > } > > @@ -800,10 +793,18 @@ again: > goto again; > } > > + qemu_co_mutex_unlock(&s->lock); > + > + /* Take the request off the list of running requests */ > + if (m->nb_clusters != 0) { > + QLIST_REMOVE(m, next_in_flight); > + } > + > + /* Meanwhile some new dependencies could have accumulated */
Am 18.09.2012 16:33, schrieb Paolo Bonzini: > Il 18/09/2012 13:40, Kevin Wolf ha scritto: >> static void run_dependent_requests(BDRVQcowState *s, QCowL2Meta *m) >> { >> - /* Take the request off the list of running requests */ >> - if (m->nb_clusters != 0) { >> - QLIST_REMOVE(m, next_in_flight); >> - } >> - >> /* Restart all dependent requests */ >> if (!qemu_co_queue_empty(&m->dependent_requests)) { >> - qemu_co_mutex_unlock(&s->lock); >> qemu_co_queue_restart_all(&m->dependent_requests); >> - qemu_co_mutex_lock(&s->lock); >> } > > The comment and if can go away. Ah, yes. And the one remaining line can be open coded in the two callers. > Perhaps this patch could be moved earlier in the series? (Just asking, > in case the rebase is not too painful). I'll check how painful it becomes. I'd expect it's doable. Kevin
diff --git a/block/qcow2.c b/block/qcow2.c index e001436..88a2020 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -739,16 +739,9 @@ fail: static void run_dependent_requests(BDRVQcowState *s, QCowL2Meta *m) { - /* Take the request off the list of running requests */ - if (m->nb_clusters != 0) { - QLIST_REMOVE(m, next_in_flight); - } - /* Restart all dependent requests */ if (!qemu_co_queue_empty(&m->dependent_requests)) { - qemu_co_mutex_unlock(&s->lock); qemu_co_queue_restart_all(&m->dependent_requests); - qemu_co_mutex_lock(&s->lock); } } @@ -800,10 +793,18 @@ again: goto again; } + qemu_co_mutex_unlock(&s->lock); + + /* Take the request off the list of running requests */ + if (m->nb_clusters != 0) { + QLIST_REMOVE(m, next_in_flight); + } + + /* Meanwhile some new dependencies could have accumulated */ run_dependent_requests(s, m); + g_free(m); - qemu_co_mutex_unlock(&s->lock); qemu_co_rwlock_unlock(&s->l2meta_flush); } @@ -961,13 +962,16 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs, ret = 0; fail: + qemu_co_mutex_unlock(&s->lock); + if (l2meta != NULL) { + if (l2meta->nb_clusters != 0) { + QLIST_REMOVE(l2meta, next_in_flight); + } run_dependent_requests(s, l2meta); g_free(l2meta); } - qemu_co_mutex_unlock(&s->lock); - qemu_iovec_destroy(&hd_qiov); qemu_vfree(cluster_data); trace_qcow2_writev_done_req(qemu_coroutine_self(), ret); @@ -1259,7 +1263,7 @@ static int preallocate(BlockDriverState *bs) /* There are no dependent requests, but we need to remove our request * from the list of in-flight requests */ if (meta != NULL) { - run_dependent_requests(bs->opaque, meta); + QLIST_REMOVE(meta, next_in_flight); } /* TODO Preallocate data if requested */
There's no reason for run_dependent_requests() to hold s->lock, and a later patch will require that in fact the lock is not held. Also, before this patch, run_dependent_requests() not only does what its name suggests, but also removes the l2meta from the list of in-flight requests. Change this, while we're touching it. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/qcow2.c | 26 +++++++++++++++----------- 1 files changed, 15 insertions(+), 11 deletions(-)