From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50225) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDY0z-0000XO-RN for qemu-devel@nongnu.org; Thu, 07 Mar 2013 05:25:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UDY0x-0004y8-9Z for qemu-devel@nongnu.org; Thu, 07 Mar 2013 05:25:33 -0500 Received: from mx1.redhat.com ([209.132.183.28]:28789) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDY0w-0004y4-Ub for qemu-devel@nongnu.org; Thu, 07 Mar 2013 05:25:31 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r27APUgs028866 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 7 Mar 2013 05:25:30 -0500 Date: Thu, 7 Mar 2013 11:25:28 +0100 From: Stefan Hajnoczi Message-ID: <20130307102528.GD14726@stefanha-thinkpad.redhat.com> References: <1362581603-21777-1-git-send-email-stefanha@redhat.com> <20130306154101.GD2285@dhcp-200-207.str.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130306154101.GD2285@dhcp-200-207.str.redhat.com> Subject: Re: [Qemu-devel] [PATCH] coroutine: use AioContext for CoQueue BH List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Paolo Bonzini , qemu-devel@nongnu.org On Wed, Mar 06, 2013 at 04:41:01PM +0100, Kevin Wolf wrote: > Am 06.03.2013 um 15:53 hat Stefan Hajnoczi geschrieben: > > CoQueue uses a BH to awake coroutines that were made ready to run again > > using qemu_co_queue_next() or qemu_co_queue_restart_all(). The BH > > currently runs in the iothread AioContext and would break coroutines > > that run in a different AioContext. > > > > This is a slightly tricky problem because the lifetime of the BH exceeds > > that of the CoQueue. This means coroutines can be awoken after CoQueue > > itself has been freed. > > Does this really happen in practice? If so, that sounds like a bug to > me. I didn't audit the callers. It seems reasonable that a CoQueue is allowed to go out of scope once qemu_co_queue_empty() returns true. I don't see what is buggy about that. > > Finally, I don't want to move unlock_queue and unlock_bh into > > AioContext. That would break encapsulation - AioContext isn't supposed > > to know about CoQueue. > > So what you would need here is "AioContext local storage". I wonder if > this will stay a requirement unique to CoQueues when AioContexts gain > wider use. I'm not convinced we need AioContext local storage. We could go all the way and make AioContext* thread-local. Then we can figure out the current AioContext* or NULL, if none, at any location in the code. Then CoQueue (and others?) could attach their resources to AioContext local storage. But let's avoid it if there are clean solutions because implicit state like globals or thread-locals makes code harder to understand. Stefan