[PATCH 0/2] migration: Store ram size value

[PATCH 0/2] migration: Store ram size value

[Juan Quintela]

Hi

Current migration code recalculates the amount of RAM each time that
is needed.  This calculation requires RCU and other operations.
During migration we disable hot/unplug of memory, so we can store it.

Notice the times difference, and specially that ram_bytes_total()
don't appears anymore in the perf output.

total time: 75852 ms
downtime: 264 ms
setup: 273 ms
transferred ram: 19671939 kbytes
throughput: 2132.28 mbps
remaining ram: 0 kbytes
total ram: 1077936904 kbytes
duplicate: 265170289 pages
skipped: 0 pages
normal: 4316628 pages
normal bytes: 17266512 kbytes
dirty sync count: 4
page size: 4 kbytes
multifd bytes: 17341329 kbytes
pages-per-second: 1236658
precopy ram: 2330608 kbytes
downtime ram: 1 kbytes

  37.97%  live_migration   qemu-system-x86_64       [.] buffer_zero_avx512
  10.42%  live_migration   qemu-system-x86_64       [.] ram_find_and_save_block.part.0
   6.67%  live_migration   qemu-system-x86_64       [.] add_to_iovec
   3.71%  live_migration   qemu-system-x86_64       [.] ram_bytes_total_common
   2.79%  live_migration   qemu-system-x86_64       [.] qemu_ram_is_migratable
   2.69%  live_migration   qemu-system-x86_64       [.] qemu_put_byte.part.0
   2.41%  live_migration   qemu-system-x86_64       [.] bitmap_test_and_clear_atomic
   1.55%  live_migration   qemu-system-x86_64       [.] qemu_put_be32
   1.26%  live_migration   qemu-system-x86_64       [.] find_next_bit
   1.07%  multifdsend_0    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   1.07%  multifdsend_13   [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   1.06%  multifdsend_6    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   1.05%  multifdsend_2    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   1.04%  multifdsend_15   [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   1.03%  multifdsend_12   [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   1.02%  live_migration   qemu-system-x86_64       [.] migrate_ignore_shared
   1.01%  multifdsend_7    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   1.01%  multifdsend_3    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   1.01%  multifdsend_10   [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.98%  live_migration   qemu-system-x86_64       [.] ram_save_iterate
   0.96%  multifdsend_4    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.93%  multifdsend_8    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.92%  multifdsend_5    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.90%  multifdsend_14   [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.88%  multifdsend_9    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.85%  multifdsend_1    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.83%  multifdsend_11   [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.61%  live_migration   qemu-system-x86_64       [.] save_zero_page_to_file.part.0
   0.48%  live_migration   [kernel.kallsyms]        [k] copy_user_enhanced_fast_string

Migration status: completed
total time: 70033 ms
downtime: 279 ms
setup: 280 ms
transferred ram: 19692747 kbytes
throughput: 2312.82 mbps
remaining ram: 0 kbytes
total ram: 1077936904 kbytes
duplicate: 265164421 pages
skipped: 0 pages
normal: 4322415 pages
normal bytes: 17289660 kbytes
dirty sync count: 3
page size: 4 kbytes
multifd bytes: 17362190 kbytes
pages-per-second: 2523447
precopy ram: 2330555 kbytes
downtime ram: 1 kbytes

  43.64%  live_migration   qemu-system-x86_64       [.] buffer_zero_avx512
  11.32%  live_migration   qemu-system-x86_64       [.] ram_find_and_save_block.part.0
   7.60%  live_migration   qemu-system-x86_64       [.] add_to_iovec
   2.95%  live_migration   qemu-system-x86_64       [.] qemu_put_byte.part.0
   2.73%  live_migration   qemu-system-x86_64       [.] bitmap_test_and_clear_atomic
   1.76%  live_migration   qemu-system-x86_64       [.] qemu_put_be32
   1.44%  live_migration   qemu-system-x86_64       [.] find_next_bit
   0.84%  multifdsend_1    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.84%  multifdsend_7    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.81%  multifdsend_15   [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.80%  multifdsend_4    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.80%  multifdsend_3    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.79%  multifdsend_12   [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.79%  multifdsend_14   [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.79%  multifdsend_11   [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.78%  multifdsend_13   [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.78%  live_migration   qemu-system-x86_64       [.] ram_save_iterate
   0.77%  multifdsend_9    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.77%  multifdsend_5    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.77%  multifdsend_10   [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.77%  multifdsend_2    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.77%  multifdsend_6    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.76%  multifdsend_8    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.71%  multifdsend_0    [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.66%  live_migration   qemu-system-x86_64       [.] save_zero_page_to_file.part.0
   0.62%  live_migration   qemu-system-x86_64       [.] qemu_ram_is_migratable
   0.54%  live_migration   [kernel.kallsyms]        [k] copy_user_enhanced_fast_string
   0.51%  live_migration   qemu-system-x86_64       [.] qemu_put_byte

Please, review.

Thanks, Juan.

Juan Quintela (2):
  migration: Split ram_bytes_total_common() in two functions
  migration: Calculate ram size once

 migration/ram.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

-- 
2.35.1

[PATCH 1/2] migration: Split ram_bytes_total_common() in two functions

[Juan Quintela]

It is just a big if in the middle of the function, and we need two
functions anways.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index a2489a2699..b3fa3d5d8f 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2304,28 +2304,30 @@ void acct_update_position(QEMUFile *f, size_t size, bool zero)
     }
 }
 
-static uint64_t ram_bytes_total_common(bool count_ignored)
+static uint64_t ram_bytes_total_with_ignored(void)
 {
     RAMBlock *block;
     uint64_t total = 0;
 
     RCU_READ_LOCK_GUARD();
 
-    if (count_ignored) {
-        RAMBLOCK_FOREACH_MIGRATABLE(block) {
-            total += block->used_length;
-        }
-    } else {
-        RAMBLOCK_FOREACH_NOT_IGNORED(block) {
-            total += block->used_length;
-        }
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
+        total += block->used_length;
     }
     return total;
 }
 
 uint64_t ram_bytes_total(void)
 {
-    return ram_bytes_total_common(false);
+    RAMBlock *block;
+    uint64_t total = 0;
+
+    RCU_READ_LOCK_GUARD();
+
+    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
+        total += block->used_length;
+    }
+    return total;
 }
 
 static void xbzrle_load_setup(void)
@@ -2924,7 +2926,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
     (*rsp)->f = f;
 
     WITH_RCU_READ_LOCK_GUARD() {
-        qemu_put_be64(f, ram_bytes_total_common(true) | RAM_SAVE_FLAG_MEM_SIZE);
+        qemu_put_be64(f, ram_bytes_total_with_ignored() | RAM_SAVE_FLAG_MEM_SIZE);
 
         RAMBLOCK_FOREACH_MIGRATABLE(block) {
             qemu_put_byte(f, strlen(block->idstr));
-- 
2.35.1

[PATCH 2/2] migration: Calculate ram size once

[Juan Quintela]

We are recalculating ram size continously, when we know that it don't
change during migration.  Create a field in RAMState to track it.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index b3fa3d5d8f..5d415834e5 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -301,6 +301,8 @@ struct RAMState {
     QEMUFile *f;
     /* UFFD file descriptor, used in 'write-tracking' migration */
     int uffdio_fd;
+    /* total ram size in bytes */
+    uint64_t ram_bytes_total;
     /* Last block that we have visited searching for dirty pages */
     RAMBlock *last_seen_block;
     /* Last block from where we have sent data */
@@ -2259,7 +2261,7 @@ static int ram_find_and_save_block(RAMState *rs)
     bool again, found;
 
     /* No dirty page as there is zero RAM */
-    if (!ram_bytes_total()) {
+    if (!rs->ram_bytes_total) {
         return pages;
     }
 
@@ -2707,13 +2709,14 @@ static int ram_state_init(RAMState **rsp)
     qemu_mutex_init(&(*rsp)->bitmap_mutex);
     qemu_mutex_init(&(*rsp)->src_page_req_mutex);
     QSIMPLEQ_INIT(&(*rsp)->src_page_requests);
+    (*rsp)->ram_bytes_total = ram_bytes_total();
 
     /*
      * Count the total number of pages used by ram blocks not including any
      * gaps due to alignment or unplugs.
      * This must match with the initial values of dirty bitmap.
      */
-    (*rsp)->migration_dirty_pages = ram_bytes_total() >> TARGET_PAGE_BITS;
+    (*rsp)->migration_dirty_pages = (*rsp)->ram_bytes_total >> TARGET_PAGE_BITS;
     ram_state_reset(*rsp);
 
     return 0;
-- 
2.35.1

Re: [PATCH 2/2] migration: Calculate ram size once

[David Hildenbrand]

On 11.05.22 01:17, Juan Quintela wrote:
> We are recalculating ram size continously, when we know that it don't
> change during migration.  Create a field in RAMState to track it.

We do have resizable RAM, which triggers ram_mig_ram_block_resized() on
resizes when changing block->used_length.

ram_mig_ram_block_resized() aborts migration when we detect a resize on
the source. I assume that is what you care about here, right?


-- 
Thanks,

David / dhildenb

Re: [PATCH 2/2] migration: Calculate ram size once

[Juan Quintela]

David Hildenbrand <david@redhat.com> wrote:
> On 11.05.22 01:17, Juan Quintela wrote:
>> We are recalculating ram size continously, when we know that it don't
>> change during migration.  Create a field in RAMState to track it.

Hi

> We do have resizable RAM, which triggers ram_mig_ram_block_resized() on
> resizes when changing block->used_length.

Yeap.

The problem is this bit of the patch:

@@ -2259,7 +2261,7 @@ static int ram_find_and_save_block(RAMState *rs)
     bool again, found;
 
     /* No dirty page as there is zero RAM */
-    if (!ram_bytes_total()) {
+    if (!rs->ram_bytes_total) {
         return pages;
     }
 
On 1TB guest, we moved form

75.8 seconds to 70.0 seconds total time just with this change.  This is
a idle guest full of zero pages.  If thue guest is busier, the effect is
less noticiable because we spend more time sending pages.

This effect is even more noticeable if you put this series on top of my
zero page detection series that I sent before.  With that, we end having
ram_bytes_total_common() as the second function that uses more CPU.

As you can see for the perf profile that I sent, with upstream we get:


  10.42%  live_migration   qemu-system-x86_64       [.] ram_find_and_save_block.part.0
   3.71%  live_migration   qemu-system-x86_64       [.] ram_bytes_total_common

(I have deleted the functions that are not reletade to this code path.
ram_find_and_save_block() is the function that calls
ram_bytes_total_common()).

After the patch, we move to:

  11.32%  live_migration   qemu-system-x86_64       [.] ram_find_and_save_block.part.0

And ram_bytes_total_common() don't even exist.  Notice that my series
only remove one call to it.  The CPU utilization here is higher for
ram_find_and_save_block() because:

- Meassuring things is difficult (TM)
- With this patch, migration goes faster, and then we call
  ram_find_and_save_block() more times in the same period of time.

> ram_mig_ram_block_resized() aborts migration when we detect a resize on
> the source. I assume that is what you care about here, right?

Ouch.  Hadn't noticed that we could change the ramsize during
migration.  Anyways, as you said, if ram size change, we cancel
migration, so we can still cache the size value. Actually, I have just
put it on ram_state_init(), because there is where we setup the bitmaps,
so if we change the bitmap, we change also the ram_size cached value.

Thanks, Juan.