2005-10-31 22:34:48

by Roland Dreier

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

Implement reporting asynchronous CQ events in Mellanox HCA driver.

Signed-off-by: Michael S. Tsirkin <[email protected]>
Signed-off-by: Roland Dreier <[email protected]>

---

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


2005-10-31 22:35:01

by Roland Dreier

[permalink] [raw]
Subject: [git patch review 2/5] [IPoIB] use spin_trylock_irqsave()

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 <[email protected]>

---

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

2005-10-31 22:35:23

by Roland Dreier

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

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 <[email protected]>

---

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

2005-10-31 22:35:48

by Roland Dreier

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

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 <[email protected]>

---

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

2005-10-31 22:36:04

by Roland Dreier

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

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 <[email protected]>

---

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