Re: [Qemu-devel] [PATCH 09/14] migration: take finer locking

[Juan Quintela <quintela@redhat.com>]

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)
>>