Re: [Qemu-devel] [PATCH v4 1/8] enable sharing of the function between migration and bitmap dump

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

* Sanidhya Kashyap (sanidhya.iiith@gmail.com) wrote:
> Added prefix qemu_ to the function as pointed out by Juan.
> 
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
>  arch_init.c             | 19 +++++++++++--------
>  include/exec/ram_addr.h |  4 ++++
>  2 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 8ddaf35..9ac4602 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,
> +                                                     bool migration_flag)
>  {
>      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 && migration_flag) {
>          migration_dirty_pages++;
>      }
>      return ret;
>  }

One small suggestion; instead of taking migration_flag you could give it
   uint64_t *counter

and then make it:
     if (!ret && counter) {
         *counter++;
     }

that keeps the functions more general.

> -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, bool migration_flag)
>  {
>      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, migration_flag);
>              }
>          }
>      }
> @@ -512,7 +514,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, true);
>      }
>      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 e9eb831..a5809b4 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -151,5 +151,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, bool migration_flag);
> +
>  #endif
>  #endif
> -- 
> 1.9.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK