| Message ID | 1270515084-24120-3-git-send-email-tamura.yoshiaki@lab.ntt.co.jp |
|---|---|
| State | New |
| Headers | show |
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. > + > + 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.
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.
On 04/12/2010 12:39 PM, Yoshiaki Tamura wrote: >> Please put in some header file, maybe qemu-common.h. > > > OK. BTW, is qemu-kvm.h planned to go upstream? No. Use kvm.h for kvm specific symbols (qemu-kvm.h includes it). >> >> 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. You can have *_DIRTY_FLAG (1, 2, 4, 8) and *_DIRTY_IDX (0, 1, 2, 3); use _FLAG for compatibility with byte-based maps, and _IDX for multiple bitmaps. Alternatively, have only _IDX, and modify the callers to shift when necessary. At the end of the patchset, we'll only use bitmaps, so the shifts will be all gone.
diff --git a/exec.c b/exec.c 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) + + 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)); + } last_ram_offset += size;