Re: [PATCH 15/15] KVM: Fix racy in kvm_free_assigned_irq

[Marcelo Tosatti <mtosatti@redhat.com>]

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.