On 07/07/2010 03:07 PM, Jiri Slaby wrote:
quoted text
>>> --- 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
--
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/