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:07:26 +0200 Message-ID: <4C347C0E.6020107@gmail.com> References: <1278507745-32746-1-git-send-email-jirislaby@gmail.com> <20100707130545.GA14934@elte.hu> 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: In-Reply-To: <20100707130545.GA14934@elte.hu> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 07/07/2010 03:05 PM, Ingo Molnar wrote: > > * Jiri Slaby wrote: > >> Stanse found that there is an omitted unlock in kvm_create_pit in one fail >> path. Add proper unlock there. >> >> Signed-off-by: Jiri Slaby >> Cc: Avi Kivity >> Cc: Marcelo Tosatti >> Cc: Thomas Gleixner >> Cc: Ingo Molnar >> Cc: "H. Peter Anvin" >> Cc: x86@kernel.org >> Cc: Gleb Natapov >> Cc: "Michael S. Tsirkin" >> Cc: Gregory Haskins >> Cc: kvm@vger.kernel.org >> --- >> arch/x86/kvm/i8254.c | 1 + >> 1 files changed, 1 insertions(+), 0 deletions(-) >> >> diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c >> index 467cc47..70db4d4 100644 >> --- 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. Do you want one? thanks, -- js