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

[Paolo Bonzini <pbonzini@redhat.com>]

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