From patchwork Thu Oct 20 11:16:21 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 120798 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 610D6B70B2 for ; Thu, 20 Oct 2011 23:03:04 +1100 (EST) Received: from localhost ([::1]:50168 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RGqcu-0001nP-6E for incoming@patchwork.ozlabs.org; Thu, 20 Oct 2011 07:17:32 -0400 Received: from eggs.gnu.org ([140.186.70.92]:55635) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RGqcG-0000Uo-Hz for qemu-devel@nongnu.org; Thu, 20 Oct 2011 07:16:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RGqcE-0001SF-Hh for qemu-devel@nongnu.org; Thu, 20 Oct 2011 07:16:52 -0400 Received: from mail-gx0-f173.google.com ([209.85.161.173]:63637) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RGqcD-0001S6-Vt for qemu-devel@nongnu.org; Thu, 20 Oct 2011 07:16:50 -0400 Received: by ggnr5 with SMTP id r5so3266012ggn.4 for ; Thu, 20 Oct 2011 04:16:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=sender:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references; bh=NPquRv3kZoywYkAfPMixtvrpP0rm9tdUBKrRjkOFvzA=; b=Sivo1CwZFYdfPLGP6uUYLac1lXvHyZoI6tQO0UHoXvaHGJExyDBsW98WcCbB8dZ4N8 cjgWrr618WvazKaFEA7MDgQdhuj92KBHAUaJrGECqImzvCZ1AXIe2MGQHZzLeF34x9GB Y0FWYHFDA/UbLfEwP4LljvxnyPI3AA/HWEO8Y= Received: by 10.42.159.1 with SMTP id j1mr18393612icx.20.1319109409029; Thu, 20 Oct 2011 04:16:49 -0700 (PDT) Received: from localhost.localdomain (93-34-218-143.ip51.fastwebnet.it. [93.34.218.143]) by mx.google.com with ESMTPS id n30sm22979754ibl.4.2011.10.20.04.16.46 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 20 Oct 2011 04:16:48 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Thu, 20 Oct 2011 13:16:21 +0200 Message-Id: <1319109385-7927-4-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.7.6 In-Reply-To: <1319109385-7927-1-git-send-email-pbonzini@redhat.com> References: <1319109385-7927-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 209.85.161.173 Cc: kwolf@redhat.com, stefanha@linux.vnet.ibm.com Subject: [Qemu-devel] [PATCH v2 3/7] block: add a CoMutex to synchronous read drivers X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 just need to add a CoMutex that is taken around affected operations. Signed-off-by: Paolo Bonzini --- block/bochs.c | 2 ++ block/cloop.c | 2 ++ block/cow.c | 2 ++ block/dmg.c | 2 ++ block/nbd.c | 2 ++ block/parallels.c | 2 ++ block/vmdk.c | 2 ++ block/vpc.c | 2 ++ block/vvfat.c | 2 ++ 9 files changed, 18 insertions(+), 0 deletions(-) diff --git a/block/bochs.c b/block/bochs.c index 3c2f8d1..b0f8072 100644 --- a/block/bochs.c +++ b/block/bochs.c @@ -80,6 +80,7 @@ struct bochs_header { }; typedef struct BDRVBochsState { + CoMutex lock; uint32_t *catalog_bitmap; int catalog_size; @@ -150,6 +151,7 @@ static int bochs_open(BlockDriverState *bs, int flags) s->extent_size = le32_to_cpu(bochs.extra.redolog.extent); + qemu_co_mutex_init(&s->lock); return 0; fail: return -1; diff --git a/block/cloop.c b/block/cloop.c index 8cff9f2..a91f372 100644 --- a/block/cloop.c +++ b/block/cloop.c @@ -27,6 +27,7 @@ #include typedef struct BDRVCloopState { + CoMutex lock; uint32_t block_size; uint32_t n_blocks; uint64_t* offsets; @@ -93,6 +94,7 @@ static int cloop_open(BlockDriverState *bs, int flags) s->sectors_per_block = s->block_size/512; bs->total_sectors = s->n_blocks*s->sectors_per_block; + qemu_co_mutex_init(&s->lock); return 0; cloop_close: diff --git a/block/cow.c b/block/cow.c index 4cf543c..2f426e7 100644 --- a/block/cow.c +++ b/block/cow.c @@ -42,6 +42,7 @@ struct cow_header_v2 { }; typedef struct BDRVCowState { + CoMutex lock; int64_t cow_sectors_offset; } BDRVCowState; @@ -84,6 +85,7 @@ static int cow_open(BlockDriverState *bs, int flags) bitmap_size = ((bs->total_sectors + 7) >> 3) + sizeof(cow_header); s->cow_sectors_offset = (bitmap_size + 511) & ~511; + qemu_co_mutex_init(&s->lock); return 0; fail: return -1; diff --git a/block/dmg.c b/block/dmg.c index 64c3cce..111aeae 100644 --- a/block/dmg.c +++ b/block/dmg.c @@ -28,6 +28,7 @@ #include typedef struct BDRVDMGState { + CoMutex lock; /* each chunk contains a certain number of sectors, * offsets[i] is the offset in the .dmg file, * lengths[i] is the length of the compressed chunk, @@ -177,6 +178,7 @@ static int dmg_open(BlockDriverState *bs, int flags) s->current_chunk = s->n_chunks; + qemu_co_mutex_init(&s->lock); return 0; fail: return -1; diff --git a/block/nbd.c b/block/nbd.c index 76f04d8..14ab225 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -47,6 +47,7 @@ #endif typedef struct BDRVNBDState { + CoMutex lock; int sock; uint32_t nbdflags; off_t size; @@ -175,6 +176,7 @@ static int nbd_open(BlockDriverState *bs, const char* filename, int flags) */ result = nbd_establish_connection(bs); + qemu_co_mutex_init(&s->lock); return result; } diff --git a/block/parallels.c b/block/parallels.c index c64103d..b86e87e 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -46,6 +46,7 @@ struct parallels_header { } QEMU_PACKED; typedef struct BDRVParallelsState { + CoMutex lock; uint32_t *catalog_bitmap; int catalog_size; @@ -95,6 +96,7 @@ static int parallels_open(BlockDriverState *bs, int flags) for (i = 0; i < s->catalog_size; i++) le32_to_cpus(&s->catalog_bitmap[i]); + qemu_co_mutex_init(&s->lock); return 0; fail: if (s->catalog_bitmap) diff --git a/block/vmdk.c b/block/vmdk.c index ace2977..1ce220d 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -90,6 +90,7 @@ typedef struct VmdkExtent { } VmdkExtent; typedef struct BDRVVmdkState { + CoMutex lock; int desc_offset; bool cid_updated; uint32_t parent_cid; @@ -646,6 +647,7 @@ static int vmdk_open(BlockDriverState *bs, int flags) goto fail; } s->parent_cid = vmdk_read_cid(bs, 1); + qemu_co_mutex_init(&s->lock); return ret; fail: diff --git a/block/vpc.c b/block/vpc.c index 549a632..5414042 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -110,6 +110,7 @@ struct vhd_dyndisk_header { }; typedef struct BDRVVPCState { + CoMutex lock; uint8_t footer_buf[HEADER_SIZE]; uint64_t free_data_block_offset; int max_table_entries; @@ -226,6 +227,7 @@ static int vpc_open(BlockDriverState *bs, int flags) s->last_pagetable = -1; #endif + qemu_co_mutex_init(&s->lock); return 0; fail: return err; diff --git a/block/vvfat.c b/block/vvfat.c index c567697..6f666d6 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -317,6 +317,7 @@ static void print_mapping(const struct mapping_t* mapping); /* here begins the real VVFAT driver */ typedef struct BDRVVVFATState { + CoMutex lock; BlockDriverState* bs; /* pointer to parent */ unsigned int first_sectors_number; /* 1 for a single partition, 0x40 for a disk with partition table */ unsigned char first_sectors[0x40*0x200]; @@ -1063,6 +1064,7 @@ DLOG(if (stderr == NULL) { } // assert(is_consistent(s)); + qemu_co_mutex_init(&s->lock); return 0; }