| Message ID | 1387293974-24718-33-git-send-email-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 12/17/2013 05:26 PM, Juan Quintela wrote: > Performance is important in this function, and we want to optimize even further. > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > kvm-all.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/kvm-all.c b/kvm-all.c > index 3937754..308dfba 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -31,6 +31,7 @@ > #include "sysemu/kvm.h" > #include "qemu/bswap.h" > #include "exec/memory.h" > +#include "exec/ram_addr.h" > #include "exec/address-spaces.h" > #include "qemu/event_notifier.h" > #include "trace.h" > @@ -382,6 +383,7 @@ static int kvm_get_dirty_pages_log_range(MemoryRegionSection *section, > unsigned int i, j; > unsigned long page_number, c; > hwaddr addr, addr1; > + ram_addr_t ram_addr; > unsigned int pages = int128_get64(section->size) / getpagesize(); > unsigned int len = (pages + HOST_LONG_BITS - 1) / HOST_LONG_BITS; > unsigned long hpratio = getpagesize() / TARGET_PAGE_SIZE; > @@ -399,8 +401,9 @@ static int kvm_get_dirty_pages_log_range(MemoryRegionSection *section, > page_number = (i * HOST_LONG_BITS + j) * hpratio; > addr1 = page_number * TARGET_PAGE_SIZE; > addr = section->offset_within_region + addr1; > - memory_region_set_dirty(section->mr, addr, > - TARGET_PAGE_SIZE * hpratio); > + ram_addr = section->mr->ram_addr + addr; > + cpu_physical_memory_set_dirty_range(ram_addr, > + TARGET_PAGE_SIZE * hpratio); > } while (c != 0); > } > } > Reviewed-by: Orit Wasserman <owasserm@redhat.com>
On 17 December 2013 15:26, Juan Quintela <quintela@redhat.com> wrote: > @@ -399,8 +401,9 @@ static int kvm_get_dirty_pages_log_range(MemoryRegionSection *section, > page_number = (i * HOST_LONG_BITS + j) * hpratio; > addr1 = page_number * TARGET_PAGE_SIZE; > addr = section->offset_within_region + addr1; > - memory_region_set_dirty(section->mr, addr, > - TARGET_PAGE_SIZE * hpratio); > + ram_addr = section->mr->ram_addr + addr; struct MemoryRegion says: /* All fields are private - violators will be prosecuted */ so fishing around in it for mr->ram_addr seems like a bad idea. Perhaps we could make memory_region_set_dirty() an inline function in memory.h instead? thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> wrote: > On 17 December 2013 15:26, Juan Quintela <quintela@redhat.com> wrote: >> @@ -399,8 +401,9 @@ static int >> kvm_get_dirty_pages_log_range(MemoryRegionSection *section, >> page_number = (i * HOST_LONG_BITS + j) * hpratio; >> addr1 = page_number * TARGET_PAGE_SIZE; >> addr = section->offset_within_region + addr1; >> - memory_region_set_dirty(section->mr, addr, >> - TARGET_PAGE_SIZE * hpratio); >> + ram_addr = section->mr->ram_addr + addr; > > struct MemoryRegion says: > /* All fields are private - violators will be prosecuted */ good spot > so fishing around in it for mr->ram_addr seems like a bad idea. > > Perhaps we could make memory_region_set_dirty() an inline > function in memory.h instead? But we already use it on two places on arch_init.c. For instance: static inline ram_addr_t migration_bitmap_find_and_reset_dirty(MemoryRegion *mr, ram_addr_t start) { unsigned long base = mr->ram_addr >> TARGET_PAGE_BITS; I tried to do the bitmap optimizations using memory_regions, and it got me nowhere. Basically we have: - TCG: _knows_ how memory bitmap/regions/etc work (actually, never got ported to MemoryRegions) - Migration: We are weird, and we _know_ about ram_addr_t, and we only use it, we don't care about memory regions. On this series we basicall went the other way around: we created include/exec/ram_addr.h, and we made _both_ migration and memory_region work on top of it. Later, Juan.
diff --git a/kvm-all.c b/kvm-all.c index 3937754..308dfba 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -31,6 +31,7 @@ #include "sysemu/kvm.h" #include "qemu/bswap.h" #include "exec/memory.h" +#include "exec/ram_addr.h" #include "exec/address-spaces.h" #include "qemu/event_notifier.h" #include "trace.h" @@ -382,6 +383,7 @@ static int kvm_get_dirty_pages_log_range(MemoryRegionSection *section, unsigned int i, j; unsigned long page_number, c; hwaddr addr, addr1; + ram_addr_t ram_addr; unsigned int pages = int128_get64(section->size) / getpagesize(); unsigned int len = (pages + HOST_LONG_BITS - 1) / HOST_LONG_BITS; unsigned long hpratio = getpagesize() / TARGET_PAGE_SIZE; @@ -399,8 +401,9 @@ static int kvm_get_dirty_pages_log_range(MemoryRegionSection *section, page_number = (i * HOST_LONG_BITS + j) * hpratio; addr1 = page_number * TARGET_PAGE_SIZE; addr = section->offset_within_region + addr1; - memory_region_set_dirty(section->mr, addr, - TARGET_PAGE_SIZE * hpratio); + ram_addr = section->mr->ram_addr + addr; + cpu_physical_memory_set_dirty_range(ram_addr, + TARGET_PAGE_SIZE * hpratio); } while (c != 0); } }
Performance is important in this function, and we want to optimize even further. Signed-off-by: Juan Quintela <quintela@redhat.com> --- kvm-all.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)