From b06ecde2997b72a41b2f2d25d55e61d30ea46bc2 Mon Sep 17 00:00:00 2001 From: Brian Foster Date: Fri, 18 Oct 2013 07:36:38 -0400 Subject: features/qemu-block: simplify coroutine model to use single synctask, ucontext The current coroutine model, mapping synctasks 1-1 with qemu internal Coroutines, has some unresolved raciness issues. This problem usually manifests as lifecycle mismatches between top-level (gluster created) synctasks and the subsequently created internal coroutines from that context. Qemu's internal queueing (and locking) can cause situations where the top-level synctask is destroyed before the internal scheduler has released references to memory, leading to use after free crashes and asserts. Simplify the coroutine model to use a single synctask as a coroutine processor and rely on the existing native ucontext coroutine implementation. The syncenv thread is donated to qemu and ensures a single top-level coroutine is processed at a time. Qemu now has complete control over coroutine scheduling. BUG: 986775 Change-Id: I38223479a608d80353128e390f243933fc946fd6 Signed-off-by: Brian Foster Reviewed-on: http://review.gluster.org/6110 Tested-by: Gluster Build System Reviewed-by: Anand Avati --- xlators/features/qemu-block/src/qb-coroutines.c | 47 ++++++------------------- 1 file changed, 10 insertions(+), 37 deletions(-) (limited to 'xlators/features/qemu-block/src/qb-coroutines.c') diff --git a/xlators/features/qemu-block/src/qb-coroutines.c b/xlators/features/qemu-block/src/qb-coroutines.c index d29117eb5..7c52adb21 100644 --- a/xlators/features/qemu-block/src/qb-coroutines.c +++ b/xlators/features/qemu-block/src/qb-coroutines.c @@ -29,7 +29,6 @@ int qb_format_and_resume (void *opaque) { - CoroutineSynctask *cs = NULL; qb_local_t *local = NULL; call_frame_t *frame = NULL; call_stub_t *stub = NULL; @@ -44,9 +43,7 @@ qb_format_and_resume (void *opaque) qb_conf_t *qb_conf = NULL; int ret = -1; - cs = opaque; - - local = DO_UPCAST(qb_local_t, cs, cs); + local = opaque; frame = local->frame; stub = local->stub; inode = local->inode; @@ -224,16 +221,13 @@ err: int qb_co_open (void *opaque) { - CoroutineSynctask *cs = NULL; qb_local_t *local = NULL; call_frame_t *frame = NULL; call_stub_t *stub = NULL; inode_t *inode = NULL; qb_inode_t *qb_inode = NULL; - cs = opaque; - - local = DO_UPCAST(qb_local_t, cs, cs); + local = opaque; frame = local->frame; stub = local->stub; inode = local->inode; @@ -262,7 +256,6 @@ qb_co_open (void *opaque) int qb_co_writev (void *opaque) { - CoroutineSynctask *cs = NULL; qb_local_t *local = NULL; call_frame_t *frame = NULL; call_stub_t *stub = NULL; @@ -271,9 +264,7 @@ qb_co_writev (void *opaque) QEMUIOVector qiov = {0, }; int ret = 0; - cs = opaque; - - local = DO_UPCAST(qb_local_t, cs, cs); + local = opaque; frame = local->frame; stub = local->stub; inode = local->inode; @@ -309,7 +300,6 @@ qb_co_writev (void *opaque) int qb_co_readv (void *opaque) { - CoroutineSynctask *cs = NULL; qb_local_t *local = NULL; call_frame_t *frame = NULL; call_stub_t *stub = NULL; @@ -320,9 +310,7 @@ qb_co_readv (void *opaque) struct iovec iov = {0, }; int ret = 0; - cs = opaque; - - local = DO_UPCAST(qb_local_t, cs, cs); + local = opaque; frame = local->frame; stub = local->stub; inode = local->inode; @@ -391,7 +379,6 @@ qb_co_readv (void *opaque) int qb_co_fsync (void *opaque) { - CoroutineSynctask *cs = NULL; qb_local_t *local = NULL; call_frame_t *frame = NULL; call_stub_t *stub = NULL; @@ -399,9 +386,7 @@ qb_co_fsync (void *opaque) qb_inode_t *qb_inode = NULL; int ret = 0; - cs = opaque; - - local = DO_UPCAST(qb_local_t, cs, cs); + local = opaque; frame = local->frame; stub = local->stub; inode = local->inode; @@ -461,7 +446,6 @@ qb_update_size_xattr (xlator_t *this, fd_t *fd, const char *fmt, off_t offset) int qb_co_truncate (void *opaque) { - CoroutineSynctask *cs = NULL; qb_local_t *local = NULL; call_frame_t *frame = NULL; call_stub_t *stub = NULL; @@ -472,9 +456,8 @@ qb_co_truncate (void *opaque) xlator_t *this = NULL; this = THIS; - cs = opaque; - local = DO_UPCAST(qb_local_t, cs, cs); + local = opaque; frame = local->frame; stub = local->stub; inode = local->inode; @@ -523,14 +506,13 @@ out: int qb_co_close (void *opaque) { - CoroutineSynctask *cs = NULL; qb_local_t *local = NULL; call_frame_t *frame = NULL; inode_t *inode = NULL; qb_inode_t *qb_inode = NULL; BlockDriverState *bs = NULL; - local = DO_UPCAST(qb_local_t, cs, cs); + local = opaque; inode = local->inode; qb_inode = qb_inode_ctx_get (THIS, inode); @@ -553,7 +535,6 @@ qb_co_close (void *opaque) int qb_snapshot_create (void *opaque) { - CoroutineSynctask *cs = NULL; qb_local_t *local = NULL; call_frame_t *frame = NULL; call_stub_t *stub = NULL; @@ -563,9 +544,7 @@ qb_snapshot_create (void *opaque) struct timeval tv = {0, }; int ret = 0; - cs = opaque; - - local = DO_UPCAST(qb_local_t, cs, cs); + local = opaque; frame = local->frame; stub = local->stub; inode = local->inode; @@ -604,7 +583,6 @@ qb_snapshot_create (void *opaque) int qb_snapshot_delete (void *opaque) { - CoroutineSynctask *cs = NULL; qb_local_t *local = NULL; call_frame_t *frame = NULL; call_stub_t *stub = NULL; @@ -612,9 +590,7 @@ qb_snapshot_delete (void *opaque) qb_inode_t *qb_inode = NULL; int ret = 0; - cs = opaque; - - local = DO_UPCAST(qb_local_t, cs, cs); + local = opaque; frame = local->frame; stub = local->stub; inode = local->inode; @@ -648,7 +624,6 @@ qb_snapshot_delete (void *opaque) int qb_snapshot_goto (void *opaque) { - CoroutineSynctask *cs = NULL; qb_local_t *local = NULL; call_frame_t *frame = NULL; call_stub_t *stub = NULL; @@ -656,9 +631,7 @@ qb_snapshot_goto (void *opaque) qb_inode_t *qb_inode = NULL; int ret = 0; - cs = opaque; - - local = DO_UPCAST(qb_local_t, cs, cs); + local = opaque; frame = local->frame; stub = local->stub; inode = local->inode; -- cgit