From 8b37b94721533f2729c79bcb6fa0bb3e2bc2f400 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Sun, 6 Nov 2005 15:47:02 -0800 Subject: [IB] umad: two small fixes Two small fixes for the umad module: - set kobject name for issm device properly - in ib_umad_add_one(), s is subtracted from the index i when initializing ports, so s should be subtracted from the index when freeing ports in the error path as well. Signed-off-by: Michael S. Tsirkin Signed-off-by: Roland Dreier --- drivers/infiniband/core/user_mad.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/infiniband/core') diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c index aed5ca23fb2..6aefeed42ab 100644 --- a/drivers/infiniband/core/user_mad.c +++ b/drivers/infiniband/core/user_mad.c @@ -801,7 +801,7 @@ static int ib_umad_init_port(struct ib_device *device, int port_num, goto err_class; port->sm_dev->owner = THIS_MODULE; port->sm_dev->ops = &umad_sm_fops; - kobject_set_name(&port->dev->kobj, "issm%d", port->dev_num); + kobject_set_name(&port->sm_dev->kobj, "issm%d", port->dev_num); if (cdev_add(port->sm_dev, base_dev + port->dev_num + IB_UMAD_MAX_PORTS, 1)) goto err_sm_cdev; @@ -913,7 +913,7 @@ static void ib_umad_add_one(struct ib_device *device) err: while (--i >= s) - ib_umad_kill_port(&umad_dev->port[i]); + ib_umad_kill_port(&umad_dev->port[i - s]); kref_put(&umad_dev->ref, ib_umad_release_dev); } -- cgit v1.2.3-18-g5258 From 2f76e82947b977a1008cfd2868351a701c93c69c Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Mon, 7 Nov 2005 10:41:29 -0800 Subject: [IB] umad: avoid potential deadlock when unregistering MAD agents ib_unregister_mad_agent() completes all pending MAD sends and waits for the agent's send_handler routine to return. umad's send_handler() calls queue_packet(), which does down_read() on the port mutex to look up the agent ID. This means that the port mutex cannot be held for writing while calling ib_unregister_mad_agent(), or else it will deadlock. This patch fixes all the calls to ib_unregister_mad_agent() in the umad module to avoid this deadlock. Signed-off-by: Roland Dreier --- drivers/infiniband/core/user_mad.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) (limited to 'drivers/infiniband/core') diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c index 6aefeed42ab..f5ed36c2a06 100644 --- a/drivers/infiniband/core/user_mad.c +++ b/drivers/infiniband/core/user_mad.c @@ -505,8 +505,6 @@ found: goto out; } - file->agent[agent_id] = agent; - file->mr[agent_id] = ib_get_dma_mr(agent->qp->pd, IB_ACCESS_LOCAL_WRITE); if (IS_ERR(file->mr[agent_id])) { ret = -ENOMEM; @@ -519,14 +517,15 @@ found: goto err_mr; } + file->agent[agent_id] = agent; ret = 0; + goto out; err_mr: ib_dereg_mr(file->mr[agent_id]); err: - file->agent[agent_id] = NULL; ib_unregister_mad_agent(agent); out: @@ -536,27 +535,33 @@ out: static int ib_umad_unreg_agent(struct ib_umad_file *file, unsigned long arg) { + struct ib_mad_agent *agent = NULL; + struct ib_mr *mr = NULL; u32 id; int ret = 0; - down_write(&file->port->mutex); + if (get_user(id, (u32 __user *) arg)) + return -EFAULT; - if (get_user(id, (u32 __user *) arg)) { - ret = -EFAULT; - goto out; - } + down_write(&file->port->mutex); if (id < 0 || id >= IB_UMAD_MAX_AGENTS || !file->agent[id]) { ret = -EINVAL; goto out; } - ib_dereg_mr(file->mr[id]); - ib_unregister_mad_agent(file->agent[id]); + agent = file->agent[id]; + mr = file->mr[id]; file->agent[id] = NULL; out: up_write(&file->port->mutex); + + if (agent) { + ib_unregister_mad_agent(agent); + ib_dereg_mr(mr); + } + return ret; } @@ -623,16 +628,16 @@ static int ib_umad_close(struct inode *inode, struct file *filp) struct ib_umad_packet *packet, *tmp; int i; - down_write(&file->port->mutex); for (i = 0; i < IB_UMAD_MAX_AGENTS; ++i) if (file->agent[i]) { - ib_dereg_mr(file->mr[i]); ib_unregister_mad_agent(file->agent[i]); + ib_dereg_mr(file->mr[i]); } list_for_each_entry_safe(packet, tmp, &file->recv_list, list) kfree(packet); + down_write(&file->port->mutex); list_del(&file->port_list); up_write(&file->port->mutex); -- cgit v1.2.3-18-g5258 From 40de2e548c225e3ef859e3c60de9785e37e1b5b1 Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Tue, 8 Nov 2005 11:10:25 -0800 Subject: [IB] Have cq_resize() method take an int, not int* Change the struct ib_device.resize_cq() method to take a plain integer that holds the new CQ size, rather than a pointer to an integer that it uses to return the new size. This makes the interface match the exported ib_resize_cq() signature, and allows the low-level driver to update the CQ size with proper locking if necessary. No in-tree drivers are exporting this method yet. Signed-off-by: Roland Dreier --- drivers/infiniband/core/verbs.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) (limited to 'drivers/infiniband/core') diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c index 72d3ef786db..4f51d797f84 100644 --- a/drivers/infiniband/core/verbs.c +++ b/drivers/infiniband/core/verbs.c @@ -324,16 +324,8 @@ EXPORT_SYMBOL(ib_destroy_cq); int ib_resize_cq(struct ib_cq *cq, int cqe) { - int ret; - - if (!cq->device->resize_cq) - return -ENOSYS; - - ret = cq->device->resize_cq(cq, &cqe); - if (!ret) - cq->cqe = cqe; - - return ret; + return cq->device->resize_cq ? + cq->device->resize_cq(cq, cqe) : -ENOSYS; } EXPORT_SYMBOL(ib_resize_cq); -- cgit v1.2.3-18-g5258 From ec914c52d6208d8752dfd85b48a9aff304911434 Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Wed, 9 Nov 2005 09:58:10 -0800 Subject: [IB] umad: get rid of unused mr array Now that ib_umad uses the new MAD sending interface, it no longer needs its own L_Key. So just delete the array of MRs that it keeps. Signed-off-by: Roland Dreier --- drivers/infiniband/core/user_mad.c | 29 ++++------------------------- 1 file changed, 4 insertions(+), 25 deletions(-) (limited to 'drivers/infiniband/core') diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c index f5ed36c2a06..d61f544f19e 100644 --- a/drivers/infiniband/core/user_mad.c +++ b/drivers/infiniband/core/user_mad.c @@ -116,7 +116,6 @@ struct ib_umad_file { spinlock_t recv_lock; wait_queue_head_t recv_wait; struct ib_mad_agent *agent[IB_UMAD_MAX_AGENTS]; - struct ib_mr *mr[IB_UMAD_MAX_AGENTS]; }; struct ib_umad_packet { @@ -505,29 +504,16 @@ found: goto out; } - file->mr[agent_id] = ib_get_dma_mr(agent->qp->pd, IB_ACCESS_LOCAL_WRITE); - if (IS_ERR(file->mr[agent_id])) { - ret = -ENOMEM; - goto err; - } - if (put_user(agent_id, (u32 __user *) (arg + offsetof(struct ib_user_mad_reg_req, id)))) { ret = -EFAULT; - goto err_mr; + ib_unregister_mad_agent(agent); + goto out; } file->agent[agent_id] = agent; ret = 0; - goto out; - -err_mr: - ib_dereg_mr(file->mr[agent_id]); - -err: - ib_unregister_mad_agent(agent); - out: up_write(&file->port->mutex); return ret; @@ -536,7 +522,6 @@ out: static int ib_umad_unreg_agent(struct ib_umad_file *file, unsigned long arg) { struct ib_mad_agent *agent = NULL; - struct ib_mr *mr = NULL; u32 id; int ret = 0; @@ -551,16 +536,13 @@ static int ib_umad_unreg_agent(struct ib_umad_file *file, unsigned long arg) } agent = file->agent[id]; - mr = file->mr[id]; file->agent[id] = NULL; out: up_write(&file->port->mutex); - if (agent) { + if (agent) ib_unregister_mad_agent(agent); - ib_dereg_mr(mr); - } return ret; } @@ -629,10 +611,8 @@ static int ib_umad_close(struct inode *inode, struct file *filp) int i; for (i = 0; i < IB_UMAD_MAX_AGENTS; ++i) - if (file->agent[i]) { + if (file->agent[i]) ib_unregister_mad_agent(file->agent[i]); - ib_dereg_mr(file->mr[i]); - } list_for_each_entry_safe(packet, tmp, &file->recv_list, list) kfree(packet); @@ -872,7 +852,6 @@ static void ib_umad_kill_port(struct ib_umad_port *port) for (id = 0; id < IB_UMAD_MAX_AGENTS; ++id) { if (!file->agent[id]) continue; - ib_dereg_mr(file->mr[id]); ib_unregister_mad_agent(file->agent[id]); file->agent[id] = NULL; } -- cgit v1.2.3-18-g5258 From 77369ed31daac51f4827c50d30f233c45480235a Mon Sep 17 00:00:00 2001 From: Jack Morgenstein Date: Wed, 9 Nov 2005 11:26:07 -0800 Subject: [IB] uverbs: have kernel return QP capabilities Move the computation of QP capabilities (max scatter/gather entries, max inline data, etc) into the kernel, and have the uverbs module return the values as part of the create QP response. This keeps precise knowledge of device limits in the low-level kernel driver. This requires an ABI bump, so while we're making changes, get rid of the max_sge parameter for the modify SRQ command -- it's not used and shouldn't be there. Signed-off-by: Jack Morgenstein Signed-off-by: Michael S. Tsirkin Signed-off-by: Roland Dreier --- drivers/infiniband/core/uverbs_cmd.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'drivers/infiniband/core') diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c index 63a74151c60..ed45da892b1 100644 --- a/drivers/infiniband/core/uverbs_cmd.c +++ b/drivers/infiniband/core/uverbs_cmd.c @@ -708,7 +708,7 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file, resp->wc[i].opcode = wc[i].opcode; resp->wc[i].vendor_err = wc[i].vendor_err; resp->wc[i].byte_len = wc[i].byte_len; - resp->wc[i].imm_data = wc[i].imm_data; + resp->wc[i].imm_data = (__u32 __force) wc[i].imm_data; resp->wc[i].qp_num = wc[i].qp_num; resp->wc[i].src_qp = wc[i].src_qp; resp->wc[i].wc_flags = wc[i].wc_flags; @@ -908,7 +908,12 @@ retry: if (ret) goto err_destroy; - resp.qp_handle = uobj->uobject.id; + resp.qp_handle = uobj->uobject.id; + resp.max_recv_sge = attr.cap.max_recv_sge; + resp.max_send_sge = attr.cap.max_send_sge; + resp.max_recv_wr = attr.cap.max_recv_wr; + resp.max_send_wr = attr.cap.max_send_wr; + resp.max_inline_data = attr.cap.max_inline_data; if (copy_to_user((void __user *) (unsigned long) cmd.response, &resp, sizeof resp)) { @@ -1135,7 +1140,7 @@ ssize_t ib_uverbs_post_send(struct ib_uverbs_file *file, next->num_sge = user_wr->num_sge; next->opcode = user_wr->opcode; next->send_flags = user_wr->send_flags; - next->imm_data = user_wr->imm_data; + next->imm_data = (__be32 __force) user_wr->imm_data; if (qp->qp_type == IB_QPT_UD) { next->wr.ud.ah = idr_find(&ib_uverbs_ah_idr, @@ -1701,7 +1706,6 @@ ssize_t ib_uverbs_modify_srq(struct ib_uverbs_file *file, } attr.max_wr = cmd.max_wr; - attr.max_sge = cmd.max_sge; attr.srq_limit = cmd.srq_limit; ret = ib_modify_srq(srq, &attr, cmd.attr_mask); -- cgit v1.2.3-18-g5258 From 94382f3562e350ed7c8f7dcd6fc968bdece31328 Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Thu, 10 Nov 2005 10:18:23 -0800 Subject: [IB] umad: further ib_unregister_mad_agent() deadlock fixes The previous umad deadlock fix left ib_umad_kill_port() still vulnerable to deadlocking. This patch fixes that by downgrading our lock to a read lock when we might end up trying to reacquire the lock for reading. Signed-off-by: Roland Dreier --- drivers/infiniband/core/user_mad.c | 87 +++++++++++++++++++++++++++----------- 1 file changed, 63 insertions(+), 24 deletions(-) (limited to 'drivers/infiniband/core') diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c index d61f544f19e..5ea741f47fc 100644 --- a/drivers/infiniband/core/user_mad.c +++ b/drivers/infiniband/core/user_mad.c @@ -31,7 +31,7 @@ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. * - * $Id: user_mad.c 2814 2005-07-06 19:14:09Z halr $ + * $Id: user_mad.c 4010 2005-11-09 23:11:56Z roland $ */ #include @@ -110,12 +110,13 @@ struct ib_umad_device { }; struct ib_umad_file { - struct ib_umad_port *port; - struct list_head recv_list; - struct list_head port_list; - spinlock_t recv_lock; - wait_queue_head_t recv_wait; - struct ib_mad_agent *agent[IB_UMAD_MAX_AGENTS]; + struct ib_umad_port *port; + struct list_head recv_list; + struct list_head port_list; + spinlock_t recv_lock; + wait_queue_head_t recv_wait; + struct ib_mad_agent *agent[IB_UMAD_MAX_AGENTS]; + int agents_dead; }; struct ib_umad_packet { @@ -144,6 +145,12 @@ static void ib_umad_release_dev(struct kref *ref) kfree(dev); } +/* caller must hold port->mutex at least for reading */ +static struct ib_mad_agent *__get_agent(struct ib_umad_file *file, int id) +{ + return file->agents_dead ? NULL : file->agent[id]; +} + static int queue_packet(struct ib_umad_file *file, struct ib_mad_agent *agent, struct ib_umad_packet *packet) @@ -151,10 +158,11 @@ static int queue_packet(struct ib_umad_file *file, int ret = 1; down_read(&file->port->mutex); + for (packet->mad.hdr.id = 0; packet->mad.hdr.id < IB_UMAD_MAX_AGENTS; packet->mad.hdr.id++) - if (agent == file->agent[packet->mad.hdr.id]) { + if (agent == __get_agent(file, packet->mad.hdr.id)) { spin_lock_irq(&file->recv_lock); list_add_tail(&packet->list, &file->recv_list); spin_unlock_irq(&file->recv_lock); @@ -326,7 +334,7 @@ static ssize_t ib_umad_write(struct file *filp, const char __user *buf, down_read(&file->port->mutex); - agent = file->agent[packet->mad.hdr.id]; + agent = __get_agent(file, packet->mad.hdr.id); if (!agent) { ret = -EINVAL; goto err_up; @@ -480,7 +488,7 @@ static int ib_umad_reg_agent(struct ib_umad_file *file, unsigned long arg) } for (agent_id = 0; agent_id < IB_UMAD_MAX_AGENTS; ++agent_id) - if (!file->agent[agent_id]) + if (!__get_agent(file, agent_id)) goto found; ret = -ENOMEM; @@ -530,7 +538,7 @@ static int ib_umad_unreg_agent(struct ib_umad_file *file, unsigned long arg) down_write(&file->port->mutex); - if (id < 0 || id >= IB_UMAD_MAX_AGENTS || !file->agent[id]) { + if (id < 0 || id >= IB_UMAD_MAX_AGENTS || !__get_agent(file, id)) { ret = -EINVAL; goto out; } @@ -608,21 +616,29 @@ static int ib_umad_close(struct inode *inode, struct file *filp) struct ib_umad_file *file = filp->private_data; struct ib_umad_device *dev = file->port->umad_dev; struct ib_umad_packet *packet, *tmp; + int already_dead; int i; - for (i = 0; i < IB_UMAD_MAX_AGENTS; ++i) - if (file->agent[i]) - ib_unregister_mad_agent(file->agent[i]); + down_write(&file->port->mutex); + + already_dead = file->agents_dead; + file->agents_dead = 1; list_for_each_entry_safe(packet, tmp, &file->recv_list, list) kfree(packet); - down_write(&file->port->mutex); list_del(&file->port_list); - up_write(&file->port->mutex); - kfree(file); + downgrade_write(&file->port->mutex); + + if (!already_dead) + for (i = 0; i < IB_UMAD_MAX_AGENTS; ++i) + if (file->agent[i]) + ib_unregister_mad_agent(file->agent[i]); + up_read(&file->port->mutex); + + kfree(file); kref_put(&dev->ref, ib_umad_release_dev); return 0; @@ -848,13 +864,36 @@ static void ib_umad_kill_port(struct ib_umad_port *port) port->ib_dev = NULL; - list_for_each_entry(file, &port->file_list, port_list) - for (id = 0; id < IB_UMAD_MAX_AGENTS; ++id) { - if (!file->agent[id]) - continue; - ib_unregister_mad_agent(file->agent[id]); - file->agent[id] = NULL; - } + /* + * Now go through the list of files attached to this port and + * unregister all of their MAD agents. We need to hold + * port->mutex while doing this to avoid racing with + * ib_umad_close(), but we can't hold the mutex for writing + * while calling ib_unregister_mad_agent(), since that might + * deadlock by calling back into queue_packet(). So we + * downgrade our lock to a read lock, and then drop and + * reacquire the write lock for the next iteration. + * + * We do list_del_init() on the file's list_head so that the + * list_del in ib_umad_close() is still OK, even after the + * file is removed from the list. + */ + while (!list_empty(&port->file_list)) { + file = list_entry(port->file_list.next, struct ib_umad_file, + port_list); + + file->agents_dead = 1; + list_del_init(&file->port_list); + + downgrade_write(&port->mutex); + + for (id = 0; id < IB_UMAD_MAX_AGENTS; ++id) + if (file->agent[id]) + ib_unregister_mad_agent(file->agent[id]); + + up_read(&port->mutex); + down_write(&port->mutex); + } up_write(&port->mutex); -- cgit v1.2.3-18-g5258