Re: [Qemu-devel] [PATCH v3] migration: change blocktime type to uint32_t

2018-02-16 Thread Dr. David Alan Gilbert 
* Alexey Perevalov (a.pereva...@samsung.com) wrote:
> Initially int64_t was used, but on PowerPC architecture,
> clang doesn't have atomic_*_8 function, so it produces
> link time error.
> 
> QEMU is working with time as with 64bit value, but by
> fact 32 bit is enough with CLOCK_REALTIME. In this case
> blocktime will keep only 1200 hours time interval.

Hi Alexey,

> Signed-off-by: Alexey Perevalov 
> Acked-by: Eric Blake 
> ---
>  hmp.c|  4 ++--
>  migration/postcopy-ram.c | 48 
> +++-
>  migration/trace-events   |  4 ++--
>  qapi/migration.json  |  4 ++--
>  4 files changed, 33 insertions(+), 27 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index c6bab53..3c376b3 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -265,7 +265,7 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>  }
>  
>  if (info->has_postcopy_blocktime) {
> -monitor_printf(mon, "postcopy blocktime: %" PRId64 "\n",
> +monitor_printf(mon, "postcopy blocktime: %u\n",
> info->postcopy_blocktime);
>  }
>  
> @@ -273,7 +273,7 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>  Visitor *v;
>  char *str;
>  v = string_output_visitor_new(false, );
> -visit_type_int64List(v, NULL, >postcopy_vcpu_blocktime, NULL);
> +visit_type_uint32List(v, NULL, >postcopy_vcpu_blocktime, NULL);
>  visit_complete(v, );
>  monitor_printf(mon, "postcopy vcpu blocktime: %s\n", str);
>  g_free(str);
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 7814da5..6694fd3 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -63,16 +63,17 @@ struct PostcopyDiscardState {
>  
>  typedef struct PostcopyBlocktimeContext {
>  /* time when page fault initiated per vCPU */
> -int64_t *page_fault_vcpu_time;
> +uint32_t *page_fault_vcpu_time;
>  /* page address per vCPU */
>  uintptr_t *vcpu_addr;
> -int64_t total_blocktime;
> +uint32_t total_blocktime;
>  /* blocktime per vCPU */
> -int64_t *vcpu_blocktime;
> +uint32_t *vcpu_blocktime;
>  /* point in time when last page fault was initiated */
> -int64_t last_begin;
> +uint32_t last_begin;
>  /* number of vCPU are suspended */
>  int smp_cpus_down;
> +uint64_t start_time;
>  
>  /*
>   * Handler for exit event, necessary for
> @@ -99,22 +100,23 @@ static void migration_exit_cb(Notifier *n, void *data)
>  static struct PostcopyBlocktimeContext *blocktime_context_new(void)
>  {
>  PostcopyBlocktimeContext *ctx = g_new0(PostcopyBlocktimeContext, 1);
> -ctx->page_fault_vcpu_time = g_new0(int64_t, smp_cpus);
> +ctx->page_fault_vcpu_time = g_new0(uint32_t, smp_cpus);
>  ctx->vcpu_addr = g_new0(uintptr_t, smp_cpus);
> -ctx->vcpu_blocktime = g_new0(int64_t, smp_cpus);
> +ctx->vcpu_blocktime = g_new0(uint32_t, smp_cpus);
>  
>  ctx->exit_notifier.notify = migration_exit_cb;
> +ctx->start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
>  qemu_add_exit_notifier(>exit_notifier);
>  return ctx;
>  }
>  
> -static int64List *get_vcpu_blocktime_list(PostcopyBlocktimeContext *ctx)
> +static uint32List *get_vcpu_blocktime_list(PostcopyBlocktimeContext *ctx)
>  {
> -int64List *list = NULL, *entry = NULL;
> +uint32List *list = NULL, *entry = NULL;
>  int i;
>  
>  for (i = smp_cpus - 1; i >= 0; i--) {
> -entry = g_new0(int64List, 1);
> +entry = g_new0(uint32List, 1);
>  entry->value = ctx->vcpu_blocktime[i];
>  entry->next = list;
>  list = entry;
> @@ -145,7 +147,7 @@ void 
> fill_destination_postcopy_migration_info(MigrationInfo *info)
>  info->postcopy_vcpu_blocktime = get_vcpu_blocktime_list(bc);
>  }
>  
> -static uint64_t get_postcopy_total_blocktime(void)
> +static uint32_t get_postcopy_total_blocktime(void)
>  {
>  MigrationIncomingState *mis = migration_incoming_get_current();
>  PostcopyBlocktimeContext *bc = mis->blocktime_ctx;
> @@ -633,7 +635,8 @@ static void mark_postcopy_blocktime_begin(uintptr_t addr, 
> uint32_t ptid,
>  int cpu, already_received;
>  MigrationIncomingState *mis = migration_incoming_get_current();
>  PostcopyBlocktimeContext *dc = mis->blocktime_ctx;
> -int64_t now_ms;
> +int64_t start_time_offset;
> +uint32_t low_time_offset;
>  
>  if (!dc || ptid == 0) {
>  return;
> @@ -643,14 +646,15 @@ static void mark_postcopy_blocktime_begin(uintptr_t 
> addr, uint32_t ptid,
>  return;
>  }
>  
> -now_ms = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +start_time_offset = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - 
> dc->start_time;
> +low_time_offset = start_time_offset & UINT32_MAX;

OK, I suggest you add a sanity check that start_time_offset >= 1 - to
cover the cases where it was unusually quick to get to this

[Qemu-devel] [PATCH v3] migration: change blocktime type to uint32_t

2018-02-16 Thread Alexey Perevalov 
Initially int64_t was used, but on PowerPC architecture,
clang doesn't have atomic_*_8 function, so it produces
link time error.

QEMU is working with time as with 64bit value, but by
fact 32 bit is enough with CLOCK_REALTIME. In this case
blocktime will keep only 1200 hours time interval.

Signed-off-by: Alexey Perevalov 
Acked-by: Eric Blake 
---
 hmp.c|  4 ++--
 migration/postcopy-ram.c | 48 +++-
 migration/trace-events   |  4 ++--
 qapi/migration.json  |  4 ++--
 4 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/hmp.c b/hmp.c
index c6bab53..3c376b3 100644
--- a/hmp.c
+++ b/hmp.c
@@ -265,7 +265,7 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
 }
 
 if (info->has_postcopy_blocktime) {
-monitor_printf(mon, "postcopy blocktime: %" PRId64 "\n",
+monitor_printf(mon, "postcopy blocktime: %u\n",
info->postcopy_blocktime);
 }
 
@@ -273,7 +273,7 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
 Visitor *v;
 char *str;
 v = string_output_visitor_new(false, );
-visit_type_int64List(v, NULL, >postcopy_vcpu_blocktime, NULL);
+visit_type_uint32List(v, NULL, >postcopy_vcpu_blocktime, NULL);
 visit_complete(v, );
 monitor_printf(mon, "postcopy vcpu blocktime: %s\n", str);
 g_free(str);
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 7814da5..6694fd3 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -63,16 +63,17 @@ struct PostcopyDiscardState {
 
 typedef struct PostcopyBlocktimeContext {
 /* time when page fault initiated per vCPU */
-int64_t *page_fault_vcpu_time;
+uint32_t *page_fault_vcpu_time;
 /* page address per vCPU */
 uintptr_t *vcpu_addr;
-int64_t total_blocktime;
+uint32_t total_blocktime;
 /* blocktime per vCPU */
-int64_t *vcpu_blocktime;
+uint32_t *vcpu_blocktime;
 /* point in time when last page fault was initiated */
-int64_t last_begin;
+uint32_t last_begin;
 /* number of vCPU are suspended */
 int smp_cpus_down;
+uint64_t start_time;
 
 /*
  * Handler for exit event, necessary for
@@ -99,22 +100,23 @@ static void migration_exit_cb(Notifier *n, void *data)
 static struct PostcopyBlocktimeContext *blocktime_context_new(void)
 {
 PostcopyBlocktimeContext *ctx = g_new0(PostcopyBlocktimeContext, 1);
-ctx->page_fault_vcpu_time = g_new0(int64_t, smp_cpus);
+ctx->page_fault_vcpu_time = g_new0(uint32_t, smp_cpus);
 ctx->vcpu_addr = g_new0(uintptr_t, smp_cpus);
-ctx->vcpu_blocktime = g_new0(int64_t, smp_cpus);
+ctx->vcpu_blocktime = g_new0(uint32_t, smp_cpus);
 
 ctx->exit_notifier.notify = migration_exit_cb;
+ctx->start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
 qemu_add_exit_notifier(>exit_notifier);
 return ctx;
 }
 
-static int64List *get_vcpu_blocktime_list(PostcopyBlocktimeContext *ctx)
+static uint32List *get_vcpu_blocktime_list(PostcopyBlocktimeContext *ctx)
 {
-int64List *list = NULL, *entry = NULL;
+uint32List *list = NULL, *entry = NULL;
 int i;
 
 for (i = smp_cpus - 1; i >= 0; i--) {
-entry = g_new0(int64List, 1);
+entry = g_new0(uint32List, 1);
 entry->value = ctx->vcpu_blocktime[i];
 entry->next = list;
 list = entry;
@@ -145,7 +147,7 @@ void fill_destination_postcopy_migration_info(MigrationInfo 
*info)
 info->postcopy_vcpu_blocktime = get_vcpu_blocktime_list(bc);
 }
 
-static uint64_t get_postcopy_total_blocktime(void)
+static uint32_t get_postcopy_total_blocktime(void)
 {
 MigrationIncomingState *mis = migration_incoming_get_current();
 PostcopyBlocktimeContext *bc = mis->blocktime_ctx;
@@ -633,7 +635,8 @@ static void mark_postcopy_blocktime_begin(uintptr_t addr, 
uint32_t ptid,
 int cpu, already_received;
 MigrationIncomingState *mis = migration_incoming_get_current();
 PostcopyBlocktimeContext *dc = mis->blocktime_ctx;
-int64_t now_ms;
+int64_t start_time_offset;
+uint32_t low_time_offset;
 
 if (!dc || ptid == 0) {
 return;
@@ -643,14 +646,15 @@ static void mark_postcopy_blocktime_begin(uintptr_t addr, 
uint32_t ptid,
 return;
 }
 
-now_ms = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+start_time_offset = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - 
dc->start_time;
+low_time_offset = start_time_offset & UINT32_MAX;
 if (dc->vcpu_addr[cpu] == 0) {
 atomic_inc(>smp_cpus_down);
 }
 
-atomic_xchg__nocheck(>last_begin, now_ms);
-atomic_xchg__nocheck(>page_fault_vcpu_time[cpu], now_ms);
-atomic_xchg__nocheck(>vcpu_addr[cpu], addr);
+atomic_xchg(>last_begin, low_time_offset);
+atomic_xchg(>page_fault_vcpu_time[cpu], low_time_offset);
+atomic_xchg(>vcpu_addr[cpu], addr);
 
 /* check it here, not at the begining of