From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57906) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZJbYD-0004aX-76 for qemu-devel@nongnu.org; Mon, 27 Jul 2015 02:06:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZJbY9-0006EF-Tg for qemu-devel@nongnu.org; Mon, 27 Jul 2015 02:06:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40610) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZJbY9-0006Df-JI for qemu-devel@nongnu.org; Mon, 27 Jul 2015 02:06:09 -0400 Date: Mon, 27 Jul 2015 11:35:54 +0530 From: Amit Shah Message-ID: <20150727060554.GA12267@grmbl.mre> References: <1434450415-11339-1-git-send-email-dgilbert@redhat.com> <1434450415-11339-33-git-send-email-dgilbert@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1434450415-11339-33-git-send-email-dgilbert@redhat.com> Subject: Re: [Qemu-devel] [PATCH v7 32/42] Page request: Consume pages off the post-copy queue List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert (git)" Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, quintela@redhat.com, liang.z.li@intel.com, qemu-devel@nongnu.org, luis@cs.umu.se, pbonzini@redhat.com, david@gibson.dropbear.id.au On (Tue) 16 Jun 2015 [11:26:45], Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" > > When transmitting RAM pages, consume pages that have been queued by > MIG_RPCOMM_REQPAGE commands and send them ahead of normal page scanning. It's slightly confusing with 'consume': we're /servicing/ requests from the dest at the src here rather than /consuming/ pages sent by src at the dest. If you find 'service' better than 'consume', please update the commit msg+log. > Note: > a) After a queued page the linear walk carries on from after the > unqueued page; there is a reasonable chance that the destination > was about to ask for other closeby pages anyway. > > b) We have to be careful of any assumptions that the page walking > code makes, in particular it does some short cuts on its first linear > walk that break as soon as we do a queued page. > > c) We have to be careful to not break up host-page size chunks, since > this makes it harder to place the pages on the destination. > > Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Amit Shah > +static int ram_save_host_page(MigrationState *ms, QEMUFile *f, RAMBlock* block, > + ram_addr_t *offset, bool last_stage, > + uint64_t *bytes_transferred, > + ram_addr_t dirty_ram_abs) > +{ > + int tmppages, pages = 0; > + do { > + /* Check the pages is dirty and if it is send it */ > + if (migration_bitmap_clear_dirty(dirty_ram_abs)) { > + if (compression_switch && migrate_use_compression()) { > + tmppages = ram_save_compressed_page(f, block, *offset, > + last_stage, > + bytes_transferred); > + } else { > + tmppages = ram_save_page(f, block, *offset, last_stage, > + bytes_transferred); > + } Something for the future: we should just have ram_save_page which does compression (or not); and even encryption (or not), and so on. > + > + if (tmppages < 0) { > + return tmppages; > + } else { > + if (ms->sentmap) { > + set_bit(dirty_ram_abs >> TARGET_PAGE_BITS, ms->sentmap); > + } > + } This else could be dropped as the if stmt returns. > + pages += tmppages; > + } > + *offset += TARGET_PAGE_SIZE; > + dirty_ram_abs += TARGET_PAGE_SIZE; > + } while (*offset & (qemu_host_page_size - 1)); > + > + /* The offset we leave with is the last one we looked at */ > + *offset -= TARGET_PAGE_SIZE; > + return pages; > +} > + > +/** > * ram_find_and_save_block: Finds a dirty page and sends it to f > * > * Called within an RCU critical section. > @@ -997,65 +1094,102 @@ err: > * @f: QEMUFile where to send the data > * @last_stage: if we are at the completion stage > * @bytes_transferred: increase it with the number of transferred bytes > + * > + * On systems where host-page-size > target-page-size it will send all the > + * pages in a host page that are dirty. > */ > > static int ram_find_and_save_block(QEMUFile *f, bool last_stage, > uint64_t *bytes_transferred) > { > + MigrationState *ms = migrate_get_current(); > RAMBlock *block = last_seen_block; > + RAMBlock *tmpblock; > ram_addr_t offset = last_offset; > + ram_addr_t tmpoffset; > bool complete_round = false; > int pages = 0; > - MemoryRegion *mr; > ram_addr_t dirty_ram_abs; /* Address of the start of the dirty page in > ram_addr_t space */ > > - if (!block) > + if (!block) { > block = QLIST_FIRST_RCU(&ram_list.blocks); > + last_was_from_queue = false; > + } > > - while (true) { > - mr = block->mr; > - offset = migration_bitmap_find_and_reset_dirty(mr, offset, > - &dirty_ram_abs); > - if (complete_round && block == last_seen_block && > - offset >= last_offset) { > - break; > - } > - if (offset >= block->used_length) { > - offset = 0; > - block = QLIST_NEXT_RCU(block, next); > - if (!block) { > - block = QLIST_FIRST_RCU(&ram_list.blocks); > - complete_round = true; > - ram_bulk_stage = false; > - if (migrate_use_xbzrle()) { > - /* If xbzrle is on, stop using the data compression at this > - * point. In theory, xbzrle can do better than compression. > - */ > - flush_compressed_data(f); > - compression_switch = false; > - } > + while (true) { /* Until we send a block or run out of stuff to send */ > + tmpblock = ram_save_unqueue_page(ms, &tmpoffset, &dirty_ram_abs); > + > + if (tmpblock) { > + /* We've got a block from the postcopy queue */ > + trace_ram_find_and_save_block_postcopy(tmpblock->idstr, > + (uint64_t)tmpoffset, > + (uint64_t)dirty_ram_abs); > + /* > + * We're sending this page, and since it's postcopy nothing else > + * will dirty it, and we must make sure it doesn't get sent again > + * even if this queue request was received after the background > + * search already sent it. > + */ > + if (!test_bit(dirty_ram_abs >> TARGET_PAGE_BITS, > + migration_bitmap)) { > + trace_ram_find_and_save_block_postcopy_not_dirty( > + tmpblock->idstr, (uint64_t)tmpoffset, > + (uint64_t)dirty_ram_abs, > + test_bit(dirty_ram_abs >> TARGET_PAGE_BITS, ms->sentmap)); > + > + continue; > } > + /* > + * As soon as we start servicing pages out of order, then we have > + * to kill the bulk stage, since the bulk stage assumes > + * in (migration_bitmap_find_and_reset_dirty) that every page is > + * dirty, that's no longer true. > + */ > + ram_bulk_stage = false; > + /* > + * We want the background search to continue from the queued page > + * since the guest is likely to want other pages near to the page > + * it just requested. > + */ > + block = tmpblock; > + offset = tmpoffset; > } else { > - if (compression_switch && migrate_use_compression()) { > - pages = ram_save_compressed_page(f, block, offset, last_stage, > - bytes_transferred); > - } else { > - pages = ram_save_page(f, block, offset, last_stage, > - bytes_transferred); > + MemoryRegion *mr; > + /* priority queue empty, so just search for something dirty */ > + mr = block->mr; > + offset = migration_bitmap_find_dirty(mr, offset, &dirty_ram_abs); > + if (complete_round && block == last_seen_block && > + offset >= last_offset) { > + break; > } > - > - /* if page is unmodified, continue to the next */ > - if (pages > 0) { > - MigrationState *ms = migrate_get_current(); > - if (ms->sentmap) { > - set_bit(dirty_ram_abs >> TARGET_PAGE_BITS, ms->sentmap); > + if (offset >= block->used_length) { > + offset = 0; > + block = QLIST_NEXT_RCU(block, next); > + if (!block) { > + block = QLIST_FIRST_RCU(&ram_list.blocks); > + complete_round = true; > + ram_bulk_stage = false; > + if (migrate_use_xbzrle()) { > + /* If xbzrle is on, stop using the data compression at > + * this point. In theory, xbzrle can do better than > + * compression. > + */ > + flush_compressed_data(f); > + compression_switch = false; > + } > } > - > - last_sent_block = block; > - break; > + continue; /* pick an offset in the new block */ > } > } > + > + pages = ram_save_host_page(ms, f, block, &offset, last_stage, > + bytes_transferred, dirty_ram_abs); > + > + /* if page is unmodified, continue to the next */ > + if (pages > 0) { > + break; > + } This function could use splitting into multiple ones. Amit