From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53010) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XH9Sk-0003dk-HJ for qemu-devel@nongnu.org; Tue, 12 Aug 2014 06:38:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XH9Se-0004X8-Bg for qemu-devel@nongnu.org; Tue, 12 Aug 2014 06:37:54 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47665) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XH9Se-0004Wq-3E for qemu-devel@nongnu.org; Tue, 12 Aug 2014 06:37:48 -0400 Date: Tue, 12 Aug 2014 11:37:40 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20140812103739.GE2384@work-vm> References: <1406862751-24008-1-git-send-email-sanidhya.iiith@gmail.com> <1406862751-24008-2-git-send-email-sanidhya.iiith@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1406862751-24008-2-git-send-email-sanidhya.iiith@gmail.com> Subject: Re: [Qemu-devel] [PATCH v5 1/6] generic function between migration and bitmap dump List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sanidhya Kashyap Cc: qemu list , Juan Quintela * Sanidhya Kashyap (sanidhya.iiith@gmail.com) wrote: > I have modified the functions to be more generic i.e. I have used the > counter variable which stores the required value. If the value of counter > is 0, it means bitmap dump process whereas the value of 1 means migration. OK, but not quite what I meant: > Signed-off-by: Sanidhya Kashyap > --- > arch_init.c | 21 +++++++++++++-------- > include/exec/ram_addr.h | 4 ++++ > 2 files changed, 17 insertions(+), 8 deletions(-) > > diff --git a/arch_init.c b/arch_init.c > index 8ddaf35..2aacb1c 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -434,20 +434,22 @@ ram_addr_t migration_bitmap_find_and_reset_dirty(MemoryRegion *mr, > return (next - base) << TARGET_PAGE_BITS; > } > > -static inline bool migration_bitmap_set_dirty(ram_addr_t addr) > +static inline bool qemu_bitmap_set_dirty(ram_addr_t addr, unsigned long *bitmap, > + uint64_t *counter) > { > bool ret; > int nr = addr >> TARGET_PAGE_BITS; > > - ret = test_and_set_bit(nr, migration_bitmap); > + ret = test_and_set_bit(nr, bitmap); > > - if (!ret) { > + if (!ret && *counter) { > migration_dirty_pages++; > } > return ret; > } If you made this: if (!ret && *counter) { *counter++; } > -static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length) > +void qemu_bitmap_sync_range(ram_addr_t start, ram_addr_t length, > + unsigned long *bitmap, uint64_t *counter) > { > ram_addr_t addr; > unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS); > @@ -461,8 +463,8 @@ static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length) > for (k = page; k < page + nr; k++) { > if (src[k]) { > unsigned long new_dirty; > - new_dirty = ~migration_bitmap[k]; > - migration_bitmap[k] |= src[k]; > + new_dirty = ~bitmap[k]; > + bitmap[k] |= src[k]; > new_dirty &= src[k]; > migration_dirty_pages += ctpopl(new_dirty); > src[k] = 0; > @@ -476,7 +478,7 @@ static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length) > cpu_physical_memory_reset_dirty(start + addr, > TARGET_PAGE_SIZE, > DIRTY_MEMORY_MIGRATION); > - migration_bitmap_set_dirty(start + addr); > + qemu_bitmap_set_dirty(start + addr, bitmap, counter); > } > } > } > @@ -497,6 +499,8 @@ static void migration_bitmap_sync(void) > int64_t bytes_xfer_now; > static uint64_t xbzrle_cache_miss_prev; > static uint64_t iterations_prev; > + /* counter = 1 indicates, this will be used for migration */ > + uint64_t counter = 1; > > bitmap_sync_count++; > > @@ -512,7 +516,8 @@ static void migration_bitmap_sync(void) > address_space_sync_dirty_bitmap(&address_space_memory); > > QTAILQ_FOREACH(block, &ram_list.blocks, next) { > - migration_bitmap_sync_range(block->mr->ram_addr, block->length); > + qemu_bitmap_sync_range(block->mr->ram_addr, block->length, > + migration_bitmap, &counter); Then this could become: qemu_bitmap_sync_range(block->mr->ram_addr, block->length, migration_bitmap, &migration_dirty_pages); so you don't have the dummy 'counter' varaible. Dave > } > trace_migration_bitmap_sync_end(migration_dirty_pages > - num_dirty_pages_init); > diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h > index 6593be1..fcc3501 100644 > --- a/include/exec/ram_addr.h > +++ b/include/exec/ram_addr.h > @@ -162,5 +162,9 @@ static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start, > void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length, > unsigned client); > > + > +void qemu_bitmap_sync_range(ram_addr_t start, ram_addr_t length, > + unsigned long *bitmap, uint64_t *counter); > + > #endif > #endif > -- > 1.9.3 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK