Re: [Qemu-devel] [PATCH v5 1/6] generic function between migration and bitmap dump

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* 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 <sanidhya.iiith@gmail.com>
> ---
>  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