Message ID | 1348236500-2565-10-git-send-email-quintela@redhat.com |
---|---|
State | New |
Headers | show |
Il 21/09/2012 16:08, Juan Quintela ha scritto: > Instead of locking the whole migration_thread inside loop, just lock > migration_fd_put_notify, that is what interacts with the rest of the > world. Wrong commit message: just lock migrate_fd_put_ready. > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > buffered_file.c | 2 -- > migration.c | 5 +++++ > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/buffered_file.c b/buffered_file.c > index 2c9859b..22b973f 100644 > --- a/buffered_file.c > +++ b/buffered_file.c > @@ -215,9 +215,7 @@ static void *buffered_file_thread(void *opaque) > /* usleep expects microseconds */ > usleep((expire_time - current_time)*1000); > } > - qemu_mutex_lock_iothread(); > buffered_put_buffer(s, NULL, 0, 0); > - qemu_mutex_unlock_iothread(); > } > g_free(s->buffer); > g_free(s); > diff --git a/migration.c b/migration.c > index 29ee710..82c2663 100644 > --- a/migration.c > +++ b/migration.c > @@ -305,8 +305,10 @@ void migrate_fd_put_ready(MigrationState *s) > int ret; > static bool first_time = true; > > + qemu_mutex_lock_iothread(); > if (s->state != MIG_STATE_ACTIVE) { > DPRINTF("put_ready returning because of non-active state\n"); > + qemu_mutex_unlock_iothread(); Please use a goto instead. > return; > } > if (first_time) { > @@ -316,6 +318,7 @@ void migrate_fd_put_ready(MigrationState *s) > if (ret < 0) { > DPRINTF("failed, %d\n", ret); > migrate_fd_error(s); > + qemu_mutex_unlock_iothread(); Same here. > return; > } > } > @@ -351,6 +354,8 @@ void migrate_fd_put_ready(MigrationState *s) > } > } > } > + qemu_mutex_unlock_iothread(); > + > } > > static void migrate_fd_cancel(MigrationState *s) >
Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 21/09/2012 16:08, Juan Quintela ha scritto: >> Instead of locking the whole migration_thread inside loop, just lock >> migration_fd_put_notify, that is what interacts with the rest of the >> world. > > Wrong commit message: just lock migrate_fd_put_ready. Fixed. >> @@ -305,8 +305,10 @@ void migrate_fd_put_ready(MigrationState *s) >> int ret; >> static bool first_time = true; >> >> + qemu_mutex_lock_iothread(); >> if (s->state != MIG_STATE_ACTIVE) { >> DPRINTF("put_ready returning because of non-active state\n"); >> + qemu_mutex_unlock_iothread(); > > Please use a goto instead. > >> return; >> } >> if (first_time) { >> @@ -316,6 +318,7 @@ void migrate_fd_put_ready(MigrationState *s) >> if (ret < 0) { >> DPRINTF("failed, %d\n", ret); >> migrate_fd_error(s); >> + qemu_mutex_unlock_iothread(); > > Same here. > Don't work. The last branch of the code drops the lock before the end. Code is: lock() while() { if (foo) { ... unlock() break; } if (bar) { ... unlock() break; } unlock() ... /* this is the interesting bit */ } .... /* more stuff needed both sides */ adding a goto would not help making things clearer. >> return; >> } >> } >> @@ -351,6 +354,8 @@ void migrate_fd_put_ready(MigrationState *s) >> } >> } >> } >> + qemu_mutex_unlock_iothread(); >> + >> } >> >> static void migrate_fd_cancel(MigrationState *s) >>
diff --git a/buffered_file.c b/buffered_file.c index 2c9859b..22b973f 100644 --- a/buffered_file.c +++ b/buffered_file.c @@ -215,9 +215,7 @@ static void *buffered_file_thread(void *opaque) /* usleep expects microseconds */ usleep((expire_time - current_time)*1000); } - qemu_mutex_lock_iothread(); buffered_put_buffer(s, NULL, 0, 0); - qemu_mutex_unlock_iothread(); } g_free(s->buffer); g_free(s); diff --git a/migration.c b/migration.c index 29ee710..82c2663 100644 --- a/migration.c +++ b/migration.c @@ -305,8 +305,10 @@ void migrate_fd_put_ready(MigrationState *s) int ret; static bool first_time = true; + qemu_mutex_lock_iothread(); if (s->state != MIG_STATE_ACTIVE) { DPRINTF("put_ready returning because of non-active state\n"); + qemu_mutex_unlock_iothread(); return; } if (first_time) { @@ -316,6 +318,7 @@ void migrate_fd_put_ready(MigrationState *s) if (ret < 0) { DPRINTF("failed, %d\n", ret); migrate_fd_error(s); + qemu_mutex_unlock_iothread(); return; } } @@ -351,6 +354,8 @@ void migrate_fd_put_ready(MigrationState *s) } } } + qemu_mutex_unlock_iothread(); + } static void migrate_fd_cancel(MigrationState *s)
Instead of locking the whole migration_thread inside loop, just lock migration_fd_put_notify, that is what interacts with the rest of the world. Signed-off-by: Juan Quintela <quintela@redhat.com> --- buffered_file.c | 2 -- migration.c | 5 +++++ 2 files changed, 5 insertions(+), 2 deletions(-)