From 18b8c8f170ce346b88884ebe4060cd6dbe64e1cc Mon Sep 17 00:00:00 2001 From: Arthur Jones Date: Fri, 29 Feb 2008 10:13:37 -0800 Subject: [PATCH 1/9] MAINTAINERS: update ipath owner I'll be leaving QLogic soon for another job and Ralph has graciously offered to take over the IPath driver maintainership. Signed-off-by: Arthur Jones Signed-off-by: Ralph Campbell Signed-off-by: Roland Dreier --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index fed09b547336..f229e16d67ed 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2143,7 +2143,7 @@ L: netdev@vger.kernel.org S: Maintained IPATH DRIVER: -P: Arthur Jones +P: Ralph Campbell M: infinipath@qlogic.com L: general@lists.openfabrics.org T: git git://git.qlogic.com/ipath-linux-2.6 From 84ba284cd78c130818e2de53150f39b92504593b Mon Sep 17 00:00:00 2001 From: Sean Hefty Date: Fri, 22 Feb 2008 10:40:45 -0800 Subject: [PATCH 2/9] IB/cm: Flush workqueue when removing device When a CM MAD is received, it is queued to a CM workqueue for processing. The queued work item references the port and device on which the MAD was received. If that device is removed from the system before the work item can execute, the work item will reference freed memory. To fix this, flush the workqueue after unregistering to receive MAD, and before the device is be freed. Signed-off-by: Sean Hefty Signed-off-by: Roland Dreier --- drivers/infiniband/core/cm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c index b10ade92efed..4df405157086 100644 --- a/drivers/infiniband/core/cm.c +++ b/drivers/infiniband/core/cm.c @@ -3759,6 +3759,7 @@ static void cm_remove_one(struct ib_device *device) port = cm_dev->port[i-1]; ib_modify_port(device, port->port_num, 0, &port_modify); ib_unregister_mad_agent(port->mad_agent); + flush_workqueue(cm.wq); cm_remove_port_fs(port); } kobject_put(&cm_dev->dev_obj); @@ -3813,6 +3814,7 @@ static void __exit ib_cm_cleanup(void) cancel_delayed_work(&timewait_info->work.work); spin_unlock_irq(&cm.lock); + ib_unregister_client(&cm_client); destroy_workqueue(cm.wq); list_for_each_entry_safe(timewait_info, tmp, &cm.timewait_list, list) { @@ -3820,7 +3822,6 @@ static void __exit ib_cm_cleanup(void) kfree(timewait_info); } - ib_unregister_client(&cm_client); class_unregister(&cm_class); idr_destroy(&cm.local_id_table); } From 35fb5340e3de5dff86923eb0cded748c3a6e05e7 Mon Sep 17 00:00:00 2001 From: Pete Wyckoff Date: Tue, 26 Feb 2008 13:27:31 -0500 Subject: [PATCH 3/9] Revert "IB/fmr_pool: ib_fmr_pool_flush() should flush all dirty FMRs" This reverts commit a3cd7d9070be417a21905c997ee32d756d999b38. The original commit breaks iSER reliably, making it complain: iser: iser_reg_page_vec:ib_fmr_pool_map_phys failed: -11 The FMR cleanup thread runs ib_fmr_batch_release() as dirty entries build up. This commit causes clean but used FMR entries also to be purged. During that process, another thread can see that there are no free FMRs and fail, even though there should always have been enough available. Signed-off-by: Pete Wyckoff Signed-off-by: Roland Dreier --- drivers/infiniband/core/fmr_pool.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/drivers/infiniband/core/fmr_pool.c b/drivers/infiniband/core/fmr_pool.c index 7f00347364f7..4044fdf62cc2 100644 --- a/drivers/infiniband/core/fmr_pool.c +++ b/drivers/infiniband/core/fmr_pool.c @@ -139,7 +139,7 @@ static inline struct ib_pool_fmr *ib_fmr_cache_lookup(struct ib_fmr_pool *pool, static void ib_fmr_batch_release(struct ib_fmr_pool *pool) { int ret; - struct ib_pool_fmr *fmr, *next; + struct ib_pool_fmr *fmr; LIST_HEAD(unmap_list); LIST_HEAD(fmr_list); @@ -158,20 +158,6 @@ static void ib_fmr_batch_release(struct ib_fmr_pool *pool) #endif } - /* - * The free_list may hold FMRs that have been put there - * because they haven't reached the max_remap count. - * Invalidate their mapping as well. - */ - list_for_each_entry_safe(fmr, next, &pool->free_list, list) { - if (fmr->remap_count == 0) - continue; - hlist_del_init(&fmr->cache_node); - fmr->remap_count = 0; - list_add_tail(&fmr->fmr->list, &fmr_list); - list_move(&fmr->list, &unmap_list); - } - list_splice(&pool->dirty_list, &unmap_list); INIT_LIST_HEAD(&pool->dirty_list); pool->dirty_len = 0; @@ -384,6 +370,11 @@ void ib_destroy_fmr_pool(struct ib_fmr_pool *pool) i = 0; list_for_each_entry_safe(fmr, tmp, &pool->free_list, list) { + if (fmr->remap_count) { + INIT_LIST_HEAD(&fmr_list); + list_add_tail(&fmr->fmr->list, &fmr_list); + ib_unmap_fmr(&fmr_list); + } ib_dealloc_fmr(fmr->fmr); list_del(&fmr->list); kfree(fmr); From 331552925d17ffa2f5c676e282d4fd37c852d9e3 Mon Sep 17 00:00:00 2001 From: Pete Wyckoff Date: Tue, 26 Feb 2008 13:27:53 -0500 Subject: [PATCH 4/9] IB/fmr_pool: Flush all dirty FMRs from ib_fmr_pool_flush() Commit a3cd7d90 ("IB/fmr_pool: ib_fmr_pool_flush() should flush all dirty FMRs") caused a regression for iSER and was reverted in e5507736. This change attempts to redo the original patch so that all used FMR entries are flushed when ib_flush_fmr_pool() is called without affecting the normal FMR pool cleaning thread. Simply move used entries from the clean list onto the dirty list in ib_flush_fmr_pool() before letting the cleanup thread do its job. Signed-off-by: Pete Wyckoff Signed-off-by: Roland Dreier --- drivers/infiniband/core/fmr_pool.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/infiniband/core/fmr_pool.c b/drivers/infiniband/core/fmr_pool.c index 4044fdf62cc2..06d502c06a4d 100644 --- a/drivers/infiniband/core/fmr_pool.c +++ b/drivers/infiniband/core/fmr_pool.c @@ -398,8 +398,23 @@ EXPORT_SYMBOL(ib_destroy_fmr_pool); */ int ib_flush_fmr_pool(struct ib_fmr_pool *pool) { - int serial = atomic_inc_return(&pool->req_ser); + int serial; + struct ib_pool_fmr *fmr, *next; + /* + * The free_list holds FMRs that may have been used + * but have not been remapped enough times to be dirty. + * Put them on the dirty list now so that the cleanup + * thread will reap them too. + */ + spin_lock_irq(&pool->pool_lock); + list_for_each_entry_safe(fmr, next, &pool->free_list, list) { + if (fmr->remap_count > 0) + list_move(&fmr->list, &pool->dirty_list); + } + spin_unlock_irq(&pool->pool_lock); + + serial = atomic_inc_return(&pool->req_ser); wake_up_process(pool->thread); if (wait_event_interruptible(pool->force_wait, From 1bab74e691d3c7845df2342d202c0f1c2344c834 Mon Sep 17 00:00:00 2001 From: Jon Mason Date: Fri, 29 Feb 2008 13:53:18 -0800 Subject: [PATCH 5/9] RDMA/cxgb3: Return correct max_inline_data when creating a QP Set cap.max_inline_data to the actual max inline data that the adapter support, so that userspace apps see the right value returned. Signed-off-by: Jon Mason Acked-by: Steve Wise Signed-off-by: Roland Dreier --- drivers/infiniband/hw/cxgb3/iwch_provider.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c index df1838f8f94d..ee3d63cd1f96 100644 --- a/drivers/infiniband/hw/cxgb3/iwch_provider.c +++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c @@ -819,8 +819,11 @@ static struct ib_qp *iwch_create_qp(struct ib_pd *pd, kfree(qhp); return ERR_PTR(-ENOMEM); } + attrs->cap.max_recv_wr = rqsize - 1; attrs->cap.max_send_wr = sqsize; + attrs->cap.max_inline_data = T3_MAX_INLINE; + qhp->rhp = rhp; qhp->attr.pd = php->pdid; qhp->attr.scq = ((struct iwch_cq *) attrs->send_cq)->cq.cqid; From 4fa45725df0f00c2bf86a0fc2670e88bfe0ceee7 Mon Sep 17 00:00:00 2001 From: Jon Mason Date: Sun, 9 Mar 2008 13:54:12 -0700 Subject: [PATCH 6/9] RDMA/cxgb3: Fix iwch_create_cq() off-by-one error The cxbg3 driver is unnecessarily decreasing the number of CQ entries by one when creating a CQ. This will cause the CQ not to have as many entries as requested by the user if the user requests a power of 2 size. Signed-off-by: Jon Mason Acked-by: Steve Wise Signed-off-by: Roland Dreier --- drivers/infiniband/hw/cxgb3/iwch_provider.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c index ee3d63cd1f96..b2ea9210467f 100644 --- a/drivers/infiniband/hw/cxgb3/iwch_provider.c +++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c @@ -189,7 +189,7 @@ static struct ib_cq *iwch_create_cq(struct ib_device *ibdev, int entries, int ve return ERR_PTR(-ENOMEM); } chp->rhp = rhp; - chp->ibcq.cqe = (1 << chp->cq.size_log2) - 1; + chp->ibcq.cqe = 1 << chp->cq.size_log2; spin_lock_init(&chp->lock); atomic_set(&chp->refcnt, 1); init_waitqueue_head(&chp->wait); From 9a378270c085080b2f38dee6308de4d8413b5141 Mon Sep 17 00:00:00 2001 From: Arne Redlich Date: Tue, 4 Mar 2008 14:07:22 +0200 Subject: [PATCH 7/9] IB/iser: Fix list iteration bug The iteration through the list of "iser_device"s during device lookup/creation is broken -- it might result in an infinite loop if more than one HCA is used with iSER. Fix this by using list_for_each_entry() instead of the open-coded flawed list iteration code. Signed-off-by: Arne Redlich Signed-off-by: Erez Zilber Signed-off-by: Roland Dreier --- drivers/infiniband/ulp/iser/iser_verbs.c | 36 +++++++++++------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c index 714b8db02b29..768ba69f2fd9 100644 --- a/drivers/infiniband/ulp/iser/iser_verbs.c +++ b/drivers/infiniband/ulp/iser/iser_verbs.c @@ -237,33 +237,29 @@ static int iser_free_ib_conn_res(struct iser_conn *ib_conn) static struct iser_device *iser_device_find_by_ib_device(struct rdma_cm_id *cma_id) { - struct list_head *p_list; - struct iser_device *device = NULL; + struct iser_device *device; mutex_lock(&ig.device_list_mutex); - p_list = ig.device_list.next; - while (p_list != &ig.device_list) { - device = list_entry(p_list, struct iser_device, ig_list); + list_for_each_entry(device, &ig.device_list, ig_list) /* find if there's a match using the node GUID */ if (device->ib_device->node_guid == cma_id->device->node_guid) - break; - } + goto out; - if (device == NULL) { - device = kzalloc(sizeof *device, GFP_KERNEL); - if (device == NULL) - goto out; - /* assign this device to the device */ - device->ib_device = cma_id->device; - /* init the device and link it into ig device list */ - if (iser_create_device_ib_res(device)) { - kfree(device); - device = NULL; - goto out; - } - list_add(&device->ig_list, &ig.device_list); + device = kzalloc(sizeof *device, GFP_KERNEL); + if (device == NULL) + goto out; + + /* assign this device to the device */ + device->ib_device = cma_id->device; + /* init the device and link it into ig device list */ + if (iser_create_device_ib_res(device)) { + kfree(device); + device = NULL; + goto out; } + list_add(&device->ig_list, &ig.device_list); + out: BUG_ON(device == NULL); device->refcount++; From d33ed425c6cc14370d8c418b504328d2c3db58b4 Mon Sep 17 00:00:00 2001 From: Arne Redlich Date: Tue, 4 Mar 2008 14:11:54 +0200 Subject: [PATCH 8/9] IB/iser: Handle iser_device allocation error gracefully "iser_device" allocation failure is "handled" with a BUG_ON() right before dereferencing the NULL-pointer - fix this! Signed-off-by: Arne Redlich Signed-off-by: Erez Zilber --- drivers/infiniband/ulp/iser/iser_verbs.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c index 768ba69f2fd9..993f0a8ff28f 100644 --- a/drivers/infiniband/ulp/iser/iser_verbs.c +++ b/drivers/infiniband/ulp/iser/iser_verbs.c @@ -244,7 +244,7 @@ struct iser_device *iser_device_find_by_ib_device(struct rdma_cm_id *cma_id) list_for_each_entry(device, &ig.device_list, ig_list) /* find if there's a match using the node GUID */ if (device->ib_device->node_guid == cma_id->device->node_guid) - goto out; + goto inc_refcnt; device = kzalloc(sizeof *device, GFP_KERNEL); if (device == NULL) @@ -260,9 +260,9 @@ struct iser_device *iser_device_find_by_ib_device(struct rdma_cm_id *cma_id) } list_add(&device->ig_list, &ig.device_list); -out: - BUG_ON(device == NULL); +inc_refcnt: device->refcount++; +out: mutex_unlock(&ig.device_list_mutex); return device; } @@ -368,6 +368,12 @@ static void iser_addr_handler(struct rdma_cm_id *cma_id) int ret; device = iser_device_find_by_ib_device(cma_id); + if (!device) { + iser_err("device lookup/creation failed\n"); + iser_connect_error(cma_id); + return; + } + ib_conn = (struct iser_conn *)cma_id->context; ib_conn->device = device; @@ -376,7 +382,6 @@ static void iser_addr_handler(struct rdma_cm_id *cma_id) iser_err("resolve route failed: %d\n", ret); iser_connect_error(cma_id); } - return; } static void iser_route_handler(struct rdma_cm_id *cma_id) From d7c1fbd6606085dbf95e47068d6bd2db8a180e38 Mon Sep 17 00:00:00 2001 From: Steve Wise Date: Tue, 4 Mar 2008 16:44:52 -0600 Subject: [PATCH 9/9] RDMA/iwcm: Don't access a cm_id after dropping reference cm_work_handler() can access cm_id_priv after it drops its reference by calling iwch_deref_id(), which might cause it to be freed. The fix is to look at whether IWCM_F_CALLBACK_DESTROY is set _before_ dropping the reference. Then if it was set, free the cm_id on this thread. Signed-off-by: Steve Wise Signed-off-by: Roland Dreier --- drivers/infiniband/core/iwcm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c index 223b1aa7d92b..81c9195b512a 100644 --- a/drivers/infiniband/core/iwcm.c +++ b/drivers/infiniband/core/iwcm.c @@ -839,6 +839,7 @@ static void cm_work_handler(struct work_struct *_work) unsigned long flags; int empty; int ret = 0; + int destroy_id; spin_lock_irqsave(&cm_id_priv->lock, flags); empty = list_empty(&cm_id_priv->work_list); @@ -857,9 +858,9 @@ static void cm_work_handler(struct work_struct *_work) destroy_cm_id(&cm_id_priv->id); } BUG_ON(atomic_read(&cm_id_priv->refcount)==0); + destroy_id = test_bit(IWCM_F_CALLBACK_DESTROY, &cm_id_priv->flags); if (iwcm_deref_id(cm_id_priv)) { - if (test_bit(IWCM_F_CALLBACK_DESTROY, - &cm_id_priv->flags)) { + if (destroy_id) { BUG_ON(!list_empty(&cm_id_priv->work_list)); free_cm_id(cm_id_priv); }