[PATCH RESEND 0/2] memory: Add tracepoints for log_sync

[PATCH RESEND 0/2] memory: Add tracepoints for log_sync

[Peter Xu]

[this is a resend to hopefully drop '\r' in cover letter caused by git-publish
 that I should have just fixed; nothing changed inside]

It can help to identify which step is slow for migration dirty sync process.
We have migration_bitmap_sync_* trace events but it's still a bit coarse.

This should help us to cut migration_bitmap_sync() into finer grained small
steps when measurement is needed.

Please review, thanks.

Peter Xu (2):
  memory: Name all the memory listeners
  memory: Add tracepoint for dirty sync

 accel/hvf/hvf-accel-ops.c         | 1 +
 accel/kvm/kvm-all.c               | 7 +++++--
 hw/i386/xen/xen-hvm.c             | 2 ++
 hw/intc/openpic_kvm.c             | 1 +
 hw/remote/proxy-memory-listener.c | 1 +
 hw/vfio/common.c                  | 1 +
 hw/vfio/spapr.c                   | 1 +
 hw/virtio/vhost-vdpa.c            | 1 +
 hw/virtio/vhost.c                 | 2 ++
 hw/virtio/virtio.c                | 1 +
 hw/xen/xen_pt.c                   | 2 ++
 include/exec/memory.h             | 8 ++++++++
 include/sysemu/kvm_int.h          | 2 +-
 softmmu/memory.c                  | 2 ++
 softmmu/physmem.c                 | 1 +
 softmmu/trace-events              | 1 +
 target/arm/kvm.c                  | 1 +
 target/i386/hax/hax-mem.c         | 1 +
 target/i386/kvm/kvm.c             | 2 +-
 target/i386/nvmm/nvmm-all.c       | 1 +
 target/i386/whpx/whpx-all.c       | 1 +
 21 files changed, 36 insertions(+), 4 deletions(-)

-- 
2.31.1

[PATCH RESEND 1/2] memory: Name all the memory listeners

[Peter Xu]

Provide a name field for all the memory listeners.  It can be used to identify
which memory listener is which.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 accel/hvf/hvf-accel-ops.c         | 1 +
 accel/kvm/kvm-all.c               | 7 +++++--
 hw/i386/xen/xen-hvm.c             | 2 ++
 hw/intc/openpic_kvm.c             | 1 +
 hw/remote/proxy-memory-listener.c | 1 +
 hw/vfio/common.c                  | 1 +
 hw/vfio/spapr.c                   | 1 +
 hw/virtio/vhost-vdpa.c            | 1 +
 hw/virtio/vhost.c                 | 2 ++
 hw/virtio/virtio.c                | 1 +
 hw/xen/xen_pt.c                   | 2 ++
 include/exec/memory.h             | 8 ++++++++
 include/sysemu/kvm_int.h          | 2 +-
 softmmu/physmem.c                 | 1 +
 target/arm/kvm.c                  | 1 +
 target/i386/hax/hax-mem.c         | 1 +
 target/i386/kvm/kvm.c             | 2 +-
 target/i386/nvmm/nvmm-all.c       | 1 +
 target/i386/whpx/whpx-all.c       | 1 +
 19 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c
index d1691be989..ecd48337b4 100644
--- a/accel/hvf/hvf-accel-ops.c
+++ b/accel/hvf/hvf-accel-ops.c
@@ -291,6 +291,7 @@ static void hvf_region_del(MemoryListener *listener,
 }
 
 static MemoryListener hvf_memory_listener = {
+    .name = "hvf",
     .priority = 10,
     .region_add = hvf_region_add,
     .region_del = hvf_region_del,
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 0125c17edb..3d6f44cb4d 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1129,6 +1129,7 @@ static void kvm_coalesce_pio_del(MemoryListener *listener,
 }
 
 static MemoryListener kvm_coalesced_pio_listener = {
+    .name = "kvm-coalesced-pio",
     .coalesced_io_add = kvm_coalesce_pio_add,
     .coalesced_io_del = kvm_coalesce_pio_del,
 };
@@ -1633,7 +1634,7 @@ static void kvm_io_ioeventfd_del(MemoryListener *listener,
 }
 
 void kvm_memory_listener_register(KVMState *s, KVMMemoryListener *kml,
-                                  AddressSpace *as, int as_id)
+                                  AddressSpace *as, int as_id, const char *name)
 {
     int i;
 
@@ -1649,6 +1650,7 @@ void kvm_memory_listener_register(KVMState *s, KVMMemoryListener *kml,
     kml->listener.log_start = kvm_log_start;
     kml->listener.log_stop = kvm_log_stop;
     kml->listener.priority = 10;
+    kml->listener.name = name;
 
     if (s->kvm_dirty_ring_size) {
         kml->listener.log_sync_global = kvm_log_sync_global;
@@ -1669,6 +1671,7 @@ void kvm_memory_listener_register(KVMState *s, KVMMemoryListener *kml,
 }
 
 static MemoryListener kvm_io_listener = {
+    .name = "kvm-io",
     .eventfd_add = kvm_io_ioeventfd_add,
     .eventfd_del = kvm_io_ioeventfd_del,
     .priority = 10,
@@ -2579,7 +2582,7 @@ static int kvm_init(MachineState *ms)
     s->memory_listener.listener.coalesced_io_del = kvm_uncoalesce_mmio_region;
 
     kvm_memory_listener_register(s, &s->memory_listener,
-                                 &address_space_memory, 0);
+                                 &address_space_memory, 0, "kvm-memory");
     if (kvm_eventfds_allowed) {
         memory_listener_register(&kvm_io_listener,
                                  &address_space_io);
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 9b432773f0..e3d3d5cf89 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -721,6 +721,7 @@ static void xen_log_global_stop(MemoryListener *listener)
 }
 
 static MemoryListener xen_memory_listener = {
+    .name = "xen-memory",
     .region_add = xen_region_add,
     .region_del = xen_region_del,
     .log_start = xen_log_start,
@@ -732,6 +733,7 @@ static MemoryListener xen_memory_listener = {
 };
 
 static MemoryListener xen_io_listener = {
+    .name = "xen-io",
     .region_add = xen_io_add,
     .region_del = xen_io_del,
     .priority = 10,
diff --git a/hw/intc/openpic_kvm.c b/hw/intc/openpic_kvm.c
index 21da680389..557dd0c2bf 100644
--- a/hw/intc/openpic_kvm.c
+++ b/hw/intc/openpic_kvm.c
@@ -234,6 +234,7 @@ static void kvm_openpic_realize(DeviceState *dev, Error **errp)
 
     opp->mem_listener.region_add = kvm_openpic_region_add;
     opp->mem_listener.region_del = kvm_openpic_region_del;
+    opp->mem_listener.name = "openpic-kvm";
     memory_listener_register(&opp->mem_listener, &address_space_memory);
 
     /* indicate pic capabilities */
diff --git a/hw/remote/proxy-memory-listener.c b/hw/remote/proxy-memory-listener.c
index 901dbf1357..882c9b4854 100644
--- a/hw/remote/proxy-memory-listener.c
+++ b/hw/remote/proxy-memory-listener.c
@@ -219,6 +219,7 @@ void proxy_memory_listener_configure(ProxyMemoryListener *proxy_listener,
     proxy_listener->listener.region_add = proxy_memory_listener_region_addnop;
     proxy_listener->listener.region_nop = proxy_memory_listener_region_addnop;
     proxy_listener->listener.priority = 10;
+    proxy_listener->listener.name = "proxy";
 
     memory_listener_register(&proxy_listener->listener,
                              &address_space_memory);
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 8728d4d5c2..3476170067 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1434,6 +1434,7 @@ static void vfio_listener_log_sync(MemoryListener *listener,
 }
 
 static const MemoryListener vfio_memory_listener = {
+    .name = "vfio",
     .region_add = vfio_listener_region_add,
     .region_del = vfio_listener_region_del,
     .log_global_start = vfio_listener_log_global_start,
diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index ea3f70bd2f..04c6e67f8f 100644
--- a/hw/vfio/spapr.c
+++ b/hw/vfio/spapr.c
@@ -136,6 +136,7 @@ static void vfio_prereg_listener_region_del(MemoryListener *listener,
 }
 
 const MemoryListener vfio_prereg_listener = {
+    .name = "vfio-pre-reg",
     .region_add = vfio_prereg_listener_region_add,
     .region_del = vfio_prereg_listener_region_del,
 };
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 4fa414feea..1ee17b9e9b 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -234,6 +234,7 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener,
  * depends on the addnop().
  */
 static const MemoryListener vhost_vdpa_memory_listener = {
+    .name = "vhost-vdpa",
     .begin = vhost_vdpa_listener_begin,
     .commit = vhost_vdpa_listener_commit,
     .region_add = vhost_vdpa_listener_region_add,
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index e8f85a5d2d..6efcfc8faf 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1341,6 +1341,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
     hdev->features = features;
 
     hdev->memory_listener = (MemoryListener) {
+        .name = "vhost",
         .begin = vhost_begin,
         .commit = vhost_commit,
         .region_add = vhost_region_addnop,
@@ -1356,6 +1357,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
     };
 
     hdev->iommu_listener = (MemoryListener) {
+        .name = "vhost-iommu",
         .region_add = vhost_iommu_region_add,
         .region_del = vhost_iommu_region_del,
     };
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 874377f37a..0dbf968c9b 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3671,6 +3671,7 @@ static void virtio_device_realize(DeviceState *dev, Error **errp)
     }
 
     vdev->listener.commit = virtio_memory_listener_commit;
+    vdev->listener.name = "virtio";
     memory_listener_register(&vdev->listener, vdev->dma_as);
 }
 
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index 232482d65f..ca0a98187e 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -689,12 +689,14 @@ static void xen_pt_io_region_del(MemoryListener *l, MemoryRegionSection *sec)
 }
 
 static const MemoryListener xen_pt_memory_listener = {
+    .name = "xen-pt-mem",
     .region_add = xen_pt_region_add,
     .region_del = xen_pt_region_del,
     .priority = 10,
 };
 
 static const MemoryListener xen_pt_io_listener = {
+    .name = "xen-pt-io",
     .region_add = xen_pt_io_region_add,
     .region_del = xen_pt_io_region_del,
     .priority = 10,
diff --git a/include/exec/memory.h b/include/exec/memory.h
index c3d417d317..ac79fee250 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -979,6 +979,14 @@ struct MemoryListener {
      */
     unsigned priority;
 
+    /**
+     * @name:
+     *
+     * Name of the listener.  It can be used in contexts where we'd like to
+     * identify one memory listener with the rest.
+     */
+    const char *name;
+
     /* private: */
     AddressSpace *address_space;
     QTAILQ_ENTRY(MemoryListener) link;
diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
index c788452cd9..1f5487d9b7 100644
--- a/include/sysemu/kvm_int.h
+++ b/include/sysemu/kvm_int.h
@@ -37,7 +37,7 @@ typedef struct KVMMemoryListener {
 } KVMMemoryListener;
 
 void kvm_memory_listener_register(KVMState *s, KVMMemoryListener *kml,
-                                  AddressSpace *as, int as_id);
+                                  AddressSpace *as, int as_id, const char *name);
 
 void kvm_set_max_memslot_size(hwaddr max_slot_size);
 
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 3c1912a1a0..3729f2537d 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -756,6 +756,7 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
     if (tcg_enabled()) {
         newas->tcg_as_listener.log_global_after_sync = tcg_log_global_after_sync;
         newas->tcg_as_listener.commit = tcg_commit;
+        newas->tcg_as_listener.name = "tcg";
         memory_listener_register(&newas->tcg_as_listener, as);
     }
 }
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index d8381ba224..97ec88a587 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -330,6 +330,7 @@ static void kvm_arm_devlistener_del(MemoryListener *listener,
 }
 
 static MemoryListener devlistener = {
+    .name = "kvm-arm",
     .region_add = kvm_arm_devlistener_add,
     .region_del = kvm_arm_devlistener_del,
 };
diff --git a/target/i386/hax/hax-mem.c b/target/i386/hax/hax-mem.c
index 8d44edbffd..a226d174d8 100644
--- a/target/i386/hax/hax-mem.c
+++ b/target/i386/hax/hax-mem.c
@@ -285,6 +285,7 @@ static void hax_log_sync(MemoryListener *listener,
 }
 
 static MemoryListener hax_memory_listener = {
+    .name = "hax",
     .begin = hax_transaction_begin,
     .commit = hax_transaction_commit,
     .region_add = hax_region_add,
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index e69abe48e3..771b06b39e 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2227,7 +2227,7 @@ static void register_smram_listener(Notifier *n, void *unused)
 
     address_space_init(&smram_address_space, &smram_as_root, "KVM-SMRAM");
     kvm_memory_listener_register(kvm_state, &smram_listener,
-                                 &smram_address_space, 1);
+                                 &smram_address_space, 1, "kvm-smram");
 }
 
 int kvm_arch_init(MachineState *ms, KVMState *s)
diff --git a/target/i386/nvmm/nvmm-all.c b/target/i386/nvmm/nvmm-all.c
index dfa690d65d..5a3f6d69c3 100644
--- a/target/i386/nvmm/nvmm-all.c
+++ b/target/i386/nvmm/nvmm-all.c
@@ -1125,6 +1125,7 @@ nvmm_log_sync(MemoryListener *listener, MemoryRegionSection *section)
 }
 
 static MemoryListener nvmm_memory_listener = {
+    .name = "nvmm",
     .begin = nvmm_transaction_begin,
     .commit = nvmm_transaction_commit,
     .region_add = nvmm_region_add,
diff --git a/target/i386/whpx/whpx-all.c b/target/i386/whpx/whpx-all.c
index f832f286ac..ded096261f 100644
--- a/target/i386/whpx/whpx-all.c
+++ b/target/i386/whpx/whpx-all.c
@@ -1600,6 +1600,7 @@ static void whpx_log_sync(MemoryListener *listener,
 }
 
 static MemoryListener whpx_memory_listener = {
+    .name = "whpx",
     .begin = whpx_transaction_begin,
     .commit = whpx_transaction_commit,
     .region_add = whpx_region_add,
-- 
2.31.1

Re: [PATCH RESEND 1/2] memory: Name all the memory listeners

[David Hildenbrand]

On 17.08.21 03:35, Peter Xu wrote:
> Provide a name field for all the memory listeners.  It can be used to identify
> which memory listener is which.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   accel/hvf/hvf-accel-ops.c         | 1 +
>   accel/kvm/kvm-all.c               | 7 +++++--
>   hw/i386/xen/xen-hvm.c             | 2 ++
>   hw/intc/openpic_kvm.c             | 1 +
>   hw/remote/proxy-memory-listener.c | 1 +
>   hw/vfio/common.c                  | 1 +
>   hw/vfio/spapr.c                   | 1 +
>   hw/virtio/vhost-vdpa.c            | 1 +
>   hw/virtio/vhost.c                 | 2 ++
>   hw/virtio/virtio.c                | 1 +
>   hw/xen/xen_pt.c                   | 2 ++
>   include/exec/memory.h             | 8 ++++++++
>   include/sysemu/kvm_int.h          | 2 +-
>   softmmu/physmem.c                 | 1 +
>   target/arm/kvm.c                  | 1 +
>   target/i386/hax/hax-mem.c         | 1 +
>   target/i386/kvm/kvm.c             | 2 +-
>   target/i386/nvmm/nvmm-all.c       | 1 +
>   target/i386/whpx/whpx-all.c       | 1 +
>   19 files changed, 33 insertions(+), 4 deletions(-)
> 


[...]

>   static const MemoryListener xen_pt_io_listener = {
> +    .name = "xen-pt-io",
>       .region_add = xen_pt_io_region_add,
>       .region_del = xen_pt_io_region_del,
>       .priority = 10,
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index c3d417d317..ac79fee250 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -979,6 +979,14 @@ struct MemoryListener {
>        */
>       unsigned priority;
>   
> +    /**
> +     * @name:
> +     *
> +     * Name of the listener.  It can be used in contexts where we'd like to
> +     * identify one memory listener with the rest. > +     */

Not a native speaker, maybe simply "Name of the listener, primarily 
useful for debugging. Names can change in the future and are not fixed."

Apart from that LGTM

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

[PATCH RESEND 2/2] memory: Add tracepoint for dirty sync

[Peter Xu]

Trace at memory_region_sync_dirty_bitmap() for log_sync() or global_log_sync()
on memory regions.  One trace line should suffice when it finishes, so as to
estimate the time used for each log sync process.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 softmmu/memory.c     | 2 ++
 softmmu/trace-events | 1 +
 2 files changed, 3 insertions(+)

diff --git a/softmmu/memory.c b/softmmu/memory.c
index bfedaf9c4d..f0c5817b97 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -2149,6 +2149,7 @@ static void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
                 }
             }
             flatview_unref(view);
+            trace_memory_region_sync_dirty(mr ? mr->name : "(all)", listener->name, 0);
         } else if (listener->log_sync_global) {
             /*
              * No matter whether MR is specified, what we can do here
@@ -2156,6 +2157,7 @@ static void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
              * sync in a finer granularity.
              */
             listener->log_sync_global(listener);
+            trace_memory_region_sync_dirty(mr ? mr->name : "(all)", listener->name, 1);
         }
     }
 }
diff --git a/softmmu/trace-events b/softmmu/trace-events
index 7b278590a0..bf1469990e 100644
--- a/softmmu/trace-events
+++ b/softmmu/trace-events
@@ -15,6 +15,7 @@ memory_region_subpage_read(int cpu_index, void *mr, uint64_t offset, uint64_t va
 memory_region_subpage_write(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u"
 memory_region_ram_device_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
 memory_region_ram_device_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
+memory_region_sync_dirty(const char *mr, const char *listener, int global) "mr '%s' listener '%s' synced (global=%d)"
 flatview_new(void *view, void *root) "%p (root %p)"
 flatview_destroy(void *view, void *root) "%p (root %p)"
 flatview_destroy_rcu(void *view, void *root) "%p (root %p)"
-- 
2.31.1

Re: [PATCH RESEND 2/2] memory: Add tracepoint for dirty sync

[David Hildenbrand]

On 17.08.21 03:37, Peter Xu wrote:
> Trace at memory_region_sync_dirty_bitmap() for log_sync() or global_log_sync()
> on memory regions.  One trace line should suffice when it finishes, so as to
> estimate the time used for each log sync process.

I wonder if a start/finish would be even nicer. At least it wouldn't 
really result in significantly more code changes :)

-- 
Thanks,

David / dhildenb

Re: [PATCH RESEND 2/2] memory: Add tracepoint for dirty sync

[Peter Xu]

On Tue, Aug 17, 2021 at 09:25:56AM +0200, David Hildenbrand wrote:
> On 17.08.21 03:37, Peter Xu wrote:
> > Trace at memory_region_sync_dirty_bitmap() for log_sync() or global_log_sync()
> > on memory regions.  One trace line should suffice when it finishes, so as to
> > estimate the time used for each log sync process.
> 
> I wonder if a start/finish would be even nicer. At least it wouldn't really
> result in significantly more code changes :)

Note that the "name"s I added is not only for not using start/end, it's about
knowing which memory listener is slow.  Start/end won't achieve that if we
don't have a name for them.  So far I just wanted to identify majorly kvm,
vhost and kvm-smram, however it'll always be good when some log_sync is missed
when tracing.

I'm also wondering whether kvm-smram needs a whole bitmap as I don't know what
RAM would be touched within system manager mode (as I thought it should only
touch a very limited range and should be defined somewhere), but that's
off-topic.

If we want to make it start/end pair, I can do that too.  But the 1st patch
will still be wanted.

Thanks,

-- 
Peter Xu

Re: [PATCH RESEND 2/2] memory: Add tracepoint for dirty sync

[David Hildenbrand]

On 17.08.21 18:05, Peter Xu wrote:
> On Tue, Aug 17, 2021 at 09:25:56AM +0200, David Hildenbrand wrote:
>> On 17.08.21 03:37, Peter Xu wrote:
>>> Trace at memory_region_sync_dirty_bitmap() for log_sync() or global_log_sync()
>>> on memory regions.  One trace line should suffice when it finishes, so as to
>>> estimate the time used for each log sync process.
>>
>> I wonder if a start/finish would be even nicer. At least it wouldn't really
>> result in significantly more code changes :)
> 
> Note that the "name"s I added is not only for not using start/end, it's about
> knowing which memory listener is slow.  Start/end won't achieve that if we
> don't have a name for them.  So far I just wanted to identify majorly kvm,
> vhost and kvm-smram, however it'll always be good when some log_sync is missed
> when tracing.
> 
> I'm also wondering whether kvm-smram needs a whole bitmap as I don't know what
> RAM would be touched within system manager mode (as I thought it should only
> touch a very limited range and should be defined somewhere), but that's
> off-topic.
> 
> If we want to make it start/end pair, I can do that too.  But the 1st patch
> will still be wanted.

Yeah, absolutely, not complaining about the name, it will be valuable to 
have!


-- 
Thanks,

David / dhildenb

Re: [PATCH RESEND 2/2] memory: Add tracepoint for dirty sync

[Paolo Bonzini]

On 17/08/21 18:07, David Hildenbrand wrote:
>>
>>
>> If we want to make it start/end pair, I can do that too.  But the 1st 
>> patch
>> will still be wanted.
> 
> Yeah, absolutely, not complaining about the name, it will be valuable to 
> have!

You could have start/end tracepoints for memory_region_sync_dirty_bitmap 
on top of this one that you are adding, so I queued the patches.

Paolo

Re: [PATCH RESEND 2/2] memory: Add tracepoint for dirty sync

[Paolo Bonzini]

On 17/08/21 18:05, Peter Xu wrote:
> 
> I'm also wondering whether kvm-smram needs a whole bitmap as I don't know what
> RAM would be touched within system manager mode (as I thought it should only
> touch a very limited range and should be defined somewhere), but that's
> off-topic.

The kvm-smram dirty bitmap will include all memory touched while the SMM 
address space is in effect, so not just SMRAM.  The two KVM dirty 
bitmaps end up in just one QEMU dirty bitmap (the one with id 
DIRTY_MEMORY_MIGRATION) but they are separate at the kernel level.

Paolo