From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=50732 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PiQqZ-0001p5-V2 for qemu-devel@nongnu.org; Thu, 27 Jan 2011 07:21:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PiQqY-000614-2L for qemu-devel@nongnu.org; Thu, 27 Jan 2011 07:21:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:23081) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PiQqX-00060n-Ri for qemu-devel@nongnu.org; Thu, 27 Jan 2011 07:21:06 -0500 Message-ID: <4D41632C.6090805@redhat.com> Date: Thu, 27 Jan 2011 14:21:00 +0200 From: Avi Kivity MIME-Version: 1.0 Subject: Re: [Qemu-devel] [RFC][PATCH 11/12] qcow2: Convert qcow2 to use coroutines for async I/O References: <1295688567-25496-1-git-send-email-stefanha@linux.vnet.ibm.com> <1295688567-25496-12-git-send-email-stefanha@linux.vnet.ibm.com> <4D40406B.2070302@redhat.com> <4D4042A8.2040903@redhat.com> <4D4046EF.3050108@codemonkey.ws> <4D40481E.9040309@redhat.com> <4D404B9F.7040801@codemonkey.ws> <4D404E1F.50809@redhat.com> <4D4055FB.6090104@codemonkey.ws> <4D413A88.2060204@redhat.com> <4D413F98.4010205@redhat.com> <4D414A51.4080601@redhat.com> <4D414BE9.5010103@redhat.com> <4D4156B3.6060800@redhat.com> In-Reply-To: <4D4156B3.6060800@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Stefan Hajnoczi , qemu-devel@nongnu.org On 01/27/2011 01:27 PM, Kevin Wolf wrote: > Am 27.01.2011 11:41, schrieb Avi Kivity: > > On 01/27/2011 12:34 PM, Kevin Wolf wrote: > >> Am 27.01.2011 10:49, schrieb Avi Kivity: > >>> On 01/27/2011 11:27 AM, Kevin Wolf wrote: > >>>> Well, but in the case of qcow2, you don't want to have a big mutex > >>>> around everything. We perfectly know which parts are asynchronous and > >>>> which are synchronous, so we'd want to do it finer grained from the > >>>> beginning. > >>> > >>> Yes we do. And the way I proposed it, the new mutex does not introduce > >>> any new serialization. > >>> > >>> To repeat, for every qcow2 callback or completion X (not qcow2 read or > >>> write operation), we transform it in the following manner: > >>> [...] > >> > >> This works fine for code that is completely synchronous today (and you > >> can't serialize it more than it already is anyway). > >> > >> It doesn't work for qemu_aio_readv/writev because these use AIO for > >> reading/writing the data. So you definitely need to rewrite that part, > >> or the AIO callback will cause the code to run outside its coroutine. > > > > The callbacks need to be wrapped in the same way. Schedule a coroutine > > to run the true callback. > > Okay, I see what you're proposing. You could schedule a new coroutine > for callbacks indeed. > > But I think it's actually easier to convert the bdrv_aio_readv into a > bdrv_co_readv (and by that removing the callback) and just make sure > that you don't hold the mutex during this call - basically what Stefan's > code does, just with mutexes instead of a request queue. My approach was for someone who isn't too familiar with qcow2 - a mindless conversion. A more integrated approach is better, since it will lead to tighter code, and if Stefan or you are able to do it without impacting concurrency, I'm all for it. > >> And during this rewrite you'll want to pay attention that you don't hold > >> the mutex for the bdrv_co_readv that was an AIO request before, or > >> you'll introduce additional serialization. > > > > I don't follow. Please elaborate. > > We were thinking of different approaches. I hope it's clearer now. I think so. -- error compiling committee.c: too many arguments to function