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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
