On Tue, Dec 30, 2008 at 10:14:09AM +0800, Sheng Yang wrote: > > There is one remaining issue: kvm_assigned_dev_interrupt_work_handler > > can re-enable the interrupt for KVM_ASSIGNED_DEV_GUEST_MSI case. Perhaps > > you need a new flag to indicate shutdown (so the host IRQ won't be > > reenabled). > > Is it already covered by disable_irq_no_sync() before cancel_work_sync()? > I've > noted this in my comment: the irq may be disabled nested(once for MSI and > twice for INTx), but I think it's fine for we're going to free it.
The problem is that the irq can be re-enabled in kvm_assigned_dev_interrupt_work_handler: context 1 | context 2 disable_irq_nosync kvm_assigned_dev_interrupt_work_handler enable_irq cancel_work_sync free_irq So between cancel_work_sync and free_irq kvm_assigned_dev_intr can run and schedule work. I guess it is OK to take the kvm mutex in kvm_free_assigned_irq (but better verify), so it can: mutex_lock assigned_dev->irq_requested_type = 0; mutex_unlock disable_irq_nosync cancel_work_sync free_irq So that the work handler won't re-enable the interrupt. Other than that the latest patchset looks good. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html