From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37831) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YKhvl-0006JK-VS for qemu-devel@nongnu.org; Mon, 09 Feb 2015 01:34:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YKhvh-0004VR-Uf for qemu-devel@nongnu.org; Mon, 09 Feb 2015 01:34:49 -0500 Received: from szxga03-in.huawei.com ([119.145.14.66]:35746) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YKhvg-0004Qf-HB for qemu-devel@nongnu.org; Mon, 09 Feb 2015 01:34:45 -0500 Message-ID: <54D854E4.4090300@huawei.com> Date: Mon, 9 Feb 2015 14:34:12 +0800 From: Bin Wu MIME-Version: 1.0 References: <1423454964-6840-1-git-send-email-wu.wubin@huawei.com> <1423454964-6840-2-git-send-email-wu.wubin@huawei.com> In-Reply-To: <1423454964-6840-2-git-send-email-wu.wubin@huawei.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qemu-coroutine: fix qemu_co_queue_run_restart error List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, famz@redhat.com, boby.chen@huawei.com, subo7@huawei.com, kathy.wangting@huawei.com, rudy.zhangmin@huawei.com, arei.gonglei@huawei.com, stefanha@redhat.com, pbonzini@redhat.com, bruce.fon@huawei.com, Bin Wu sorry, there is a mistake in this patch: the "ret" variable is not defined :< I will send a new patch to fix this problem. On 2015/2/9 12:09, Bin Wu wrote: > From: Bin Wu > > The error scenario is as follow: coroutine C1 enters C2, C2 yields > back to C1, then C1 ternimates and the related coroutine memory > becomes invalid. After a while, the C2 coroutine is entered again. > At this point, C1 is used as a parameter passed to > qemu_co_queue_run_restart. Therefore, qemu_co_queue_run_restart > accesses an invalid memory and a segfault error ocurrs. > > The qemu_co_queue_run_restart function re-enters coroutines waiting > in the co_queue. However, this function should be only used int the > qemu_coroutine_enter context. Only in this context, when the current > coroutine gets execution control again(after the execution of > qemu_coroutine_switch), we can restart the target coutine because the > target coutine has yielded back to the current coroutine or it has > terminated. > > First we want to put qemu_co_queue_run_restart in qemu_coroutine_enter, > but we find we can not access the target coroutine if it terminates. > > Signed-off-by: Bin Wu > --- > qemu-coroutine.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/qemu-coroutine.c b/qemu-coroutine.c > index 525247b..9a294c4 100644 > --- a/qemu-coroutine.c > +++ b/qemu-coroutine.c > @@ -99,24 +99,25 @@ static void coroutine_delete(Coroutine *co) > qemu_coroutine_delete(co); > } > > -static void coroutine_swap(Coroutine *from, Coroutine *to) > +static CoroutineAction coroutine_swap(Coroutine *from, Coroutine *to) > { > CoroutineAction ret; > > ret = qemu_coroutine_switch(from, to, COROUTINE_YIELD); > > - qemu_co_queue_run_restart(to); > - > switch (ret) { > case COROUTINE_YIELD: > - return; > + break; > case COROUTINE_TERMINATE: > trace_qemu_coroutine_terminate(to); > + qemu_co_queue_run_restart(to); > coroutine_delete(to); > - return; > + break; > default: > abort(); > } > + > + return ret; > } > > void qemu_coroutine_enter(Coroutine *co, void *opaque) > @@ -133,6 +134,8 @@ void qemu_coroutine_enter(Coroutine *co, void *opaque) > co->caller = self; > co->entry_arg = opaque; > coroutine_swap(self, co); > + if (ret == COROUTINE_YIELD) > + qemu_co_queue_run_restart(co); > } > > void coroutine_fn qemu_coroutine_yield(void) > -- Bin Wu