Thanks to Marcelo's observation, The following code have potential issue:
if (cancel_work_sync(&assigned_dev->interrupt_work))
kvm_put_kvm(kvm);
In fact, cancel_work_sync() would return true either work struct is only
scheduled or the callback of work struct is executed. This code only
consider the former situation.
Also, we have a window between cancel_work_sync() and free_irq. This patch fixs
them two.
Signed-off-by: Sheng Yang <[email protected]>
---
virt/kvm/kvm_main.c | 26 ++++++++++++++++++++++----
1 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1863942..ed10f15 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -186,10 +186,28 @@ static void kvm_free_assigned_irq(struct kvm *kvm,
if (!assigned_dev->irq_requested_type)
return;
- if (cancel_work_sync(&assigned_dev->interrupt_work))
- /* We had pending work. That means we will have to take
- * care of kvm_put_kvm.
- */
+ /*
+ * We need to ensure: kvm_put_kvm() paired with kvm_get_kvm() in
+ * kvm_assigned_dev_intr, and no more interrupt after we cancelled
+ * current one.
+ *
+ * Here we have two possiblities for cancel_work_sync() return true:
+ * 1. The work is scheduled, but callback haven't been called. We need
+ * to call kvm_put_kvm() here. And IRQ is already disabled without
+ * doubt.
+ *
+ * 2. The callback have executed, here we don't need to call
+ * kvm_put_kvm(), but we may need to disable irq(e.g. for MSI).
+ *
+ * We judge the two condition according to if we have pending IRQs in
+ * irq_fifo.
+ */
+ if (kfifo_len(assigned_dev->irq_fifo) == 0 &&
+ (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI))
+ disable_irq_nosync(assigned_dev->host_irq);
+
+ if (cancel_work_sync(&assigned_dev->interrupt_work) &&
+ kfifo_len(assigned_dev->irq_fifo) != 0)
kvm_put_kvm(kvm);
free_irq(assigned_dev->host_irq, (void *)assigned_dev);
--
1.5.4.5
--
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
