From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Slaby Subject: Re: [PATCH] x86: KVM, fix lock imbalance Date: Wed, 07 Jul 2010 15:12:15 +0200 Message-ID: <4C347D2F.3080908@gmail.com> References: <1278507745-32746-1-git-send-email-jirislaby@gmail.com> <20100707130545.GA14934@elte.hu> <4C347C0E.6020107@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: avi@redhat.com, linux-kernel@vger.kernel.org, Marcelo Tosatti , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, Gleb Natapov , "Michael S. Tsirkin" , Gregory Haskins , kvm@vger.kernel.org To: Ingo Molnar Return-path: Received: from mail-bw0-f46.google.com ([209.85.214.46]:49377 "EHLO mail-bw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752838Ab0GGNMU (ORCPT ); Wed, 7 Jul 2010 09:12:20 -0400 In-Reply-To: <4C347C0E.6020107@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: 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