[git patch review 1/5] [IB] mthca: report asynchronous CQ events

[git patch review 1/5] [IB] mthca: report asynchronous CQ events

[Roland Dreier]

Implement reporting asynchronous CQ events in Mellanox HCA driver.

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>
Signed-off-by: Roland Dreier <rolandd@cisco.com>

---

 drivers/infiniband/hw/mthca/mthca_cq.c  |   31 ++++++++++++++++++++++++++++++-
 drivers/infiniband/hw/mthca/mthca_dev.h |    4 +++-
 drivers/infiniband/hw/mthca/mthca_eq.c  |    4 +++-
 3 files changed, 36 insertions(+), 3 deletions(-)

applies-to: d918cd1ba0ef9afa692cef281afee2f6d6634a1e
affcd50546d4788b7849e2b2e2ec7bc50d64c5f8
diff --git a/drivers/infiniband/hw/mthca/mthca_cq.c b/drivers/infiniband/hw/mthca/mthca_cq.c
index 8600b6c..f98e235 100644
--- a/drivers/infiniband/hw/mthca/mthca_cq.c
+++ b/drivers/infiniband/hw/mthca/mthca_cq.c
@@ -208,7 +208,7 @@ static inline void update_cons_index(str
 	}
 }
 
-void mthca_cq_event(struct mthca_dev *dev, u32 cqn)
+void mthca_cq_completion(struct mthca_dev *dev, u32 cqn)
 {
 	struct mthca_cq *cq;
 
@@ -224,6 +224,35 @@ void mthca_cq_event(struct mthca_dev *de
 	cq->ibcq.comp_handler(&cq->ibcq, cq->ibcq.cq_context);
 }
 
+void mthca_cq_event(struct mthca_dev *dev, u32 cqn,
+		    enum ib_event_type event_type)
+{
+	struct mthca_cq *cq;
+	struct ib_event event;
+
+	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);
+
+	if (!cq) {
+		mthca_warn(dev, "Async event for bogus CQ %08x\n", cqn);
+		return;
+	}
+
+	event.device      = &dev->ib_dev;
+	event.event       = event_type;
+	event.element.cq  = &cq->ibcq;
+	if (cq->ibcq.event_handler)
+		cq->ibcq.event_handler(&event, 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,
 		    struct mthca_srq *srq)
 {
diff --git a/drivers/infiniband/hw/mthca/mthca_dev.h b/drivers/infiniband/hw/mthca/mthca_dev.h
index 7e68bd4..e7e5d3b 100644
--- a/drivers/infiniband/hw/mthca/mthca_dev.h
+++ b/drivers/infiniband/hw/mthca/mthca_dev.h
@@ -460,7 +460,9 @@ int mthca_init_cq(struct mthca_dev *dev,
 		  struct mthca_cq *cq);
 void mthca_free_cq(struct mthca_dev *dev,
 		   struct mthca_cq *cq);
-void mthca_cq_event(struct mthca_dev *dev, u32 cqn);
+void mthca_cq_completion(struct mthca_dev *dev, u32 cqn);
+void mthca_cq_event(struct mthca_dev *dev, u32 cqn,
+		    enum ib_event_type event_type);
 void mthca_cq_clean(struct mthca_dev *dev, u32 cqn, u32 qpn,
 		    struct mthca_srq *srq);
 
diff --git a/drivers/infiniband/hw/mthca/mthca_eq.c b/drivers/infiniband/hw/mthca/mthca_eq.c
index e5a047a..34d68e5 100644
--- a/drivers/infiniband/hw/mthca/mthca_eq.c
+++ b/drivers/infiniband/hw/mthca/mthca_eq.c
@@ -292,7 +292,7 @@ static int mthca_eq_int(struct mthca_dev
 		case MTHCA_EVENT_TYPE_COMP:
 			disarm_cqn = be32_to_cpu(eqe->event.comp.cqn) & 0xffffff;
 			disarm_cq(dev, eq->eqn, disarm_cqn);
-			mthca_cq_event(dev, disarm_cqn);
+			mthca_cq_completion(dev, disarm_cqn);
 			break;
 
 		case MTHCA_EVENT_TYPE_PATH_MIG:
@@ -364,6 +364,8 @@ static int mthca_eq_int(struct mthca_dev
 				   eqe->event.cq_err.syndrome == 1 ?
 				   "overrun" : "access violation",
 				   be32_to_cpu(eqe->event.cq_err.cqn) & 0xffffff);
+			mthca_cq_event(dev, be32_to_cpu(eqe->event.cq_err.cqn),
+				       IB_EVENT_CQ_ERR);
 			break;
 
 		case MTHCA_EVENT_TYPE_EQ_OVERFLOW:
---
0.99.9

[git patch review 2/5] [IPoIB] use spin_trylock_irqsave()

[Roland Dreier]

Use spin_trylock_irqsave() in ipoib_start_xmit() instead of
reinventing it out of local_irq_save(), spin_trylock() and
local_irq_restore().

Signed-off-by: Roland Dreier <rolandd@cisco.com>

---

 drivers/infiniband/ulp/ipoib/ipoib_main.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

applies-to: e4e6a0f5f2203569b6ada4c101a146c3a4f24c28
a20583a7c2e35d80b1dfc1f60c9729498838725e
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index cd4f423..273d5f4 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -551,11 +551,8 @@ static int ipoib_start_xmit(struct sk_bu
 	struct ipoib_neigh *neigh;
 	unsigned long flags;
 
-	local_irq_save(flags);
-	if (!spin_trylock(&priv->tx_lock)) {
-		local_irq_restore(flags);
+	if (!spin_trylock_irqsave(&priv->tx_lock, flags))
 		return NETDEV_TX_LOCKED;
-	}
 
 	/*
 	 * Check if our queue is stopped.  Since we have the LLTX bit
---
0.99.9

[git patch review 4/5] [IB] mthca: Avoid SRQ free WQE list corruption

[Roland Dreier]

Fix wqe_to_link() to use a structure field that we know is definitely
always unused for receive work requests, so that it really avoids the
free list corruption bug that the comment claims it does.

Signed-off-by: Roland Dreier <rolandd@cisco.com>

---

 drivers/infiniband/hw/mthca/mthca_srq.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

applies-to: ffd7eba03f29dd2932dd32ac4adc2921bde7644b
e5b251a24a9cd34a7ef98e361eb94e7ab122a554
diff --git a/drivers/infiniband/hw/mthca/mthca_srq.c b/drivers/infiniband/hw/mthca/mthca_srq.c
index 64f70aa..292f55b 100644
--- a/drivers/infiniband/hw/mthca/mthca_srq.c
+++ b/drivers/infiniband/hw/mthca/mthca_srq.c
@@ -75,15 +75,16 @@ static void *get_wqe(struct mthca_srq *s
 
 /*
  * Return a pointer to the location within a WQE that we're using as a
- * link when the WQE is in the free list.  We use an offset of 4
- * because in the Tavor case, posting a WQE may overwrite the first
- * four bytes of the previous WQE.  The offset avoids corrupting our
- * free list if the WQE has already completed and been put on the free
- * list when we post the next WQE.
+ * link when the WQE is in the free list.  We use the imm field
+ * because in the Tavor case, posting a WQE may overwrite the next
+ * segment of the previous WQE, but a receive WQE will never touch the
+ * imm field.  This avoids corrupting our free list if the previous
+ * WQE has already completed and been put on the free list when we
+ * post the next WQE.
  */
 static inline int *wqe_to_link(void *wqe)
 {
-	return (int *) (wqe + 4);
+	return (int *) (wqe + offsetof(struct mthca_next_seg, imm));
 }
 
 static void mthca_tavor_init_srq_context(struct mthca_dev *dev,
---
0.99.9

[git patch review 3/5] [IB] uverbs: Avoid NULL pointer deref on CQ async event

[Roland Dreier]

Userspace CQs that have no completion event channel attached end up
with their cq_context set to NULL.  However, asynchronous events like
"CQ overrun" can still occur on such CQs, so add a uverbs_file member
to struct ib_ucq_object that we can follow to deliver these events.

Signed-off-by: Roland Dreier <rolandd@cisco.com>

---

 drivers/infiniband/core/uverbs.h      |    1 +
 drivers/infiniband/core/uverbs_cmd.c  |    1 +
 drivers/infiniband/core/uverbs_main.c |    9 +++------
 3 files changed, 5 insertions(+), 6 deletions(-)

applies-to: e7fbd856e7522b65d309e9dfd425541d8f45a0bd
7162a3e0db34e914a8bc5bf74bbae0b386310cf8
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index 031cdf3..ecb8301 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -113,6 +113,7 @@ struct ib_uevent_object {
 
 struct ib_ucq_object {
 	struct ib_uobject	uobject;
+	struct ib_uverbs_file  *uverbs_file;
 	struct list_head	comp_list;
 	struct list_head	async_list;
 	u32			comp_events_reported;
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 8c89abc..63a7415 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -602,6 +602,7 @@ ssize_t ib_uverbs_create_cq(struct ib_uv
 
 	uobj->uobject.user_handle   = cmd.user_handle;
 	uobj->uobject.context       = file->ucontext;
+	uobj->uverbs_file	    = file;
 	uobj->comp_events_reported  = 0;
 	uobj->async_events_reported = 0;
 	INIT_LIST_HEAD(&uobj->comp_list);
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 0eb38f4..e58a7b2 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -442,13 +442,10 @@ static void ib_uverbs_async_handler(stru
 
 void ib_uverbs_cq_event_handler(struct ib_event *event, void *context_ptr)
 {
-	struct ib_uverbs_event_file *ev_file = context_ptr;
-	struct ib_ucq_object *uobj;
+	struct ib_ucq_object *uobj = container_of(event->element.cq->uobject,
+						  struct ib_ucq_object, uobject);
 
-	uobj = container_of(event->element.cq->uobject,
-			    struct ib_ucq_object, uobject);
-
-	ib_uverbs_async_handler(ev_file->uverbs_file, uobj->uobject.user_handle,
+	ib_uverbs_async_handler(uobj->uverbs_file, uobj->uobject.user_handle,
 				event->event, &uobj->async_list,
 				&uobj->async_events_reported);
 				
---
0.99.9

[git patch review 5/5] [IPoIB] cleanups: fix comment, remove useless variables

[Roland Dreier]

Minor cleanups: fix a misleading comment, and get rid of attr_mask
variables that are only used to hold constants (just use the constants
directly).

Signed-off-by: Roland Dreier <rolandd@cisco.com>

---

 drivers/infiniband/ulp/ipoib/ipoib_ib.c    |   12 ++++++------
 drivers/infiniband/ulp/ipoib/ipoib_verbs.c |    4 +---
 2 files changed, 7 insertions(+), 9 deletions(-)

applies-to: c29760bafd7107252389712965ad7e4ed0791a82
3bc12e75b23c0499cc2c0873a5f77494be173761
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
index 192fef8..0a6f578 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
@@ -486,15 +486,16 @@ int ipoib_ib_dev_stop(struct net_device 
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	struct ib_qp_attr qp_attr;
-	int attr_mask;
 	unsigned long begin;
 	struct ipoib_tx_buf *tx_req;
 	int i;
 
-	/* Kill the existing QP and allocate a new one */
+	/*
+	 * Move our QP to the error state and then reinitialize in
+	 * when all work requests have completed or have been flushed.
+	 */
 	qp_attr.qp_state = IB_QPS_ERR;
-	attr_mask        = IB_QP_STATE;
-	if (ib_modify_qp(priv->qp, &qp_attr, attr_mask))
+	if (ib_modify_qp(priv->qp, &qp_attr, IB_QP_STATE))
 		ipoib_warn(priv, "Failed to modify QP to ERROR state\n");
 
 	/* Wait for all sends and receives to complete */
@@ -541,8 +542,7 @@ int ipoib_ib_dev_stop(struct net_device 
 
 timeout:
 	qp_attr.qp_state = IB_QPS_RESET;
-	attr_mask        = IB_QP_STATE;
-	if (ib_modify_qp(priv->qp, &qp_attr, attr_mask))
+	if (ib_modify_qp(priv->qp, &qp_attr, IB_QP_STATE))
 		ipoib_warn(priv, "Failed to modify QP to RESET state\n");
 
 	/* Wait for all AHs to be reaped */
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_verbs.c b/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
index b5902a7..e829e10 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
@@ -41,7 +41,6 @@ int ipoib_mcast_attach(struct net_device
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	struct ib_qp_attr *qp_attr;
-	int attr_mask;
 	int ret;
 	u16 pkey_index;
 
@@ -59,8 +58,7 @@ int ipoib_mcast_attach(struct net_device
 
 	/* set correct QKey for QP */
 	qp_attr->qkey = priv->qkey;
-	attr_mask = IB_QP_QKEY;
-	ret = ib_modify_qp(priv->qp, qp_attr, attr_mask);
+	ret = ib_modify_qp(priv->qp, qp_attr, IB_QP_QKEY);
 	if (ret) {
 		ipoib_warn(priv, "failed to modify QP, ret = %d\n", ret);
 		goto out;
---
0.99.9