[openib-general] [PATCH] (final) reduce qp locking on cq poll

[Michael S. Tsirkin]

Here it is, fixed. Untested, but I only touched the comments ...

> >     Michael> Locking during the poll cq operation can be reduced, by
> >     Michael> locking the cq while qp is being removed from the qp
> >     Michael> array.  This also avoids an extra atomic operation for
> >     Michael> reference counting.

Signed-off-by: Michael S. Tsirkin <mst at mellanox.co.il>

Index: hw/mthca/mthca_cq.c
===================================================================
--- hw/mthca/mthca_cq.c	(revision 1663)
+++ hw/mthca/mthca_cq.c	(working copy)
@@ -420,14 +420,11 @@ static inline int mthca_poll_one(struct 
 				wake_up(&(*cur_qp)->wait);
 		}
 
-		spin_lock(&dev->qp_table.lock);
+		/* We do not have to take the qp table lock here, because
+		   cq was locked while qp was removed from the array */
 		*cur_qp = mthca_array_get(&dev->qp_table.qp,
 					  be32_to_cpu(cqe->my_qpn) &
 					  (dev->limits.num_qps - 1));
-		if (*cur_qp)
-			atomic_inc(&(*cur_qp)->refcount);
-		spin_unlock(&dev->qp_table.lock);
-
 		if (!*cur_qp) {
 			mthca_warn(dev, "CQ entry for unknown QP %06x\n",
 				   be32_to_cpu(cqe->my_qpn) & 0xffffff);
@@ -541,8 +538,6 @@ int mthca_poll_cq(struct ib_cq *ibcq, in
 
 	if (qp) {
 		spin_unlock(&qp->lock);
-		if (atomic_dec_and_test(&qp->refcount))
-			wake_up(&qp->wait);
 	}
 
 
Index: hw/mthca/TODO
===================================================================
--- hw/mthca/TODO	(revision 1663)
+++ hw/mthca/TODO	(working copy)
@@ -11,11 +11,6 @@ Small projects (extensions to existing c
         into HCA memory.
     Miscellaneous verbs: At least query AH, query QP and resize CQ are
         not implemented.
-    Reduce locking for CQ poll: The reference counting used to prevent
-        CQs and QPs from being destroyed while events are being
-        dispatched could be eliminated by locking EQs (and, for QPs,
-        the associated CQs) during destroy operations.  This saves an
-        atomic access in the CQ poll fast path.
     Reduce doorbell locking on 32-bit architectures: By using a
         a doorbell page dedicated to each EQ, no lock is required in
         the EQ poll path.
Index: hw/mthca/mthca_qp.c
===================================================================
--- hw/mthca/mthca_qp.c	(revision 1663)
+++ hw/mthca/mthca_qp.c	(working copy)
@@ -1083,9 +1083,19 @@ int mthca_alloc_sqp(struct mthca_dev *de
 	return 0;
 
  err_out_free:
-	spin_lock_irq(&dev->qp_table.lock);
+	/* Lock cq here, so that cq polling code
+	   can do qp lookup without taking a lock */
+	spin_lock_irq(&send_cq->lock);
+	if (send_cq != recv_cq)
+		spin_lock(&recv_cq->lock);
+
+	spin_lock(&dev->qp_table.lock);
 	mthca_array_clear(&dev->qp_table.qp, mqpn);
-	spin_unlock_irq(&dev->qp_table.lock);
+	spin_unlock(&dev->qp_table.lock);
+
+	if (send_cq != recv_cq)
+		spin_unlock(&recv_cq->lock);
+	spin_unlock_irq(&send_cq->lock);
 
  err_out:
 	dma_free_coherent(&dev->pdev->dev, sqp->header_buf_size,
@@ -1100,11 +1110,26 @@ void mthca_free_qp(struct mthca_dev *dev
 	u8 status;
 	int size;
 	int i;
+	struct mthca_cq *send_cq;
+	struct mthca_cq *recv_cq;
+
+	send_cq = to_mcq(qp->ibqp.send_cq);
+	recv_cq = to_mcq(qp->ibqp.recv_cq);
 
-	spin_lock_irq(&dev->qp_table.lock);
+	/* Lock cq here, so that cq polling code
+	   can do qp lookup without taking a lock */
+	spin_lock_irq(&send_cq->lock);
+	if (send_cq != recv_cq)
+		spin_lock(&recv_cq->lock);
+
+	spin_lock(&dev->qp_table.lock);
 	mthca_array_clear(&dev->qp_table.qp,
 			  qp->qpn & (dev->limits.num_qps - 1));
-	spin_unlock_irq(&dev->qp_table.lock);
+	spin_unlock(&dev->qp_table.lock);
+
+	if (send_cq != recv_cq)
+		spin_unlock(&recv_cq->lock);
+	spin_unlock_irq(&send_cq->lock);
 
 	atomic_dec(&qp->refcount);
 	wait_event(qp->wait, !atomic_read(&qp->refcount));
-- 
I dont speak for Mellanox