From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:46253) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ShydN-0004Hb-8g for qemu-devel@nongnu.org; Fri, 22 Jun 2012 03:50:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ShydI-0003JX-Hd for qemu-devel@nongnu.org; Fri, 22 Jun 2012 03:50:24 -0400 Received: from david.siemens.de ([192.35.17.14]:20352) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ShydI-0003FC-7S for qemu-devel@nongnu.org; Fri, 22 Jun 2012 03:50:20 -0400 Message-ID: <4FE423B0.9020702@siemens.com> Date: Fri, 22 Jun 2012 09:50:08 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1340347459-29861-1-git-send-email-peter.crosthwaite@petalogix.com> In-Reply-To: <1340347459-29861-1-git-send-email-peter.crosthwaite@petalogix.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC] block: Removed coroutine ownership assumption List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Peter A. G. Crosthwaite" Cc: kwolf@redhat.com, edgar.iglesias@gmail.com, qemu-devel@nongnu.org, john.williams@petalogix.com, stefanha@linux.vnet.ibm.com On 2012-06-22 08:44, Peter A. G. Crosthwaite wrote: > The block layer assumes that it is the only user of coroutines - > The qemu_in_coroutine() is used to determine if a function is in one of the > block layers coroutines, which is flawed. I.E. If a client (e.g. a device or > a machine model) of the block layer uses couroutine itself, the block layer > will identify the callers coroutines as its own, and may falsely yield the > calling coroutine (instead of creating its own to yield). > > AFAICT, there are no conflicts in the QEMU master here yet, but its kind of an > issue, as anyone who comes along and used coroutines and the block layer > together is going to run into some very obscure and hard to debug race > conditions. Not sure if I understood the intention yet: Is this supposed to fix an issue with the current usage of coroutines or to extend their usage beyond that? In the latter case, please don't do this. We'd rather like to get rid of them long term. Jan > > Signed-off-by: Peter A. G. Crosthwaite > --- > block.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/block.c b/block.c > index 0acdcac..b50af15 100644 > --- a/block.c > +++ b/block.c > @@ -380,7 +380,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, > return -ENOTSUP; > } > > - if (qemu_in_coroutine()) { > + if (0) { > /* Fast-path if already in coroutine context */ > bdrv_create_co_entry(&cco); > } else { > @@ -1590,7 +1590,7 @@ static int bdrv_rw_co(BlockDriverState *bs, int64_t sector_num, uint8_t *buf, > bdrv_io_limits_disable(bs); > } > > - if (qemu_in_coroutine()) { > + if (0) { > /* Fast-path if already in coroutine context */ > bdrv_rw_co_entry(&rwco); > } else { > @@ -3813,7 +3813,7 @@ int bdrv_flush(BlockDriverState *bs) > .ret = NOT_DONE, > }; > > - if (qemu_in_coroutine()) { > + if (0) { > /* Fast-path if already in coroutine context */ > bdrv_flush_co_entry(&rwco); > } else { > @@ -3874,7 +3874,7 @@ int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors) > .ret = NOT_DONE, > }; > > - if (qemu_in_coroutine()) { > + if (0) { > /* Fast-path if already in coroutine context */ > bdrv_discard_co_entry(&rwco); > } else { > -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux