Re: [RFC PATCH v2 2/3] fine grained qemu_mutex locking for migration

Paolo Bonzini
2011-08-01T09:39:54Z

On 07/29/2011 10:57 PM, Umesh Deshpande wrote:
> + qemu_mutex_unlock_iothread();
>
> while (s->state == MIG_STATE_ACTIVE) {
> if (migrate_fd_check_expire()) {
> + qemu_mutex_lock_iothread();
> buffered_rate_tick(s->file);
> + qemu_mutex_unlock_iothread();
> }
>
> if (s->state != MIG_STATE_ACTIVE) {
> @@ -392,12 +396,11 @@ void migrate_fd_begin(void *arg)
>
> if (s->callback) {
> migrate_fd_wait_for_unfreeze(s);
> + qemu_mutex_lock_iothread();
> s->callback(s);
> + qemu_mutex_unlock_iothread();
> }
> }
> -
> -out:
> - qemu_mutex_unlock_iothread();

I think it's clearer to unlock explicitly around the waiting points (see
review of 1/3). In fact, I think you're working around the busy wait by
accessing s->state outside the lock, right? I don't think this is
provably safe; moving the knowledge of the thread entirely within
buffered_file.c also fixes this, because then the lifetimes of the
thread and the QEMUFile are much clearer.

Thanks,

Paolo