From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38409) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZyFp3-0006fO-LI for qemu-devel@nongnu.org; Mon, 16 Nov 2015 04:11:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZyFoy-0000mg-OS for qemu-devel@nongnu.org; Mon, 16 Nov 2015 04:11:37 -0500 Received: from szxga02-in.huawei.com ([119.145.14.65]:18916) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZyFoy-0000mX-4t for qemu-devel@nongnu.org; Mon, 16 Nov 2015 04:11:32 -0500 References: <1446551816-15768-1-git-send-email-zhang.zhanghailiang@huawei.com> <1446551816-15768-16-git-send-email-zhang.zhanghailiang@huawei.com> <20151113161928.GL2456@work-vm> From: zhanghailiang Message-ID: <56499CE4.8060308@huawei.com> Date: Mon, 16 Nov 2015 17:07:48 +0800 MIME-Version: 1.0 In-Reply-To: <20151113161928.GL2456@work-vm> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame v10 15/38] ram/COLO: Record pages received from PVM by re-using migration dirty bitmap List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: lizhijian@cn.fujitsu.com, quintela@redhat.com, yunhong.jiang@intel.com, eddie.dong@intel.com, peter.huangpeng@huawei.com, qemu-devel@nongnu.org, arei.gonglei@huawei.com, stefanha@redhat.com, amit.shah@redhat.com On 2015/11/14 0:19, Dr. David Alan Gilbert wrote: > * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote: >> We need to record the address of the dirty pages that received from PVM, >> It will help flushing pages that cached into SVM. >> >> Signed-off-by: zhanghailiang >> --- >> v10: >> - New patch split from v9's patch 13 >> - Rebase to master to use 'migration_bitmap_rcu' >> --- >> migration/ram.c | 35 +++++++++++++++++++++++++++++++++++ >> 1 file changed, 35 insertions(+) >> >> diff --git a/migration/ram.c b/migration/ram.c >> index b094dc3..70879bd 100644 >> --- a/migration/ram.c >> +++ b/migration/ram.c >> @@ -1448,6 +1448,18 @@ static inline void *host_from_stream_offset(QEMUFile *f, >> } >> >> if (ram_cache_enable) { >> + unsigned long *bitmap; >> + long k = (block->mr->ram_addr + offset) >> TARGET_PAGE_BITS; >> + >> + bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; >> + /* >> + * During colo checkpoint, we need bitmap of these migrated pages. >> + * It help us to decide which pages in ram cache should be flushed >> + * into VM's RAM later. >> + */ >> + if (!test_and_set_bit(k, bitmap)) { >> + migration_dirty_pages++; >> + } > > I don't like having this in host_from_stream_offset; if you look > at the current ram_load there is only a single call to host_from_stream_offset, so it's > now much easier for you to move it into a separate function. > Hmm, that's really a good idea, i will split it in next version. >> return block->host_cache + offset; >> } else { >> return block->host + offset; >> @@ -1462,6 +1474,13 @@ static inline void *host_from_stream_offset(QEMUFile *f, >> if (!strncmp(id, block->idstr, sizeof(id)) && >> block->max_length > offset) { >> if (ram_cache_enable) { >> + unsigned long *bitmap; >> + long k = (block->mr->ram_addr + offset) >> TARGET_PAGE_BITS; >> + >> + bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; >> + if (!test_and_set_bit(k, bitmap)) { >> + migration_dirty_pages++; >> + } >> return block->host_cache + offset; >> } else { >> return block->host + offset; >> @@ -1723,6 +1742,7 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) >> int colo_init_ram_cache(void) >> { >> RAMBlock *block; >> + int64_t ram_cache_pages = last_ram_offset() >> TARGET_PAGE_BITS; >> >> rcu_read_lock(); >> QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { >> @@ -1734,6 +1754,15 @@ int colo_init_ram_cache(void) >> } >> rcu_read_unlock(); >> ram_cache_enable = true; >> + /* >> + * Record the dirty pages that sent by PVM, we use this dirty bitmap together >> + * with to decide which page in cache should be flushed into SVM's RAM. Here >> + * we use the same name 'migration_bitmap_rcu' as for migration. >> + */ >> + migration_bitmap_rcu = g_new(struct BitmapRcu, 1); > > Please update that to g_new0 (I changed the other use when I added postcopy). > OK, thanks. > Dave > >> + migration_bitmap_rcu->bmap = bitmap_new(ram_cache_pages); >> + migration_dirty_pages = 0; >> + >> return 0; >> >> out_locked: >> @@ -1751,9 +1780,15 @@ out_locked: >> void colo_release_ram_cache(void) >> { >> RAMBlock *block; >> + struct BitmapRcu *bitmap = migration_bitmap_rcu; >> >> ram_cache_enable = false; >> >> + atomic_rcu_set(&migration_bitmap_rcu, NULL); >> + if (bitmap) { >> + call_rcu(bitmap, migration_bitmap_free, rcu); >> + } >> + >> rcu_read_lock(); >> QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { >> if (block->host_cache) { >> -- >> 1.8.3.1 >> >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > . >