[PATCH 0/3] migration: postcopy blocktime cleanups and assert fix

[PATCH 0/3] migration: postcopy blocktime cleanups and assert fix

[Bin Guo]

This series fixes a couple of issues in the postcopy blocktime
fine-grained tracking feature:

  - Patch 1 fixes an ineffective overflow assert in vcpu_faults_current[].
  - Patch 2 fixes the const/gchar mismatch in format_time_str().
  - Patch 3 collects minor cleanups: remove a redundant memset() and
    fix the HMP label for non-vCPU latency.

Bin Guo (3):
  migration: fix ineffective overflow assert in postcopy blocktime
  migration: fix const mismatch in format_time_str()
  migration: clean up postcopy blocktime presentation

 migration/migration-hmp-cmds.c |  8 ++++----
 migration/postcopy-ram.c       | 10 ++++------
 2 files changed, 8 insertions(+), 10 deletions(-)

-- 
2.50.1 (Apple Git-155)

[PATCH 1/3] migration: fix ineffective overflow assert in postcopy blocktime

[Bin Guo]

vcpu_faults_current[] is uint8_t.  The overflow assert was checked
after the post-increment, so 255 would wrap to 0 and the assert
would pass silently.  Move the check before the increment and use
< 255.

Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
---
 migration/postcopy-ram.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index f5ef93f193..341cad264a 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1093,7 +1093,11 @@ void mark_postcopy_blocktime_begin(uintptr_t addr, uint32_t ptid,
         /*
          * Account how many concurrent faults on this vCPU we trapped.  See
          * comments above vcpu_faults_current[] on why it can be more than one.
+         *
+         * vcpu_faults_current[] is uint8_t, so assert before incrementing to
+         * catch overflow before it wraps.
          */
+        assert(dc->vcpu_faults_current[cpu] < 255);
         if (dc->vcpu_faults_current[cpu]++ == 0) {
             dc->smp_cpus_down++;
             /*
@@ -1103,9 +1107,6 @@ void mark_postcopy_blocktime_begin(uintptr_t addr, uint32_t ptid,
              */
             dc->last_begin = current;
         }
-
-        /* Making sure it won't overflow - it really should never! */
-        assert(dc->vcpu_faults_current[cpu] <= 255);
     } else {
         /*
          * For non-vCPU thread faults, we don't care about tid or cpu index
-- 
2.50.1 (Apple Git-155)

[PATCH 2/3] migration: fix const mismatch in format_time_str()

[Bin Guo]

format_time_str() returns a newly allocated string from
g_strdup_printf(), so the caller owns it.  Return gchar * rather
than const gchar * to match the g_autofree callers.

Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
---
 migration/migration-hmp-cmds.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 502ca704da..6bedf786df 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -54,7 +54,7 @@ static void migration_global_dump(Monitor *mon)
                    ms->clear_bitmap_shift);
 }
 
-static const gchar *format_time_str(uint64_t us)
+static gchar *format_time_str(uint64_t us)
 {
     const char *units[] = {"us", "ms", "sec"};
     int index = 0;
@@ -123,8 +123,8 @@ static void migration_dump_blocktime(Monitor *mon, MigrationInfo *info)
         monitor_printf(mon, "Postcopy Latency Distribution:\n");
 
         while (item) {
-            g_autofree const gchar *from = format_time_str(1UL << count);
-            g_autofree const gchar *to = format_time_str(1UL << (count + 1));
+            g_autofree gchar *from = format_time_str(1UL << count);
+            g_autofree gchar *to = format_time_str(1UL << (count + 1));
 
             monitor_printf(mon, "  [ %8s - %8s ]: %10"PRIu64"\n",
                            from, to, item->value);
-- 
2.50.1 (Apple Git-155)

[PATCH 3/3] migration: clean up postcopy blocktime presentation

[Bin Guo]

Small cleanups for the postcopy blocktime fine-grained tracking
feature:

* Remove a redundant memset() on latency_buckets after g_new0().
* Use singular "Latency" in the HMP label for postcopy non-vCPU
  latency, consistent with other single-value labels.

Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
---
 migration/migration-hmp-cmds.c | 2 +-
 migration/postcopy-ram.c       | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 6bedf786df..4a18ca8bba 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -96,7 +96,7 @@ static void migration_dump_blocktime(Monitor *mon, MigrationInfo *info)
     }
 
     if (info->has_postcopy_non_vcpu_latency) {
-        monitor_printf(mon, "Postcopy non-vCPU Latencies (ns): %" PRIu64 "\n",
+        monitor_printf(mon, "Postcopy non-vCPU Latency (ns): %" PRIu64 "\n",
                        info->postcopy_non_vcpu_latency);
     }
 
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 341cad264a..28d698260e 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -297,9 +297,6 @@ static struct PostcopyBlocktimeContext *blocktime_context_new(void)
     unsigned int smp_cpus = ms->smp.cpus;
     PostcopyBlocktimeContext *ctx = g_new0(PostcopyBlocktimeContext, 1);
 
-    /* Initialize all counters to be zeros */
-    memset(ctx->latency_buckets, 0, sizeof(ctx->latency_buckets));
-
     ctx->vcpu_blocktime_total = g_new0(uint64_t, smp_cpus);
     ctx->vcpu_faults_count = g_new0(uint64_t, smp_cpus);
     ctx->vcpu_faults_current = g_new0(uint8_t, smp_cpus);
-- 
2.50.1 (Apple Git-155)