From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:52661) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RGpB9-00080G-7f for qemu-devel@nongnu.org; Thu, 20 Oct 2011 05:44:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RGpB8-0000eB-5Q for qemu-devel@nongnu.org; Thu, 20 Oct 2011 05:44:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40193) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RGpB7-0000e0-Ph for qemu-devel@nongnu.org; Thu, 20 Oct 2011 05:44:46 -0400 Message-ID: <4E9FEE44.7050905@redhat.com> Date: Thu, 20 Oct 2011 11:47:48 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1319036398-14320-1-git-send-email-pbonzini@redhat.com> <1319036398-14320-5-git-send-email-pbonzini@redhat.com> In-Reply-To: <1319036398-14320-5-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/8] block: add a Rwlock to synchronous read/write drivers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com Am 19.10.2011 16:59, schrieb Paolo Bonzini: > The big conversion of bdrv_read/write to coroutines caused the two > homonymous callbacks in BlockDriver to become reentrant. It goes > like this: > > 1) bdrv_read is now called in a coroutine, and calls bdrv_read or > bdrv_pread. > > 2) the nested bdrv_read goes through the fast path in bdrv_rw_co_entry; > > 3) in the common case when the protocol is file, bdrv_co_do_readv calls > bdrv_co_readv_em (and from here goes to bdrv_co_io_em), which yields > until the AIO operation is complete; > > 4) if bdrv_read had been called from a bottom half, the main loop > is free to iterate again: a device model or another bottom half > can then come and call bdrv_read again. > > This applies to all four of read/write/flush/discard. It would also > apply to is_allocated, but it is not used from within coroutines: > besides qemu-img.c and qemu-io.c, which operate synchronously, the > only user is the monitor. Copy-on-read will introduce a use in the > block layer, and will require converting it. > > The solution is "simply" to convert all drivers to coroutines! We > have nothing to do for the read-only drivers. For the others, we > add a Rwlock that is taken around affected operations. > > Signed-off-by: Paolo Bonzini For the record, we just discussed this on IRC: We'll probably need a mutex in some/most cases instead. Also, cloop and dmg need locking. That leaves only bochs and parallels that should be okay without a lock. We decided to play it the safe way and add locking in every driver and make it a mutex everywhere. Kevin