[Qemu-devel] [PATCH v3 00/29] Dirty bitmap atomic access and optimizations

[Qemu-devel] [PATCH v3 00/29] Dirty bitmap atomic access and optimizations

[Paolo Bonzini]

QEMU is currently accessing the dirty bitmaps very liberally,
which is understandable since the accesses are cheap.  This is
however not good for squeezing maximum performance out of dataplane,
and is also not good if the accesses become more expensive---as is
the case when they use atomic primitives.

This patch series does the following optimizations and cleanups:

1) it lets KVM code treat migration as "just another dirty bitmap
client" instead of needing the special global_log_start/stop callbacks.
These remain in use in Xen and vhost.  This removes code and avoids
bugs such as the one fixed in commit 4cc856f (kvm-all: Sync dirty-bitmap
from kvm before kvm destroy the corresponding dirty_bitmap, 2015-04-02).

2) it avoids modifications to unused dirty bitmaps: code if TCG
is disabled, migration if no migration is in progress, VGA for
regions other than VRAM.

and on top of this makes dirty bitmap access atomic.  I'm not including
the patch to make the migration thread synchronize the bitmap outside
the big QEMU lock (thus removing the last source of jitter during the
RAM copy phase of migration) but it is also enabled by these patches.

Patches 1-4 are cleanups to DIRTY_MEMORY_VGA users.

Patches 5-12 are the first cleanup (KVM treats migration as just
another client).  Patches 13-14 are a simple optimization that is enabled
by these patches.

Patches 15-18 are bonus cleanups to translate-all.c's dirty memory
tracking for TCG.

Patches 19-22 are the second cleanup (avoid modifications to unused
dirty bitmaps).

Patches 23-28 are Stefan's patches for atomic access to the dirty
bitmap, which has no performance impact in the common case thanks to
the previous work.

Patch 29 is an unrelated strengthening of assertions, that mst spotted
while reviewing v1.

v2->v3: 22 patches reviewed by Fam
	fixed tcx24_update_display (patch 4, Fam)
	fixed comments (patch 5, Fam)
	improved commit messages (patches 7/10/11, Fam)
	fixed logic for full word loops (patches 23/24, Fam)
	avoid duplicated ~(ram_addr_t)0 constant (patch 29, Fam)

v1->v2: completed work on removing global_start/global_stop from KVM
	listener

        extra spelunking of TCG history so that the exec.c code
	makes more sense

        extra splitting of patches (Stefan)

        keep memory_region_is_logging and memory_region_get_dirty_log_mask
	APIs separate (mst)


Paolo Bonzini (23):
  memory: the only dirty memory flag for users is DIRTY_MEMORY_VGA
  g364fb: remove pointless call to memory_region_set_coalescing
  display: enable DIRTY_MEMORY_VGA tracking explicitly
  display: add memory_region_sync_dirty_bitmap calls
  memory: differentiate memory_region_is_logging and
    memory_region_get_dirty_log_mask
  memory: prepare for multiple bits in the dirty log mask
  framebuffer: check memory_region_is_logging
  ui/console: remove dpy_gfx_update_dirty
  memory: track DIRTY_MEMORY_CODE in mr->dirty_log_mask
  kvm: accept non-mapped memory in kvm_dirty_pages_log_change
  memory: include DIRTY_MEMORY_MIGRATION in the dirty log mask
  kvm: remove special handling of DIRTY_MEMORY_MIGRATION in the dirty
    log mask
  ram_addr: tweaks to xen_modified_memory
  exec: use memory_region_get_dirty_log_mask to optimize dirty tracking
  exec: move functions to translate-all.h
  translate-all: remove unnecessary argument to tb_invalidate_phys_range
  cputlb: remove useless arguments to tlb_unprotect_code_phys, rename
  translate-all: make less of tb_invalidate_phys_page_range depend on
    is_cpu_write_access
  exec: pass client mask to cpu_physical_memory_set_dirty_range
  exec: invert return value of cpu_physical_memory_get_clean, rename
  exec: only check relevant bitmaps for cleanliness
  memory: do not touch code dirty bitmap unless TCG is enabled
  memory: use mr->ram_addr in "is this RAM?" assertions

Stefan Hajnoczi (6):
  bitmap: add atomic set functions
  bitmap: add atomic test and clear
  memory: use atomic ops for setting dirty memory bits
  migration: move dirty bitmap sync to ram_addr.h
  memory: replace cpu_physical_memory_reset_dirty() with test-and-clear
  memory: make cpu_physical_memory_sync_dirty_bitmap() fully atomic

 arch_init.c                  |  46 +--------------
 cputlb.c                     |   7 +--
 exec.c                       |  99 +++++++++++++++----------------
 hw/display/cg3.c             |   2 +
 hw/display/exynos4210_fimd.c |  20 ++++---
 hw/display/framebuffer.c     |   4 ++
 hw/display/g364fb.c          |   3 +-
 hw/display/sm501.c           |   2 +
 hw/display/tcx.c             |   3 +
 hw/display/vmware_vga.c      |   2 +-
 hw/virtio/dataplane/vring.c  |   2 +-
 hw/virtio/vhost.c            |   9 ++-
 include/exec/cputlb.h        |   3 +-
 include/exec/exec-all.h      |   6 +-
 include/exec/memory.h        |  25 ++++++--
 include/exec/ram_addr.h      | 138 ++++++++++++++++++++++++++++---------------
 include/qemu/bitmap.h        |   4 ++
 include/qemu/bitops.h        |  14 +++++
 include/ui/console.h         |   4 --
 kvm-all.c                    |  77 ++++++------------------
 linux-user/mmap.c            |   7 ++-
 memory.c                     |  81 +++++++++++++++++--------
 translate-all.c              |  20 +++----
 translate-all.h              |   7 +++
 ui/console.c                 |  61 -------------------
 user-exec.c                  |   1 +
 util/bitmap.c                |  83 ++++++++++++++++++++++++++
 xen-hvm.c                    |  22 ++++---
 28 files changed, 408 insertions(+), 344 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v3 01/29] memory: the only dirty memory flag for users is DIRTY_MEMORY_VGA

[Paolo Bonzini]

DIRTY_MEMORY_MIGRATION is triggered by memory_global_dirty_log_start
and memory_global_dirty_log_stop, so it cannot be used with
memory_region_set_log.

Specify this in the documentation and assert it.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/exec/memory.h | 3 +--
 memory.c              | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index b61c84f..7b0929d 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -647,8 +647,7 @@ void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize,
  *
  * @mr: the memory region being updated.
  * @log: whether dirty logging is to be enabled or disabled.
- * @client: the user of the logging information; %DIRTY_MEMORY_MIGRATION or
- *          %DIRTY_MEMORY_VGA.
+ * @client: the user of the logging information; %DIRTY_MEMORY_VGA only.
  */
 void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client);
 
diff --git a/memory.c b/memory.c
index 03c536b..a8f8599 100644
--- a/memory.c
+++ b/memory.c
@@ -1425,6 +1425,7 @@ void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
 {
     uint8_t mask = 1 << client;
 
+    assert(client == DIRTY_MEMORY_VGA);
     memory_region_transaction_begin();
     mr->dirty_log_mask = (mr->dirty_log_mask & ~mask) | (log * mask);
     memory_region_update_pending |= mr->enabled;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 02/29] g364fb: remove pointless call to memory_region_set_coalescing

[Paolo Bonzini]

Coalescing work on MMIO, not RAM, thus this call has no effect.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/display/g364fb.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/display/g364fb.c b/hw/display/g364fb.c
index 46f7b41..6543f2f 100644
--- a/hw/display/g364fb.c
+++ b/hw/display/g364fb.c
@@ -489,7 +489,6 @@ static void g364fb_init(DeviceState *dev, G364State *s)
     memory_region_init_ram_ptr(&s->mem_vram, NULL, "vram",
                                s->vram_size, s->vram);
     vmstate_register_ram(&s->mem_vram, dev);
-    memory_region_set_coalescing(&s->mem_vram);
 }
 
 #define TYPE_G364 "sysbus-g364"
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 03/29] display: enable DIRTY_MEMORY_VGA tracking explicitly

[Paolo Bonzini]

This will be required soon by the memory core.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/display/cg3.c             |  1 +
 hw/display/exynos4210_fimd.c | 20 +++++++++++++-------
 hw/display/g364fb.c          |  1 +
 hw/display/sm501.c           |  1 +
 hw/display/tcx.c             |  1 +
 5 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/hw/display/cg3.c b/hw/display/cg3.c
index 1e6ff2b..cbcf518 100644
--- a/hw/display/cg3.c
+++ b/hw/display/cg3.c
@@ -309,6 +309,7 @@ static void cg3_realizefn(DeviceState *dev, Error **errp)
 
     memory_region_init_ram(&s->vram_mem, NULL, "cg3.vram", s->vram_size,
                            &error_abort);
+    memory_region_set_log(&s->vram_mem, true, DIRTY_MEMORY_VGA);
     vmstate_register_ram_global(&s->vram_mem);
     sysbus_init_mmio(sbd, &s->vram_mem);
 
diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
index 45c62af..72b3a1d 100644
--- a/hw/display/exynos4210_fimd.c
+++ b/hw/display/exynos4210_fimd.c
@@ -1109,6 +1109,12 @@ static inline int fimd_get_buffer_id(Exynos4210fimdWindow *w)
     }
 }
 
+static void exynos4210_fimd_invalidate(void *opaque)
+{
+    Exynos4210fimdState *s = (Exynos4210fimdState *)opaque;
+    s->invalidate = true;
+}
+
 /* Updates specified window's MemorySection based on values of WINCON,
  * VIDOSDA, VIDOSDB, VIDWADDx and SHADOWCON registers */
 static void fimd_update_memory_section(Exynos4210fimdState *s, unsigned win)
@@ -1136,7 +1142,11 @@ static void fimd_update_memory_section(Exynos4210fimdState *s, unsigned win)
     /* TODO: add .exit and unref the region there.  Not needed yet since sysbus
      * does not support hot-unplug.
      */
-    memory_region_unref(w->mem_section.mr);
+    if (w->mem_section.mr) {
+        memory_region_set_log(w->mem_section.mr, false, DIRTY_MEMORY_VGA);
+        memory_region_unref(w->mem_section.mr);
+    }
+
     w->mem_section = memory_region_find(sysbus_address_space(sbd),
                                         fb_start_addr, w->fb_len);
     assert(w->mem_section.mr);
@@ -1162,6 +1172,8 @@ static void fimd_update_memory_section(Exynos4210fimdState *s, unsigned win)
         cpu_physical_memory_unmap(w->host_fb_addr, fb_mapped_len, 0, 0);
         goto error_return;
     }
+    memory_region_set_log(w->mem_section.mr, true, DIRTY_MEMORY_VGA);
+    exynos4210_fimd_invalidate(s);
     return;
 
 error_return:
@@ -1224,12 +1236,6 @@ static void exynos4210_fimd_update_irq(Exynos4210fimdState *s)
     }
 }
 
-static void exynos4210_fimd_invalidate(void *opaque)
-{
-    Exynos4210fimdState *s = (Exynos4210fimdState *)opaque;
-    s->invalidate = true;
-}
-
 static void exynos4210_update_resolution(Exynos4210fimdState *s)
 {
     DisplaySurface *surface = qemu_console_surface(s->console);
diff --git a/hw/display/g364fb.c b/hw/display/g364fb.c
index 6543f2f..be62dd6 100644
--- a/hw/display/g364fb.c
+++ b/hw/display/g364fb.c
@@ -489,6 +489,7 @@ static void g364fb_init(DeviceState *dev, G364State *s)
     memory_region_init_ram_ptr(&s->mem_vram, NULL, "vram",
                                s->vram_size, s->vram);
     vmstate_register_ram(&s->mem_vram, dev);
+    memory_region_set_log(&s->mem_vram, true, DIRTY_MEMORY_VGA);
 }
 
 #define TYPE_G364 "sysbus-g364"
diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index c72154b..43f8538 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1412,6 +1412,7 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base,
     memory_region_init_ram(&s->local_mem_region, NULL, "sm501.local",
                            local_mem_bytes, &error_abort);
     vmstate_register_ram_global(&s->local_mem_region);
+    memory_region_set_log(&s->local_mem_region, true, DIRTY_MEMORY_VGA);
     s->local_mem = memory_region_get_ram_ptr(&s->local_mem_region);
     memory_region_add_subregion(address_space_mem, base, &s->local_mem_region);
 
diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index a9f9f66..58faa96 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -1006,6 +1006,7 @@ static void tcx_realizefn(DeviceState *dev, Error **errp)
     memory_region_init_ram(&s->vram_mem, OBJECT(s), "tcx.vram",
                            s->vram_size * (1 + 4 + 4), &error_abort);
     vmstate_register_ram_global(&s->vram_mem);
+    memory_region_set_log(&s->vram_mem, true, DIRTY_MEMORY_VGA);
     vram_base = memory_region_get_ram_ptr(&s->vram_mem);
 
     /* 10/ROM : FCode ROM */
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 04/29] display: add memory_region_sync_dirty_bitmap calls

[Paolo Bonzini]

These are strictly speaking only needed for KVM and Xen, but it's still
nice to be consistent.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/display/cg3.c    | 1 +
 hw/display/g364fb.c | 1 +
 hw/display/sm501.c  | 1 +
 hw/display/tcx.c    | 2 ++
 4 files changed, 5 insertions(+)

diff --git a/hw/display/cg3.c b/hw/display/cg3.c
index cbcf518..b94e5e0 100644
--- a/hw/display/cg3.c
+++ b/hw/display/cg3.c
@@ -106,6 +106,7 @@ static void cg3_update_display(void *opaque)
     pix = memory_region_get_ram_ptr(&s->vram_mem);
     data = (uint32_t *)surface_data(surface);
 
+    memory_region_sync_dirty_bitmap(&s->vram_mem);
     for (y = 0; y < height; y++) {
         int update = s->full_update;
 
diff --git a/hw/display/g364fb.c b/hw/display/g364fb.c
index be62dd6..52a9733 100644
--- a/hw/display/g364fb.c
+++ b/hw/display/g364fb.c
@@ -260,6 +260,7 @@ static void g364fb_update_display(void *opaque)
         qemu_console_resize(s->con, s->width, s->height);
     }
 
+    memory_region_sync_dirty_bitmap(&s->mem_vram);
     if (s->ctla & CTLA_FORCE_BLANK) {
         g364fb_draw_blank(s);
     } else if (s->depth == 8) {
diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 43f8538..15a5ba8 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1322,6 +1322,7 @@ static void sm501_draw_crt(SM501State * s)
     }
 
     /* draw each line according to conditions */
+    memory_region_sync_dirty_bitmap(&s->local_mem_region);
     for (y = 0; y < height; y++) {
 	int update_hwc = draw_hwc_line ? within_hwc_y_range(s, y, 1) : 0;
 	int update = full_update || update_hwc;
diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 58faa96..f3faf78 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -353,6 +353,7 @@ static void tcx_update_display(void *opaque)
         return;
     }
 
+    memory_region_sync_dirty_bitmap(&ts->vram_mem);
     for (y = 0; y < ts->height; page += TARGET_PAGE_SIZE) {
         if (memory_region_get_dirty(&ts->vram_mem, page, TARGET_PAGE_SIZE,
                                     DIRTY_MEMORY_VGA)) {
@@ -446,6 +447,7 @@ static void tcx24_update_display(void *opaque)
     dd = surface_stride(surface);
     ds = 1024;
 
+    memory_region_sync_dirty_bitmap(&ts->vram_mem);
     for (y = 0; y < ts->height; page += TARGET_PAGE_SIZE,
             page24 += TARGET_PAGE_SIZE, cpage += TARGET_PAGE_SIZE) {
         if (tcx24_check_dirty(ts, page, page24, cpage)) {
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 05/29] memory: differentiate memory_region_is_logging and memory_region_get_dirty_log_mask

[Paolo Bonzini]

For now memory regions only track DIRTY_MEMORY_VGA individually, but
this will change soon.  To support this, split memory_region_is_logging
in two functions: one that returns a given bit from dirty_log_mask,
and one that returns the entire mask.  memory_region_is_logging gets an
extra parameter so that the compiler flags misuse.

While VGA-specific users (including the Xen listener!) will want to keep
checking that bit, KVM and vhost check for "any bit except migration"
(because migration is handled via the global start/stop listener
callbacks).

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/display/vmware_vga.c     |  2 +-
 hw/virtio/dataplane/vring.c |  2 +-
 hw/virtio/vhost.c           |  3 ++-
 include/exec/memory.h       | 16 ++++++++++++++--
 kvm-all.c                   |  3 ++-
 memory.c                    |  7 ++++++-
 xen-hvm.c                   |  2 +-
 7 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index c17ddd1..7f397d3 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -1124,7 +1124,7 @@ static void vmsvga_update_display(void *opaque)
      * Is it more efficient to look at vram VGA-dirty bits or wait
      * for the driver to issue SVGA_CMD_UPDATE?
      */
-    if (memory_region_is_logging(&s->vga.vram)) {
+    if (memory_region_is_logging(&s->vga.vram, DIRTY_MEMORY_VGA)) {
         vga_sync_dirty_bitmap(&s->vga);
         dirty = memory_region_get_dirty(&s->vga.vram, 0,
             surface_stride(surface) * surface_height(surface),
diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c
index 5c7b8c2..e378733 100644
--- a/hw/virtio/dataplane/vring.c
+++ b/hw/virtio/dataplane/vring.c
@@ -42,7 +42,7 @@ static void *vring_map(MemoryRegion **mr, hwaddr phys, hwaddr len,
     }
 
     /* Ignore regions with dirty logging, we cannot mark them dirty */
-    if (memory_region_is_logging(section.mr)) {
+    if (memory_region_get_dirty_log_mask(section.mr)) {
         goto out;
     }
 
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 54851b7..5f698a1 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -377,7 +377,8 @@ static void vhost_set_memory(MemoryListener *listener,
                                          memory_listener);
     hwaddr start_addr = section->offset_within_address_space;
     ram_addr_t size = int128_get64(section->size);
-    bool log_dirty = memory_region_is_logging(section->mr);
+    bool log_dirty =
+        memory_region_get_dirty_log_mask(section->mr) & ~(1 << DIRTY_MEMORY_MIGRATION);
     int s = offsetof(struct vhost_memory, regions) +
         (dev->mem->nregions + 1) * sizeof dev->mem->regions[0];
     void *ram;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 7b0929d..156d506 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -591,11 +591,23 @@ const char *memory_region_name(const MemoryRegion *mr);
 /**
  * memory_region_is_logging: return whether a memory region is logging writes
  *
- * Returns %true if the memory region is logging writes
+ * Returns %true if the memory region is logging writes for the given client
  *
  * @mr: the memory region being queried
+ * @client: the client being queried
  */
-bool memory_region_is_logging(MemoryRegion *mr);
+bool memory_region_is_logging(MemoryRegion *mr, uint8_t client);
+
+/**
+ * memory_region_get_dirty_log_mask: return the clients for which a
+ * memory region is logging writes.
+ *
+ * Returns a bitmap of clients, which right now will be either 0 or
+ * (1 << DIRTY_MEMORY_VGA).
+ *
+ * @mr: the memory region being queried
+ */
+uint8_t memory_region_get_dirty_log_mask(MemoryRegion *mr);
 
 /**
  * memory_region_is_rom: check whether a memory region is ROM
diff --git a/kvm-all.c b/kvm-all.c
index 17a3771..48c8665 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -663,7 +663,8 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
     KVMSlot *mem, old;
     int err;
     MemoryRegion *mr = section->mr;
-    bool log_dirty = memory_region_is_logging(mr);
+    bool log_dirty =
+        memory_region_get_dirty_log_mask(mr) & ~(1 << DIRTY_MEMORY_MIGRATION);
     bool writeable = !mr->readonly && !mr->rom_device;
     bool readonly_flag = mr->readonly || memory_region_is_romd(mr);
     hwaddr start_addr = section->offset_within_address_space;
diff --git a/memory.c b/memory.c
index a8f8599..72e33e8 100644
--- a/memory.c
+++ b/memory.c
@@ -1389,11 +1389,16 @@ bool memory_region_is_skip_dump(MemoryRegion *mr)
     return mr->skip_dump;
 }
 
-bool memory_region_is_logging(MemoryRegion *mr)
+uint8_t memory_region_get_dirty_log_mask(MemoryRegion *mr)
 {
     return mr->dirty_log_mask;
 }
 
+bool memory_region_is_logging(MemoryRegion *mr, uint8_t client)
+{
+    return memory_region_get_dirty_log_mask(mr) & (1 << client);
+}
+
 bool memory_region_is_rom(MemoryRegion *mr)
 {
     return mr->ram && mr->readonly;
diff --git a/xen-hvm.c b/xen-hvm.c
index 315864c..338ab29 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -488,7 +488,7 @@ static void xen_set_memory(struct MemoryListener *listener,
     XenIOState *state = container_of(listener, XenIOState, memory_listener);
     hwaddr start_addr = section->offset_within_address_space;
     ram_addr_t size = int128_get64(section->size);
-    bool log_dirty = memory_region_is_logging(section->mr);
+    bool log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA);
     hvmmem_type_t mem_type;
 
     if (section->mr == &ram_memory) {
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 06/29] memory: prepare for multiple bits in the dirty log mask

[Paolo Bonzini]

When the dirty log mask will also cover other bits than DIRTY_MEMORY_VGA,
some listeners may be interested in the overall zero/non-zero value of
the dirty log mask; others may be interested in the value of single bits.

For this reason, always call log_start/log_stop if bits have respectively
appeared or disappeared, and pass the old and new values of the dirty log
mask so that listeners can distinguish the kinds of change.

For example, KVM checks if dirty logging used to be completely disabled
(in log_start) or is now completely disabled (in log_stop).  On the
other hand, Xen has to check manually if DIRTY_MEMORY_VGA changed,
since that is the only bit it cares about.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/vhost.c     |  6 ++++--
 include/exec/memory.h |  6 ++++--
 kvm-all.c             | 14 ++++++++++++--
 memory.c              | 17 +++++++++++------
 xen-hvm.c             | 20 +++++++++++++-------
 5 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 5f698a1..6364b25 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -637,13 +637,15 @@ static void vhost_log_global_stop(MemoryListener *listener)
 }
 
 static void vhost_log_start(MemoryListener *listener,
-                            MemoryRegionSection *section)
+                            MemoryRegionSection *section,
+                            int old, int new)
 {
     /* FIXME: implement */
 }
 
 static void vhost_log_stop(MemoryListener *listener,
-                           MemoryRegionSection *section)
+                           MemoryRegionSection *section,
+                           int old, int new)
 {
     /* FIXME: implement */
 }
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 156d506..55dc11d 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -206,8 +206,10 @@ struct MemoryListener {
     void (*region_add)(MemoryListener *listener, MemoryRegionSection *section);
     void (*region_del)(MemoryListener *listener, MemoryRegionSection *section);
     void (*region_nop)(MemoryListener *listener, MemoryRegionSection *section);
-    void (*log_start)(MemoryListener *listener, MemoryRegionSection *section);
-    void (*log_stop)(MemoryListener *listener, MemoryRegionSection *section);
+    void (*log_start)(MemoryListener *listener, MemoryRegionSection *section,
+                      int old, int new);
+    void (*log_stop)(MemoryListener *listener, MemoryRegionSection *section,
+                     int old, int new);
     void (*log_sync)(MemoryListener *listener, MemoryRegionSection *section);
     void (*log_global_start)(MemoryListener *listener);
     void (*log_global_stop)(MemoryListener *listener);
diff --git a/kvm-all.c b/kvm-all.c
index 48c8665..b544e95 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -344,10 +344,15 @@ static int kvm_dirty_pages_log_change(hwaddr phys_addr,
 }
 
 static void kvm_log_start(MemoryListener *listener,
-                          MemoryRegionSection *section)
+                          MemoryRegionSection *section,
+                          int old, int new)
 {
     int r;
 
+    if (old != 0) {
+        return;
+    }
+
     r = kvm_dirty_pages_log_change(section->offset_within_address_space,
                                    int128_get64(section->size), true);
     if (r < 0) {
@@ -356,10 +361,15 @@ static void kvm_log_start(MemoryListener *listener,
 }
 
 static void kvm_log_stop(MemoryListener *listener,
-                          MemoryRegionSection *section)
+                          MemoryRegionSection *section,
+                          int old, int new)
 {
     int r;
 
+    if (new != 0) {
+        return;
+    }
+
     r = kvm_dirty_pages_log_change(section->offset_within_address_space,
                                    int128_get64(section->size), false);
     if (r < 0) {
diff --git a/memory.c b/memory.c
index 72e33e8..3e3d5b8 100644
--- a/memory.c
+++ b/memory.c
@@ -152,7 +152,7 @@ static bool memory_listener_match(MemoryListener *listener,
     } while (0)
 
 /* No need to ref/unref .mr, the FlatRange keeps it alive.  */
-#define MEMORY_LISTENER_UPDATE_REGION(fr, as, dir, callback)            \
+#define MEMORY_LISTENER_UPDATE_REGION(fr, as, dir, callback, _args...)  \
     MEMORY_LISTENER_CALL(callback, dir, (&(MemoryRegionSection) {       \
         .mr = (fr)->mr,                                                 \
         .address_space = (as),                                          \
@@ -160,7 +160,7 @@ static bool memory_listener_match(MemoryListener *listener,
         .size = (fr)->addr.size,                                        \
         .offset_within_address_space = int128_get64((fr)->addr.start),  \
         .readonly = (fr)->readonly,                                     \
-              }))
+              }), ##_args)
 
 struct CoalescedMemoryRange {
     AddrRange addr;
@@ -774,10 +774,15 @@ static void address_space_update_topology_pass(AddressSpace *as,
 
             if (adding) {
                 MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, region_nop);
-                if (frold->dirty_log_mask && !frnew->dirty_log_mask) {
-                    MEMORY_LISTENER_UPDATE_REGION(frnew, as, Reverse, log_stop);
-                } else if (frnew->dirty_log_mask && !frold->dirty_log_mask) {
-                    MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, log_start);
+                if (frnew->dirty_log_mask & ~frold->dirty_log_mask) {
+                    MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, log_start,
+                                                  frold->dirty_log_mask,
+                                                  frnew->dirty_log_mask);
+                }
+                if (frold->dirty_log_mask & ~frnew->dirty_log_mask) {
+                    MEMORY_LISTENER_UPDATE_REGION(frnew, as, Reverse, log_stop,
+                                                  frold->dirty_log_mask,
+                                                  frnew->dirty_log_mask);
                 }
             }
 
diff --git a/xen-hvm.c b/xen-hvm.c
index 338ab29..cc55faa 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -646,21 +646,27 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
 }
 
 static void xen_log_start(MemoryListener *listener,
-                          MemoryRegionSection *section)
+                          MemoryRegionSection *section,
+                          int old, int new)
 {
     XenIOState *state = container_of(listener, XenIOState, memory_listener);
 
-    xen_sync_dirty_bitmap(state, section->offset_within_address_space,
-                          int128_get64(section->size));
+    if (new & ~old & (1 << DIRTY_MIGRATION_VGA)) {
+        xen_sync_dirty_bitmap(state, section->offset_within_address_space,
+                              int128_get64(section->size));
+    }
 }
 
-static void xen_log_stop(MemoryListener *listener, MemoryRegionSection *section)
+static void xen_log_stop(MemoryListener *listener, MemoryRegionSection *section,
+                         int old, int new)
 {
     XenIOState *state = container_of(listener, XenIOState, memory_listener);
 
-    state->log_for_dirtybit = NULL;
-    /* Disable dirty bit tracking */
-    xc_hvm_track_dirty_vram(xen_xc, xen_domid, 0, 0, NULL);
+    if (old & ~new & (1 << DIRTY_MIGRATION_VGA)) {
+        state->log_for_dirtybit = NULL;
+        /* Disable dirty bit tracking */
+        xc_hvm_track_dirty_vram(xen_xc, xen_domid, 0, 0, NULL);
+    }
 }
 
 static void xen_log_sync(MemoryListener *listener, MemoryRegionSection *section)
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 07/29] framebuffer: check memory_region_is_logging

[Paolo Bonzini]

framebuffer.c expects DIRTY_MEMORY_VGA logging to be always on, but that
will not be the case soon.  Because framebuffer.c computes the memory
region on the fly for every update (with memory_region_find), it cannot
enable/disable logging by itself.

Instead, always treat updates as invalidations if dirty logging is
not enabled, assuming that the board will enable logging on the
RAM region that includes the framebuffer.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/display/framebuffer.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/display/framebuffer.c b/hw/display/framebuffer.c
index 4546e42..2cabced 100644
--- a/hw/display/framebuffer.c
+++ b/hw/display/framebuffer.c
@@ -63,6 +63,10 @@ void framebuffer_update_display(
     assert(mem_section.offset_within_address_space == base);
 
     memory_region_sync_dirty_bitmap(mem);
+    if (!memory_region_is_logging(mem, DIRTY_MEMORY_VGA)) {
+        invalidate = true;
+    }
+
     src_base = cpu_physical_memory_map(base, &src_len, 0);
     /* If we can't map the framebuffer then bail.  We could try harder,
        but it's not really worth it as dirty flag tracking will probably
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 08/29] ui/console: remove dpy_gfx_update_dirty

[Paolo Bonzini]

dpy_gfx_update_dirty expects DIRTY_MEMORY_VGA logging to be always on,
but that will not be the case soon.  Because it computes the memory
region on the fly for every update (with memory_region_find), it cannot
enable/disable logging by itself.

We could always treat updates as invalidations if dirty logging is
not enabled, assuming that the board will enable logging on the
RAM region that includes the framebuffer.

However, the function is unused, so just drop it.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/ui/console.h |  4 ----
 ui/console.c         | 61 ----------------------------------------------------
 2 files changed, 65 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index e8b3a9e..be90091 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -242,10 +242,6 @@ void dpy_text_resize(QemuConsole *con, int w, int h);
 void dpy_mouse_set(QemuConsole *con, int x, int y, int on);
 void dpy_cursor_define(QemuConsole *con, QEMUCursor *cursor);
 bool dpy_cursor_define_supported(QemuConsole *con);
-void dpy_gfx_update_dirty(QemuConsole *con,
-                          MemoryRegion *address_space,
-                          uint64_t base,
-                          bool invalidate);
 bool dpy_gfx_check_format(QemuConsole *con,
                           pixman_format_code_t format);
 
diff --git a/ui/console.c b/ui/console.c
index 406c36b..75fc492 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1619,67 +1619,6 @@ bool dpy_cursor_define_supported(QemuConsole *con)
     return false;
 }
 
-/*
- * Call dpy_gfx_update for all dirity scanlines.  Works for
- * DisplaySurfaces backed by guest memory (i.e. the ones created
- * using qemu_create_displaysurface_guestmem).
- */
-void dpy_gfx_update_dirty(QemuConsole *con,
-                          MemoryRegion *address_space,
-                          hwaddr base,
-                          bool invalidate)
-{
-    DisplaySurface *ds = qemu_console_surface(con);
-    int width = surface_stride(ds);
-    int height = surface_height(ds);
-    hwaddr size = width * height;
-    MemoryRegionSection mem_section;
-    MemoryRegion *mem;
-    ram_addr_t addr;
-    int first, last, i;
-    bool dirty;
-
-    mem_section = memory_region_find(address_space, base, size);
-    mem = mem_section.mr;
-    if (int128_get64(mem_section.size) != size ||
-        !memory_region_is_ram(mem_section.mr)) {
-        goto out;
-    }
-    assert(mem);
-
-    memory_region_sync_dirty_bitmap(mem);
-    addr = mem_section.offset_within_region;
-
-    first = -1;
-    last = -1;
-    for (i = 0; i < height; i++, addr += width) {
-        dirty = invalidate ||
-            memory_region_get_dirty(mem, addr, width, DIRTY_MEMORY_VGA);
-        if (dirty) {
-            if (first == -1) {
-                first = i;
-            }
-            last = i;
-        }
-        if (first != -1 && !dirty) {
-            assert(last != -1 && last >= first);
-            dpy_gfx_update(con, 0, first, surface_width(ds),
-                           last - first + 1);
-            first = -1;
-        }
-    }
-    if (first != -1) {
-        assert(last != -1 && last >= first);
-        dpy_gfx_update(con, 0, first, surface_width(ds),
-                       last - first + 1);
-    }
-
-    memory_region_reset_dirty(mem, mem_section.offset_within_region, size,
-                              DIRTY_MEMORY_VGA);
-out:
-    memory_region_unref(mem);
-}
-
 /***********************************************************/
 /* register display */
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 09/29] memory: track DIRTY_MEMORY_CODE in mr->dirty_log_mask

[Paolo Bonzini]

DIRTY_MEMORY_CODE is only needed for TCG.  By adding it directly to
mr->dirty_log_mask, we avoid testing for TCG everywhere a region is
checked for the enabled/disabled state of dirty logging.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/exec/memory.h | 4 ++--
 memory.c              | 4 ++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 55dc11d..8ae004e 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -604,8 +604,8 @@ bool memory_region_is_logging(MemoryRegion *mr, uint8_t client);
  * memory_region_get_dirty_log_mask: return the clients for which a
  * memory region is logging writes.
  *
- * Returns a bitmap of clients, which right now will be either 0 or
- * (1 << DIRTY_MEMORY_VGA).
+ * Returns a bitmap of clients, in which the DIRTY_MEMORY_* constants
+ * are the bit indices.
  *
  * @mr: the memory region being queried
  */
diff --git a/memory.c b/memory.c
index 3e3d5b8..47cc181 100644
--- a/memory.c
+++ b/memory.c
@@ -1212,6 +1212,7 @@ void memory_region_init_ram(MemoryRegion *mr,
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
     mr->ram_addr = qemu_ram_alloc(size, mr, errp);
+    mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 
 void memory_region_init_resizeable_ram(MemoryRegion *mr,
@@ -1229,6 +1230,7 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
     mr->ram_addr = qemu_ram_alloc_resizeable(size, max_size, resized, mr, errp);
+    mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 
 #ifdef __linux__
@@ -1245,6 +1247,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
     mr->ram_addr = qemu_ram_alloc_from_file(size, mr, share, path, errp);
+    mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 #endif
 
@@ -1258,6 +1261,7 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram_from_ptr;
+    mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 
     /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
     assert(ptr != NULL);
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 10/29] kvm: accept non-mapped memory in kvm_dirty_pages_log_change

[Paolo Bonzini]

It is okay if memory is not mapped into the guest but has dirty logging
enabled.  When this happens, KVM will not do anything and only accesses
from the host will be logged.

This can be triggered by iofuzz.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 kvm-all.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index b544e95..f4d66d1 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -335,12 +335,10 @@ static int kvm_dirty_pages_log_change(hwaddr phys_addr,
     KVMSlot *mem = kvm_lookup_matching_slot(s, phys_addr, phys_addr + size);
 
     if (mem == NULL)  {
-        fprintf(stderr, "BUG: %s: invalid parameters " TARGET_FMT_plx "-"
-                TARGET_FMT_plx "\n", __func__, phys_addr,
-                (hwaddr)(phys_addr + size - 1));
-        return -EINVAL;
+        return 0;
+    } else {
+        return kvm_slot_dirty_pages_log_change(mem, log_dirty);
     }
-    return kvm_slot_dirty_pages_log_change(mem, log_dirty);
 }
 
 static void kvm_log_start(MemoryListener *listener,
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 11/29] memory: include DIRTY_MEMORY_MIGRATION in the dirty log mask

[Paolo Bonzini]

The separate handling of DIRTY_MEMORY_MIGRATION, which does not
call log_start/log_stop callbacks when it changes in a region's
dirty logging mask, has caused several bugs.

One recent example is commit 4cc856f (kvm-all: Sync dirty-bitmap from
kvm before kvm destroy the corresponding dirty_bitmap, 2015-04-02).
Another performance problem is that KVM keeps tracking dirty pages
after a failed live migration, which causes bad performance due to
disallowing huge page mapping.

This patch removes the root cause of the problem by reporting
DIRTY_MEMORY_MIGRATION changes via log_start and log_stop.
Note that we now have to rebuild the FlatView when global dirty
logging is enabled or disabled; this ensures that log_start and
log_stop callbacks are invoked.

This will also be used to make the setting of bitmaps conditional.
In general, this patch lets users of the memory API ignore the
global state of dirty logging if they handle dirty logging
generically per region.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/memory.c b/memory.c
index 47cc181..b7ca987 100644
--- a/memory.c
+++ b/memory.c
@@ -588,7 +588,7 @@ static void render_memory_region(FlatView *view,
     remain = clip.size;
 
     fr.mr = mr;
-    fr.dirty_log_mask = mr->dirty_log_mask;
+    fr.dirty_log_mask = memory_region_get_dirty_log_mask(mr);
     fr.romd_mode = mr->romd_mode;
     fr.readonly = readonly;
 
@@ -1400,7 +1400,11 @@ bool memory_region_is_skip_dump(MemoryRegion *mr)
 
 uint8_t memory_region_get_dirty_log_mask(MemoryRegion *mr)
 {
-    return mr->dirty_log_mask;
+    uint8_t mask = mr->dirty_log_mask;
+    if (global_dirty_log) {
+        mask |= (1 << DIRTY_MEMORY_MIGRATION);
+    }
+    return mask;
 }
 
 bool memory_region_is_logging(MemoryRegion *mr, uint8_t client)
@@ -1962,12 +1966,24 @@ void address_space_sync_dirty_bitmap(AddressSpace *as)
 void memory_global_dirty_log_start(void)
 {
     global_dirty_log = true;
+
     MEMORY_LISTENER_CALL_GLOBAL(log_global_start, Forward);
+
+    /* Refresh DIRTY_LOG_MIGRATION bit.  */
+    memory_region_transaction_begin();
+    memory_region_update_pending = true;
+    memory_region_transaction_commit();
 }
 
 void memory_global_dirty_log_stop(void)
 {
     global_dirty_log = false;
+
+    /* Refresh DIRTY_LOG_MIGRATION bit.  */
+    memory_region_transaction_begin();
+    memory_region_update_pending = true;
+    memory_region_transaction_commit();
+
     MEMORY_LISTENER_CALL_GLOBAL(log_global_stop, Reverse);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 12/29] kvm: remove special handling of DIRTY_MEMORY_MIGRATION in the dirty log mask

[Paolo Bonzini]

One recent example is commit 4cc856f (kvm-all: Sync dirty-bitmap from
kvm before kvm destroy the corresponding dirty_bitmap, 2015-04-02).
Another performance problem is that KVM keeps tracking dirty pages
after a failed live migration, which causes bad performance due to
disallowing huge page mapping.

Thanks to the previous patch, KVM can now stop hooking into
log_global_start/stop.  This simplifies the KVM code noticeably.

Reported-by: Wanpeng Li <wanpeng.li@linux.intel.com>
Reported-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 kvm-all.c | 56 ++------------------------------------------------------
 1 file changed, 2 insertions(+), 54 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index f4d66d1..e84c319 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -83,7 +83,6 @@ struct KVMState
     struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
     bool coalesced_flush_in_progress;
     int broken_set_mem_region;
-    int migration_log;
     int vcpu_events;
     int robust_singlestep;
     int debugregs;
@@ -234,9 +233,6 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
     mem.guest_phys_addr = slot->start_addr;
     mem.userspace_addr = (unsigned long)slot->ram;
     mem.flags = slot->flags;
-    if (s->migration_log) {
-        mem.flags |= KVM_MEM_LOG_DIRTY_PAGES;
-    }
 
     if (slot->memory_size && mem.flags & KVM_MEM_READONLY) {
         /* Set the slot size to 0 before setting the slot to the desired
@@ -317,10 +313,6 @@ static int kvm_slot_dirty_pages_log_change(KVMSlot *mem, bool log_dirty)
     mem->flags = flags;
 
     /* If nothing changed effectively, no need to issue ioctl */
-    if (s->migration_log) {
-        flags |= KVM_MEM_LOG_DIRTY_PAGES;
-    }
-
     if (flags == old_flags) {
         return 0;
     }
@@ -375,31 +367,6 @@ static void kvm_log_stop(MemoryListener *listener,
     }
 }
 
-static int kvm_set_migration_log(bool enable)
-{
-    KVMState *s = kvm_state;
-    KVMSlot *mem;
-    int i, err;
-
-    s->migration_log = enable;
-
-    for (i = 0; i < s->nr_slots; i++) {
-        mem = &s->slots[i];
-
-        if (!mem->memory_size) {
-            continue;
-        }
-        if (!!(mem->flags & KVM_MEM_LOG_DIRTY_PAGES) == enable) {
-            continue;
-        }
-        err = kvm_set_user_memory_region(s, mem);
-        if (err) {
-            return err;
-        }
-    }
-    return 0;
-}
-
 /* get kvm's dirty pages bitmap and update qemu's */
 static int kvm_get_dirty_pages_log_range(MemoryRegionSection *section,
                                          unsigned long *bitmap)
@@ -671,8 +638,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
     KVMSlot *mem, old;
     int err;
     MemoryRegion *mr = section->mr;
-    bool log_dirty =
-        memory_region_get_dirty_log_mask(mr) & ~(1 << DIRTY_MEMORY_MIGRATION);
+    bool log_dirty = memory_region_get_dirty_log_mask(mr) != 0;
     bool writeable = !mr->readonly && !mr->rom_device;
     bool readonly_flag = mr->readonly || memory_region_is_romd(mr);
     hwaddr start_addr = section->offset_within_address_space;
@@ -724,7 +690,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
 
         old = *mem;
 
-        if ((mem->flags & KVM_MEM_LOG_DIRTY_PAGES) || s->migration_log) {
+        if (mem->flags & KVM_MEM_LOG_DIRTY_PAGES) {
             kvm_physical_sync_dirty_bitmap(section);
         }
 
@@ -853,22 +819,6 @@ static void kvm_log_sync(MemoryListener *listener,
     }
 }
 
-static void kvm_log_global_start(struct MemoryListener *listener)
-{
-    int r;
-
-    r = kvm_set_migration_log(1);
-    assert(r >= 0);
-}
-
-static void kvm_log_global_stop(struct MemoryListener *listener)
-{
-    int r;
-
-    r = kvm_set_migration_log(0);
-    assert(r >= 0);
-}
-
 static void kvm_mem_ioeventfd_add(MemoryListener *listener,
                                   MemoryRegionSection *section,
                                   bool match_data, uint64_t data,
@@ -944,8 +894,6 @@ static MemoryListener kvm_memory_listener = {
     .log_start = kvm_log_start,
     .log_stop = kvm_log_stop,
     .log_sync = kvm_log_sync,
-    .log_global_start = kvm_log_global_start,
-    .log_global_stop = kvm_log_global_stop,
     .eventfd_add = kvm_mem_ioeventfd_add,
     .eventfd_del = kvm_mem_ioeventfd_del,
     .coalesced_mmio_add = kvm_coalesce_mmio_region,
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 13/29] ram_addr: tweaks to xen_modified_memory

[Paolo Bonzini]

Invoke xen_modified_memory from cpu_physical_memory_set_dirty_range_nocode;
it is akin to DIRTY_MEMORY_MIGRATION, so set it together with that bitmap.
The remaining call from invalidate_and_set_dirty's "else" branch will go
away soon.

Second, fix the second argument to the function in the
cpu_physical_memory_set_dirty_lebitmap call site.  That function is only used
by KVM, but it is better to be clean anyway.

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c                  | 3 ++-
 include/exec/ram_addr.h | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index e19ab22..c90dc72 100644
--- a/exec.c
+++ b/exec.c
@@ -2285,8 +2285,9 @@ static void invalidate_and_set_dirty(hwaddr addr,
     if (cpu_physical_memory_range_includes_clean(addr, length)) {
         tb_invalidate_phys_range(addr, addr + length, 0);
         cpu_physical_memory_set_dirty_range_nocode(addr, length);
+    } else {
+        xen_modified_memory(addr, length);
     }
-    xen_modified_memory(addr, length);
 }
 
 static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index ff558a4..7f6e928 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -112,6 +112,7 @@ static inline void cpu_physical_memory_set_dirty_range_nocode(ram_addr_t start,
     page = start >> TARGET_PAGE_BITS;
     bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION], page, end - page);
     bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_VGA], page, end - page);
+    xen_modified_memory(start, length);
 }
 
 static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start,
@@ -155,7 +156,7 @@ static inline void cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
                 ram_list.dirty_memory[DIRTY_MEMORY_CODE][page + k] |= temp;
             }
         }
-        xen_modified_memory(start, pages);
+        xen_modified_memory(start, pages << TARGET_PAGE_BITS);
     } else {
         /*
          * bitmap-traveling is faster than memory-traveling (for addr...)
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 14/29] exec: use memory_region_get_dirty_log_mask to optimize dirty tracking

[Paolo Bonzini]

The memory API can now return the exact set of bitmaps that have to
be tracked.  Use it instead of the in_migration variable.

In the next patches, we will also use it to set only DIRTY_MEMORY_VGA
or DIRTY_MEMORY_MIGRATION if necessary.  This can make a difference
for dataplane, especially after the dirty bitmap is changed to use
more expensive atomic operations.

Of some interest is the change to stl_phys_notdirty.  When migration
was introduced, stl_phys_notdirty was changed to effectively behave
as stl_phys during migration.  In fact, if one looks at the function as it
was in the beginning (commit 8df1cd0, physical memory access functions,
2005-01-28), at the time the dirty bitmap was the equivalent of
DIRTY_MEMORY_CODE nowadays; hence, the function simply should not touch
the dirty code bits.  This patch changes it to do the intended thing.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c | 59 +++++++++++++++++++----------------------------------------
 1 file changed, 19 insertions(+), 40 deletions(-)

diff --git a/exec.c b/exec.c
index c90dc72..9486347 100644
--- a/exec.c
+++ b/exec.c
@@ -59,8 +59,6 @@
 //#define DEBUG_SUBPAGE
 
 #if !defined(CONFIG_USER_ONLY)
-static bool in_migration;
-
 /* ram_list is read under rcu_read_lock()/rcu_read_unlock().  Writes
  * are protected by the ramlist lock.
  */
@@ -870,11 +868,6 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length,
     }
 }
 
-static void cpu_physical_memory_set_dirty_tracking(bool enable)
-{
-    in_migration = enable;
-}
-
 /* Called from RCU critical section */
 hwaddr memory_region_section_get_iotlb(CPUState *cpu,
                                        MemoryRegionSection *section,
@@ -2165,22 +2158,6 @@ static void tcg_commit(MemoryListener *listener)
     }
 }
 
-static void core_log_global_start(MemoryListener *listener)
-{
-    cpu_physical_memory_set_dirty_tracking(true);
-}
-
-static void core_log_global_stop(MemoryListener *listener)
-{
-    cpu_physical_memory_set_dirty_tracking(false);
-}
-
-static MemoryListener core_memory_listener = {
-    .log_global_start = core_log_global_start,
-    .log_global_stop = core_log_global_stop,
-    .priority = 1,
-};
-
 void address_space_init_dispatch(AddressSpace *as)
 {
     as->dispatch = NULL;
@@ -2220,8 +2197,6 @@ static void memory_map_init(void)
     memory_region_init_io(system_io, NULL, &unassigned_io_ops, NULL, "io",
                           65536);
     address_space_init(&address_space_io, system_io, "I/O");
-
-    memory_listener_register(&core_memory_listener, &address_space_memory);
 }
 
 MemoryRegion *get_system_memory(void)
@@ -2279,12 +2254,18 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
 
 #else
 
-static void invalidate_and_set_dirty(hwaddr addr,
+static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr,
                                      hwaddr length)
 {
     if (cpu_physical_memory_range_includes_clean(addr, length)) {
-        tb_invalidate_phys_range(addr, addr + length, 0);
-        cpu_physical_memory_set_dirty_range_nocode(addr, length);
+        uint8_t dirty_log_mask = memory_region_get_dirty_log_mask(mr);
+        if (dirty_log_mask & (1 << DIRTY_MEMORY_CODE)) {
+            tb_invalidate_phys_range(addr, addr + length, 0);
+            dirty_log_mask &= ~(1 << DIRTY_MEMORY_CODE);
+        }
+        if (dirty_log_mask) {
+            cpu_physical_memory_set_dirty_range_nocode(addr, length);
+        }
     } else {
         xen_modified_memory(addr, length);
     }
@@ -2372,7 +2353,7 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
                 /* RAM case */
                 ptr = qemu_get_ram_ptr(addr1);
                 memcpy(ptr, buf, l);
-                invalidate_and_set_dirty(addr1, l);
+                invalidate_and_set_dirty(mr, addr1, l);
             }
         } else {
             if (!memory_access_is_direct(mr, is_write)) {
@@ -2469,7 +2450,7 @@ static inline void cpu_physical_memory_write_rom_internal(AddressSpace *as,
             switch (type) {
             case WRITE_DATA:
                 memcpy(ptr, buf, l);
-                invalidate_and_set_dirty(addr1, l);
+                invalidate_and_set_dirty(mr, addr1, l);
                 break;
             case FLUSH_CACHE:
                 flush_icache_range((uintptr_t)ptr, (uintptr_t)ptr + l);
@@ -2694,7 +2675,7 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
         mr = qemu_ram_addr_from_host(buffer, &addr1);
         assert(mr != NULL);
         if (is_write) {
-            invalidate_and_set_dirty(addr1, access_len);
+            invalidate_and_set_dirty(mr, addr1, access_len);
         }
         if (xen_enabled()) {
             xen_invalidate_map_cache_entry(buffer);
@@ -3023,6 +3004,7 @@ void address_space_stl_notdirty(AddressSpace *as, hwaddr addr, uint32_t val,
     hwaddr l = 4;
     hwaddr addr1;
     MemTxResult r;
+    uint8_t dirty_log_mask;
 
     rcu_read_lock();
     mr = address_space_translate(as, addr, &addr1, &l,
@@ -3034,13 +3016,10 @@ void address_space_stl_notdirty(AddressSpace *as, hwaddr addr, uint32_t val,
         ptr = qemu_get_ram_ptr(addr1);
         stl_p(ptr, val);
 
-        if (unlikely(in_migration)) {
-            if (cpu_physical_memory_is_clean(addr1)) {
-                /* invalidate code */
-                tb_invalidate_phys_page_range(addr1, addr1 + 4, 0);
-                /* set dirty bit */
-                cpu_physical_memory_set_dirty_range_nocode(addr1, 4);
-            }
+        dirty_log_mask = memory_region_get_dirty_log_mask(mr);
+        dirty_log_mask &= ~(1 << DIRTY_MEMORY_CODE);
+        if (dirty_log_mask) {
+            cpu_physical_memory_set_dirty_range_nocode(addr1, 4);
         }
         r = MEMTX_OK;
     }
@@ -3097,7 +3076,7 @@ static inline void address_space_stl_internal(AddressSpace *as,
             stl_p(ptr, val);
             break;
         }
-        invalidate_and_set_dirty(addr1, 4);
+        invalidate_and_set_dirty(mr, addr1, 4);
         r = MEMTX_OK;
     }
     if (result) {
@@ -3201,7 +3180,7 @@ static inline void address_space_stw_internal(AddressSpace *as,
             stw_p(ptr, val);
             break;
         }
-        invalidate_and_set_dirty(addr1, 2);
+        invalidate_and_set_dirty(mr, addr1, 2);
         r = MEMTX_OK;
     }
     if (result) {
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 15/29] exec: move functions to translate-all.h

[Paolo Bonzini]

Remove them from the sundry exec-all.h header, since they are only used by
the TCG runtime in exec.c and user-exec.c.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/exec/exec-all.h | 6 +-----
 linux-user/mmap.c       | 1 +
 translate-all.h         | 8 ++++++++
 user-exec.c             | 1 +
 4 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index b58cd47..2f7a4f1 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -90,11 +90,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
                               int cflags);
 void cpu_exec_init(CPUArchState *env);
 void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
-int page_unprotect(target_ulong address, uintptr_t pc, void *puc);
-void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
-                                   int is_cpu_write_access);
-void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end,
-                              int is_cpu_write_access);
+
 #if !defined(CONFIG_USER_ONLY)
 bool qemu_in_vcpu_thread(void);
 void cpu_reload_memory_map(CPUState *cpu);
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index a249f0c..959ff4d 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -30,6 +30,7 @@
 
 #include "qemu.h"
 #include "qemu-common.h"
+#include "translate-all.h"
 
 //#define DEBUG_MMAP
 
diff --git a/translate-all.h b/translate-all.h
index 02832b2..9d115a6 100644
--- a/translate-all.h
+++ b/translate-all.h
@@ -21,7 +21,15 @@
 
 /* translate-all.c */
 void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len);
+void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
+                                   int is_cpu_write_access);
+void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end,
+                              int is_cpu_write_access);
 void cpu_unlink_tb(CPUState *cpu);
 void tb_check_watchpoint(CPUState *cpu);
 
+#ifdef CONFIG_USER_ONLY
+int page_unprotect(target_ulong address, uintptr_t pc, void *puc);
+#endif
+
 #endif /* TRANSLATE_ALL_H */
diff --git a/user-exec.c b/user-exec.c
index 8f57e8a..ed9a07f 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -22,6 +22,7 @@
 #include "tcg.h"
 #include "qemu/bitops.h"
 #include "exec/cpu_ldst.h"
+#include "translate-all.h"
 
 #undef EAX
 #undef ECX
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 16/29] translate-all: remove unnecessary argument to tb_invalidate_phys_range

[Paolo Bonzini]

The is_cpu_write_access argument is always 0, remove it.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c            | 2 +-
 linux-user/mmap.c | 6 +++---
 translate-all.c   | 5 ++---
 translate-all.h   | 3 +--
 4 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/exec.c b/exec.c
index 9486347..0a0b89e 100644
--- a/exec.c
+++ b/exec.c
@@ -2260,7 +2260,7 @@ static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr,
     if (cpu_physical_memory_range_includes_clean(addr, length)) {
         uint8_t dirty_log_mask = memory_region_get_dirty_log_mask(mr);
         if (dirty_log_mask & (1 << DIRTY_MEMORY_CODE)) {
-            tb_invalidate_phys_range(addr, addr + length, 0);
+            tb_invalidate_phys_range(addr, addr + length);
             dirty_log_mask &= ~(1 << DIRTY_MEMORY_CODE);
         }
         if (dirty_log_mask) {
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 959ff4d..78e1b2d 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -575,7 +575,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
     page_dump(stdout);
     printf("\n");
 #endif
-    tb_invalidate_phys_range(start, start + len, 0);
+    tb_invalidate_phys_range(start, start + len);
     mmap_unlock();
     return start;
 fail:
@@ -680,7 +680,7 @@ int target_munmap(abi_ulong start, abi_ulong len)
 
     if (ret == 0) {
         page_set_flags(start, start + len, 0);
-        tb_invalidate_phys_range(start, start + len, 0);
+        tb_invalidate_phys_range(start, start + len);
     }
     mmap_unlock();
     return ret;
@@ -759,7 +759,7 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong old_size,
         page_set_flags(old_addr, old_addr + old_size, 0);
         page_set_flags(new_addr, new_addr + new_size, prot | PAGE_VALID);
     }
-    tb_invalidate_phys_range(new_addr, new_addr + new_size, 0);
+    tb_invalidate_phys_range(new_addr, new_addr + new_size);
     mmap_unlock();
     return new_addr;
 }
diff --git a/translate-all.c b/translate-all.c
index 536008f..d118c6c 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1042,11 +1042,10 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
  * access: the virtual CPU will exit the current TB if code is modified inside
  * this TB.
  */
-void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end,
-                              int is_cpu_write_access)
+void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end)
 {
     while (start < end) {
-        tb_invalidate_phys_page_range(start, end, is_cpu_write_access);
+        tb_invalidate_phys_page_range(start, end, 0);
         start &= TARGET_PAGE_MASK;
         start += TARGET_PAGE_SIZE;
     }
diff --git a/translate-all.h b/translate-all.h
index 9d115a6..ef9166c 100644
--- a/translate-all.h
+++ b/translate-all.h
@@ -23,8 +23,7 @@
 void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len);
 void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
                                    int is_cpu_write_access);
-void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end,
-                              int is_cpu_write_access);
+void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end);
 void cpu_unlink_tb(CPUState *cpu);
 void tb_check_watchpoint(CPUState *cpu);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 17/29] cputlb: remove useless arguments to tlb_unprotect_code_phys, rename

[Paolo Bonzini]

These days modification of the TLB is done in notdirty_mem_write,
so the virtual address and env pointer as unnecessary.

The new name of the function, tlb_unprotect_code, is consistent with
tlb_protect_code.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cputlb.c              | 3 +--
 include/exec/cputlb.h | 3 +--
 translate-all.c       | 2 +-
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 7606548..dd1203b 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -131,8 +131,7 @@ void tlb_protect_code(ram_addr_t ram_addr)
 
 /* update the TLB so that writes in physical page 'phys_addr' are no longer
    tested for self modifying code */
-void tlb_unprotect_code_phys(CPUState *cpu, ram_addr_t ram_addr,
-                             target_ulong vaddr)
+void tlb_unprotect_code(ram_addr_t ram_addr)
 {
     cpu_physical_memory_set_dirty_flag(ram_addr, DIRTY_MEMORY_CODE);
 }
diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
index e0da9d7..360815e 100644
--- a/include/exec/cputlb.h
+++ b/include/exec/cputlb.h
@@ -22,8 +22,7 @@
 #if !defined(CONFIG_USER_ONLY)
 /* cputlb.c */
 void tlb_protect_code(ram_addr_t ram_addr);
-void tlb_unprotect_code_phys(CPUState *cpu, ram_addr_t ram_addr,
-                             target_ulong vaddr);
+void tlb_unprotect_code(ram_addr_t ram_addr);
 void tlb_reset_dirty_range(CPUTLBEntry *tlb_entry, uintptr_t start,
                            uintptr_t length);
 void cpu_tlb_reset_dirty_all(ram_addr_t start1, ram_addr_t length);
diff --git a/translate-all.c b/translate-all.c
index d118c6c..0e2ad8a 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1158,7 +1158,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
     if (!p->first_tb) {
         invalidate_page_bitmap(p);
         if (is_cpu_write_access) {
-            tlb_unprotect_code_phys(cpu, start, cpu->mem_io_vaddr);
+            tlb_unprotect_code(start);
         }
     }
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 18/29] translate-all: make less of tb_invalidate_phys_page_range depend on is_cpu_write_access

[Paolo Bonzini]

is_cpu_write_access is only set if tb_invalidate_phys_page_range is called
from tb_invalidate_phys_page_fast, and hence from notdirty_mem_write.
However:

- the code bitmap can be built directly in tb_invalidate_phys_page_fast
  (unconditionally, since is_cpu_write_access would always be passed as 1);

- the virtual address is not needed to mark the page as "not containing
  code" (dirty code bitmap = 1), so we can also remove that use of
  is_cpu_write_access.  For calls of tb_invalidate_phys_page_range
  that do not come from notdirty_mem_write, the next call to
  notdirty_mem_write will notice that the page does not contain code
  anymore, and will fix up the TLB entry.

The parameter needs to remain in order to guard accesses to cpu->mem_io_pc.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 translate-all.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index 0e2ad8a..e2e7422 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1082,12 +1082,6 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
     if (!p) {
         return;
     }
-    if (!p->code_bitmap &&
-        ++p->code_write_count >= SMC_BITMAP_USE_THRESHOLD &&
-        is_cpu_write_access) {
-        /* build code bitmap */
-        build_page_bitmap(p);
-    }
 #if defined(TARGET_HAS_PRECISE_SMC)
     if (cpu != NULL) {
         env = cpu->env_ptr;
@@ -1157,9 +1151,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
     /* if no code remaining, no need to continue to use slow writes */
     if (!p->first_tb) {
         invalidate_page_bitmap(p);
-        if (is_cpu_write_access) {
-            tlb_unprotect_code(start);
-        }
+        tlb_unprotect_code(start);
     }
 #endif
 #ifdef TARGET_HAS_PRECISE_SMC
@@ -1192,6 +1184,11 @@ void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)
     if (!p) {
         return;
     }
+    if (!p->code_bitmap &&
+        ++p->code_write_count >= SMC_BITMAP_USE_THRESHOLD) {
+        /* build code bitmap */
+        build_page_bitmap(p);
+    }
     if (p->code_bitmap) {
         unsigned int nr;
         unsigned long b;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 19/29] exec: pass client mask to cpu_physical_memory_set_dirty_range

[Paolo Bonzini]

This cuts in half the cost of bitmap operations (which will become more
expensive when made atomic) during migration on non-VRAM regions.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c                  | 20 +++++++++++---------
 include/exec/ram_addr.h | 33 ++++++++++++++++-----------------
 memory.c                |  3 ++-
 3 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/exec.c b/exec.c
index 0a0b89e..9d342fe 100644
--- a/exec.c
+++ b/exec.c
@@ -1355,7 +1355,8 @@ int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, Error **errp)
 
     cpu_physical_memory_clear_dirty_range(block->offset, block->used_length);
     block->used_length = newsize;
-    cpu_physical_memory_set_dirty_range(block->offset, block->used_length);
+    cpu_physical_memory_set_dirty_range(block->offset, block->used_length,
+                                        DIRTY_CLIENTS_ALL);
     memory_region_set_size(block->mr, newsize);
     if (block->resized) {
         block->resized(block->idstr, newsize, block->host);
@@ -1429,7 +1430,8 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
        }
     }
     cpu_physical_memory_set_dirty_range(new_block->offset,
-                                        new_block->used_length);
+                                        new_block->used_length,
+                                        DIRTY_CLIENTS_ALL);
 
     if (new_block->host) {
         qemu_ram_setup_dump(new_block->host, new_block->max_length);
@@ -1817,7 +1819,11 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
     default:
         abort();
     }
-    cpu_physical_memory_set_dirty_range_nocode(ram_addr, size);
+    /* Set both VGA and migration bits for simplicity and to remove
+     * the notdirty callback faster.
+     */
+    cpu_physical_memory_set_dirty_range(ram_addr, size,
+                                        DIRTY_CLIENTS_NOCODE);
     /* we remove the notdirty callback only if the code has been
        flushed */
     if (!cpu_physical_memory_is_clean(ram_addr)) {
@@ -2263,9 +2269,7 @@ static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr,
             tb_invalidate_phys_range(addr, addr + length);
             dirty_log_mask &= ~(1 << DIRTY_MEMORY_CODE);
         }
-        if (dirty_log_mask) {
-            cpu_physical_memory_set_dirty_range_nocode(addr, length);
-        }
+        cpu_physical_memory_set_dirty_range(addr, length, dirty_log_mask);
     } else {
         xen_modified_memory(addr, length);
     }
@@ -3018,9 +3022,7 @@ void address_space_stl_notdirty(AddressSpace *as, hwaddr addr, uint32_t val,
 
         dirty_log_mask = memory_region_get_dirty_log_mask(mr);
         dirty_log_mask &= ~(1 << DIRTY_MEMORY_CODE);
-        if (dirty_log_mask) {
-            cpu_physical_memory_set_dirty_range_nocode(addr1, 4);
-        }
+        cpu_physical_memory_set_dirty_range(addr1, 4, dirty_log_mask);
         r = MEMTX_OK;
     }
     if (result) {
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 7f6e928..b0ea03f 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -41,6 +41,9 @@ void qemu_ram_free_from_ptr(ram_addr_t addr);
 
 int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, Error **errp);
 
+#define DIRTY_CLIENTS_ALL	((1 << DIRTY_MEMORY_NUM) - 1)
+#define DIRTY_CLIENTS_NOCODE	(DIRTY_CLIENTS_ALL & ~ (1 << DIRTY_MEMORY_CODE))
+
 static inline bool cpu_physical_memory_get_dirty(ram_addr_t start,
                                                  ram_addr_t length,
                                                  unsigned client)
@@ -103,28 +106,23 @@ static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr,
     set_bit(addr >> TARGET_PAGE_BITS, ram_list.dirty_memory[client]);
 }
 
-static inline void cpu_physical_memory_set_dirty_range_nocode(ram_addr_t start,
-                                                              ram_addr_t length)
-{
-    unsigned long end, page;
-
-    end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS;
-    page = start >> TARGET_PAGE_BITS;
-    bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION], page, end - page);
-    bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_VGA], page, end - page);
-    xen_modified_memory(start, length);
-}
-
 static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start,
-                                                       ram_addr_t length)
+                                                       ram_addr_t length,
+                                                       uint8_t mask)
 {
     unsigned long end, page;
 
     end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS;
     page = start >> TARGET_PAGE_BITS;
-    bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION], page, end - page);
-    bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_VGA], page, end - page);
-    bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_CODE], page, end - page);
+    if (likely(mask & (1 << DIRTY_MEMORY_MIGRATION))) {
+        bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION], page, end - page);
+    }
+    if (unlikely(mask & (1 << DIRTY_MEMORY_VGA))) {
+        bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_VGA], page, end - page);
+    }
+    if (unlikely(mask & (1 << DIRTY_MEMORY_CODE))) {
+        bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_CODE], page, end - page);
+    }
     xen_modified_memory(start, length);
 }
 
@@ -172,7 +170,8 @@ static inline void cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
                     addr = page_number * TARGET_PAGE_SIZE;
                     ram_addr = start + addr;
                     cpu_physical_memory_set_dirty_range(ram_addr,
-                                       TARGET_PAGE_SIZE * hpratio);
+                                       TARGET_PAGE_SIZE * hpratio,
+                                       DIRTY_CLIENTS_ALL);
                 } while (c != 0);
             }
         }
diff --git a/memory.c b/memory.c
index b7ca987..418cac7 100644
--- a/memory.c
+++ b/memory.c
@@ -1461,7 +1461,8 @@ void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr,
                              hwaddr size)
 {
     assert(mr->terminates);
-    cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size);
+    cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size,
+                                        memory_region_get_dirty_log_mask(mr));
 }
 
 bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr,
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 20/29] exec: invert return value of cpu_physical_memory_get_clean, rename

[Paolo Bonzini]

While it is obvious that cpu_physical_memory_get_dirty returns true even if
a single page is dirty, the same is not true for cpu_physical_memory_get_clean;
one would expect that it returns true only if all the pages are clean, but
it actually looks for even one clean page.  (By contrast, the caller of that
function, cpu_physical_memory_range_includes_clean, has a good name).

To clarify, rename the function to cpu_physical_memory_all_dirty and return
true if _all_ the pages are dirty.  This is the opposite of the previous
meaning, because "all are 1" is the same as "not (any is 0)", so we have to
modify cpu_physical_memory_range_includes_clean as well.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/exec/ram_addr.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index b0ea03f..4e3b081 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -59,7 +59,7 @@ static inline bool cpu_physical_memory_get_dirty(ram_addr_t start,
     return next < end;
 }
 
-static inline bool cpu_physical_memory_get_clean(ram_addr_t start,
+static inline bool cpu_physical_memory_all_dirty(ram_addr_t start,
                                                  ram_addr_t length,
                                                  unsigned client)
 {
@@ -71,7 +71,7 @@ static inline bool cpu_physical_memory_get_clean(ram_addr_t start,
     page = start >> TARGET_PAGE_BITS;
     next = find_next_zero_bit(ram_list.dirty_memory[client], end, page);
 
-    return next < end;
+    return next >= end;
 }
 
 static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr,
@@ -92,10 +92,10 @@ static inline bool cpu_physical_memory_is_clean(ram_addr_t addr)
 static inline bool cpu_physical_memory_range_includes_clean(ram_addr_t start,
                                                             ram_addr_t length)
 {
-    bool vga = cpu_physical_memory_get_clean(start, length, DIRTY_MEMORY_VGA);
-    bool code = cpu_physical_memory_get_clean(start, length, DIRTY_MEMORY_CODE);
+    bool vga = !cpu_physical_memory_all_dirty(start, length, DIRTY_MEMORY_VGA);
+    bool code = !cpu_physical_memory_all_dirty(start, length, DIRTY_MEMORY_CODE);
     bool migration =
-        cpu_physical_memory_get_clean(start, length, DIRTY_MEMORY_MIGRATION);
+        !cpu_physical_memory_all_dirty(start, length, DIRTY_MEMORY_MIGRATION);
     return vga || code || migration;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 21/29] exec: only check relevant bitmaps for cleanliness

[Paolo Bonzini]

Most of the time, not all bitmaps have to be marked as dirty;
do not do anything if the interesting ones are already dirty.
Previously, any clean bitmap would have cause all the bitmaps to be
marked dirty.

In fact, unless running TCG most of the time bitmap operations need
not be done at all, because memory_region_is_logging returns zero.
In this case, skip the call to cpu_physical_memory_range_includes_clean
altogether as well.

With this patch, cpu_physical_memory_set_dirty_range is called
unconditionally, so there need not be anymore a separate call to
xen_modified_memory.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c                  | 22 +++++++++++++---------
 include/exec/ram_addr.h | 25 ++++++++++++++++++-------
 2 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/exec.c b/exec.c
index 9d342fe..4cd18ff 100644
--- a/exec.c
+++ b/exec.c
@@ -2263,16 +2263,20 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
 static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr,
                                      hwaddr length)
 {
-    if (cpu_physical_memory_range_includes_clean(addr, length)) {
-        uint8_t dirty_log_mask = memory_region_get_dirty_log_mask(mr);
-        if (dirty_log_mask & (1 << DIRTY_MEMORY_CODE)) {
-            tb_invalidate_phys_range(addr, addr + length);
-            dirty_log_mask &= ~(1 << DIRTY_MEMORY_CODE);
-        }
-        cpu_physical_memory_set_dirty_range(addr, length, dirty_log_mask);
-    } else {
-        xen_modified_memory(addr, length);
+    uint8_t dirty_log_mask = memory_region_get_dirty_log_mask(mr);
+    /* No early return if dirty_log_mask is or becomes 0, because
+     * cpu_physical_memory_set_dirty_range will still call
+     * xen_modified_memory.
+     */
+    if (dirty_log_mask) {
+        dirty_log_mask =
+            cpu_physical_memory_range_includes_clean(addr, length, dirty_log_mask);
+    }
+    if (dirty_log_mask & (1 << DIRTY_MEMORY_CODE)) {
+        tb_invalidate_phys_range(addr, addr + length);
+        dirty_log_mask &= ~(1 << DIRTY_MEMORY_CODE);
     }
+    cpu_physical_memory_set_dirty_range(addr, length, dirty_log_mask);
 }
 
 static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 4e3b081..9778398 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -89,14 +89,25 @@ static inline bool cpu_physical_memory_is_clean(ram_addr_t addr)
     return !(vga && code && migration);
 }
 
-static inline bool cpu_physical_memory_range_includes_clean(ram_addr_t start,
-                                                            ram_addr_t length)
+static inline uint8_t cpu_physical_memory_range_includes_clean(ram_addr_t start,
+                                                               ram_addr_t length,
+                                                               uint8_t mask)
 {
-    bool vga = !cpu_physical_memory_all_dirty(start, length, DIRTY_MEMORY_VGA);
-    bool code = !cpu_physical_memory_all_dirty(start, length, DIRTY_MEMORY_CODE);
-    bool migration =
-        !cpu_physical_memory_all_dirty(start, length, DIRTY_MEMORY_MIGRATION);
-    return vga || code || migration;
+    uint8_t ret = 0;
+
+    if (mask & (1 << DIRTY_MEMORY_VGA) &&
+        !cpu_physical_memory_all_dirty(start, length, DIRTY_MEMORY_VGA)) {
+        ret |= (1 << DIRTY_MEMORY_VGA);
+    }
+    if (mask & (1 << DIRTY_MEMORY_CODE) &&
+        !cpu_physical_memory_all_dirty(start, length, DIRTY_MEMORY_CODE)) {
+        ret |= (1 << DIRTY_MEMORY_CODE);
+    }
+    if (mask & (1 << DIRTY_MEMORY_MIGRATION) &&
+        !cpu_physical_memory_all_dirty(start, length, DIRTY_MEMORY_MIGRATION)) {
+        ret |= (1 << DIRTY_MEMORY_MIGRATION);
+    }
+    return ret;
 }
 
 static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr,
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 22/29] memory: do not touch code dirty bitmap unless TCG is enabled

[Paolo Bonzini]

cpu_physical_memory_set_dirty_lebitmap unconditionally syncs the
DIRTY_MEMORY_CODE bitmap.  This however is unused unless TCG is
enabled.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/exec/ram_addr.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 9778398..c2037f7 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -162,11 +162,14 @@ static inline void cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
 
                 ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION][page + k] |= temp;
                 ram_list.dirty_memory[DIRTY_MEMORY_VGA][page + k] |= temp;
-                ram_list.dirty_memory[DIRTY_MEMORY_CODE][page + k] |= temp;
+                if (tcg_enabled()) {
+                    ram_list.dirty_memory[DIRTY_MEMORY_CODE][page + k] |= temp;
+                }
             }
         }
         xen_modified_memory(start, pages << TARGET_PAGE_BITS);
     } else {
+        uint8_t clients = tcg_enabled() ? DIRTY_CLIENTS_ALL : DIRTY_CLIENTS_NOCODE;
         /*
          * bitmap-traveling is faster than memory-traveling (for addr...)
          * especially when most of the memory is not dirty.
@@ -181,8 +184,7 @@ static inline void cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
                     addr = page_number * TARGET_PAGE_SIZE;
                     ram_addr = start + addr;
                     cpu_physical_memory_set_dirty_range(ram_addr,
-                                       TARGET_PAGE_SIZE * hpratio,
-                                       DIRTY_CLIENTS_ALL);
+                                       TARGET_PAGE_SIZE * hpratio, clients);
                 } while (c != 0);
             }
         }
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 23/29] bitmap: add atomic set functions

[Paolo Bonzini]

From: Stefan Hajnoczi <stefanha@redhat.com>

Use atomic_or() for atomic bitmaps where several threads may set bits at
the same time.  This avoids the race condition between threads loading
an element, bitwise ORing, and then storing the element.

When setting all bits in a word we can avoid atomic ops and instead just
use an smp_mb() at the end.

Most bitmap users don't need atomicity so introduce new functions.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <1417519399-3166-2-git-send-email-stefanha@redhat.com>
[Avoid barrier in the single word case, use full barrier instead of write.
 - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/bitmap.h |  2 ++
 include/qemu/bitops.h | 14 ++++++++++++++
 util/bitmap.c         | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+)

diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index f0273c9..3e0a4f3 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -39,6 +39,7 @@
  * bitmap_empty(src, nbits)			Are all bits zero in *src?
  * bitmap_full(src, nbits)			Are all bits set in *src?
  * bitmap_set(dst, pos, nbits)			Set specified bit area
+ * bitmap_set_atomic(dst, pos, nbits)   Set specified bit area with atomic ops
  * bitmap_clear(dst, pos, nbits)		Clear specified bit area
  * bitmap_find_next_zero_area(buf, len, pos, n, mask)	Find bit free area
  */
@@ -226,6 +227,7 @@ static inline int bitmap_intersects(const unsigned long *src1,
 }
 
 void bitmap_set(unsigned long *map, long i, long len);
+void bitmap_set_atomic(unsigned long *map, long i, long len);
 void bitmap_clear(unsigned long *map, long start, long nr);
 unsigned long bitmap_find_next_zero_area(unsigned long *map,
                                          unsigned long size,
diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index 8abdcf9..8164225 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -16,6 +16,7 @@
 #include <assert.h>
 
 #include "host-utils.h"
+#include "atomic.h"
 
 #define BITS_PER_BYTE           CHAR_BIT
 #define BITS_PER_LONG           (sizeof (unsigned long) * BITS_PER_BYTE)
@@ -39,6 +40,19 @@ static inline void set_bit(long nr, unsigned long *addr)
 }
 
 /**
+ * set_bit_atomic - Set a bit in memory atomically
+ * @nr: the bit to set
+ * @addr: the address to start counting from
+ */
+static inline void set_bit_atomic(long nr, unsigned long *addr)
+{
+    unsigned long mask = BIT_MASK(nr);
+    unsigned long *p = addr + BIT_WORD(nr);
+
+    atomic_or(p, mask);
+}
+
+/**
  * clear_bit - Clears a bit in memory
  * @nr: Bit to clear
  * @addr: Address to start counting from
diff --git a/util/bitmap.c b/util/bitmap.c
index 9c6bb52..39994af 100644
--- a/util/bitmap.c
+++ b/util/bitmap.c
@@ -11,6 +11,7 @@
 
 #include "qemu/bitops.h"
 #include "qemu/bitmap.h"
+#include "qemu/atomic.h"
 
 /*
  * bitmaps provide an array of bits, implemented using an an
@@ -177,6 +178,43 @@ void bitmap_set(unsigned long *map, long start, long nr)
     }
 }
 
+void bitmap_set_atomic(unsigned long *map, long start, long nr)
+{
+    unsigned long *p = map + BIT_WORD(start);
+    const long size = start + nr;
+    int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
+    unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
+
+    /* First word */
+    if (nr - bits_to_set > 0) {
+        atomic_or(p, mask_to_set);
+        nr -= bits_to_set;
+        bits_to_set = BITS_PER_LONG;
+        mask_to_set = ~0UL;
+        p++;
+    }
+
+    /* Full words */
+    if (bits_to_set == BITS_PER_LONG) {
+        while (nr >= BITS_PER_LONG) {
+            *p = ~0UL;
+            nr -= BITS_PER_LONG;
+            p++;
+        }
+    }
+
+    /* Last word */
+    if (nr) {
+        mask_to_set &= BITMAP_LAST_WORD_MASK(size);
+        atomic_or(p, mask_to_set);
+    } else {
+        /* If we avoided the full barrier in atomic_or(), issue a
+         * barrier to account for the assignments in the while loop.
+         */
+        smp_mb();
+    }
+}
+
 void bitmap_clear(unsigned long *map, long start, long nr)
 {
     unsigned long *p = map + BIT_WORD(start);
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 24/29] bitmap: add atomic test and clear

[Paolo Bonzini]

From: Stefan Hajnoczi <stefanha@redhat.com>

The new bitmap_test_and_clear_atomic() function clears a range and
returns whether or not the bits were set.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <1417519399-3166-3-git-send-email-stefanha@redhat.com>
[Test before xchg; then a full barrier is needed at the end just like
 in the previous patch.  The barrier can be avoided if we did at least
 one xchg.  - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/bitmap.h |  2 ++
 util/bitmap.c         | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index 3e0a4f3..86dd9cd 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -41,6 +41,7 @@
  * bitmap_set(dst, pos, nbits)			Set specified bit area
  * bitmap_set_atomic(dst, pos, nbits)   Set specified bit area with atomic ops
  * bitmap_clear(dst, pos, nbits)		Clear specified bit area
+ * bitmap_test_and_clear_atomic(dst, pos, nbits)    Test and clear area
  * bitmap_find_next_zero_area(buf, len, pos, n, mask)	Find bit free area
  */
 
@@ -229,6 +230,7 @@ static inline int bitmap_intersects(const unsigned long *src1,
 void bitmap_set(unsigned long *map, long i, long len);
 void bitmap_set_atomic(unsigned long *map, long i, long len);
 void bitmap_clear(unsigned long *map, long start, long nr);
+bool bitmap_test_and_clear_atomic(unsigned long *map, long start, long nr);
 unsigned long bitmap_find_next_zero_area(unsigned long *map,
                                          unsigned long size,
                                          unsigned long start,
diff --git a/util/bitmap.c b/util/bitmap.c
index 39994af..300a68e 100644
--- a/util/bitmap.c
+++ b/util/bitmap.c
@@ -235,6 +235,51 @@ void bitmap_clear(unsigned long *map, long start, long nr)
     }
 }
 
+bool bitmap_test_and_clear_atomic(unsigned long *map, long start, long nr)
+{
+    unsigned long *p = map + BIT_WORD(start);
+    const long size = start + nr;
+    int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
+    unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
+    unsigned long dirty = 0;
+    unsigned long old_bits;
+
+    /* First word */
+    if (nr - bits_to_clear > 0) {
+        old_bits = atomic_fetch_and(p, ~mask_to_clear);
+        dirty |= old_bits & mask_to_clear;
+        nr -= bits_to_clear;
+        bits_to_clear = BITS_PER_LONG;
+        mask_to_clear = ~0UL;
+        p++;
+    }
+
+    /* Full words */
+    if (bits_to_clear == BITS_PER_LONG) {
+        while (nr >= BITS_PER_LONG) {
+            if (*p) {
+                old_bits = atomic_xchg(p, 0);
+                dirty |= old_bits;
+            }
+            nr -= BITS_PER_LONG;
+            p++;
+        }
+    }
+
+    /* Last word */
+    if (nr) {
+        mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
+        old_bits = atomic_fetch_and(p, ~mask_to_clear);
+        dirty |= old_bits & mask_to_clear;
+    } else {
+        if (!dirty) {
+            smp_mb();
+        }
+    }
+
+    return dirty != 0;
+}
+
 #define ALIGN_MASK(x,mask)      (((x)+(mask))&~(mask))
 
 /**
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 25/29] memory: use atomic ops for setting dirty memory bits

[Paolo Bonzini]

From: Stefan Hajnoczi <stefanha@redhat.com>

Use set_bit_atomic() and bitmap_set_atomic() so that multiple threads
can dirty memory without race conditions.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <1417519399-3166-4-git-send-email-stefanha@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/exec/ram_addr.h | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index c2037f7..891c7e6 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -114,7 +114,7 @@ static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr,
                                                       unsigned client)
 {
     assert(client < DIRTY_MEMORY_NUM);
-    set_bit(addr >> TARGET_PAGE_BITS, ram_list.dirty_memory[client]);
+    set_bit_atomic(addr >> TARGET_PAGE_BITS, ram_list.dirty_memory[client]);
 }
 
 static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start,
@@ -122,17 +122,18 @@ static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start,
                                                        uint8_t mask)
 {
     unsigned long end, page;
+    unsigned long **d = ram_list.dirty_memory;
 
     end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS;
     page = start >> TARGET_PAGE_BITS;
     if (likely(mask & (1 << DIRTY_MEMORY_MIGRATION))) {
-        bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION], page, end - page);
+        bitmap_set_atomic(d[DIRTY_MEMORY_MIGRATION], page, end - page);
     }
     if (unlikely(mask & (1 << DIRTY_MEMORY_VGA))) {
-        bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_VGA], page, end - page);
+        bitmap_set_atomic(d[DIRTY_MEMORY_VGA], page, end - page);
     }
     if (unlikely(mask & (1 << DIRTY_MEMORY_CODE))) {
-        bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_CODE], page, end - page);
+        bitmap_set_atomic(d[DIRTY_MEMORY_CODE], page, end - page);
     }
     xen_modified_memory(start, length);
 }
@@ -159,11 +160,12 @@ static inline void cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
         for (k = 0; k < nr; k++) {
             if (bitmap[k]) {
                 unsigned long temp = leul_to_cpu(bitmap[k]);
+                unsigned long **d = ram_list.dirty_memory;
 
-                ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION][page + k] |= temp;
-                ram_list.dirty_memory[DIRTY_MEMORY_VGA][page + k] |= temp;
+                atomic_or(&d[DIRTY_MEMORY_MIGRATION][page + k], temp);
+                atomic_or(&d[DIRTY_MEMORY_VGA][page + k], temp);
                 if (tcg_enabled()) {
-                    ram_list.dirty_memory[DIRTY_MEMORY_CODE][page + k] |= temp;
+                    atomic_or(&d[DIRTY_MEMORY_CODE][page + k], temp);
                 }
             }
         }
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 26/29] migration: move dirty bitmap sync to ram_addr.h

[Paolo Bonzini]

From: Stefan Hajnoczi <stefanha@redhat.com>

The dirty memory bitmap is managed by ram_addr.h and copied to
migration_bitmap[] periodically during live migration.

Move the code to sync the bitmap to ram_addr.h where related code lives.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <1417519399-3166-5-git-send-email-stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch_init.c             | 46 ++--------------------------------------------
 include/exec/ram_addr.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 44 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 23d3feb..db8c7b6 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -610,52 +610,10 @@ 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)
-{
-    bool ret;
-    int nr = addr >> TARGET_PAGE_BITS;
-
-    ret = test_and_set_bit(nr, migration_bitmap);
-
-    if (!ret) {
-        migration_dirty_pages++;
-    }
-    return ret;
-}
-
 static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
 {
-    ram_addr_t addr;
-    unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS);
-
-    /* start address is aligned at the start of a word? */
-    if (((page * BITS_PER_LONG) << TARGET_PAGE_BITS) == start) {
-        int k;
-        int nr = BITS_TO_LONGS(length >> TARGET_PAGE_BITS);
-        unsigned long *src = ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION];
-
-        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 &= src[k];
-                migration_dirty_pages += ctpopl(new_dirty);
-                src[k] = 0;
-            }
-        }
-    } else {
-        for (addr = 0; addr < length; addr += TARGET_PAGE_SIZE) {
-            if (cpu_physical_memory_get_dirty(start + addr,
-                                              TARGET_PAGE_SIZE,
-                                              DIRTY_MEMORY_MIGRATION)) {
-                cpu_physical_memory_reset_dirty(start + addr,
-                                                TARGET_PAGE_SIZE,
-                                                DIRTY_MEMORY_MIGRATION);
-                migration_bitmap_set_dirty(start + addr);
-            }
-        }
-    }
+    migration_dirty_pages +=
+        cpu_physical_memory_sync_dirty_bitmap(migration_bitmap, start, length);
 }
 
 
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 891c7e6..2dab337 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -218,5 +218,49 @@ 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);
 
+static inline
+uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned long *dest,
+                                               ram_addr_t start,
+                                               ram_addr_t length)
+{
+    ram_addr_t addr;
+    unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS);
+    uint64_t num_dirty = 0;
+
+    /* start address is aligned at the start of a word? */
+    if (((page * BITS_PER_LONG) << TARGET_PAGE_BITS) == start) {
+        int k;
+        int nr = BITS_TO_LONGS(length >> TARGET_PAGE_BITS);
+        unsigned long *src = ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION];
+
+        for (k = page; k < page + nr; k++) {
+            if (src[k]) {
+                unsigned long new_dirty;
+                new_dirty = ~dest[k];
+                dest[k] |= src[k];
+                new_dirty &= src[k];
+                num_dirty += ctpopl(new_dirty);
+                src[k] = 0;
+            }
+        }
+    } else {
+        for (addr = 0; addr < length; addr += TARGET_PAGE_SIZE) {
+            if (cpu_physical_memory_get_dirty(start + addr,
+                                              TARGET_PAGE_SIZE,
+                                              DIRTY_MEMORY_MIGRATION)) {
+                long k = (start + addr) >> TARGET_PAGE_BITS;
+                if (!test_and_set_bit(k, dest)) {
+                    num_dirty++;
+                }
+                cpu_physical_memory_reset_dirty(start + addr,
+                                                TARGET_PAGE_SIZE,
+                                                DIRTY_MEMORY_MIGRATION);
+            }
+        }
+    }
+
+    return num_dirty;
+}
+
 #endif
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 27/29] memory: replace cpu_physical_memory_reset_dirty() with test-and-clear

[Paolo Bonzini]

From: Stefan Hajnoczi <stefanha@redhat.com>

The cpu_physical_memory_reset_dirty() function is sometimes used
together with cpu_physical_memory_get_dirty().  This is not atomic since
two separate accesses to the dirty memory bitmap are made.

Turn cpu_physical_memory_reset_dirty() and
cpu_physical_memory_clear_dirty_range_type() into the atomic
cpu_physical_memory_test_and_clear_dirty().

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <1417519399-3166-6-git-send-email-stefanha@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cputlb.c                |  4 ++--
 exec.c                  | 23 +++++++++++++++++------
 include/exec/ram_addr.h | 33 ++++++++++-----------------------
 memory.c                | 11 ++++-------
 4 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index dd1203b..a506086 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -125,8 +125,8 @@ void tlb_flush_page(CPUState *cpu, target_ulong addr)
    can be detected */
 void tlb_protect_code(ram_addr_t ram_addr)
 {
-    cpu_physical_memory_reset_dirty(ram_addr, TARGET_PAGE_SIZE,
-                                    DIRTY_MEMORY_CODE);
+    cpu_physical_memory_test_and_clear_dirty(ram_addr, TARGET_PAGE_SIZE,
+                                             DIRTY_MEMORY_CODE);
 }
 
 /* update the TLB so that writes in physical page 'phys_addr' are no longer
diff --git a/exec.c b/exec.c
index 4cd18ff..7552363 100644
--- a/exec.c
+++ b/exec.c
@@ -856,16 +856,27 @@ static void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length)
 }
 
 /* Note: start and end must be within the same ram block.  */
-void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length,
-                                     unsigned client)
+bool cpu_physical_memory_test_and_clear_dirty(ram_addr_t start,
+                                              ram_addr_t length,
+                                              unsigned client)
 {
-    if (length == 0)
-        return;
-    cpu_physical_memory_clear_dirty_range_type(start, length, client);
+    unsigned long end, page;
+    bool dirty;
+ 
+    if (length == 0) {
+        return false;
+    }
 
-    if (tcg_enabled()) {
+    end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS;
+    page = start >> TARGET_PAGE_BITS;
+    dirty = bitmap_test_and_clear_atomic(ram_list.dirty_memory[client],
+                                         page, end - page);
+
+    if (dirty && tcg_enabled()) {
         tlb_reset_dirty_range_all(start, length);
     }
+
+    return dirty;
 }
 
 /* Called from RCU critical section */
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 2dab337..e93207d 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -194,30 +194,19 @@ static inline void cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
 }
 #endif /* not _WIN32 */
 
-static inline void cpu_physical_memory_clear_dirty_range_type(ram_addr_t start,
-                                                              ram_addr_t length,
-                                                              unsigned client)
-{
-    unsigned long end, page;
-
-    assert(client < DIRTY_MEMORY_NUM);
-    end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS;
-    page = start >> TARGET_PAGE_BITS;
-    bitmap_clear(ram_list.dirty_memory[client], page, end - page);
-}
+bool cpu_physical_memory_test_and_clear_dirty(ram_addr_t start,
+                                              ram_addr_t length,
+                                              unsigned client);
 
 static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start,
                                                          ram_addr_t length)
 {
-    cpu_physical_memory_clear_dirty_range_type(start, length, DIRTY_MEMORY_MIGRATION);
-    cpu_physical_memory_clear_dirty_range_type(start, length, DIRTY_MEMORY_VGA);
-    cpu_physical_memory_clear_dirty_range_type(start, length, DIRTY_MEMORY_CODE);
+    cpu_physical_memory_test_and_clear_dirty(start, length, DIRTY_MEMORY_MIGRATION);
+    cpu_physical_memory_test_and_clear_dirty(start, length, DIRTY_MEMORY_VGA);
+    cpu_physical_memory_test_and_clear_dirty(start, length, DIRTY_MEMORY_CODE);
 }
 
 
-void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length,
-                                     unsigned client);
-
 static inline
 uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned long *dest,
                                                ram_addr_t start,
@@ -245,16 +234,14 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned long *dest,
         }
     } else {
         for (addr = 0; addr < length; addr += TARGET_PAGE_SIZE) {
-            if (cpu_physical_memory_get_dirty(start + addr,
-                                              TARGET_PAGE_SIZE,
-                                              DIRTY_MEMORY_MIGRATION)) {
+            if (cpu_physical_memory_test_and_clear_dirty(
+                        start + addr,
+                        TARGET_PAGE_SIZE,
+                        DIRTY_MEMORY_MIGRATION)) {
                 long k = (start + addr) >> TARGET_PAGE_BITS;
                 if (!test_and_set_bit(k, dest)) {
                     num_dirty++;
                 }
-                cpu_physical_memory_reset_dirty(start + addr,
-                                                TARGET_PAGE_SIZE,
-                                                DIRTY_MEMORY_MIGRATION);
             }
         }
     }
diff --git a/memory.c b/memory.c
index 418cac7..0c5d328 100644
--- a/memory.c
+++ b/memory.c
@@ -1468,13 +1468,9 @@ void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr,
 bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr,
                                         hwaddr size, unsigned client)
 {
-    bool ret;
     assert(mr->terminates);
-    ret = cpu_physical_memory_get_dirty(mr->ram_addr + addr, size, client);
-    if (ret) {
-        cpu_physical_memory_reset_dirty(mr->ram_addr + addr, size, client);
-    }
-    return ret;
+    return cpu_physical_memory_test_and_clear_dirty(mr->ram_addr + addr,
+                                                    size, client);
 }
 
 
@@ -1518,7 +1514,8 @@ void memory_region_reset_dirty(MemoryRegion *mr, hwaddr addr,
                                hwaddr size, unsigned client)
 {
     assert(mr->terminates);
-    cpu_physical_memory_reset_dirty(mr->ram_addr + addr, size, client);
+    cpu_physical_memory_test_and_clear_dirty(mr->ram_addr + addr, size,
+                                             client);
 }
 
 int memory_region_get_fd(MemoryRegion *mr)
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 28/29] memory: make cpu_physical_memory_sync_dirty_bitmap() fully atomic

[Paolo Bonzini]

From: Stefan Hajnoczi <stefanha@redhat.com>

The fast path of cpu_physical_memory_sync_dirty_bitmap() directly
manipulates the dirty bitmap.  Use atomic_xchg() to make the
test-and-clear atomic.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <1417519399-3166-7-git-send-email-stefanha@redhat.com>
[Only do xchg on nonzero words. - Paolo]
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/exec/ram_addr.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index e93207d..ffd7710 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -224,12 +224,12 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned long *dest,
 
         for (k = page; k < page + nr; k++) {
             if (src[k]) {
+                unsigned long bits = atomic_xchg(&src[k], 0);
                 unsigned long new_dirty;
                 new_dirty = ~dest[k];
-                dest[k] |= src[k];
-                new_dirty &= src[k];
+                dest[k] |= bits;
+                new_dirty &= bits;
                 num_dirty += ctpopl(new_dirty);
-                src[k] = 0;
             }
         }
     } else {
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 29/29] memory: use mr->ram_addr in "is this RAM?" assertions

[Paolo Bonzini]

mr->terminates alone doesn't guarantee that we are looking at a RAM region.
mr->ram_addr also has to be checked, in order to distinguish RAM and I/O
regions.

So, do the following:

1) add a new define RAM_ADDR_INVALID, and test it in the assertions
instead of mr->terminates

2) IOMMU regions were not setting mr->ram_addr to a bogus value, initialize
it in the instance_init function so that the new assertions would fire
for IOMMU regions as well.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/memory.c b/memory.c
index 0c5d328..3ac0bd2 100644
--- a/memory.c
+++ b/memory.c
@@ -28,6 +28,8 @@
 
 //#define DEBUG_UNASSIGNED
 
+#define RAM_ADDR_INVALID (~(ram_addr_t)0)
+
 static unsigned memory_region_transaction_depth;
 static bool memory_region_update_pending;
 static bool ioeventfd_update_pending;
@@ -1007,6 +1009,7 @@ static void memory_region_initfn(Object *obj)
     ObjectProperty *op;
 
     mr->ops = &unassigned_mem_ops;
+    mr->ram_addr = RAM_ADDR_INVALID;
     mr->enabled = true;
     mr->romd_mode = true;
     mr->destructor = memory_region_destructor_none;
@@ -1198,7 +1201,6 @@ void memory_region_init_io(MemoryRegion *mr,
     mr->ops = ops;
     mr->opaque = opaque;
     mr->terminates = true;
-    mr->ram_addr = ~(ram_addr_t)0;
 }
 
 void memory_region_init_ram(MemoryRegion *mr,
@@ -1453,14 +1455,14 @@ void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
 bool memory_region_get_dirty(MemoryRegion *mr, hwaddr addr,
                              hwaddr size, unsigned client)
 {
-    assert(mr->terminates);
+    assert(mr->ram_addr != RAM_ADDR_INVALID);
     return cpu_physical_memory_get_dirty(mr->ram_addr + addr, size, client);
 }
 
 void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr,
                              hwaddr size)
 {
-    assert(mr->terminates);
+    assert(mr->ram_addr != RAM_ADDR_INVALID);
     cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size,
                                         memory_region_get_dirty_log_mask(mr));
 }
@@ -1468,7 +1470,7 @@ void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr,
 bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr,
                                         hwaddr size, unsigned client)
 {
-    assert(mr->terminates);
+    assert(mr->ram_addr != RAM_ADDR_INVALID);
     return cpu_physical_memory_test_and_clear_dirty(mr->ram_addr + addr,
                                                     size, client);
 }
@@ -1513,7 +1515,7 @@ void memory_region_rom_device_set_romd(MemoryRegion *mr, bool romd_mode)
 void memory_region_reset_dirty(MemoryRegion *mr, hwaddr addr,
                                hwaddr size, unsigned client)
 {
-    assert(mr->terminates);
+    assert(mr->ram_addr != RAM_ADDR_INVALID);
     cpu_physical_memory_test_and_clear_dirty(mr->ram_addr + addr, size,
                                              client);
 }
@@ -1524,7 +1526,7 @@ int memory_region_get_fd(MemoryRegion *mr)
         return memory_region_get_fd(mr->alias);
     }
 
-    assert(mr->terminates);
+    assert(mr->ram_addr != RAM_ADDR_INVALID);
 
     return qemu_get_ram_fd(mr->ram_addr & TARGET_PAGE_MASK);
 }
@@ -1535,14 +1537,14 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr)
         return memory_region_get_ram_ptr(mr->alias) + mr->alias_offset;
     }
 
-    assert(mr->terminates);
+    assert(mr->ram_addr != RAM_ADDR_INVALID);
 
     return qemu_get_ram_ptr(mr->ram_addr & TARGET_PAGE_MASK);
 }
 
 void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp)
 {
-    assert(mr->terminates);
+    assert(mr->ram_addr != RAM_ADDR_INVALID);
 
     qemu_ram_resize(mr->ram_addr, newsize, errp);
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 27/29] memory: replace cpu_physical_memory_reset_dirty() with test-and-clear

[Aurelien Jarno]

On 2015-05-26 18:55, Paolo Bonzini wrote:
> From: Stefan Hajnoczi <stefanha@redhat.com>
> 
> The cpu_physical_memory_reset_dirty() function is sometimes used
> together with cpu_physical_memory_get_dirty().  This is not atomic since
> two separate accesses to the dirty memory bitmap are made.
> 
> Turn cpu_physical_memory_reset_dirty() and
> cpu_physical_memory_clear_dirty_range_type() into the atomic
> cpu_physical_memory_test_and_clear_dirty().
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Message-Id: <1417519399-3166-6-git-send-email-stefanha@redhat.com>
> Reviewed-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  cputlb.c                |  4 ++--
>  exec.c                  | 23 +++++++++++++++++------
>  include/exec/ram_addr.h | 33 ++++++++++-----------------------
>  memory.c                | 11 ++++-------
>  4 files changed, 33 insertions(+), 38 deletions(-)

...

> diff --git a/exec.c b/exec.c
> index 4cd18ff..7552363 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -856,16 +856,27 @@ static void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length)
>  }
>  
>  /* Note: start and end must be within the same ram block.  */
> -void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length,
> -                                     unsigned client)
> +bool cpu_physical_memory_test_and_clear_dirty(ram_addr_t start,
> +                                              ram_addr_t length,
> +                                              unsigned client)
>  {
> -    if (length == 0)
> -        return;
> -    cpu_physical_memory_clear_dirty_range_type(start, length, client);
> +    unsigned long end, page;
> +    bool dirty;
> + 

   ^

git am complains about the above added space.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH v3 00/29] Dirty bitmap atomic access and optimizations

[Aurelien Jarno]

On 2015-05-26 18:54, Paolo Bonzini wrote:
> QEMU is currently accessing the dirty bitmaps very liberally,
> which is understandable since the accesses are cheap.  This is
> however not good for squeezing maximum performance out of dataplane,
> and is also not good if the accesses become more expensive---as is
> the case when they use atomic primitives.
> 
> This patch series does the following optimizations and cleanups:
> 
> 1) it lets KVM code treat migration as "just another dirty bitmap
> client" instead of needing the special global_log_start/stop callbacks.
> These remain in use in Xen and vhost.  This removes code and avoids
> bugs such as the one fixed in commit 4cc856f (kvm-all: Sync dirty-bitmap
> from kvm before kvm destroy the corresponding dirty_bitmap, 2015-04-02).
> 
> 2) it avoids modifications to unused dirty bitmaps: code if TCG
> is disabled, migration if no migration is in progress, VGA for
> regions other than VRAM.
> 
> and on top of this makes dirty bitmap access atomic.  I'm not including
> the patch to make the migration thread synchronize the bitmap outside
> the big QEMU lock (thus removing the last source of jitter during the
> RAM copy phase of migration) but it is also enabled by these patches.
> 
> Patches 1-4 are cleanups to DIRTY_MEMORY_VGA users.
> 
> Patches 5-12 are the first cleanup (KVM treats migration as just
> another client).  Patches 13-14 are a simple optimization that is enabled
> by these patches.
> 
> Patches 15-18 are bonus cleanups to translate-all.c's dirty memory
> tracking for TCG.
> 
> Patches 19-22 are the second cleanup (avoid modifications to unused
> dirty bitmaps).
> 
> Patches 23-28 are Stefan's patches for atomic access to the dirty
> bitmap, which has no performance impact in the common case thanks to
> the previous work.
> 
> Patch 29 is an unrelated strengthening of assertions, that mst spotted
> while reviewing v1.

I don't feel qualified to review this kind of patch set. That said, I
have tested the SH4 machine (and hence the sm501 display) and the MIPS
magnum machine up to the ARCsystem (and hence the g364fb display) and
they both seems to work correctly with this patchset applied.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH v3 03/29] display: enable DIRTY_MEMORY_VGA tracking explicitly

[Fam Zheng]

On Tue, 05/26 18:54, Paolo Bonzini wrote:
> This will be required soon by the memory core.
> 
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Fam Zheng <famz@redhat.com>

Re: [Qemu-devel] [PATCH v3 04/29] display: add memory_region_sync_dirty_bitmap calls

[Fam Zheng]

On Tue, 05/26 18:54, Paolo Bonzini wrote:
> These are strictly speaking only needed for KVM and Xen, but it's still
> nice to be consistent.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Fam Zheng <famz@redhat.com>

Re: [Qemu-devel] [PATCH v3 07/29] framebuffer: check memory_region_is_logging

[Fam Zheng]

On Tue, 05/26 18:54, Paolo Bonzini wrote:
> framebuffer.c expects DIRTY_MEMORY_VGA logging to be always on, but that
> will not be the case soon.  Because framebuffer.c computes the memory
> region on the fly for every update (with memory_region_find), it cannot
> enable/disable logging by itself.
> 
> Instead, always treat updates as invalidations if dirty logging is
> not enabled, assuming that the board will enable logging on the
> RAM region that includes the framebuffer.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Fam Zheng <famz@redhat.com>

> ---
>  hw/display/framebuffer.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/display/framebuffer.c b/hw/display/framebuffer.c
> index 4546e42..2cabced 100644
> --- a/hw/display/framebuffer.c
> +++ b/hw/display/framebuffer.c
> @@ -63,6 +63,10 @@ void framebuffer_update_display(
>      assert(mem_section.offset_within_address_space == base);
>  
>      memory_region_sync_dirty_bitmap(mem);
> +    if (!memory_region_is_logging(mem, DIRTY_MEMORY_VGA)) {
> +        invalidate = true;
> +    }
> +
>      src_base = cpu_physical_memory_map(base, &src_len, 0);
>      /* If we can't map the framebuffer then bail.  We could try harder,
>         but it's not really worth it as dirty flag tracking will probably
> -- 
> 1.8.3.1
> 
> 
> 

Re: [Qemu-devel] [PATCH v3 14/29] exec: use memory_region_get_dirty_log_mask to optimize dirty tracking

[Fam Zheng]

On Tue, 05/26 18:54, Paolo Bonzini wrote:
> The memory API can now return the exact set of bitmaps that have to
> be tracked.  Use it instead of the in_migration variable.
> 
> In the next patches, we will also use it to set only DIRTY_MEMORY_VGA
> or DIRTY_MEMORY_MIGRATION if necessary.  This can make a difference
> for dataplane, especially after the dirty bitmap is changed to use
> more expensive atomic operations.
> 
> Of some interest is the change to stl_phys_notdirty.  When migration
> was introduced, stl_phys_notdirty was changed to effectively behave
> as stl_phys during migration.  In fact, if one looks at the function as it
> was in the beginning (commit 8df1cd0, physical memory access functions,
> 2005-01-28), at the time the dirty bitmap was the equivalent of
> DIRTY_MEMORY_CODE nowadays; hence, the function simply should not touch
> the dirty code bits.  This patch changes it to do the intended thing.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Fam Zheng <famz@redhat.com>

Re: [Qemu-devel] [PATCH v3 23/29] bitmap: add atomic set functions

[Fam Zheng]

On Tue, 05/26 18:54, Paolo Bonzini wrote:
> From: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Use atomic_or() for atomic bitmaps where several threads may set bits at
> the same time.  This avoids the race condition between threads loading
> an element, bitwise ORing, and then storing the element.
> 
> When setting all bits in a word we can avoid atomic ops and instead just
> use an smp_mb() at the end.
> 
> Most bitmap users don't need atomicity so introduce new functions.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Message-Id: <1417519399-3166-2-git-send-email-stefanha@redhat.com>
> [Avoid barrier in the single word case, use full barrier instead of write.
>  - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/bitmap.h |  2 ++
>  include/qemu/bitops.h | 14 ++++++++++++++
>  util/bitmap.c         | 38 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 54 insertions(+)
> 
> diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
> index f0273c9..3e0a4f3 100644
> --- a/include/qemu/bitmap.h
> +++ b/include/qemu/bitmap.h
> @@ -39,6 +39,7 @@
>   * bitmap_empty(src, nbits)			Are all bits zero in *src?
>   * bitmap_full(src, nbits)			Are all bits set in *src?
>   * bitmap_set(dst, pos, nbits)			Set specified bit area
> + * bitmap_set_atomic(dst, pos, nbits)   Set specified bit area with atomic ops

The tab/space mix is not consistent. Fixing can be on top.

>   * bitmap_clear(dst, pos, nbits)		Clear specified bit area
>   * bitmap_find_next_zero_area(buf, len, pos, n, mask)	Find bit free area
>   */
> @@ -226,6 +227,7 @@ static inline int bitmap_intersects(const unsigned long *src1,
>  }
>  
>  void bitmap_set(unsigned long *map, long i, long len);
> +void bitmap_set_atomic(unsigned long *map, long i, long len);
>  void bitmap_clear(unsigned long *map, long start, long nr);
>  unsigned long bitmap_find_next_zero_area(unsigned long *map,
>                                           unsigned long size,
> diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
> index 8abdcf9..8164225 100644
> --- a/include/qemu/bitops.h
> +++ b/include/qemu/bitops.h
> @@ -16,6 +16,7 @@
>  #include <assert.h>
>  
>  #include "host-utils.h"
> +#include "atomic.h"
>  
>  #define BITS_PER_BYTE           CHAR_BIT
>  #define BITS_PER_LONG           (sizeof (unsigned long) * BITS_PER_BYTE)
> @@ -39,6 +40,19 @@ static inline void set_bit(long nr, unsigned long *addr)
>  }
>  
>  /**
> + * set_bit_atomic - Set a bit in memory atomically
> + * @nr: the bit to set
> + * @addr: the address to start counting from
> + */
> +static inline void set_bit_atomic(long nr, unsigned long *addr)
> +{
> +    unsigned long mask = BIT_MASK(nr);
> +    unsigned long *p = addr + BIT_WORD(nr);
> +
> +    atomic_or(p, mask);
> +}

Where is this function used?

Fam

Re: [Qemu-devel] [PATCH v3 24/29] bitmap: add atomic test and clear

[Fam Zheng]

On Tue, 05/26 18:55, Paolo Bonzini wrote:
> From: Stefan Hajnoczi <stefanha@redhat.com>
> 
> The new bitmap_test_and_clear_atomic() function clears a range and
> returns whether or not the bits were set.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Message-Id: <1417519399-3166-3-git-send-email-stefanha@redhat.com>
> [Test before xchg; then a full barrier is needed at the end just like
>  in the previous patch.  The barrier can be avoided if we did at least
>  one xchg.  - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Fam Zheng <famz@redhat.com>

> ---
>  include/qemu/bitmap.h |  2 ++
>  util/bitmap.c         | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
> index 3e0a4f3..86dd9cd 100644
> --- a/include/qemu/bitmap.h
> +++ b/include/qemu/bitmap.h
> @@ -41,6 +41,7 @@
>   * bitmap_set(dst, pos, nbits)			Set specified bit area
>   * bitmap_set_atomic(dst, pos, nbits)   Set specified bit area with atomic ops
>   * bitmap_clear(dst, pos, nbits)		Clear specified bit area
> + * bitmap_test_and_clear_atomic(dst, pos, nbits)    Test and clear area
>   * bitmap_find_next_zero_area(buf, len, pos, n, mask)	Find bit free area
>   */
>  
> @@ -229,6 +230,7 @@ static inline int bitmap_intersects(const unsigned long *src1,
>  void bitmap_set(unsigned long *map, long i, long len);
>  void bitmap_set_atomic(unsigned long *map, long i, long len);
>  void bitmap_clear(unsigned long *map, long start, long nr);
> +bool bitmap_test_and_clear_atomic(unsigned long *map, long start, long nr);
>  unsigned long bitmap_find_next_zero_area(unsigned long *map,
>                                           unsigned long size,
>                                           unsigned long start,
> diff --git a/util/bitmap.c b/util/bitmap.c
> index 39994af..300a68e 100644
> --- a/util/bitmap.c
> +++ b/util/bitmap.c
> @@ -235,6 +235,51 @@ void bitmap_clear(unsigned long *map, long start, long nr)
>      }
>  }
>  
> +bool bitmap_test_and_clear_atomic(unsigned long *map, long start, long nr)
> +{
> +    unsigned long *p = map + BIT_WORD(start);
> +    const long size = start + nr;
> +    int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
> +    unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
> +    unsigned long dirty = 0;
> +    unsigned long old_bits;
> +
> +    /* First word */
> +    if (nr - bits_to_clear > 0) {
> +        old_bits = atomic_fetch_and(p, ~mask_to_clear);
> +        dirty |= old_bits & mask_to_clear;
> +        nr -= bits_to_clear;
> +        bits_to_clear = BITS_PER_LONG;
> +        mask_to_clear = ~0UL;
> +        p++;
> +    }
> +
> +    /* Full words */
> +    if (bits_to_clear == BITS_PER_LONG) {
> +        while (nr >= BITS_PER_LONG) {
> +            if (*p) {
> +                old_bits = atomic_xchg(p, 0);
> +                dirty |= old_bits;
> +            }
> +            nr -= BITS_PER_LONG;
> +            p++;
> +        }
> +    }
> +
> +    /* Last word */
> +    if (nr) {
> +        mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
> +        old_bits = atomic_fetch_and(p, ~mask_to_clear);
> +        dirty |= old_bits & mask_to_clear;
> +    } else {
> +        if (!dirty) {
> +            smp_mb();
> +        }
> +    }
> +
> +    return dirty != 0;
> +}
> +
>  #define ALIGN_MASK(x,mask)      (((x)+(mask))&~(mask))
>  
>  /**
> -- 
> 1.8.3.1
> 
> 
> 

Re: [Qemu-devel] [PATCH v3 26/29] migration: move dirty bitmap sync to ram_addr.h

[Fam Zheng]

On Tue, 05/26 18:55, Paolo Bonzini wrote:
> From: Stefan Hajnoczi <stefanha@redhat.com>
> 
> The dirty memory bitmap is managed by ram_addr.h and copied to
> migration_bitmap[] periodically during live migration.
> 
> Move the code to sync the bitmap to ram_addr.h where related code lives.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Message-Id: <1417519399-3166-5-git-send-email-stefanha@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Fam Zheng <famz@redhat.com>

Re: [Qemu-devel] [PATCH v3 23/29] bitmap: add atomic set functions

[Paolo Bonzini]

On 27/05/2015 09:58, Fam Zheng wrote:
> On Tue, 05/26 18:54, Paolo Bonzini wrote:
>> From: Stefan Hajnoczi <stefanha@redhat.com>
>>
>> Use atomic_or() for atomic bitmaps where several threads may set bits at
>> the same time.  This avoids the race condition between threads loading
>> an element, bitwise ORing, and then storing the element.
>>
>> When setting all bits in a word we can avoid atomic ops and instead just
>> use an smp_mb() at the end.
>>
>> Most bitmap users don't need atomicity so introduce new functions.
>>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Message-Id: <1417519399-3166-2-git-send-email-stefanha@redhat.com>
>> [Avoid barrier in the single word case, use full barrier instead of write.
>>  - Paolo]
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  include/qemu/bitmap.h |  2 ++
>>  include/qemu/bitops.h | 14 ++++++++++++++
>>  util/bitmap.c         | 38 ++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 54 insertions(+)
>>
>> diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
>> index f0273c9..3e0a4f3 100644
>> --- a/include/qemu/bitmap.h
>> +++ b/include/qemu/bitmap.h
>> @@ -39,6 +39,7 @@
>>   * bitmap_empty(src, nbits)			Are all bits zero in *src?
>>   * bitmap_full(src, nbits)			Are all bits set in *src?
>>   * bitmap_set(dst, pos, nbits)			Set specified bit area
>> + * bitmap_set_atomic(dst, pos, nbits)   Set specified bit area with atomic ops
> 
> The tab/space mix is not consistent. Fixing can be on top.
> 
>>   * bitmap_clear(dst, pos, nbits)		Clear specified bit area
>>   * bitmap_find_next_zero_area(buf, len, pos, n, mask)	Find bit free area
>>   */
>> @@ -226,6 +227,7 @@ static inline int bitmap_intersects(const unsigned long *src1,
>>  }
>>  
>>  void bitmap_set(unsigned long *map, long i, long len);
>> +void bitmap_set_atomic(unsigned long *map, long i, long len);
>>  void bitmap_clear(unsigned long *map, long start, long nr);
>>  unsigned long bitmap_find_next_zero_area(unsigned long *map,
>>                                           unsigned long size,
>> diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
>> index 8abdcf9..8164225 100644
>> --- a/include/qemu/bitops.h
>> +++ b/include/qemu/bitops.h
>> @@ -16,6 +16,7 @@
>>  #include <assert.h>
>>  
>>  #include "host-utils.h"
>> +#include "atomic.h"
>>  
>>  #define BITS_PER_BYTE           CHAR_BIT
>>  #define BITS_PER_LONG           (sizeof (unsigned long) * BITS_PER_BYTE)
>> @@ -39,6 +40,19 @@ static inline void set_bit(long nr, unsigned long *addr)
>>  }
>>  
>>  /**
>> + * set_bit_atomic - Set a bit in memory atomically
>> + * @nr: the bit to set
>> + * @addr: the address to start counting from
>> + */
>> +static inline void set_bit_atomic(long nr, unsigned long *addr)
>> +{
>> +    unsigned long mask = BIT_MASK(nr);
>> +    unsigned long *p = addr + BIT_WORD(nr);
>> +
>> +    atomic_or(p, mask);
>> +}
> 
> Where is this function used?

In cpu_physical_memory_set_dirty_flag (patch 25).

Paolo

Re: [Qemu-devel] [PATCH v3 23/29] bitmap: add atomic set functions

[Fam Zheng]

On Tue, 05/26 18:54, Paolo Bonzini wrote:
> From: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Use atomic_or() for atomic bitmaps where several threads may set bits at
> the same time.  This avoids the race condition between threads loading
> an element, bitwise ORing, and then storing the element.
> 
> When setting all bits in a word we can avoid atomic ops and instead just
> use an smp_mb() at the end.
> 
> Most bitmap users don't need atomicity so introduce new functions.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Message-Id: <1417519399-3166-2-git-send-email-stefanha@redhat.com>
> [Avoid barrier in the single word case, use full barrier instead of write.
>  - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/bitmap.h |  2 ++
>  include/qemu/bitops.h | 14 ++++++++++++++
>  util/bitmap.c         | 38 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 54 insertions(+)
> 
> diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
> index f0273c9..3e0a4f3 100644
> --- a/include/qemu/bitmap.h
> +++ b/include/qemu/bitmap.h
> @@ -39,6 +39,7 @@
>   * bitmap_empty(src, nbits)			Are all bits zero in *src?
>   * bitmap_full(src, nbits)			Are all bits set in *src?
>   * bitmap_set(dst, pos, nbits)			Set specified bit area
> + * bitmap_set_atomic(dst, pos, nbits)   Set specified bit area with atomic ops
>   * bitmap_clear(dst, pos, nbits)		Clear specified bit area
>   * bitmap_find_next_zero_area(buf, len, pos, n, mask)	Find bit free area
>   */
> @@ -226,6 +227,7 @@ static inline int bitmap_intersects(const unsigned long *src1,
>  }
>  
>  void bitmap_set(unsigned long *map, long i, long len);
> +void bitmap_set_atomic(unsigned long *map, long i, long len);
>  void bitmap_clear(unsigned long *map, long start, long nr);
>  unsigned long bitmap_find_next_zero_area(unsigned long *map,
>                                           unsigned long size,
> diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
> index 8abdcf9..8164225 100644
> --- a/include/qemu/bitops.h
> +++ b/include/qemu/bitops.h
> @@ -16,6 +16,7 @@
>  #include <assert.h>
>  
>  #include "host-utils.h"
> +#include "atomic.h"
>  
>  #define BITS_PER_BYTE           CHAR_BIT
>  #define BITS_PER_LONG           (sizeof (unsigned long) * BITS_PER_BYTE)
> @@ -39,6 +40,19 @@ static inline void set_bit(long nr, unsigned long *addr)
>  }
>  
>  /**
> + * set_bit_atomic - Set a bit in memory atomically
> + * @nr: the bit to set
> + * @addr: the address to start counting from
> + */
> +static inline void set_bit_atomic(long nr, unsigned long *addr)
> +{
> +    unsigned long mask = BIT_MASK(nr);
> +    unsigned long *p = addr + BIT_WORD(nr);
> +
> +    atomic_or(p, mask);
> +}
> +
> +/**
>   * clear_bit - Clears a bit in memory
>   * @nr: Bit to clear
>   * @addr: Address to start counting from
> diff --git a/util/bitmap.c b/util/bitmap.c
> index 9c6bb52..39994af 100644
> --- a/util/bitmap.c
> +++ b/util/bitmap.c
> @@ -11,6 +11,7 @@
>  
>  #include "qemu/bitops.h"
>  #include "qemu/bitmap.h"
> +#include "qemu/atomic.h"
>  
>  /*
>   * bitmaps provide an array of bits, implemented using an an
> @@ -177,6 +178,43 @@ void bitmap_set(unsigned long *map, long start, long nr)
>      }
>  }
>  
> +void bitmap_set_atomic(unsigned long *map, long start, long nr)
> +{
> +    unsigned long *p = map + BIT_WORD(start);
> +    const long size = start + nr;
> +    int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
> +    unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
> +
> +    /* First word */
> +    if (nr - bits_to_set > 0) {
> +        atomic_or(p, mask_to_set);
> +        nr -= bits_to_set;
> +        bits_to_set = BITS_PER_LONG;
> +        mask_to_set = ~0UL;
> +        p++;
> +    }
> +
> +    /* Full words */
> +    if (bits_to_set == BITS_PER_LONG) {
> +        while (nr >= BITS_PER_LONG) {
> +            *p = ~0UL;
> +            nr -= BITS_PER_LONG;
> +            p++;

Out of curiosity: why not use a memset here?

Reviewed-by: Fam Zheng <famz@redhat.com>

> +        }
> +    }
> +
> +    /* Last word */
> +    if (nr) {
> +        mask_to_set &= BITMAP_LAST_WORD_MASK(size);
> +        atomic_or(p, mask_to_set);
> +    } else {
> +        /* If we avoided the full barrier in atomic_or(), issue a
> +         * barrier to account for the assignments in the while loop.
> +         */
> +        smp_mb();
> +    }
> +}
> +
>  void bitmap_clear(unsigned long *map, long start, long nr)
>  {
>      unsigned long *p = map + BIT_WORD(start);
> -- 
> 1.8.3.1
> 
> 
>