[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 07 Jul 2010 15:12:15 +0200
From: Jiri Slaby <jirislaby@...il.com>
To: Ingo Molnar <mingo@...e.hu>
CC: avi@...hat.com, linux-kernel@...r.kernel.org,
Marcelo Tosatti <mtosatti@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
Gleb Natapov <gleb@...hat.com>,
"Michael S. Tsirkin" <mst@...hat.com>,
Gregory Haskins <ghaskins@...ell.com>, kvm@...r.kernel.org
Subject: Re: [PATCH] x86: KVM, fix lock imbalance
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 linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists