diff mbox

[14/42] migration: prepare to access s->state outside critical sections

Message ID 1361551008-12430-15-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Feb. 22, 2013, 4:36 p.m. UTC 
Accessing s->state outside the big QEMU lock will simplify a bit the
locking/unlocking of the iothread lock.

We need to keep the lock in migrate_fd_error and migrate_fd_completed,
however, because they call migrate_fd_cleanup.

Reviewed-by: Orit Wasserman <owasserm@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 migration.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

Comments

Stefan Hajnoczi March 25, 2013, 9:44 a.m. UTC | #1 
On Fri, Feb 22, 2013 at 5:36 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Accessing s->state outside the big QEMU lock will simplify a bit the
> locking/unlocking of the iothread lock.
>
> We need to keep the lock in migrate_fd_error and migrate_fd_completed,
> however, because they call migrate_fd_cleanup.
>
> Reviewed-by: Orit Wasserman <owasserm@redhat.com>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  migration.c | 33 +++++++++++++++++++++------------
>  1 file changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index b091532..b40755f 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -279,19 +279,25 @@ static void migrate_fd_cleanup(MigrationState *s)
>      notifier_list_notify(&migration_state_notifiers, s);
>  }
>
> +static void migrate_finish_set_state(MigrationState *s, int new_state)
> +{
> +    if (__sync_val_compare_and_swap(&s->state, MIG_STATE_ACTIVE,

kraxel_rhel61's mingw build fails:

  LINK  i386-softmmu/qemu-system-i386.exe
../migration.o:migration.c:(.text+0x408): undefined reference to
`__sync_val_compare_and_swap_4'
../migration.o:migration.c:(.text+0x7af): undefined reference to
`__sync_val_compare_and_swap_4'
../migration.o:migration.c:(.text+0x827): undefined reference to
`__sync_val_compare_and_swap_4'

http://buildbot.b1-systems.de/qemu/builders/default_mingw32/builds/566/steps/compile/logs/stdio

The latest RHEL6 mingw gcc is version 4.4 but this buildslave is
RHEL6.1 so perhaps it's an older version that is missing the atomics
builtins?

Stefan
Gerd Hoffmann March 25, 2013, 9:52 a.m. UTC | #2 
Hi,

> http://buildbot.b1-systems.de/qemu/builders/default_mingw32/builds/566/steps/compile/logs/stdio
> 
> The latest RHEL6 mingw gcc is version 4.4 but this buildslave is
> RHEL6.1 so perhaps it's an older version that is missing the atomics
> builtins?

No, it's RHEL-6.4 actually, even though the name suggests otherwise (was
a bad idea to include the minor rev in the buildslave name ...).

There is a detailed slave info page btw:
http://buildbot.b1-systems.de/qemu/buildslaves/kraxel_rhel61

cheers,
  Gerd
diff mbox

Patch

diff --git a/migration.c b/migration.c
index b091532..b40755f 100644
--- a/migration.c
+++ b/migration.c
@@ -279,19 +279,25 @@  static void migrate_fd_cleanup(MigrationState *s)
     notifier_list_notify(&migration_state_notifiers, s);
 }
 
+static void migrate_finish_set_state(MigrationState *s, int new_state)
+{
+    if (__sync_val_compare_and_swap(&s->state, MIG_STATE_ACTIVE,
+                                    new_state) == new_state) {
+        trace_migrate_set_state(new_state);
+    }
+}
+
 void migrate_fd_error(MigrationState *s)
 {
     DPRINTF("setting error state\n");
-    s->state = MIG_STATE_ERROR;
-    trace_migrate_set_state(MIG_STATE_ERROR);
+    migrate_finish_set_state(s, MIG_STATE_ERROR);
     migrate_fd_cleanup(s);
 }
 
 static void migrate_fd_completed(MigrationState *s)
 {
     DPRINTF("setting completed state\n");
-    s->state = MIG_STATE_COMPLETED;
-    trace_migrate_set_state(MIG_STATE_COMPLETED);
+    migrate_finish_set_state(s, MIG_STATE_COMPLETED);
     migrate_fd_cleanup(s);
 }
 
@@ -316,13 +322,9 @@  static ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
 
 static void migrate_fd_cancel(MigrationState *s)
 {
-    if (s->state != MIG_STATE_ACTIVE)
-        return;
-
     DPRINTF("cancelling migration\n");
 
-    s->state = MIG_STATE_CANCELLED;
-    trace_migrate_set_state(MIG_STATE_CANCELLED);
+    migrate_finish_set_state(s, MIG_STATE_CANCELLED);
     migrate_fd_cleanup(s);
 }
 
@@ -657,12 +659,14 @@  static void *buffered_file_thread(void *opaque)
     qemu_mutex_lock_iothread();
     DPRINTF("beginning savevm\n");
     qemu_savevm_state_begin(s->file, &s->params);
+    qemu_mutex_unlock_iothread();
 
     while (s->state == MIG_STATE_ACTIVE) {
         int64_t current_time;
         uint64_t pending_size;
 
         if (s->bytes_xfer < s->xfer_limit) {
+            qemu_mutex_lock_iothread();
             DPRINTF("iterate\n");
             pending_size = qemu_savevm_state_pending(s->file, max_size);
             DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
@@ -678,8 +682,9 @@  static void *buffered_file_thread(void *opaque)
                 qemu_savevm_state_complete(s->file);
                 last_round = true;
             }
+            qemu_mutex_unlock_iothread();
         }
-        qemu_mutex_unlock_iothread();
+
         current_time = qemu_get_clock_ms(rt_clock);
         if (current_time >= initial_time + BUFFER_DELAY) {
             uint64_t transferred_bytes = s->bytes_xfer;
@@ -706,14 +711,18 @@  static void *buffered_file_thread(void *opaque)
             sleep_time += qemu_get_clock_ms(rt_clock) - current_time;
         }
         buffered_flush(s);
-        qemu_mutex_lock_iothread();
         if (qemu_file_get_error(s->file)) {
+            qemu_mutex_lock_iothread();
             migrate_fd_error(s);
+            qemu_mutex_unlock_iothread();
         } else if (last_round && s->buffer_size == 0) {
+            qemu_mutex_lock_iothread();
             migrate_fd_completed(s);
+            qemu_mutex_unlock_iothread();
         }
     }
 
+    qemu_mutex_lock_iothread();
     if (s->state == MIG_STATE_COMPLETED) {
         int64_t end_time = qemu_get_clock_ms(rt_clock);
         s->total_time = end_time - s->total_time;
@@ -725,8 +734,8 @@  static void *buffered_file_thread(void *opaque)
             vm_start();
         }
     }
-
     qemu_mutex_unlock_iothread();
+
     g_free(s->buffer);
     return NULL;
 }