[openib-general] [PATCH] remove locking for cq event (was: rcu for event speedup)

[Michael S. Tsirkin]

Quoting r. Roland Dreier <roland at topspin.com>:
> Subject: Re: [PATCH] rcu for event speedup
> 
>     Michael> Hi!  The following patch adds an option to avoid taking
>     Michael> the cq table lock on data path, by using RCU.  When
>     Michael> enabled, this gets a nice speedup (about 4%) in IP over
>     Michael> IB on my hardware.
> 
> I don't think this requires using RCU, since you're not really using
> anything from rcupdate.h except synchronize_kernel(), and even that is
> unnecessary.  I think this could be made a lot simpler by just using
> explicit memory barriers and synchronize_irq().  That should also
> remove the problem with slowing down the free operation so that this
> doesn't need to be made conditional.

I think you are right. Attached.

> (By the way, did you hand edit the patch?  There's no filename header
> between the Kconfig chunk and the mthca_cq.c chunks, so the patch
> won't apply)
> 
>  - R.

Yes, sorry.

Index: hw/mthca/mthca_cq.c
===================================================================
--- hw/mthca/mthca_cq.c	(revision 1653)
+++ hw/mthca/mthca_cq.c	(working copy)
@@ -180,11 +180,8 @@ void mthca_cq_event(struct mthca_dev *de
 {
 	struct mthca_cq *cq;
 
-	spin_lock(&dev->cq_table.lock);
 	cq = mthca_array_get(&dev->cq_table.cq, cqn & (dev->limits.num_cqs - 1));
-	if (cq)
-		atomic_inc(&cq->refcount);
-	spin_unlock(&dev->cq_table.lock);
+	read_barrier_depends();
 
 	if (!cq) {
 		mthca_warn(dev, "Completion event for bogus CQ %08x\n", cqn);
@@ -192,9 +189,6 @@ void mthca_cq_event(struct mthca_dev *de
 	}
 
 	cq->ibcq.comp_handler(&cq->ibcq, cq->ibcq.cq_context);
-
-	if (atomic_dec_and_test(&cq->refcount))
-		wake_up(&cq->wait);
 }
 
 void mthca_cq_clean(struct mthca_dev *dev, u32 cqn, u32 qpn)
@@ -785,6 +779,15 @@ void mthca_free_cq(struct mthca_dev *dev
 			  cq->cqn & (dev->limits.num_cqs - 1));
 	spin_unlock_irq(&dev->cq_table.lock);
 
+	wmb();
+
+	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);
+
+
+
 	atomic_dec(&cq->refcount);
 	wait_event(cq->wait, !atomic_read(&cq->refcount));
 
-- 
I dont speak for Mellanox