[ofa-general] Re: [PATCHv3 for-2.6.21] IB/mthca: fix race in QP destroy

[Michael S. Tsirkin]

> Quoting Roland Dreier <rdreier at cisco.com>:
> Subject: Re: [PATCHv3 for-2.6.21] IB/mthca: fix race in QP destroy
> 
>  > Looks good, except that spin_lock/spin_unlock need a lock pointer.
> 
> Yep, caught that as soon as I tried to build it...
> 
>  > Here's a (compile tested only) patch for merging async/command queues.
> 
> OK, can you test that and my patch with the test that started this
> thread? If it looks good we can merge it for 2.6.21.

OK. And I think we also need the following on top of this.
Please comment.

commit 1cbe0687501636891ae367ae94b37fce9f73275d
Author: Michael S. Tsirkin <mst at mellanox.co.il>
Date:   Thu Mar 8 08:33:23 2007 +0200

    IB/mthca: sync IRQs on QP reset and destroy
    
    It is common practice to put a pointer/index to a per-QP
    structure inside the wrid. This data is available after poll_cq
    returns, when cq lock is not taken.  If this pointer is used
    directly inside the event handler, the ULP that is moving QP to
    reset has no way to know when is it safe to free data it points to,
    unless the verbs provider synchronizes with the IRQ handler
    before the verbs returns.
    
    Signed-off-by: Michael S. Tsirkin <mst at mellanox.co.il>

---

diff --git a/drivers/infiniband/hw/mthca/mthca_qp.c b/drivers/infiniband/hw/mthca/mthca_qp.c
index 0cba284..3d6591b 100644
--- a/drivers/infiniband/hw/mthca/mthca_qp.c
+++ b/drivers/infiniband/hw/mthca/mthca_qp.c
@@ -37,6 +37,7 @@
 
 #include <linux/string.h>
 #include <linux/slab.h>
+#include <linux/hardirq.h>
 
 #include <asm/io.h>
 
@@ -864,6 +865,11 @@ int mthca_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr, int attr_mask,
 		if (qp->ibqp.send_cq != qp->ibqp.recv_cq)
 			mthca_cq_clean(dev, to_mcq(qp->ibqp.send_cq), qp->qpn, NULL);
 
+		if (dev->mthca_flags & MTHCA_FLAG_MSI_X)
+			synchronize_irq(dev->eq_table.eq[MTHCA_EQ_COMP].msi_x_vector);
+		else
+			synchronize_irq(dev->pdev->irq);
+
 		mthca_wq_reset(&qp->sq);
 		qp->sq.last = get_send_wqe(qp, qp->sq.max - 1);
 
@@ -1420,6 +1426,11 @@ void mthca_free_qp(struct mthca_dev *dev,
 
 	mthca_unlock_cqs(send_cq, recv_cq);
 
+	if (dev->mthca_flags & MTHCA_FLAG_MSI_X)
+		synchronize_irq(dev->eq_table.eq[MTHCA_EQ_COMP].msi_x_vector);
+	else
+		synchronize_irq(dev->pdev->irq);
+
 	wait_event(qp->wait, !get_qp_refcount(dev, qp));
 
 	/*

-- 
MST