From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48937) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YKlo8-00066j-WE for qemu-devel@nongnu.org; Mon, 09 Feb 2015 05:43:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YKlo2-0005FS-Nl for qemu-devel@nongnu.org; Mon, 09 Feb 2015 05:43:12 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39504) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YKlo2-0005FE-G8 for qemu-devel@nongnu.org; Mon, 09 Feb 2015 05:43:06 -0500 Date: Mon, 9 Feb 2015 10:42:52 +0100 From: Kevin Wolf Message-ID: <20150209094252.GA3963@noname.str.redhat.com> References: <1423464639-4764-1-git-send-email-wu.wubin@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1423464639-4764-1-git-send-email-wu.wubin@huawei.com> Subject: Re: [Qemu-devel] [PATCH v2] qemu-coroutine: segfault when restarting co_queue List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bin Wu Cc: famz@redhat.com, boby.chen@huawei.com, subo7@huawei.com, kathy.wangting@huawei.com, rudy.zhangmin@huawei.com, qemu-devel@nongnu.org, arei.gonglei@huawei.com, stefanha@redhat.com, pbonzini@redhat.com, bruce.fon@huawei.com Am 09.02.2015 um 07:50 hat Bin Wu geschrieben: > From: Bin Wu > > We tested VMs migration with their disk images by drive_mirror. With > migration, two VMs copyed large files between each other. During the > test, a segfault occured. The stack was as follow: > > (gdb) bt > qemu-coroutine-lock.c:66 > to=0x7fa5a1798648) at qemu-coroutine.c:97 > request=0x7fa28c2ffa10, reply=0x7fa28c2ffa30, qiov=0x0, offset=0) at > block/nbd-client.c:165 > sector_num=8552704, nb_sectors=2040, qiov=0x7fa5a1757468, offset=0) at > block/nbd-client.c:262 > sector_num=8552704, nb_sectors=2048, qiov=0x7fa5a1757468) at > block/nbd-client.c:296 > nb_sectors=2048, qiov=0x7fa5a1757468) at block/nbd.c:291 > req=0x7fa28c2ffbb0, offset=4378984448, bytes=1048576, qiov=0x7fa5a1757468, > flags=0) at block.c:3321 > offset=4378984448, bytes=1048576, qiov=0x7fa5a1757468, flags=(unknown: 0)) at > block.c:3447 > sector_num=8552704, nb_sectors=2048, qiov=0x7fa5a1757468, flags=(unknown: 0)) at > block.c:3471 > nb_sectors=2048, qiov=0x7fa5a1757468) at block.c:3480 > nb_sectors=2048, qiov=0x7fa5a1757468) at block/raw_bsd.c:62 > req=0x7fa28c2ffe30, offset=4378984448, bytes=1048576, qiov=0x7fa5a1757468, > flags=0) at block.c:3321 > offset=4378984448, bytes=1048576, qiov=0x7fa5a1757468, flags=(unknown: 0)) at > block.c:3447 > sector_num=8552704, nb_sectors=2048, qiov=0x7fa5a1757468, flags=(unknown: 0)) at > block.c:3471 > coroutine-ucontext.c:121 > > After analyzing the stack and reviewing the code, we find the > qemu_co_queue_run_restart should not be put in the coroutine_swap function which > can be invoked by qemu_coroutine_enter or qemu_coroutine_yield. Only > qemu_coroutine_enter needs to restart the co_queue. > > 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. Why would C1 be used for the qemu_co_queue_run_restart() call? There is only one caller, coroutine_swap(). It calls the function for the coroutine that has just yielded or terminated. In the case of termination, the coroutine is deleted only afterwards, in the switch block in coroutine_swap(). > 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. It shouldn't make a difference. Any coroutine waiting in a CoQueue should be ready to be restarted at either point. Kevin