| Message ID | 20180813022006.7216-10-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | mirror: Mainly coroutine refinements | expand |
On 13/08/2018 04:19, Max Reitz wrote: > Signed-off-by: Max Reitz <mreitz@redhat.com> Locking AioContext should not be needed anywhere here. mirror_run is called via aio_co_enter or aio_co_wake, so the lock is actually already taken every time you call aio_context_acquire. It was needed only because AIO callbacks do *not* take the lock, but it's not needed anymore since the conversion to coroutines. Paolo > block/mirror.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/block/mirror.c b/block/mirror.c > index 85b08086cc..6330269156 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -196,7 +196,6 @@ static void coroutine_fn mirror_write_complete(MirrorOp *op, int ret) > { > MirrorBlockJob *s = op->s; > > - aio_context_acquire(blk_get_aio_context(s->common.blk)); > if (ret < 0) { > BlockErrorAction action; > > @@ -207,14 +206,12 @@ static void coroutine_fn mirror_write_complete(MirrorOp *op, int ret) > } > } > mirror_iteration_done(op, ret); > - aio_context_release(blk_get_aio_context(s->common.blk)); > } > > static void coroutine_fn mirror_read_complete(MirrorOp *op, int ret) > { > MirrorBlockJob *s = op->s; > > - aio_context_acquire(blk_get_aio_context(s->common.blk)); > if (ret < 0) { > BlockErrorAction action; > > @@ -230,7 +227,6 @@ static void coroutine_fn mirror_read_complete(MirrorOp *op, int ret) > op->qiov.size, &op->qiov, 0); > mirror_write_complete(op, ret); > } > - aio_context_release(blk_get_aio_context(s->common.blk)); > } > > /* Clip bytes relative to offset to not exceed end-of-file */ > @@ -387,12 +383,16 @@ static void coroutine_fn mirror_co_perform(void *opaque) > { > MirrorOp *op = opaque; > MirrorBlockJob *s = op->s; > + AioContext *aio_context; > int ret; > > + aio_context = blk_get_aio_context(s->common.blk); > + aio_context_acquire(aio_context); > + > switch (op->mirror_method) { > case MIRROR_METHOD_COPY: > mirror_co_read(opaque); > - return; > + goto done; > case MIRROR_METHOD_ZERO: > ret = mirror_co_zero(s, op->offset, op->bytes); > break; > @@ -404,6 +404,9 @@ static void coroutine_fn mirror_co_perform(void *opaque) > } > > mirror_write_complete(op, ret); > + > +done: > + aio_context_release(aio_context); > } > > /* If mirror_method == MIRROR_METHOD_COPY, *offset and *bytes will be >
On 2018-08-13 16:43, Paolo Bonzini wrote: > On 13/08/2018 04:19, Max Reitz wrote: >> Signed-off-by: Max Reitz <mreitz@redhat.com> > > Locking AioContext should not be needed anywhere here. mirror_run is > called via aio_co_enter or aio_co_wake, so the lock is actually already > taken every time you call aio_context_acquire. > > It was needed only because AIO callbacks do *not* take the lock, but > it's not needed anymore since the conversion to coroutines. Uh, nice. Thanks for letting me know! :-) I'll drop the locks in v2. Max
diff --git a/block/mirror.c b/block/mirror.c index 85b08086cc..6330269156 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -196,7 +196,6 @@ static void coroutine_fn mirror_write_complete(MirrorOp *op, int ret) { MirrorBlockJob *s = op->s; - aio_context_acquire(blk_get_aio_context(s->common.blk)); if (ret < 0) { BlockErrorAction action; @@ -207,14 +206,12 @@ static void coroutine_fn mirror_write_complete(MirrorOp *op, int ret) } } mirror_iteration_done(op, ret); - aio_context_release(blk_get_aio_context(s->common.blk)); } static void coroutine_fn mirror_read_complete(MirrorOp *op, int ret) { MirrorBlockJob *s = op->s; - aio_context_acquire(blk_get_aio_context(s->common.blk)); if (ret < 0) { BlockErrorAction action; @@ -230,7 +227,6 @@ static void coroutine_fn mirror_read_complete(MirrorOp *op, int ret) op->qiov.size, &op->qiov, 0); mirror_write_complete(op, ret); } - aio_context_release(blk_get_aio_context(s->common.blk)); } /* Clip bytes relative to offset to not exceed end-of-file */ @@ -387,12 +383,16 @@ static void coroutine_fn mirror_co_perform(void *opaque) { MirrorOp *op = opaque; MirrorBlockJob *s = op->s; + AioContext *aio_context; int ret; + aio_context = blk_get_aio_context(s->common.blk); + aio_context_acquire(aio_context); + switch (op->mirror_method) { case MIRROR_METHOD_COPY: mirror_co_read(opaque); - return; + goto done; case MIRROR_METHOD_ZERO: ret = mirror_co_zero(s, op->offset, op->bytes); break; @@ -404,6 +404,9 @@ static void coroutine_fn mirror_co_perform(void *opaque) } mirror_write_complete(op, ret); + +done: + aio_context_release(aio_context); } /* If mirror_method == MIRROR_METHOD_COPY, *offset and *bytes will be
Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/mirror.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)