Re: [PATCH v2 2/6] Introduce bit-based phys_ram_dirty for VGA, CODE, MIGRATION and MASTER.

[Avi Kivity <avi@redhat.com>]

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.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.