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

Sheng Yang
2008-12-30T11:11:14Z

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