From patchwork Fri Feb 22 16:36:27 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 222651 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 6247D2C0094 for ; Sat, 23 Feb 2013 08:01:07 +1100 (EST) Received: from localhost ([::1]:56212 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1U8vds-0005uH-AO for incoming@patchwork.ozlabs.org; Fri, 22 Feb 2013 11:38:36 -0500 Received: from eggs.gnu.org ([208.118.235.92]:58945) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1U8vdB-0005Cb-A2 for qemu-devel@nongnu.org; Fri, 22 Feb 2013 11:38:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1U8vd4-0006oe-Hk for qemu-devel@nongnu.org; Fri, 22 Feb 2013 11:37:53 -0500 Received: from mail-ve0-f177.google.com ([209.85.128.177]:61641) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1U8vd4-0006oU-Cf for qemu-devel@nongnu.org; Fri, 22 Feb 2013 11:37:46 -0500 Received: by mail-ve0-f177.google.com with SMTP id m1so707819ves.36 for ; Fri, 22 Feb 2013 08:37:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:sender:from:to:cc:subject:date:message-id:x-mailer :in-reply-to:references; bh=GBRZM8TPN2WAHwky7KfP46ob04aaRc2v9tj6pZYVJBg=; b=QmlH+SxfmzNtBldJaV1roZrM2NkczMyZHk/JPs6ETQQychzhEeFh6i+wij/Pcof/ps A1pj9yUlgkMKzJQ9fJjWtt4sQaTqi1ufskmDuNOVm19EbLUWuzToh8NJ7mmnhz7KXTvu WPcyV4l2xgwg4U8LBAWHrhkYKfcabAkrYFYbf6lp675MuOkvoJJBLuO7rwcGKzBPnTio cOQmmgxu7OunMAtssFnSLY1MjD8hqxe/M6+swI0l0oOR0ch/IPXz32Zwsr0rpMRKJFgy vlysgJjsJYpzPv7GgyuOHe2d0ci7+onmMWvH5eXQ3S5CUNVR5pUsDS2AuUeVZ5qnPWuq x9/g== X-Received: by 10.52.69.232 with SMTP id h8mr3118165vdu.26.1361551066024; Fri, 22 Feb 2013 08:37:46 -0800 (PST) Received: from yakj.usersys.redhat.com (93-34-179-137.ip50.fastwebnet.it. [93.34.179.137]) by mx.google.com with ESMTPS id tp10sm4291733vec.1.2013.02.22.08.37.44 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 22 Feb 2013 08:37:45 -0800 (PST) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Fri, 22 Feb 2013 17:36:27 +0100 Message-Id: <1361551008-12430-22-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.8.1.2 In-Reply-To: <1361551008-12430-1-git-send-email-pbonzini@redhat.com> References: <1361551008-12430-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 209.85.128.177 Cc: owasserm@redhat.com, quintela@redhat.com Subject: [Qemu-devel] [PATCH 21/42] migration: run pending/iterate callbacks out of big lock 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 This makes it possible to do blocking writes directly to the socket, with no buffer in the middle. For RAM, only the migration_bitmap_sync() call needs the iothread lock. For block migration, it is needed by the block layer (including bdrv_drain_all and dirty bitmap access), but because some code is shared between iterate and complete, all of mig_save_device_dirty is run with the lock taken. In the savevm case, the iterate callback runs within the big lock. This is annoying because it complicates the rules. Luckily we do not need to do anything about it: the RAM iterate callback does not need the iothread lock, and block migration never runs during savevm. Reviewed-by: Orit Wasserman Reviewed-by: Juan Quintela Signed-off-by: Paolo Bonzini --- arch_init.c | 4 ++++ block-migration.c | 37 +++++++++++++++++++++++++++++++++++-- include/migration/vmstate.h | 11 +++++++++++ migration.c | 4 ++-- 4 files changed, 52 insertions(+), 4 deletions(-) diff --git a/arch_init.c b/arch_init.c index 8daeafa..32b4378 100644 --- a/arch_init.c +++ b/arch_init.c @@ -379,6 +379,8 @@ static inline bool migration_bitmap_set_dirty(MemoryRegion *mr, return ret; } +/* Needs iothread lock! */ + static void migration_bitmap_sync(void) { RAMBlock *block; @@ -690,7 +692,9 @@ static uint64_t ram_save_pending(QEMUFile *f, void *opaque, uint64_t max_size) remaining_size = ram_save_remaining() * TARGET_PAGE_SIZE; if (remaining_size < max_size) { + qemu_mutex_lock_iothread(); migration_bitmap_sync(); + qemu_mutex_unlock_iothread(); remaining_size = ram_save_remaining() * TARGET_PAGE_SIZE; } return remaining_size; diff --git a/block-migration.c b/block-migration.c index b726c6c..8da5f86 100644 --- a/block-migration.c +++ b/block-migration.c @@ -107,6 +107,10 @@ static void blk_mig_unlock(void) qemu_mutex_unlock(&block_mig_state.lock); } +/* Must run outside of the iothread lock during the bulk phase, + * or the VM will stall. + */ + static void blk_send(QEMUFile *f, BlkMigBlock * blk) { int len; @@ -226,6 +230,8 @@ static void blk_mig_read_cb(void *opaque, int ret) blk_mig_unlock(); } +/* Called with no lock taken. */ + static int mig_save_device_bulk(QEMUFile *f, BlkMigDevState *bmds) { int64_t total_sectors = bmds->total_sectors; @@ -235,11 +241,13 @@ static int mig_save_device_bulk(QEMUFile *f, BlkMigDevState *bmds) int nr_sectors; if (bmds->shared_base) { + qemu_mutex_lock_iothread(); while (cur_sector < total_sectors && !bdrv_is_allocated(bs, cur_sector, MAX_IS_ALLOCATED_SEARCH, &nr_sectors)) { cur_sector += nr_sectors; } + qemu_mutex_unlock_iothread(); } if (cur_sector >= total_sectors) { @@ -272,15 +280,19 @@ static int mig_save_device_bulk(QEMUFile *f, BlkMigDevState *bmds) block_mig_state.submitted++; blk_mig_unlock(); + qemu_mutex_lock_iothread(); blk->aiocb = bdrv_aio_readv(bs, cur_sector, &blk->qiov, nr_sectors, blk_mig_read_cb, blk); bdrv_reset_dirty(bs, cur_sector, nr_sectors); - bmds->cur_sector = cur_sector + nr_sectors; + qemu_mutex_unlock_iothread(); + bmds->cur_sector = cur_sector + nr_sectors; return (bmds->cur_sector >= total_sectors); } +/* Called with iothread lock taken. */ + static void set_dirty_tracking(int enable) { BlkMigDevState *bmds; @@ -336,6 +348,8 @@ static void init_blk_migration(QEMUFile *f) bdrv_iterate(init_blk_migration_it, NULL); } +/* Called with no lock taken. */ + static int blk_mig_save_bulked_block(QEMUFile *f) { int64_t completed_sector_sum = 0; @@ -382,6 +396,8 @@ static void blk_mig_reset_dirty_cursor(void) } } +/* Called with iothread lock taken. */ + static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds, int is_async) { @@ -451,7 +467,9 @@ error: return ret; } -/* return value: +/* Called with iothread lock taken. + * + * return value: * 0: too much data for max_downtime * 1: few enough data for max_downtime */ @@ -470,6 +488,8 @@ static int blk_mig_save_dirty_block(QEMUFile *f, int is_async) return ret; } +/* Called with no locks taken. */ + static int flush_blks(QEMUFile *f) { BlkMigBlock *blk; @@ -509,6 +529,8 @@ static int flush_blks(QEMUFile *f) return ret; } +/* Called with iothread lock taken. */ + static int64_t get_remaining_dirty(void) { BlkMigDevState *bmds; @@ -521,6 +543,8 @@ static int64_t get_remaining_dirty(void) return dirty << BDRV_SECTOR_BITS; } +/* Called with iothread lock taken. */ + static void blk_mig_cleanup(void) { BlkMigDevState *bmds; @@ -600,7 +624,12 @@ static int block_save_iterate(QEMUFile *f, void *opaque) } ret = 0; } else { + /* Always called with iothread lock taken for + * simplicity, block_save_complete also calls it. + */ + qemu_mutex_lock_iothread(); ret = blk_mig_save_dirty_block(f, 1); + qemu_mutex_unlock_iothread(); } if (ret < 0) { return ret; @@ -622,6 +651,8 @@ static int block_save_iterate(QEMUFile *f, void *opaque) return qemu_ftell(f) - last_ftell; } +/* Called with iothread lock taken. */ + static int block_save_complete(QEMUFile *f, void *opaque) { int ret; @@ -665,6 +696,7 @@ static uint64_t block_save_pending(QEMUFile *f, void *opaque, uint64_t max_size) /* Estimate pending number of bytes to send */ uint64_t pending; + qemu_mutex_lock_iothread(); blk_mig_lock(); pending = get_remaining_dirty() + block_mig_state.submitted * BLOCK_SIZE + @@ -675,6 +707,7 @@ static uint64_t block_save_pending(QEMUFile *f, void *opaque, uint64_t max_size) pending = BLOCK_SIZE; } blk_mig_unlock(); + qemu_mutex_unlock_iothread(); DPRINTF("Enter save live pending %" PRIu64 "\n", pending); return pending; diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 6229569..5f803f5 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -30,14 +30,25 @@ typedef void SaveStateHandler(QEMUFile *f, void *opaque); typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id); typedef struct SaveVMHandlers { + /* This runs inside the iothread lock. */ void (*set_params)(const MigrationParams *params, void * opaque); SaveStateHandler *save_state; int (*save_live_setup)(QEMUFile *f, void *opaque); void (*cancel)(void *opaque); int (*save_live_complete)(QEMUFile *f, void *opaque); + + /* This runs both outside and inside the iothread lock. */ bool (*is_active)(void *opaque); + + /* This runs outside the iothread lock in the migration case, and + * within the lock in the savevm case. The callback had better only + * use data that is local to the migration thread or protected + * by other locks. + */ int (*save_live_iterate)(QEMUFile *f, void *opaque); + + /* This runs outside the iothread lock! */ uint64_t (*save_live_pending)(QEMUFile *f, void *opaque, uint64_t max_size); LoadStateHandler *load_state; diff --git a/migration.c b/migration.c index 729578b..92a7152 100644 --- a/migration.c +++ b/migration.c @@ -670,7 +670,6 @@ static void *buffered_file_thread(void *opaque) uint64_t pending_size; if (s->bytes_xfer < s->xfer_limit) { - qemu_mutex_lock_iothread(); DPRINTF("iterate\n"); pending_size = qemu_savevm_state_pending(s->file, max_size); DPRINTF("pending size %lu max %lu\n", pending_size, max_size); @@ -678,6 +677,7 @@ static void *buffered_file_thread(void *opaque) qemu_savevm_state_iterate(s->file); } else { DPRINTF("done iterating\n"); + qemu_mutex_lock_iothread(); start_time = qemu_get_clock_ms(rt_clock); qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER); old_vm_running = runstate_is_running(); @@ -685,8 +685,8 @@ static void *buffered_file_thread(void *opaque) s->xfer_limit = INT_MAX; qemu_savevm_state_complete(s->file); last_round = true; + qemu_mutex_unlock_iothread(); } - qemu_mutex_unlock_iothread(); } current_time = qemu_get_clock_ms(rt_clock);