From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 15/15] KVM: Fix racy in kvm_free_assigned_irq Date: Mon, 29 Dec 2008 15:20:09 +0200 Message-ID: <4958CE89.4050600@redhat.com> References: <20081225115609.GA10087@syang10-desktop> <1230258607-15208-1-git-send-email-sheng@linux.intel.com> <20081227200626.GA4095@amt.cnet> <20081228112402.GB2610@syang10-desktop> <20081229054222.GB13166@shell.devel.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Cc: Sheng Yang , Marcelo Tosatti , kvm@vger.kernel.org, Amit Shah , "Han, Weidong" To: Amit Shah Return-path: Received: from mx2.redhat.com ([66.187.237.31]:54390 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752483AbYL2NT4 (ORCPT ); Mon, 29 Dec 2008 08:19:56 -0500 In-Reply-To: <20081229054222.GB13166@shell.devel.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Amit Shah wrote: > I see from the call chain kvm_put_kvm->...->kvm_arch_destroy_vm, no locks are > taken to actually destroy the vm. We can't be in ioctls, sure. But shouldn't > the mutex be taken to ensure there's nothing else going on while destroying? > > Locks are useless to guard against something happening concurrent with destruction, since we're about to destroy the lock. > At least with the workqueue model, we could be called asynchronously in kernel > context and I would have held the mutex and about to inject interrupts while > everything is being wiped off underneath. However, the workqueue model tries > its best to schedule the work on the same CPU, though we can't use that > guarantee to ensure things will be fine. > > --- > So I had to get a ref to the current vm till we had any pending work scheduled. I think that's the right thing to do. > I think I put in comments in the code, but sadly most of my comments we stripped out before the merge. > Pity. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.