| Message ID | 1360153926-9492-26-git-send-email-benoit@irqsave.net |
|---|---|
| State | New |
| Headers | show |
On 02/06/2013 05:31 AM, Benoît Canet wrote: > This fix the sub cluster sized writes race conditions while waiting s/fix/fixes/ > for a more faster solution. s/more// Would it be worth describing the race condition in more detail?
On Wed, Feb 06, 2013 at 01:31:58PM +0100, Benoît Canet wrote: > This fix the sub cluster sized writes race conditions while waiting > for a more faster solution. > > Signed-off-by: Benoit Canet <benoit@irqsave.net> > --- > block/qcow2.c | 14 +++++++++++++- > block/qcow2.h | 1 + > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/block/qcow2.c b/block/qcow2.c > index f39b6d5..5cfffd2 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -524,6 +524,7 @@ static int qcow2_open(BlockDriverState *bs, int flags) > > /* Initialise locks */ > qemu_co_mutex_init(&s->lock); > + qemu_co_mutex_init(&s->dedup_lock); > > /* Repair image if dirty */ > if (!(flags & BDRV_O_CHECK) && !bs->read_only && > @@ -815,8 +816,15 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs, > s->cluster_cache_offset = -1; /* disable compressed cache */ > > qemu_co_mutex_lock(&s->lock); > - > atomic_dedup_is_running = qcow2_dedup_is_running(bs); > + qemu_co_mutex_unlock(&s->lock); No need to lock/unlock around qcow2_dedup_is_running(bs) because it doesn't yield. No other coroutine can run while our thread of control is active here. > + > + if (atomic_dedup_is_running) { > + qemu_co_mutex_lock(&s->dedup_lock); > + } What exactly does dedup_lock protect? Please document it. > + > + qemu_co_mutex_lock(&s->lock); > + > if (atomic_dedup_is_running) { > QTAILQ_INIT(&ds.undedupables); > ds.phash.reuse = false; > @@ -983,6 +991,10 @@ fail: > g_free(l2meta); > } > > + if (atomic_dedup_is_running) { > + qemu_co_mutex_unlock(&s->dedup_lock); > + } > + > qemu_iovec_destroy(&hd_qiov); > qemu_vfree(cluster_data); > qemu_vfree(dedup_cluster_data); > diff --git a/block/qcow2.h b/block/qcow2.h > index a13ec75..81dc0ea 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -236,6 +236,7 @@ typedef struct BDRVQcowState { > GTree *dedup_tree_by_sect; > > CoMutex lock; > + CoMutex dedup_lock; > > uint32_t crypt_method; /* current crypt method, 0 if no key yet */ > uint32_t crypt_method_header; > -- > 1.7.10.4 >
diff --git a/block/qcow2.c b/block/qcow2.c index f39b6d5..5cfffd2 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -524,6 +524,7 @@ static int qcow2_open(BlockDriverState *bs, int flags) /* Initialise locks */ qemu_co_mutex_init(&s->lock); + qemu_co_mutex_init(&s->dedup_lock); /* Repair image if dirty */ if (!(flags & BDRV_O_CHECK) && !bs->read_only && @@ -815,8 +816,15 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs, s->cluster_cache_offset = -1; /* disable compressed cache */ qemu_co_mutex_lock(&s->lock); - atomic_dedup_is_running = qcow2_dedup_is_running(bs); + qemu_co_mutex_unlock(&s->lock); + + if (atomic_dedup_is_running) { + qemu_co_mutex_lock(&s->dedup_lock); + } + + qemu_co_mutex_lock(&s->lock); + if (atomic_dedup_is_running) { QTAILQ_INIT(&ds.undedupables); ds.phash.reuse = false; @@ -983,6 +991,10 @@ fail: g_free(l2meta); } + if (atomic_dedup_is_running) { + qemu_co_mutex_unlock(&s->dedup_lock); + } + qemu_iovec_destroy(&hd_qiov); qemu_vfree(cluster_data); qemu_vfree(dedup_cluster_data); diff --git a/block/qcow2.h b/block/qcow2.h index a13ec75..81dc0ea 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -236,6 +236,7 @@ typedef struct BDRVQcowState { GTree *dedup_tree_by_sect; CoMutex lock; + CoMutex dedup_lock; uint32_t crypt_method; /* current crypt method, 0 if no key yet */ uint32_t crypt_method_header;
This fix the sub cluster sized writes race conditions while waiting for a more faster solution. Signed-off-by: Benoit Canet <benoit@irqsave.net> --- block/qcow2.c | 14 +++++++++++++- block/qcow2.h | 1 + 2 files changed, 14 insertions(+), 1 deletion(-)