From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46408) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZrOqe-0006EA-1z for qemu-devel@nongnu.org; Wed, 28 Oct 2015 07:24:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZrOqa-0003vC-Pu for qemu-devel@nongnu.org; Wed, 28 Oct 2015 07:24:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41836) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZrOqa-0003v1-Fj for qemu-devel@nongnu.org; Wed, 28 Oct 2015 07:24:52 -0400 From: Juan Quintela In-Reply-To: <1443515898-3594-46-git-send-email-dgilbert@redhat.com> (David Alan Gilbert's message of "Tue, 29 Sep 2015 09:38:09 +0100") References: <1443515898-3594-1-git-send-email-dgilbert@redhat.com> <1443515898-3594-46-git-send-email-dgilbert@redhat.com> Date: Wed, 28 Oct 2015 12:24:39 +0100 Message-ID: <87wpu78cco.fsf@neno.neno> 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 (git)" 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 (git)" wrote: > From: "Dr. David Alan Gilbert" > > 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 > + struct RAMBlock *block; > + unsigned int host_ratio = qemu_host_page_size / TARGET_PAGE_SIZE; > + > + 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; It should be enough with the last one, right? if you put last_seen/sent_block to NULL, you will return from the beggining each time that you do a migration bitmap sync, penalizing the pages on the begining of the cycle. Even better than: last_offset = 0 is doing a: last_offset &= HOST_PAGE_MASK or whatever is the constant, no? > + > + QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { > + 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 found_set; > + unsigned long search_start; next_search? search_next? > + > + PostcopyDiscardState *pds = > + postcopy_discard_send_init(ms, first, block->idstr); > + > + /* First pass: Discard all partially sent host pages */ > + found_set = find_next_bit(ms->sentmap, last + 1, first); > + while (found_set <= last) { > + bool do_discard = false; > + unsigned long discard_start_addr; > + /* > + * If the start of this run of pages is in the middle of a host > + * page, then we need to discard this host page. > + */ > + if (found_set % host_ratio) { > + do_discard = true; > + found_set -= found_set % host_ratio; please, create a PAGE_HOST_ALIGN() macro, or whatever you want to call it? > + discard_start_addr = found_set; > + search_start = found_set + host_ratio; > + } else { > + /* Find the end of this run */ > + unsigned long found_zero; > + found_zero = find_next_zero_bit(ms->sentmap, last + 1, > + found_set + 1); > + /* > + * If the 0 isn't at the start of a host page, then the > + * run of 1's doesn't finish at the end of a host page > + * and we need to discard. > + */ > + if (found_zero % host_ratio) { > + do_discard = true; > + discard_start_addr = found_zero - (found_zero % host_ratio); > + /* > + * This host page has gone, the next loop iteration starts > + * from the next page with a 1 bit > + */ > + search_start = discard_start_addr + host_ratio; > + } else { > + /* > + * No discards on this iteration, next loop starts from > + * next 1 bit > + */ > + search_start = found_zero + 1; change for this found_set = found_zero + 1; > + } > + } > + /* Find the next 1 for the next iteration */ > + found_set = find_next_bit(ms->sentmap, last + 1, search_start); and move previous line to: > + if (do_discard) { > + unsigned long page; > + > + /* Tell the destination to discard this page */ > + postcopy_discard_send_range(ms, pds, discard_start_addr, > + discard_start_addr + host_ratio - 1); > + /* Clean up the bitmap */ > + for (page = discard_start_addr; > + page < discard_start_addr + host_ratio; page++) { > + /* All pages in this host page are now not sent */ > + clear_bit(page, ms->sentmap); > + > + /* > + * Remark them as dirty, updating the count for any pages > + * that weren't previously dirty. > + */ > + migration_dirty_pages += !test_and_set_bit(page, > + migration_bitmap); > + } to here /* Find the next 1 for the next iteration */ found_set = find_next_bit(ms->sentmap, last + 1, search_start); } > + } ? > + > + /* > + * Second pass: Ensure that all partially dirty host pages are made > + * fully dirty. > + */ > + found_set = find_next_bit(migration_bitmap, last + 1, first); > + while (found_set <= last) { > + bool do_dirty = false; > + unsigned long dirty_start_addr; > + /* > + * If the start of this run of pages is in the middle of a host > + * page, then we need to mark the whole of this host page dirty > + */ > + if (found_set % host_ratio) { > + do_dirty = true; > + found_set -= found_set % host_ratio; > + dirty_start_addr = found_set; > + search_start = found_set + host_ratio; > + } else { > + /* Find the end of this run */ > + unsigned long found_zero; > + found_zero = find_next_zero_bit(migration_bitmap, last + 1, > + found_set + 1); > + /* > + * If the 0 isn't at the start of a host page, then the > + * run of 1's doesn't finish at the end of a host page > + * and we need to discard. > + */ > + if (found_zero % host_ratio) { > + do_dirty = true; > + dirty_start_addr = found_zero - (found_zero % host_ratio); > + /* > + * This host page has gone, the next loop iteration starts > + * from the next page with a 1 bit > + */ > + search_start = dirty_start_addr + host_ratio; > + } else { > + /* > + * No discards on this iteration, next loop starts from > + * next 1 bit > + */ > + search_start = found_zero + 1; > + } > + } > + > + /* Find the next 1 for the next iteration */ > + found_set = find_next_bit(migration_bitmap, last + 1, search_start); > + > + if (do_dirty) { > + unsigned long page; > + > + if (test_bit(dirty_start_addr, ms->sentmap)) { > + /* > + * If the page being redirtied is marked as sent, then it > + * must have been fully sent (otherwise it would have been > + * discarded by the previous pass.) > + * Discard it now. > + */ > + postcopy_discard_send_range(ms, pds, dirty_start_addr, > + dirty_start_addr + > + host_ratio - 1); > + } > + > + /* Clean up the bitmap */ > + for (page = dirty_start_addr; > + page < dirty_start_addr + host_ratio; page++) { > + > + /* Clear the sentmap bits for the discard case above */ > + clear_bit(page, ms->sentmap); > + > + /* > + * Mark them as dirty, updating the count for any pages > + * that weren't previously dirty. > + */ > + migration_dirty_pages += !test_and_set_bit(page, > + migration_bitmap); > + } > + } > + } 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?