From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yoshiaki Tamura Subject: Re: [PATCH v2 2/6] Introduce bit-based phys_ram_dirty for VGA, CODE, MIGRATION and MASTER. Date: Mon, 12 Apr 2010 18:39:48 +0900 Message-ID: <4BC2EA64.6060700@lab.ntt.co.jp> References: <1270515084-24120-1-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <1270515084-24120-3-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <4BC2D340.2030402@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org, anthony@codemonkey.ws, aliguori@us.ibm.com, mtosatti@redhat.com, ohmura.kei@lab.ntt.co.jp To: Avi Kivity Return-path: Received: from tama50.ecl.ntt.co.jp ([129.60.39.147]:35188 "EHLO tama50.ecl.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751219Ab0DLJkS (ORCPT ); Mon, 12 Apr 2010 05:40:18 -0400 In-Reply-To: <4BC2D340.2030402@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Avi Kivity wrote: > On 04/06/2010 03:51 AM, Yoshiaki Tamura wrote: >> Replaces byte-based phys_ram_dirty bitmap with three bit-based >> phys_ram_dirty >> bitmap. On allocation, it sets all bits in the bitmap. >> >> > >> index c74b0a4..9733892 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -110,7 +110,7 @@ uint8_t *code_gen_ptr; >> >> #if !defined(CONFIG_USER_ONLY) >> int phys_ram_fd; >> -uint8_t *phys_ram_dirty; >> +unsigned long *phys_ram_dirty[NUM_DIRTY_FLAGS]; >> static int in_migration; >> >> typedef struct RAMBlock { >> @@ -2825,10 +2825,32 @@ ram_addr_t qemu_ram_alloc(ram_addr_t size) >> new_block->next = ram_blocks; >> ram_blocks = new_block; >> >> - phys_ram_dirty = qemu_realloc(phys_ram_dirty, >> - (last_ram_offset + size)>> TARGET_PAGE_BITS); >> - memset(phys_ram_dirty + (last_ram_offset>> TARGET_PAGE_BITS), >> - 0xff, size>> TARGET_PAGE_BITS); >> +/* temporarily copy from qemu-kvm.git/qemu-kvm.h */ >> +#define ALIGN(x, y) (((x)+(y)-1)& ~((y)-1)) >> +#define BITMAP_SIZE(m) (ALIGN(((m)>>TARGET_PAGE_BITS), >> HOST_LONG_BITS) / 8) > > Please put in some header file, maybe qemu-common.h. OK. BTW, is qemu-kvm.h planned to go upstream? >> + >> + if (BITMAP_SIZE(last_ram_offset + size) != >> BITMAP_SIZE(last_ram_offset)) { >> + phys_ram_dirty[MASTER_DIRTY_FLAG] = >> + qemu_realloc(phys_ram_dirty[MASTER_DIRTY_FLAG], >> + BITMAP_SIZE(last_ram_offset + size)); >> + phys_ram_dirty[VGA_DIRTY_FLAG] >> + = qemu_realloc(phys_ram_dirty[VGA_DIRTY_FLAG], >> + BITMAP_SIZE(last_ram_offset + size)); >> + phys_ram_dirty[CODE_DIRTY_FLAG] = >> + qemu_realloc(phys_ram_dirty[CODE_DIRTY_FLAG], >> + BITMAP_SIZE(last_ram_offset + size)); >> + phys_ram_dirty[MIGRATION_DIRTY_FLAG] = >> + qemu_realloc(phys_ram_dirty[MIGRATION_DIRTY_FLAG], >> + BITMAP_SIZE(last_ram_offset + size)); >> + memset((uint8_t *)phys_ram_dirty[MASTER_DIRTY_FLAG] + >> + BITMAP_SIZE(last_ram_offset), 0xff, BITMAP_SIZE(size)); >> + memset((uint8_t *)phys_ram_dirty[VGA_DIRTY_FLAG] + >> + BITMAP_SIZE(last_ram_offset), 0xff, BITMAP_SIZE(size)); >> + memset((uint8_t *)phys_ram_dirty[CODE_DIRTY_FLAG] + >> + BITMAP_SIZE(last_ram_offset), 0xff, BITMAP_SIZE(size)); >> + memset((uint8_t *)phys_ram_dirty[MIGRATION_DIRTY_FLAG] + >> + BITMAP_SIZE(last_ram_offset), 0xff, BITMAP_SIZE(size)); >> + } >> > > Should be nicer as a loop calling a helper to allocate each bitmap. This > patch won't work by itself, will it? I think you need to merge it with > the succeeding patches. Yeah. I originally wrote as you suggested, but I needed to skip 0x03 because of the reason written below. > +/* Use DIRTY_FLAG as indexes of bit-based phys_ram_dirty. > + 0x03 is empty to make it compatible with byte-based bitmap. */ > +#define MASTER_DIRTY_FLAG 0x00 > #define VGA_DIRTY_FLAG 0x01 > #define CODE_DIRTY_FLAG 0x02 > -#define MIGRATION_DIRTY_FLAG 0x08 > +#define MIGRATION_DIRTY_FLAG 0x04 If you think if (i != 0x03) is better, I would modify it to a loop. And sorry, you need to merge the succeeding patches to make it work. From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O1G7h-0002NC-3I for qemu-devel@nongnu.org; Mon, 12 Apr 2010 05:40:05 -0400 Received: from [140.186.70.92] (port=36617 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O1G7e-0002Mz-FO for qemu-devel@nongnu.org; Mon, 12 Apr 2010 05:40:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O1G7c-0001VY-PL for qemu-devel@nongnu.org; Mon, 12 Apr 2010 05:40:02 -0400 Received: from tama50.ecl.ntt.co.jp ([129.60.39.147]:35182) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O1G7c-0001V0-7u for qemu-devel@nongnu.org; Mon, 12 Apr 2010 05:40:00 -0400 Message-ID: <4BC2EA64.6060700@lab.ntt.co.jp> Date: Mon, 12 Apr 2010 18:39:48 +0900 From: Yoshiaki Tamura MIME-Version: 1.0 References: <1270515084-24120-1-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <1270515084-24120-3-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <4BC2D340.2030402@redhat.com> In-Reply-To: <4BC2D340.2030402@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH v2 2/6] Introduce bit-based phys_ram_dirty for VGA, CODE, MIGRATION and MASTER. List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: aliguori@us.ibm.com, kvm@vger.kernel.org, ohmura.kei@lab.ntt.co.jp, mtosatti@redhat.com, qemu-devel@nongnu.org Avi Kivity wrote: > On 04/06/2010 03:51 AM, Yoshiaki Tamura wrote: >> Replaces byte-based phys_ram_dirty bitmap with three bit-based >> phys_ram_dirty >> bitmap. On allocation, it sets all bits in the bitmap. >> >> > >> index c74b0a4..9733892 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -110,7 +110,7 @@ uint8_t *code_gen_ptr; >> >> #if !defined(CONFIG_USER_ONLY) >> int phys_ram_fd; >> -uint8_t *phys_ram_dirty; >> +unsigned long *phys_ram_dirty[NUM_DIRTY_FLAGS]; >> static int in_migration; >> >> typedef struct RAMBlock { >> @@ -2825,10 +2825,32 @@ ram_addr_t qemu_ram_alloc(ram_addr_t size) >> new_block->next = ram_blocks; >> ram_blocks = new_block; >> >> - phys_ram_dirty = qemu_realloc(phys_ram_dirty, >> - (last_ram_offset + size)>> TARGET_PAGE_BITS); >> - memset(phys_ram_dirty + (last_ram_offset>> TARGET_PAGE_BITS), >> - 0xff, size>> TARGET_PAGE_BITS); >> +/* temporarily copy from qemu-kvm.git/qemu-kvm.h */ >> +#define ALIGN(x, y) (((x)+(y)-1)& ~((y)-1)) >> +#define BITMAP_SIZE(m) (ALIGN(((m)>>TARGET_PAGE_BITS), >> HOST_LONG_BITS) / 8) > > Please put in some header file, maybe qemu-common.h. OK. BTW, is qemu-kvm.h planned to go upstream? >> + >> + if (BITMAP_SIZE(last_ram_offset + size) != >> BITMAP_SIZE(last_ram_offset)) { >> + phys_ram_dirty[MASTER_DIRTY_FLAG] = >> + qemu_realloc(phys_ram_dirty[MASTER_DIRTY_FLAG], >> + BITMAP_SIZE(last_ram_offset + size)); >> + phys_ram_dirty[VGA_DIRTY_FLAG] >> + = qemu_realloc(phys_ram_dirty[VGA_DIRTY_FLAG], >> + BITMAP_SIZE(last_ram_offset + size)); >> + phys_ram_dirty[CODE_DIRTY_FLAG] = >> + qemu_realloc(phys_ram_dirty[CODE_DIRTY_FLAG], >> + BITMAP_SIZE(last_ram_offset + size)); >> + phys_ram_dirty[MIGRATION_DIRTY_FLAG] = >> + qemu_realloc(phys_ram_dirty[MIGRATION_DIRTY_FLAG], >> + BITMAP_SIZE(last_ram_offset + size)); >> + memset((uint8_t *)phys_ram_dirty[MASTER_DIRTY_FLAG] + >> + BITMAP_SIZE(last_ram_offset), 0xff, BITMAP_SIZE(size)); >> + memset((uint8_t *)phys_ram_dirty[VGA_DIRTY_FLAG] + >> + BITMAP_SIZE(last_ram_offset), 0xff, BITMAP_SIZE(size)); >> + memset((uint8_t *)phys_ram_dirty[CODE_DIRTY_FLAG] + >> + BITMAP_SIZE(last_ram_offset), 0xff, BITMAP_SIZE(size)); >> + memset((uint8_t *)phys_ram_dirty[MIGRATION_DIRTY_FLAG] + >> + BITMAP_SIZE(last_ram_offset), 0xff, BITMAP_SIZE(size)); >> + } >> > > Should be nicer as a loop calling a helper to allocate each bitmap. This > patch won't work by itself, will it? I think you need to merge it with > the succeeding patches. Yeah. I originally wrote as you suggested, but I needed to skip 0x03 because of the reason written below. > +/* Use DIRTY_FLAG as indexes of bit-based phys_ram_dirty. > + 0x03 is empty to make it compatible with byte-based bitmap. */ > +#define MASTER_DIRTY_FLAG 0x00 > #define VGA_DIRTY_FLAG 0x01 > #define CODE_DIRTY_FLAG 0x02 > -#define MIGRATION_DIRTY_FLAG 0x08 > +#define MIGRATION_DIRTY_FLAG 0x04 If you think if (i != 0x03) is better, I would modify it to a loop. And sorry, you need to merge the succeeding patches to make it work.