[prev in list] [next in list] [prev in thread] [next in thread] 

List:       qemu-devel
Subject:    Re: [Qemu-devel] [PATCH 12/13] dirty bitmap: abstract its use
From:       Blue Swirl <blauwirbel () gmail ! com>
Date:       2012-06-29 20:18:40
Message-ID: CAAu8pHv5MdD8NSrfij6BuMAPxP-SfkGAbgx1BEPHH1z-gY6gmQ () mail ! gmail ! com
[Download RAW message or body]

On Fri, Jun 29, 2012 at 4:44 PM, Juan Quintela <quintela@redhat.com> wrote:
> Always use accessors to read/set the dirty bitmap.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>   exec-obsolete.h |    40 ++++++++++++++++++++--------------------
>   exec.c               |      3 +--
>   2 files changed, 21 insertions(+), 22 deletions(-)
>
> diff --git a/exec-obsolete.h b/exec-obsolete.h
> index 792c831..f8ffce6 100644
> --- a/exec-obsolete.h
> +++ b/exec-obsolete.h
> @@ -45,15 +45,15 @@ int cpu_physical_memory_set_dirty_tracking(int enable);
>   #define CODE_DIRTY_FLAG         0x02
>   #define MIGRATION_DIRTY_FLAG 0x08
>
> -/* read dirty bit (return 0 or 1) */
> -static inline int cpu_physical_memory_is_dirty(ram_addr_t addr)
> +static inline int cpu_physical_memory_get_dirty_flags(ram_addr_t addr)
>   {
> -      return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] == 0xff;
> +      return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS];
>   }
>
> -static inline int cpu_physical_memory_get_dirty_flags(ram_addr_t addr)
> +/* read dirty bit (return 0 or 1) */
> +static inline int cpu_physical_memory_is_dirty(ram_addr_t addr)
>   {
> -      return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS];
> +      return cpu_physical_memory_get_dirty_flags(addr) == 0xff;
>   }

This shuffling does not look helpful. At least it's difficult to see
if anything changes.

>
>   static inline int cpu_physical_memory_get_dirty(ram_addr_t start,
> @@ -61,41 +61,45 @@ static inline int cpu_physical_memory_get_dirty(ram_addr_t start,
>                                                                         int dirty_flags)
>   {
>       int ret = 0;
> -      uint8_t *p;
>       ram_addr_t addr, end;
>
>       end = TARGET_PAGE_ALIGN(start + length);
>       start &= TARGET_PAGE_MASK;
> -      p = ram_list.phys_dirty + (start >> TARGET_PAGE_BITS);
>       for (addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
> -            ret |= *p++ & dirty_flags;
> +            ret |= cpu_physical_memory_get_dirty_flags(addr) & dirty_flags;
>       }
>       return ret;
>   }
>
> +static inline int cpu_physical_memory_set_dirty_flags(ram_addr_t addr,
> +                                                                                 int dirty_flags)
> +{
> +      return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] |= dirty_flags;
> +}
> +
>   static inline void cpu_physical_memory_set_dirty(ram_addr_t addr)
>   {
> -      ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] = 0xff;
> +      cpu_physical_memory_set_dirty_flags(addr, 0xff);
>   }
>
> -static inline int cpu_physical_memory_set_dirty_flags(ram_addr_t addr,
> -                                                                                 int dirty_flags)
> +static inline int cpu_physical_memory_clear_dirty_flags(ram_addr_t addr,
> +                                                                                    int dirty_flags)
>   {
> -      return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] |= dirty_flags;
> +      int mask = ~dirty_flags;
> +
> +      return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] &= mask;
>   }

Ditto.

>
>   static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start,
>                                                                                    ram_addr_t length,
>                                                                                    int dirty_flags)
>   {
> -      uint8_t *p;
>       ram_addr_t addr, end;
>
>       end = TARGET_PAGE_ALIGN(start + length);
>       start &= TARGET_PAGE_MASK;
> -      p = ram_list.phys_dirty + (start >> TARGET_PAGE_BITS);
>       for (addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
> -            *p++ |= dirty_flags;
> +            cpu_physical_memory_set_dirty_flags(addr, dirty_flags);
>       }
>   }
>
> @@ -103,16 +107,12 @@ static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start,
>                                                                                     ram_addr_t length,
>                                                                                     int dirty_flags)
>   {
> -      int mask;
> -      uint8_t *p;
>       ram_addr_t addr, end;
>
>       end = TARGET_PAGE_ALIGN(start + length);
>       start &= TARGET_PAGE_MASK;
> -      mask = ~dirty_flags;
> -      p = ram_list.phys_dirty + (start >> TARGET_PAGE_BITS);
>       for (addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
> -            *p++ &= mask;
> +            cpu_physical_memory_clear_dirty_flags(addr, dirty_flags);
>       }
>   }
>
> diff --git a/exec.c b/exec.c
> index a68b65c..dd4833d 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2565,8 +2565,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
>
>       ram_list.phys_dirty = g_realloc(ram_list.phys_dirty,
>                                                            last_ram_offset() >> TARGET_PAGE_BITS);
> -      memset(ram_list.phys_dirty + (new_block->offset >> TARGET_PAGE_BITS),
> -                0xff, size >> TARGET_PAGE_BITS);
> +      cpu_physical_memory_set_dirty_range(new_block->offset, size, 0xff);
>
>       if (kvm_enabled())
>             kvm_setup_guest_memory(new_block->host, size);
> --
> 1.7.10.4
>
>

[prev in list] [next in list] [prev in thread] [next in thread]