From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42641) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtgLz-0003ZV-2q for qemu-devel@nongnu.org; Tue, 03 Nov 2015 13:30:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZtgLw-0006pf-9M for qemu-devel@nongnu.org; Tue, 03 Nov 2015 13:30:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51499) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtgLv-0006pZ-M1 for qemu-devel@nongnu.org; Tue, 03 Nov 2015 13:30:39 -0500 From: Juan Quintela In-Reply-To: <20151103173200.GA31958@work-vm> (David Alan Gilbert's message of "Tue, 3 Nov 2015 17:32:01 +0000") References: <1443515898-3594-1-git-send-email-dgilbert@redhat.com> <1443515898-3594-46-git-send-email-dgilbert@redhat.com> <87wpu78cco.fsf@neno.neno> <20151103173200.GA31958@work-vm> Date: Tue, 03 Nov 2015 19:30:36 +0100 Message-ID: <87vb9jlyur.fsf@emacs.mitica> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v8 45/54] Host page!=target page: Cleanup bitmaps Reply-To: quintela@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: aarcange@redhat.com, liang.z.li@intel.com, qemu-devel@nongnu.org, luis@cs.umu.se, bharata@linux.vnet.ibm.com, amit.shah@redhat.com, pbonzini@redhat.com "Dr. David Alan Gilbert" wrote: > * Juan Quintela (quintela@redhat.com) wrote: >> "Dr. David Alan Gilbert (git)" wrote: >> > From: "Dr. David Alan Gilbert" >> >> This is exactly the same code than the previous half of the function, >> you just need to factor out in a function? >> >> walk_btimap_host_page_chunks or whatever, and pass the two bits that >> change? the bitmap, and what to do with the ranges that are not there? > > Split out; see below - it gets a little bit more hairy since sentmap is > now unsentmap, so we need a few if's; but it's still lost the duplication: > (build tested only so far): > > Dave > > commit 15003123520ee5c358b2233c0bc30635aa90eb75 > Author: Dr. David Alan Gilbert > Date: Fri Sep 26 15:15:14 2014 +0100 > > Host page!=target page: Cleanup bitmaps > > Prior to the start of postcopy, ensure that everything that will > be transferred later is a whole host-page in size. > > This is accomplished by discarding partially transferred host pages > and marking any that are partially dirty as fully dirty. > > Signed-off-by: Dr. David Alan Gilbert > See, when we changed the sentbtmap to unsentbitmap, we made this code more complex, but I still think that new code is easier to undertand. Thanks. Reviewed-by: Juan Quintela > diff --git a/migration/ram.c b/migration/ram.c > index fe782e7..e30ed2b 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -1576,6 +1576,167 @@ static int postcopy_each_ram_send_discard(MigrationState *ms) > } > > /* > + * Helper for postcopy_chunk_hostpages; it's called twice to cleanup > + * the two bitmaps, that are similar, but one is inverted. > + * > + * We search for runs of target-pages that don't start or end on a > + * host page boundary; > + * unsent_pass=true: Cleans up partially unsent host pages by searching > + * the unsentmap > + * unsent_pass=false: Cleans up partially dirty host pages by searching > + * the main migration bitmap > + * > + */ > +static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass, > + RAMBlock *block, > + PostcopyDiscardState *pds) > +{ > + unsigned long *bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; > + unsigned int host_ratio = qemu_host_page_size / TARGET_PAGE_SIZE; > + unsigned long first = block->offset >> TARGET_PAGE_BITS; > + unsigned long len = block->used_length >> TARGET_PAGE_BITS; > + unsigned long last = first + (len - 1); > + unsigned long run_start; > + > + if (unsent_pass) { > + /* Find a sent page */ > + run_start = find_next_zero_bit(ms->unsentmap, last + 1, first); > + } else { > + /* Find a dirty page */ > + run_start = find_next_bit(bitmap, last + 1, first); > + } > + > + while (run_start <= last) { > + bool do_fixup = false; > + unsigned long fixup_start_addr; > + unsigned long host_offset; > + > + /* > + * If the start of this run of pages is in the middle of a host > + * page, then we need to fixup this host page. > + */ > + host_offset = run_start % host_ratio; > + if (host_offset) { > + do_fixup = true; > + run_start -= host_offset; > + fixup_start_addr = run_start; > + /* For the next pass */ > + run_start = run_start + host_ratio; > + } else { > + /* Find the end of this run */ > + unsigned long run_end; > + if (unsent_pass) { > + run_end = find_next_bit(ms->unsentmap, last + 1, run_start + 1); > + } else { > + run_end = find_next_zero_bit(bitmap, last + 1, run_start + 1); > + } > + /* > + * If the end isn't at the start of a host page, then the > + * run doesn't finish at the end of a host page > + * and we need to discard. > + */ > + host_offset = run_end % host_ratio; > + if (host_offset) { > + do_fixup = true; > + fixup_start_addr = run_end - host_offset; > + /* > + * This host page has gone, the next loop iteration starts > + * from after the fixup > + */ > + run_start = fixup_start_addr + host_ratio; > + } else { > + /* > + * No discards on this iteration, next loop starts from > + * next sent/dirty page > + */ > + run_start = run_end + 1; > + } > + } > + > + if (do_fixup) { > + unsigned long page; > + > + /* Tell the destination to discard this page */ > + if (unsent_pass || !test_bit(fixup_start_addr, ms->unsentmap)) { > + /* For the unsent_pass we: > + * discard partially sent pages > + * For the !unsent_pass (dirty) we: > + * discard partially dirty pages that were sent > + * (any partially sent pages were already discarded > + * by the previous unsent_pass) > + */ > + postcopy_discard_send_range(ms, pds, fixup_start_addr, > + host_ratio); > + } > + > + /* Clean up the bitmap */ > + for (page = fixup_start_addr; > + page < fixup_start_addr + host_ratio; page++) { > + /* All pages in this host page are now not sent */ > + set_bit(page, ms->unsentmap); > + > + /* > + * Remark them as dirty, updating the count for any pages > + * that weren't previously dirty. > + */ > + migration_dirty_pages += !test_and_set_bit(page, bitmap); > + } > + } > + > + if (unsent_pass) { > + /* Find the next sent page for the next iteration */ > + run_start = find_next_zero_bit(ms->unsentmap, last + 1, > + run_start); > + } else { > + /* Find the next dirty page for the next iteration */ > + run_start = find_next_bit(bitmap, last + 1, run_start); > + } > + } > +} > + > +/* > + * Utility for the outgoing postcopy code. > + * > + * Discard any partially sent host-page size chunks, mark any partially > + * dirty host-page size chunks as all dirty. > + * > + * Returns: 0 on success > + */ > +static int postcopy_chunk_hostpages(MigrationState *ms) > +{ > + struct RAMBlock *block; > + > + if (qemu_host_page_size == TARGET_PAGE_SIZE) { > + /* Easy case - TPS==HPS - nothing to be done */ > + return 0; > + } > + > + /* Easiest way to make sure we don't resume in the middle of a host-page */ > + last_seen_block = NULL; > + last_sent_block = NULL; > + last_offset = 0; > + > + QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { > + unsigned long first = block->offset >> TARGET_PAGE_BITS; > + > + PostcopyDiscardState *pds = > + postcopy_discard_send_init(ms, first, block->idstr); > + > + /* First pass: Discard all partially sent host pages */ > + postcopy_chunk_hostpages_pass(ms, true, block, pds); > + /* > + * Second pass: Ensure that all partially dirty host pages are made > + * fully dirty. > + */ > + postcopy_chunk_hostpages_pass(ms, false, block, pds); > + > + postcopy_discard_send_finish(ms, pds); > + } /* ram_list loop */ > + > + return 0; > +} > + > +/* > * Transmit the set of pages to be discarded after precopy to the target > * these are pages that: > * a) Have been previously transmitted but are now dirty again > @@ -1594,6 +1755,13 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms) > /* This should be our last sync, the src is now paused */ > migration_bitmap_sync(); > > + /* Deal with TPS != HPS */ > + ret = postcopy_chunk_hostpages(ms); > + if (ret) { > + rcu_read_unlock(); > + return ret; > + } > + > /* > * Update the unsentmap to be unsentmap = unsentmap | dirty > */ >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK