[openib-general] [PATCH] rcu for event speedup

[Michael S. Tsirkin]

Hi!
The following patch adds an option to avoid taking the cq table lock
on data path, by using RCU.
When enabled, this gets a nice speedup (about 4%) in IP over IB on my
hardware.

But, if you select this option, closing cqs will likely
get much slower.  For this reason I decided we need this
as an option, at least until we are sure its not a problem
for all ulps.

Someone on the list said it might be useful to be able to get code that
does not use RCU at all, looking at the patch you will see this is trivially
easy too.

Roland, you mentioned you will only consider RCU if speedup is
significant. Its up to you now, please decide whether this is
significant enough.

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

Index: hw/mthca/Kconfig
===================================================================
--- hw/mthca/Kconfig	(revision 1653)
+++ hw/mthca/Kconfig	(working copy)
@@ -6,6 +6,15 @@ config INFINIBAND_MTHCA
 	  channel adapters (HCAs), including the MT23108 PCI-X HCA
 	  ("Tavor") and the MT25208 PCI Express HCA ("Arbel").
 
+config INFINIBAND_MTHCA_CQ_RCU
+	bool "Use RCU for CQ table protection"
+	depends on INFINIBAND_MTHCA
+	default y
+	---help---
+	  This option causes the mthca driver to use RCU for
+	  protecting CQ tables.
+	  Using CQs will be faster but destroying them will be slower.
+
 config INFINIBAND_MTHCA_DEBUG
 	bool "Verbose debugging output"
 	depends on INFINIBAND_MTHCA
@@ -180,12 +181,17 @@ void mthca_cq_event(struct mthca_dev *de
 {
 	struct mthca_cq *cq;
 
+#ifdef CONFIG_INFINIBAND_MTHCA_CQ_RCU
+	rcu_read_lock();
+	cq = mthca_array_get(&dev->cq_table.cq, cqn & (dev->limits.num_cqs - 1));
+	rcu_dereference(cq);
+#else
 	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);
-
+#endif
 	if (!cq) {
 		mthca_warn(dev, "Completion event for bogus CQ %08x\n", cqn);
 		return;
@@ -193,8 +199,12 @@ void mthca_cq_event(struct mthca_dev *de
 
 	cq->ibcq.comp_handler(&cq->ibcq, cq->ibcq.cq_context);
 
+#ifdef CONFIG_INFINIBAND_MTHCA_CQ_RCU
+	rcu_read_unlock();
+#else
 	if (atomic_dec_and_test(&cq->refcount))
 		wake_up(&cq->wait);
+#endif
 }
 
 void mthca_cq_clean(struct mthca_dev *dev, u32 cqn, u32 qpn)
@@ -785,6 +794,10 @@ void mthca_free_cq(struct mthca_dev *dev
 			  cq->cqn & (dev->limits.num_cqs - 1));
 	spin_unlock_irq(&dev->cq_table.lock);
 
+#ifdef CONFIG_INFINIBAND_MTHCA_CQ_RCU
+	synchronize_kernel();
+#endif
+
 	atomic_dec(&cq->refcount);
 	wait_event(cq->wait, !atomic_read(&cq->refcount));
 

-- 
I dont speak for Mellanox