On 07/07/2010 03:05 PM, Ingo Molnar wrote: > > * Jiri Slaby <jirislaby@xxxxxxxxx> 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 <jirislaby@xxxxxxxxx> >> Cc: Avi Kivity <avi@xxxxxxxxxx> >> Cc: Marcelo Tosatti <mtosatti@xxxxxxxxxx> >> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx> >> Cc: Ingo Molnar <mingo@xxxxxxxxxx> >> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx> >> Cc: x86@xxxxxxxxxx >> Cc: Gleb Natapov <gleb@xxxxxxxxxx> >> Cc: "Michael S. Tsirkin" <mst@xxxxxxxxxx> >> Cc: Gregory Haskins <ghaskins@xxxxxxxxxx> >> Cc: kvm@xxxxxxxxxxxxxxx >> --- >> 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html