Re: [PATCH 2/3] KVM: Use kvm_free_assigned_irq() for free irq

[Sheng Yang <sheng@linux.intel.com>]

On Tuesday 30 December 2008 18:28:21 Avi Kivity wrote:
> Sheng Yang wrote:
> > Which is more convenient...
> >
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > ---
> >  virt/kvm/kvm_main.c |   10 ++--------
> >  1 files changed, 2 insertions(+), 8 deletions(-)
> >
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index ffd261d..cd84b3e 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -284,11 +284,7 @@ static int assigned_device_update_intx(struct kvm
> > *kvm, return 0;
> >
> >  	if (irqchip_in_kernel(kvm)) {
> > -		if (!msi2intx &&
> > -		    adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI) {
> > -			free_irq(adev->host_irq, (void *)kvm);
> > -			pci_disable_msi(adev->dev);
> > -		}
> > +		kvm_free_assigned_irq(kvm, adev);
>
> This will call cancel_work_sync(), which may wait upon kvm->lock, which
> I think we hold here -> deadlock.
>
> I think that the current code has even bigger problems (races), since a
> scheduled work can arrive after the interrupt has been freed and
> reallocated.

For the race problem, how about put a cancel_work_sync() for all devices at 
the beginning of kvm_destroy_vm? Something named kvm_arch_cancel_work_sync...

-- 
regards
Yang, Sheng