[openib-general] [PATCH] fix cleanup in MAD code when unloading HCA driver
Sean Hefty
mshefty at ichips.intel.com
Mon Nov 15 15:48:26 PST 2004
After looking at the code, I believe that there's a race condition
cleaning up in the MAD code when unloading the HCA driver. The
MAD layer can be processing a received MAD when the driver unloads,
which can result in accessing the receive queue after all MADs
on the receive queue have been freed.
This patch should correct that issue, by delaying cleanup of
the receive queues until after processing completions. A
similar fix is applied recovering from errors when initializing
the port.
- Sean
Index: core/mad.c
===================================================================
--- core/mad.c (revision 1237)
+++ core/mad.c (working copy)
@@ -1677,7 +1677,7 @@
/*
* Return all the posted receive MADs
*/
-static void ib_mad_return_posted_recv_mads(struct ib_mad_qp_info *qp_info)
+static void cleanup_recv_queue(struct ib_mad_qp_info *qp_info)
{
struct ib_mad_private_header *mad_priv_hdr;
struct ib_mad_private *recv;
@@ -1737,7 +1737,7 @@
if (ret) {
printk(KERN_ERR PFX "Couldn't change QP%d state to "
"INIT: %d\n", i, ret);
- goto error;
+ goto out;
}
attr->qp_state = IB_QPS_RTR;
@@ -1745,7 +1745,7 @@
if (ret) {
printk(KERN_ERR PFX "Couldn't change QP%d state to "
"RTR: %d\n", i, ret);
- goto error;
+ goto out;
}
attr->qp_state = IB_QPS_RTS;
@@ -1754,7 +1754,7 @@
if (ret) {
printk(KERN_ERR PFX "Couldn't change QP%d state to "
"RTS: %d\n", i, ret);
- goto error;
+ goto out;
}
}
@@ -1762,58 +1762,21 @@
if (ret) {
printk(KERN_ERR PFX "Failed to request completion "
"notification: %d\n", ret);
- goto error;
+ goto out;
}
for (i = 0; i < IB_MAD_QPS_CORE; i++) {
ret = ib_mad_post_receive_mads(&port_priv->qp_info[i], NULL);
if (ret) {
printk(KERN_ERR PFX "Couldn't post receive WRs\n");
- goto error;
+ goto out;
}
}
- goto out;
-
-error:
- for (i = 0; i < IB_MAD_QPS_CORE; i++) {
- attr->qp_state = IB_QPS_RESET;
- ret = ib_modify_qp(port_priv->qp_info[i].qp, attr, IB_QP_STATE);
- ib_mad_return_posted_recv_mads(&port_priv->qp_info[i]);
- }
-
out:
kfree(attr);
return ret;
}
-/*
- * Stop the port
- */
-static void ib_mad_port_stop(struct ib_mad_port_private *port_priv)
-{
- int i, ret;
- struct ib_qp_attr *attr;
-
- attr = kmalloc(sizeof *attr, GFP_KERNEL);
- if (attr) {
- attr->qp_state = IB_QPS_RESET;
- for (i = 0; i < IB_MAD_QPS_CORE; i++) {
- ret = ib_modify_qp(port_priv->qp_info[i].qp, attr,
- IB_QP_STATE);
- if (ret)
- printk(KERN_ERR PFX "ib_mad_port_stop: "
- "Couldn't change %s port %d QP%d "
- "state to RESET\n",
- port_priv->device->name,
- port_priv->port_num, i);
- }
- kfree(attr);
- }
-
- for (i = 0; i < IB_MAD_QPS_CORE; i++)
- ib_mad_return_posted_recv_mads(&port_priv->qp_info[i]);
-}
-
static void qp_event_handler(struct ib_event *event, void *qp_context)
{
struct ib_mad_qp_info *qp_info = qp_context;
@@ -1832,21 +1795,24 @@
INIT_LIST_HEAD(&mad_queue->list);
}
-static int create_mad_qp(struct ib_mad_port_private *port_priv,
- struct ib_mad_qp_info *qp_info,
- enum ib_qp_type qp_type)
+static void init_mad_qp(struct ib_mad_port_private *port_priv,
+ struct ib_mad_qp_info *qp_info)
{
- struct ib_qp_init_attr qp_init_attr;
- int ret;
-
qp_info->port_priv = port_priv;
init_mad_queue(qp_info, &qp_info->send_queue);
init_mad_queue(qp_info, &qp_info->recv_queue);
INIT_LIST_HEAD(&qp_info->overflow_list);
+}
+
+static int create_mad_qp(struct ib_mad_qp_info *qp_info,
+ enum ib_qp_type qp_type)
+{
+ struct ib_qp_init_attr qp_init_attr;
+ int ret;
memset(&qp_init_attr, 0, sizeof qp_init_attr);
- qp_init_attr.send_cq = port_priv->cq;
- qp_init_attr.recv_cq = port_priv->cq;
+ qp_init_attr.send_cq = qp_info->port_priv->cq;
+ qp_init_attr.recv_cq = qp_info->port_priv->cq;
qp_init_attr.sq_sig_type = IB_SIGNAL_ALL_WR;
qp_init_attr.rq_sig_type = IB_SIGNAL_ALL_WR;
qp_init_attr.cap.max_send_wr = IB_MAD_QP_SEND_SIZE;
@@ -1854,10 +1820,10 @@
qp_init_attr.cap.max_send_sge = IB_MAD_SEND_REQ_MAX_SG;
qp_init_attr.cap.max_recv_sge = IB_MAD_RECV_REQ_MAX_SG;
qp_init_attr.qp_type = qp_type;
- qp_init_attr.port_num = port_priv->port_num;
+ qp_init_attr.port_num = qp_info->port_priv->port_num;
qp_init_attr.qp_context = qp_info;
qp_init_attr.event_handler = qp_event_handler;
- qp_info->qp = ib_create_qp(port_priv->pd, &qp_init_attr);
+ qp_info->qp = ib_create_qp(qp_info->port_priv->pd, &qp_init_attr);
if (IS_ERR(qp_info->qp)) {
printk(KERN_ERR PFX "Couldn't create ib_mad QP%d\n",
get_spl_qp_index(qp_type));
@@ -1903,11 +1869,13 @@
printk(KERN_ERR PFX "No memory for ib_mad_port_private\n");
return -ENOMEM;
}
-
memset(port_priv, 0, sizeof *port_priv);
port_priv->device = device;
port_priv->port_num = port_num;
spin_lock_init(&port_priv->reg_lock);
+ INIT_LIST_HEAD(&port_priv->agent_list);
+ init_mad_qp(port_priv, &port_priv->qp_info[0]);
+ init_mad_qp(port_priv, &port_priv->qp_info[1]);
cq_size = (IB_MAD_QP_SEND_SIZE + IB_MAD_QP_RECV_SIZE) * 2;
port_priv->cq = ib_create_cq(port_priv->device,
@@ -1934,16 +1902,13 @@
goto error5;
}
- ret = create_mad_qp(port_priv, &port_priv->qp_info[0], IB_QPT_SMI);
+ ret = create_mad_qp(&port_priv->qp_info[0], IB_QPT_SMI);
if (ret)
goto error6;
- ret = create_mad_qp(port_priv, &port_priv->qp_info[1], IB_QPT_GSI);
+ ret = create_mad_qp(&port_priv->qp_info[1], IB_QPT_GSI);
if (ret)
goto error7;
- spin_lock_init(&port_priv->reg_lock);
- INIT_LIST_HEAD(&port_priv->agent_list);
-
port_priv->wq = create_workqueue("ib_mad");
if (!port_priv->wq) {
ret = -ENOMEM;
@@ -1974,6 +1939,8 @@
ib_dealloc_pd(port_priv->pd);
error4:
ib_destroy_cq(port_priv->cq);
+ cleanup_recv_queue(&port_priv->qp_info[1]);
+ cleanup_recv_queue(&port_priv->qp_info[0]);
error3:
kfree(port_priv);
@@ -2000,7 +1967,7 @@
list_del(&port_priv->port_list);
spin_unlock_irqrestore(&ib_mad_port_list_lock, flags);
- ib_mad_port_stop(port_priv);
+ /* Stop processing completions. */
flush_workqueue(port_priv->wq);
destroy_workqueue(port_priv->wq);
destroy_mad_qp(&port_priv->qp_info[1]);
@@ -2008,6 +1975,8 @@
ib_dereg_mr(port_priv->mr);
ib_dealloc_pd(port_priv->pd);
ib_destroy_cq(port_priv->cq);
+ cleanup_recv_queue(&port_priv->qp_info[1]);
+ cleanup_recv_queue(&port_priv->qp_info[0]);
/* XXX: Handle deallocation of MAD registration tables */
kfree(port_priv);
More information about the general
mailing list