* [PATCH][0/11] InfiniBand fixes @ 2005-03-03 5:31 Roland Dreier 2005-03-03 5:31 ` [PATCH][1/11] IB: simplify MAD code Roland Dreier 0 siblings, 1 reply; 12+ messages in thread From: Roland Dreier @ 2005-03-03 5:31 UTC (permalink / raw) To: akpm; +Cc: linux-kernel, openib-general Here is a batch of fixes from the OpenIB subversion tree for merging. Thanks, Roland ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH][1/11] IB: simplify MAD code 2005-03-03 5:31 [PATCH][0/11] InfiniBand fixes Roland Dreier @ 2005-03-03 5:31 ` Roland Dreier 2005-03-03 5:31 ` [PATCH][2/11] IB: fix vendor MAD deregistration Roland Dreier 0 siblings, 1 reply; 12+ messages in thread From: Roland Dreier @ 2005-03-03 5:31 UTC (permalink / raw) To: akpm; +Cc: linux-kernel, openib-general From: Hal Rosenstock <halr@voltaire.com> Remove unneeded MAD agent registration by using a single agent for both directed-route and LID-routed MADs. Signed-off-by: Hal Rosenstock <halr@voltaire.com> Signed-off-by: Roland Dreier <roland@topspin.com> --- linux-export.orig/drivers/infiniband/core/agent.c 2005-03-02 20:26:03.280776011 -0800 +++ linux-export/drivers/infiniband/core/agent.c 2005-03-02 20:26:10.599187430 -0800 @@ -66,14 +66,13 @@ if (device) { list_for_each_entry(entry, &ib_agent_port_list, port_list) { - if (entry->dr_smp_agent->device == device && + if (entry->smp_agent->device == device && entry->port_num == port_num) return entry; } } else { list_for_each_entry(entry, &ib_agent_port_list, port_list) { - if ((entry->dr_smp_agent == mad_agent) || - (entry->lr_smp_agent == mad_agent) || + if ((entry->smp_agent == mad_agent) || (entry->perf_mgmt_agent == mad_agent)) return entry; } @@ -111,7 +110,7 @@ return 1; } - return smi_check_local_smp(port_priv->dr_smp_agent, smp); + return smi_check_local_smp(port_priv->smp_agent, smp); } static int agent_mad_send(struct ib_mad_agent *mad_agent, @@ -231,10 +230,8 @@ /* Get mad agent based on mgmt_class in MAD */ switch (mad->mad.mad.mad_hdr.mgmt_class) { case IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE: - mad_agent = port_priv->dr_smp_agent; - break; case IB_MGMT_CLASS_SUBN_LID_ROUTED: - mad_agent = port_priv->lr_smp_agent; + mad_agent = port_priv->smp_agent; break; case IB_MGMT_CLASS_PERF_MGMT: mad_agent = port_priv->perf_mgmt_agent; @@ -284,7 +281,6 @@ { int ret; struct ib_agent_port_private *port_priv; - struct ib_mad_reg_req reg_req; unsigned long flags; /* First, check if port already open for SMI */ @@ -308,35 +304,19 @@ spin_lock_init(&port_priv->send_list_lock); INIT_LIST_HEAD(&port_priv->send_posted_list); - /* Obtain MAD agent for directed route SM class */ - reg_req.mgmt_class = IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE; - reg_req.mgmt_class_version = 1; - - port_priv->dr_smp_agent = ib_register_mad_agent(device, port_num, - IB_QPT_SMI, - NULL, 0, - &agent_send_handler, - NULL, NULL); + /* Obtain send only MAD agent for SM class (SMI QP) */ + port_priv->smp_agent = ib_register_mad_agent(device, port_num, + IB_QPT_SMI, + NULL, 0, + &agent_send_handler, + NULL, NULL); - if (IS_ERR(port_priv->dr_smp_agent)) { - ret = PTR_ERR(port_priv->dr_smp_agent); + if (IS_ERR(port_priv->smp_agent)) { + ret = PTR_ERR(port_priv->smp_agent); goto error2; } - /* Obtain MAD agent for LID routed SM class */ - reg_req.mgmt_class = IB_MGMT_CLASS_SUBN_LID_ROUTED; - port_priv->lr_smp_agent = ib_register_mad_agent(device, port_num, - IB_QPT_SMI, - NULL, 0, - &agent_send_handler, - NULL, NULL); - if (IS_ERR(port_priv->lr_smp_agent)) { - ret = PTR_ERR(port_priv->lr_smp_agent); - goto error3; - } - - /* Obtain MAD agent for PerfMgmt class */ - reg_req.mgmt_class = IB_MGMT_CLASS_PERF_MGMT; + /* Obtain send only MAD agent for PerfMgmt class (GSI QP) */ port_priv->perf_mgmt_agent = ib_register_mad_agent(device, port_num, IB_QPT_GSI, NULL, 0, @@ -344,15 +324,15 @@ NULL, NULL); if (IS_ERR(port_priv->perf_mgmt_agent)) { ret = PTR_ERR(port_priv->perf_mgmt_agent); - goto error4; + goto error3; } - port_priv->mr = ib_get_dma_mr(port_priv->dr_smp_agent->qp->pd, + port_priv->mr = ib_get_dma_mr(port_priv->smp_agent->qp->pd, IB_ACCESS_LOCAL_WRITE); if (IS_ERR(port_priv->mr)) { printk(KERN_ERR SPFX "Couldn't get DMA MR\n"); ret = PTR_ERR(port_priv->mr); - goto error5; + goto error4; } spin_lock_irqsave(&ib_agent_port_list_lock, flags); @@ -361,12 +341,10 @@ return 0; -error5: - ib_unregister_mad_agent(port_priv->perf_mgmt_agent); error4: - ib_unregister_mad_agent(port_priv->lr_smp_agent); + ib_unregister_mad_agent(port_priv->perf_mgmt_agent); error3: - ib_unregister_mad_agent(port_priv->dr_smp_agent); + ib_unregister_mad_agent(port_priv->smp_agent); error2: kfree(port_priv); error1: @@ -391,8 +369,7 @@ ib_dereg_mr(port_priv->mr); ib_unregister_mad_agent(port_priv->perf_mgmt_agent); - ib_unregister_mad_agent(port_priv->lr_smp_agent); - ib_unregister_mad_agent(port_priv->dr_smp_agent); + ib_unregister_mad_agent(port_priv->smp_agent); kfree(port_priv); return 0; --- linux-export.orig/drivers/infiniband/core/agent_priv.h 2005-03-02 20:26:03.280776011 -0800 +++ linux-export/drivers/infiniband/core/agent_priv.h 2005-03-02 20:26:10.599187430 -0800 @@ -55,8 +55,7 @@ struct list_head send_posted_list; spinlock_t send_list_lock; int port_num; - struct ib_mad_agent *dr_smp_agent; /* DR SM class */ - struct ib_mad_agent *lr_smp_agent; /* LR SM class */ + struct ib_mad_agent *smp_agent; /* SM class */ struct ib_mad_agent *perf_mgmt_agent; /* PerfMgmt class */ struct ib_mr *mr; }; ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH][2/11] IB: fix vendor MAD deregistration 2005-03-03 5:31 ` [PATCH][1/11] IB: simplify MAD code Roland Dreier @ 2005-03-03 5:31 ` Roland Dreier 2005-03-03 5:31 ` [PATCH][3/11] IB: sparse fixes Roland Dreier 0 siblings, 1 reply; 12+ messages in thread From: Roland Dreier @ 2005-03-03 5:31 UTC (permalink / raw) To: akpm; +Cc: linux-kernel, openib-general From: Shahar Frank <shaharf@voltaire.com> Fix bug when deregistering a vendor class MAD agent. Signed-off-by: Roland Dreier <roland@topspin.com> --- linux-export.orig/drivers/infiniband/core/mad.c 2005-03-02 20:26:03.185796628 -0800 +++ linux-export/drivers/infiniband/core/mad.c 2005-03-02 20:26:10.980104746 -0800 @@ -41,7 +41,6 @@ #include "smi.h" #include "agent.h" - MODULE_LICENSE("Dual BSD/GPL"); MODULE_DESCRIPTION("kernel IB MAD API"); MODULE_AUTHOR("Hal Rosenstock"); @@ -490,6 +489,7 @@ cancel_mads(mad_agent_priv); port_priv = mad_agent_priv->qp_info->port_priv; + cancel_delayed_work(&mad_agent_priv->timed_work); flush_workqueue(port_priv->wq); @@ -1266,12 +1266,12 @@ } port_priv = agent_priv->qp_info->port_priv; + mgmt_class = convert_mgmt_class(agent_priv->reg_req->mgmt_class); class = port_priv->version[ agent_priv->reg_req->mgmt_class_version].class; if (!class) goto vendor_check; - mgmt_class = convert_mgmt_class(agent_priv->reg_req->mgmt_class); method = class->method_table[mgmt_class]; if (method) { /* Remove any methods for this mad agent */ @@ -1293,16 +1293,21 @@ } vendor_check: + if (!is_vendor_class(mgmt_class)) + goto out; + + /* normalize mgmt_class to vendor range 2 */ + mgmt_class = vendor_class_index(agent_priv->reg_req->mgmt_class); vendor = port_priv->version[ agent_priv->reg_req->mgmt_class_version].vendor; + if (!vendor) goto out; - mgmt_class = vendor_class_index(agent_priv->reg_req->mgmt_class); vendor_class = vendor->vendor_class[mgmt_class]; if (vendor_class) { index = find_vendor_oui(vendor_class, agent_priv->reg_req->oui); - if (index == -1) + if (index < 0) goto out; method = vendor_class->method_table[index]; if (method) { --- linux-export.orig/drivers/infiniband/core/mad_priv.h 2005-03-02 20:26:03.185796628 -0800 +++ linux-export/drivers/infiniband/core/mad_priv.h 2005-03-02 20:26:10.980104746 -0800 @@ -58,8 +58,8 @@ #define MAX_MGMT_CLASS 80 #define MAX_MGMT_VERSION 8 #define MAX_MGMT_OUI 8 -#define MAX_MGMT_VENDOR_RANGE2 IB_MGMT_CLASS_VENDOR_RANGE2_END - \ - IB_MGMT_CLASS_VENDOR_RANGE2_START + 1 +#define MAX_MGMT_VENDOR_RANGE2 (IB_MGMT_CLASS_VENDOR_RANGE2_END - \ + IB_MGMT_CLASS_VENDOR_RANGE2_START + 1) struct ib_mad_list_head { struct list_head list; ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH][3/11] IB: sparse fixes 2005-03-03 5:31 ` [PATCH][2/11] IB: fix vendor MAD deregistration Roland Dreier @ 2005-03-03 5:31 ` Roland Dreier 2005-03-03 5:31 ` [PATCH][4/11] IB/mthca: add missing break Roland Dreier 0 siblings, 1 reply; 12+ messages in thread From: Roland Dreier @ 2005-03-03 5:31 UTC (permalink / raw) To: akpm; +Cc: linux-kernel, openib-general From: Tom Duffy <tduffy@sun.com> Fix some sparse warnings by making sure we have appropriate "extern" declarations visible. Signed-off-by: Tom Duffy <tduffy@sun.com> Signed-off-by: Hal Rosenstock (<halr@voltaire.com> Signed-off-by: Roland Dreier <roland@topspin.com> --- linux-export.orig/drivers/infiniband/core/agent.c 2005-03-02 20:26:10.599187430 -0800 +++ linux-export/drivers/infiniband/core/agent.c 2005-03-02 20:26:11.456001445 -0800 @@ -45,14 +45,11 @@ #include "smi.h" #include "agent_priv.h" #include "mad_priv.h" - +#include "agent.h" spinlock_t ib_agent_port_list_lock; static LIST_HEAD(ib_agent_port_list); -extern kmem_cache_t *ib_mad_cache; - - /* * Caller must hold ib_agent_port_list_lock */ --- linux-export.orig/drivers/infiniband/core/cache.c 2005-03-02 20:26:03.085818330 -0800 +++ linux-export/drivers/infiniband/core/cache.c 2005-03-02 20:26:11.456001445 -0800 @@ -37,6 +37,8 @@ #include <linux/errno.h> #include <linux/slab.h> +#include <ib_cache.h> + #include "core_priv.h" struct ib_pkey_cache { --- linux-export.orig/drivers/infiniband/core/mad_priv.h 2005-03-02 20:26:10.980104746 -0800 +++ linux-export/drivers/infiniband/core/mad_priv.h 2005-03-02 20:26:11.457001228 -0800 @@ -192,4 +192,6 @@ struct ib_mad_qp_info qp_info[IB_MAD_QPS_CORE]; }; +extern kmem_cache_t *ib_mad_cache; + #endif /* __IB_MAD_PRIV_H__ */ --- linux-export.orig/drivers/infiniband/core/smi.c 2005-03-02 20:26:03.085818330 -0800 +++ linux-export/drivers/infiniband/core/smi.c 2005-03-02 20:26:11.458001011 -0800 @@ -37,7 +37,7 @@ */ #include <ib_smi.h> - +#include "smi.h" /* * Fixup a directed route SMP for sending ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH][4/11] IB/mthca: add missing break 2005-03-03 5:31 ` [PATCH][3/11] IB: sparse fixes Roland Dreier @ 2005-03-03 5:31 ` Roland Dreier 2005-03-03 5:31 ` [PATCH][5/11] IB/mthca: fix reset value endianness Roland Dreier 0 siblings, 1 reply; 12+ messages in thread From: Roland Dreier @ 2005-03-03 5:31 UTC (permalink / raw) To: akpm; +Cc: linux-kernel, openib-general Add missing break statements in switch in mthca_profile.c (pointed out by Michael Tsirkin). Signed-off-by: Roland Dreier <roland@topspin.com> --- linux-export.orig/drivers/infiniband/hw/mthca/mthca_profile.c 2005-03-02 20:26:03.023831785 -0800 +++ linux-export/drivers/infiniband/hw/mthca/mthca_profile.c 2005-03-02 20:26:11.904904003 -0800 @@ -241,10 +241,12 @@ case MTHCA_RES_UDAV: dev->av_table.ddr_av_base = profile[i].start; dev->av_table.num_ddr_avs = profile[i].num; + break; case MTHCA_RES_UARC: init_hca->uarc_base = profile[i].start; init_hca->log_uarc_sz = ffs(request->uarc_size) - 13; init_hca->log_uar_sz = ffs(request->num_uar) - 1; + break; default: break; } ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH][5/11] IB/mthca: fix reset value endianness 2005-03-03 5:31 ` [PATCH][4/11] IB/mthca: add missing break Roland Dreier @ 2005-03-03 5:31 ` Roland Dreier 2005-03-03 5:31 ` [PATCH][6/11] IB/ipoib: fix rx memory leak Roland Dreier 0 siblings, 1 reply; 12+ messages in thread From: Roland Dreier @ 2005-03-03 5:31 UTC (permalink / raw) To: akpm; +Cc: linux-kernel, openib-general MTHCA_RESET_VALUE must always be swapped, since the HCA expects to see it in big-endian order and we write it with writel. This means on little-endian systems we have to swap it to big-endian order before writing, and on big-endian systems we need to swap it to make up for the additional swap that writel will do. This fixes resetting the HCA on big-endian machines. Signed-off-by: Roland Dreier <roland@topspin.com> --- linux-export.orig/drivers/infiniband/hw/mthca/mthca_reset.c 2005-03-02 20:26:02.970843287 -0800 +++ linux-export/drivers/infiniband/hw/mthca/mthca_reset.c 2005-03-02 20:26:12.219835642 -0800 @@ -50,7 +50,7 @@ struct pci_dev *bridge = NULL; #define MTHCA_RESET_OFFSET 0xf0010 -#define MTHCA_RESET_VALUE cpu_to_be32(1) +#define MTHCA_RESET_VALUE swab32(1) /* * Reset the chip. This is somewhat ugly because we have to ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH][6/11] IB/ipoib: fix rx memory leak 2005-03-03 5:31 ` [PATCH][5/11] IB/mthca: fix reset value endianness Roland Dreier @ 2005-03-03 5:31 ` Roland Dreier 2005-03-03 5:31 ` [PATCH][7/11] IB/ipoib: use list_for_each_entry_safe when required Roland Dreier 0 siblings, 1 reply; 12+ messages in thread From: Roland Dreier @ 2005-03-03 5:31 UTC (permalink / raw) To: akpm; +Cc: linux-kernel, openib-general Fix memory leak when posting a receive buffer (pointed out by Shirley Ma). Signed-off-by: Roland Dreier <roland@topspin.com> --- linux-export.orig/drivers/infiniband/ulp/ipoib/ipoib_ib.c 2005-03-02 20:26:02.919854355 -0800 +++ linux-export/drivers/infiniband/ulp/ipoib/ipoib_ib.c 2005-03-02 20:26:12.514771621 -0800 @@ -137,6 +137,9 @@ if (ret) { ipoib_warn(priv, "ipoib_ib_receive failed for buf %d (%d)\n", id, ret); + dma_unmap_single(priv->ca->dma_device, addr, + IPOIB_BUF_SIZE, DMA_FROM_DEVICE); + dev_kfree_skb_any(skb); priv->rx_ring[id].skb = NULL; } ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH][7/11] IB/ipoib: use list_for_each_entry_safe when required 2005-03-03 5:31 ` [PATCH][6/11] IB/ipoib: fix rx memory leak Roland Dreier @ 2005-03-03 5:31 ` Roland Dreier 2005-03-03 5:31 ` [PATCH][8/11] IB/ipoib: rename global symbols Roland Dreier 0 siblings, 1 reply; 12+ messages in thread From: Roland Dreier @ 2005-03-03 5:31 UTC (permalink / raw) To: akpm; +Cc: linux-kernel, openib-general From: Shirley Ma <xma@us.ibm.com> Change uses of list_for_each_entry() where the loop variable is freed inside the loop to list_for_each_entry_safe(). Signed-off-by: Shirley Ma <xma@us.ibm.com> Signed-off-by: Roland Dreier <roland@topspin.com> --- linux-export.orig/drivers/infiniband/ulp/ipoib/ipoib_multicast.c 2005-03-02 20:26:02.832873236 -0800 +++ linux-export/drivers/infiniband/ulp/ipoib/ipoib_multicast.c 2005-03-02 20:26:12.799709771 -0800 @@ -790,7 +790,7 @@ spin_unlock_irqrestore(&priv->lock, flags); - list_for_each_entry(mcast, &remove_list, list) { + list_for_each_entry_safe(mcast, tmcast, &remove_list, list) { ipoib_mcast_leave(dev, mcast); ipoib_mcast_free(mcast); } @@ -902,7 +902,7 @@ spin_unlock_irqrestore(&priv->lock, flags); /* We have to cancel outside of the spinlock */ - list_for_each_entry(mcast, &remove_list, list) { + list_for_each_entry_safe(mcast, tmcast, &remove_list, list) { ipoib_mcast_leave(mcast->dev, mcast); ipoib_mcast_free(mcast); } ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH][8/11] IB/ipoib: rename global symbols 2005-03-03 5:31 ` [PATCH][7/11] IB/ipoib: use list_for_each_entry_safe when required Roland Dreier @ 2005-03-03 5:31 ` Roland Dreier 2005-03-03 5:31 ` [PATCH][9/11] IB/ipoib: small fixes Roland Dreier 0 siblings, 1 reply; 12+ messages in thread From: Roland Dreier @ 2005-03-03 5:31 UTC (permalink / raw) To: akpm; +Cc: linux-kernel, openib-general Make IPoIB data_debug_level module parameter static to the single file where it is used. Also Rename IPoIB module parameter variable from "debug_level" to "ipoib_debug_level". This avoids possible name clashes if IPoIB is built into the kernel. We use module_param_named so that the user-visible parameter names remain the same. Signed-off-by: Tom Duffy <tduffy@sun.com> Signed-off-by: Roland Dreier <roland@topspin.com> --- linux-export.orig/drivers/infiniband/ulp/ipoib/ipoib.h 2005-03-02 20:26:02.744892334 -0800 +++ linux-export/drivers/infiniband/ulp/ipoib/ipoib.h 2005-03-02 20:26:13.207621227 -0800 @@ -308,11 +308,11 @@ #ifdef CONFIG_INFINIBAND_IPOIB_DEBUG -extern int debug_level; +extern int ipoib_debug_level; #define ipoib_dbg(priv, format, arg...) \ do { \ - if (debug_level > 0) \ + if (ipoib_debug_level > 0) \ ipoib_printk(KERN_DEBUG, priv, format , ## arg); \ } while (0) #define ipoib_dbg_mcast(priv, format, arg...) \ --- linux-export.orig/drivers/infiniband/ulp/ipoib/ipoib_ib.c 2005-03-02 20:26:12.514771621 -0800 +++ linux-export/drivers/infiniband/ulp/ipoib/ipoib_ib.c 2005-03-02 20:26:13.208621010 -0800 @@ -40,7 +40,7 @@ #include "ipoib.h" #ifdef CONFIG_INFINIBAND_IPOIB_DEBUG_DATA -int data_debug_level; +static int data_debug_level; module_param(data_debug_level, int, 0644); MODULE_PARM_DESC(data_debug_level, --- linux-export.orig/drivers/infiniband/ulp/ipoib/ipoib_main.c 2005-03-02 20:26:02.744892334 -0800 +++ linux-export/drivers/infiniband/ulp/ipoib/ipoib_main.c 2005-03-02 20:26:13.207621227 -0800 @@ -51,9 +51,9 @@ MODULE_LICENSE("Dual BSD/GPL"); #ifdef CONFIG_INFINIBAND_IPOIB_DEBUG -int debug_level; +int ipoib_debug_level; -module_param(debug_level, int, 0644); +module_param_named(debug_level, ipoib_debug_level, int, 0644); MODULE_PARM_DESC(debug_level, "Enable debug tracing if > 0"); #endif ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH][9/11] IB/ipoib: small fixes 2005-03-03 5:31 ` [PATCH][8/11] IB/ipoib: rename global symbols Roland Dreier @ 2005-03-03 5:31 ` Roland Dreier 2005-03-03 5:31 ` [PATCH][10/11] IB/ipoib: don't call ipoib_put_ah with lock held Roland Dreier 0 siblings, 1 reply; 12+ messages in thread From: Roland Dreier @ 2005-03-03 5:31 UTC (permalink / raw) To: akpm; +Cc: linux-kernel, openib-general From: Shirley Ma <xma@us.ibm.com> IPoIB small fixes: Initialize path->ah to NULL, and fix dereference after free of neigh in error path of neigh_add_path(). Signed-off-by: Shirley Ma <xma@us.ibm.com> Signed-off-by: Roland Dreier <roland@topspin.com> --- linux-export.orig/drivers/infiniband/ulp/ipoib/ipoib_main.c 2005-03-02 20:26:13.207621227 -0800 +++ linux-export/drivers/infiniband/ulp/ipoib/ipoib_main.c 2005-03-02 20:26:13.653524436 -0800 @@ -346,8 +346,9 @@ if (!path) return NULL; - path->dev = dev; + path->dev = dev; path->pathrec.dlid = 0; + path->ah = NULL; skb_queue_head_init(&path->queue); @@ -450,8 +451,8 @@ err: *to_ipoib_neigh(skb->dst->neighbour) = NULL; list_del(&neigh->list); - kfree(neigh); neigh->neighbour->ops->destructor = NULL; + kfree(neigh); ++priv->stats.tx_dropped; dev_kfree_skb_any(skb); ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH][10/11] IB/ipoib: don't call ipoib_put_ah with lock held 2005-03-03 5:31 ` [PATCH][9/11] IB/ipoib: small fixes Roland Dreier @ 2005-03-03 5:31 ` Roland Dreier 2005-03-03 5:31 ` [PATCH][11/11] IB/ipoib: fix locking on path deletion Roland Dreier 0 siblings, 1 reply; 12+ messages in thread From: Roland Dreier @ 2005-03-03 5:31 UTC (permalink / raw) To: akpm; +Cc: linux-kernel, openib-general From: Shirley Ma <xma@us.ibm.com> ipoib_put_ah() may call ipoib_free_ah(), which might take the device's lock. Therefore we need to make sure we don't call ipoib_put_ah() when holding the lock already. Signed-off-by: Shirley Ma <xma@us.ibm.com> Signed-off-by: Roland Dreier <roland@topspin.com> --- linux-export.orig/drivers/infiniband/ulp/ipoib/ipoib_main.c 2005-03-02 20:26:13.653524436 -0800 +++ linux-export/drivers/infiniband/ulp/ipoib/ipoib_main.c 2005-03-02 20:26:13.977454122 -0800 @@ -661,6 +661,7 @@ struct ipoib_neigh *neigh = *to_ipoib_neigh(n); struct ipoib_dev_priv *priv = netdev_priv(n->dev); unsigned long flags; + struct ipoib_ah *ah = NULL; ipoib_dbg(priv, "neigh_destructor for %06x " IPOIB_GID_FMT "\n", @@ -671,13 +672,16 @@ if (neigh) { if (neigh->ah) - ipoib_put_ah(neigh->ah); + ah = neigh->ah; list_del(&neigh->list); *to_ipoib_neigh(n) = NULL; kfree(neigh); } spin_unlock_irqrestore(&priv->lock, flags); + + if (ah) + ipoib_put_ah(ah); } static int ipoib_neigh_setup(struct neighbour *neigh) --- linux-export.orig/drivers/infiniband/ulp/ipoib/ipoib_multicast.c 2005-03-02 20:26:12.799709771 -0800 +++ linux-export/drivers/infiniband/ulp/ipoib/ipoib_multicast.c 2005-03-02 20:26:13.977454122 -0800 @@ -93,6 +93,8 @@ struct ipoib_dev_priv *priv = netdev_priv(dev); struct ipoib_neigh *neigh, *tmp; unsigned long flags; + LIST_HEAD(ah_list); + struct ipoib_ah *ah, *tah; ipoib_dbg_mcast(netdev_priv(dev), "deleting multicast group " IPOIB_GID_FMT "\n", @@ -101,7 +103,8 @@ spin_lock_irqsave(&priv->lock, flags); list_for_each_entry_safe(neigh, tmp, &mcast->neigh_list, list) { - ipoib_put_ah(neigh->ah); + if (neigh->ah) + list_add_tail(&neigh->ah->list, &ah_list); *to_ipoib_neigh(neigh->neighbour) = NULL; neigh->neighbour->ops->destructor = NULL; kfree(neigh); @@ -109,6 +112,9 @@ spin_unlock_irqrestore(&priv->lock, flags); + list_for_each_entry_safe(ah, tah, &ah_list, list) + ipoib_put_ah(ah); + if (mcast->ah) ipoib_put_ah(mcast->ah); ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH][11/11] IB/ipoib: fix locking on path deletion 2005-03-03 5:31 ` [PATCH][10/11] IB/ipoib: don't call ipoib_put_ah with lock held Roland Dreier @ 2005-03-03 5:31 ` Roland Dreier 0 siblings, 0 replies; 12+ messages in thread From: Roland Dreier @ 2005-03-03 5:31 UTC (permalink / raw) To: akpm; +Cc: linux-kernel, openib-general Fix up locking for IPoIB path table. Make sure that destruction of address handles, neighbour info and path structs is locked properly to avoid races and deadlocks. (Problem originally diagnosed by Shirley Ma) Signed-off-by: Roland Dreier <roland@topspin.com> --- linux-export.orig/drivers/infiniband/ulp/ipoib/ipoib_main.c 2005-03-02 20:26:13.977454122 -0800 +++ linux-export/drivers/infiniband/ulp/ipoib/ipoib_main.c 2005-03-02 20:26:14.301383808 -0800 @@ -215,16 +215,25 @@ return 0; } -static void __path_free(struct net_device *dev, struct ipoib_path *path) +static void path_free(struct net_device *dev, struct ipoib_path *path) { struct ipoib_dev_priv *priv = netdev_priv(dev); struct ipoib_neigh *neigh, *tn; struct sk_buff *skb; + unsigned long flags; while ((skb = __skb_dequeue(&path->queue))) dev_kfree_skb_irq(skb); + spin_lock_irqsave(&priv->lock, flags); + list_for_each_entry_safe(neigh, tn, &path->neigh_list, list) { + /* + * It's safe to call ipoib_put_ah() inside priv->lock + * here, because we know that path->ah will always + * hold one more reference, so ipoib_put_ah() will + * never do more than decrement the ref count. + */ if (neigh->ah) ipoib_put_ah(neigh->ah); *to_ipoib_neigh(neigh->neighbour) = NULL; @@ -232,11 +241,11 @@ kfree(neigh); } + spin_unlock_irqrestore(&priv->lock, flags); + if (path->ah) ipoib_put_ah(path->ah); - rb_erase(&path->rb_node, &priv->path_tree); - list_del(&path->list); kfree(path); } @@ -248,15 +257,20 @@ unsigned long flags; spin_lock_irqsave(&priv->lock, flags); + list_splice(&priv->path_list, &remove_list); INIT_LIST_HEAD(&priv->path_list); + + list_for_each_entry(path, &remove_list, list) + rb_erase(&path->rb_node, &priv->path_tree); + spin_unlock_irqrestore(&priv->lock, flags); list_for_each_entry_safe(path, tp, &remove_list, list) { if (path->query) ib_sa_cancel_query(path->query_id, path->query); wait_for_completion(&path->done); - __path_free(dev, path); + path_free(dev, path); } } @@ -361,8 +375,6 @@ path->pathrec.pkey = cpu_to_be16(priv->pkey); path->pathrec.numb_path = 1; - __path_add(dev, path); - return path; } @@ -422,6 +434,8 @@ (union ib_gid *) (skb->dst->neighbour->ha + 4)); if (!path) goto err; + + __path_add(dev, path); } list_add_tail(&neigh->list, &path->neigh_list); @@ -497,8 +511,12 @@ skb_push(skb, sizeof *phdr); __skb_queue_tail(&path->queue, skb); - if (path_rec_start(dev, path)) - __path_free(dev, path); + if (path_rec_start(dev, path)) { + spin_unlock(&priv->lock); + path_free(dev, path); + return; + } else + __path_add(dev, path); } else { ++priv->stats.tx_dropped; dev_kfree_skb_any(skb); @@ -658,7 +676,7 @@ static void ipoib_neigh_destructor(struct neighbour *n) { - struct ipoib_neigh *neigh = *to_ipoib_neigh(n); + struct ipoib_neigh *neigh; struct ipoib_dev_priv *priv = netdev_priv(n->dev); unsigned long flags; struct ipoib_ah *ah = NULL; @@ -670,6 +688,7 @@ spin_lock_irqsave(&priv->lock, flags); + neigh = *to_ipoib_neigh(n); if (neigh) { if (neigh->ah) ah = neigh->ah; ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2005-03-03 6:08 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2005-03-03 5:31 [PATCH][0/11] InfiniBand fixes Roland Dreier 2005-03-03 5:31 ` [PATCH][1/11] IB: simplify MAD code Roland Dreier 2005-03-03 5:31 ` [PATCH][2/11] IB: fix vendor MAD deregistration Roland Dreier 2005-03-03 5:31 ` [PATCH][3/11] IB: sparse fixes Roland Dreier 2005-03-03 5:31 ` [PATCH][4/11] IB/mthca: add missing break Roland Dreier 2005-03-03 5:31 ` [PATCH][5/11] IB/mthca: fix reset value endianness Roland Dreier 2005-03-03 5:31 ` [PATCH][6/11] IB/ipoib: fix rx memory leak Roland Dreier 2005-03-03 5:31 ` [PATCH][7/11] IB/ipoib: use list_for_each_entry_safe when required Roland Dreier 2005-03-03 5:31 ` [PATCH][8/11] IB/ipoib: rename global symbols Roland Dreier 2005-03-03 5:31 ` [PATCH][9/11] IB/ipoib: small fixes Roland Dreier 2005-03-03 5:31 ` [PATCH][10/11] IB/ipoib: don't call ipoib_put_ah with lock held Roland Dreier 2005-03-03 5:31 ` [PATCH][11/11] IB/ipoib: fix locking on path deletion Roland Dreier
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.