From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:58469) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Quexv-000810-He for qemu-devel@nongnu.org; Sat, 20 Aug 2011 02:23:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Quexu-0001S7-MN for qemu-devel@nongnu.org; Sat, 20 Aug 2011 02:23:31 -0400 Received: from mail-ww0-f41.google.com ([74.125.82.41]:61169) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Quexu-0001Rm-Hp for qemu-devel@nongnu.org; Sat, 20 Aug 2011 02:23:30 -0400 Received: by wwj26 with SMTP id 26so986097wwj.4 for ; Fri, 19 Aug 2011 23:23:29 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <4E4E5C3E.4010603@redhat.com> Date: Fri, 19 Aug 2011 05:51:10 -0700 From: Paolo Bonzini MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH v4 3/5] separate migration bitmap List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Umesh Deshpande , qemu-devel On 08/16/2011 08:56 PM, Umesh Deshpande wrote: > @@ -2128,8 +2132,61 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end, > start1, length); > } > } > + > } > > +void migration_bitmap_reset_dirty(ram_addr_t start, ram_addr_t end, > + int dirty_flags) > +{ > + unsigned long length, start1; > + > + start &= TARGET_PAGE_MASK; > + end = TARGET_PAGE_ALIGN(end); > + > + length = end - start; > + if (length == 0) { > + return; > + } > + > + migration_bitmap_mask_dirty_range(start, length, dirty_flags); > + > + /* we modify the TLB cache so that the dirty bit will be set again > + when accessing the range */ The comment does not apply here, and the code below can also be safely deleted. > + start1 = (unsigned long)qemu_safe_ram_ptr(start); > + /* Check that we don't span multiple blocks - this breaks the > + address comparisons below. */ > + if ((unsigned long)qemu_safe_ram_ptr(end - 1) - start1 > + != (end - 1) - start) { > + abort(); > + } > +} > + > +void sync_migration_bitmap(ram_addr_t start, ram_addr_t end) > +{ > + unsigned long length, len, i; > + ram_addr_t addr; > + start &= TARGET_PAGE_MASK; > + end = TARGET_PAGE_ALIGN(end); > + > + length = end - start; > + if (length == 0) { > + return; > + } > + > + len = length >> TARGET_PAGE_BITS; > + for (i = 0; i < len; i++) { > + addr = i << TARGET_PAGE_BITS; > + if (cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG)) { > + migration_bitmap_set_dirty(addr); > + cpu_physical_memory_reset_dirty(addr, addr + TARGET_PAGE_SIZE, > + MIGRATION_DIRTY_FLAG); This should be run under the iothread lock. Pay attention to avoiding lock inversion: the I/O thread always takes the iothread lock outside and the ramlist lock within, so the migration thread must do the same. BTW, I think this code in the migration thread patch also needs the iothread lock: > if (stage < 0) { > cpu_physical_memory_set_dirty_tracking(0); > return 0; > } > > if (cpu_physical_sync_dirty_bitmap(0, TARGET_PHYS_ADDR_MAX) != 0) { > qemu_file_set_error(f); > return 0; > } > Finally, here: > /* Make sure all dirty bits are set */ > QLIST_FOREACH(block, &ram_list.blocks, next) { > for (addr = block->offset; addr < block->offset + block->length; > addr += TARGET_PAGE_SIZE) { > if (!migration_bitmap_get_dirty(addr, > MIGRATION_DIRTY_FLAG)) { > migration_bitmap_set_dirty(addr); > } > } > } > ... you can skip the get_dirty operation since we are not interested in other flags than the migration flag for the migration-specific bitmap. Paolo