On 07/07/2010 03:07 PM, Jiri Slaby wrote:
>>> --- a/arch/x86/kvm/i8254.c
>>> +++ b/arch/x86/kvm/i8254.c
>>> @@ -696,6 +696,7 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 
>>> flags)
>>>  
>>>     pit->wq = create_singlethread_workqueue("kvm-pit-wq");
>>>     if (!pit->wq) {
>>> +           mutex_unlock(&pit->pit_state.lock);
>>>             kfree(pit);
>>>             return NULL;
>>>     }
>>
>> A cleanliness comment: why is that tear-down/dealloc sequence open-coded? It 
>> should be at the end of the function, with goto labels, like we do it in 
>> similar cases.
> 
> Because the lock is around a block only. I usually don't create a goto
> fail-paths in these cases.

To be more precise what I mean by that:
if ()
  return;

lock();
...
if () { [single if inside the crit section]
  unlock();
  return;
}
...
unlock()

...
if ()
  return;
...
if ()
  return;

-- 
js
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to