From: AMEET M. PARANJAPE <aparanja@redhat.com> Date: Tue, 12 May 2009 09:20:24 -0400 Subject: [net] ehea: fix circular locking problem Message-id: 20090512131734.5710.54692.sendpatchset@squad5-lp1.lab.bos.redhat.com O-Subject: [PATCH RHEL5.4 BZ493359] ehea: fix circular locking problem Bugzilla: 493359 RH-Acked-by: David Howells <dhowells@redhat.com> RHBZ#: ====== https://bugzilla.redhat.com/show_bug.cgi?id=493359 Description: =========== This patch fixes the circular locking problem by changing the locking strategy concerning the logging of firmware handles. RHEL Version Found: ================ RHEL 5.3 kABI Status: ============ No symbols were harmed. Brew: ===== Built on all platforms. http://brewweb.devel.redhat.com/brew/taskinfo?taskID=1792650 Upstream Status: ================ http://www.spinics.net/lists/netdev/msg91286.html Test Status: ============ The driver has executed the following tests succesfully: - ipv4: ping, flood ping, broadcast ping - ipv6: ping, flood ping, TCP traffic - flood ping with big pakets - ftp tests with large files using 4 connections with TSO on/off - ftp long run using 4 connections with TSO on/off - netpipe - netperf - netperf via IPv6 - netperf/udp - vlan ping - multicast basic =============================================================== Ameet Paranjape 978-392-3903 ext 23903 IBM on-site partner Proposed Patch: =============== diff --git a/drivers/net/ehea/ehea.h b/drivers/net/ehea/ehea.h index f51c878..4b37a9a 100644 --- a/drivers/net/ehea/ehea.h +++ b/drivers/net/ehea/ehea.h @@ -40,7 +40,7 @@ #include <asm/io.h> #define DRV_NAME "ehea" -#define DRV_VERSION "EHEA_0092-03" +#define DRV_VERSION "EHEA_0092-04" /* eHEA capability flags */ #define DLPAR_PORT_ADD_REM 1 diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c index bb6f832..a139a32 100644 --- a/drivers/net/ehea/ehea_main.c +++ b/drivers/net/ehea/ehea_main.c @@ -154,6 +154,8 @@ static void ehea_update_firmware_handles(void) int num_fw_handles, k, l; /* Determine number of handles */ + mutex_lock(&ehea_fw_handles.lock); + list_for_each_entry(adapter, &adapter_list, list) { num_adapters++; @@ -175,15 +177,19 @@ static void ehea_update_firmware_handles(void) if (num_fw_handles) { arr = kzalloc(num_fw_handles * sizeof(*arr), GFP_KERNEL); if (!arr) - return; /* Keep the existing array */ + goto out; /* Keep the existing array */ } else goto out_update; list_for_each_entry(adapter, &adapter_list, list) { + if (num_adapters == 0) + break; + for (k = 0; k < EHEA_MAX_PORTS; k++) { struct ehea_port *port = adapter->port[k]; - if (!port || (port->state != EHEA_PORT_UP)) + if (!port || (port->state != EHEA_PORT_UP) + || (num_ports == 0)) continue; for (l = 0; @@ -206,6 +212,7 @@ static void ehea_update_firmware_handles(void) } arr[i].adh = adapter->handle; arr[i++].fwh = port->qp_eq->fw_handle; + num_ports--; } arr[i].adh = adapter->handle; @@ -215,16 +222,20 @@ static void ehea_update_firmware_handles(void) arr[i].adh = adapter->handle; arr[i++].fwh = adapter->mr.handle; } + num_adapters--; } out_update: kfree(ehea_fw_handles.arr); ehea_fw_handles.arr = arr; ehea_fw_handles.num_entries = i; +out: + mutex_unlock(&ehea_fw_handles.lock); } static void ehea_update_bcmc_registrations(void) { + unsigned long flags; struct ehea_bcmc_reg_entry *arr = NULL; struct ehea_adapter *adapter; struct ehea_mc_list *mc_entry; @@ -232,6 +243,8 @@ static void ehea_update_bcmc_registrations(void) int i = 0; int k; + spin_lock_irqsave(&ehea_bcmc_regs.lock, flags); + /* Determine number of registrations */ list_for_each_entry(adapter, &adapter_list, list) for (k = 0; k < EHEA_MAX_PORTS; k++) { @@ -249,7 +262,7 @@ static void ehea_update_bcmc_registrations(void) if (num_registrations) { arr = kzalloc(num_registrations * sizeof(*arr), GFP_ATOMIC); if (!arr) - return; /* Keep the existing array */ + goto out; /* Keep the existing array */ } else goto out_update; @@ -260,6 +273,9 @@ static void ehea_update_bcmc_registrations(void) if (!port || (port->state != EHEA_PORT_UP)) continue; + if (num_registrations == 0) + goto out_update; + arr[i].adh = adapter->handle; arr[i].port_id = port->logical_port_id; arr[i].reg_type = EHEA_BCMC_BROADCAST | @@ -271,9 +287,13 @@ static void ehea_update_bcmc_registrations(void) arr[i].reg_type = EHEA_BCMC_BROADCAST | EHEA_BCMC_VLANID_ALL; arr[i++].macaddr = port->mac_addr; + num_registrations -= 2; list_for_each_entry(mc_entry, &port->mc_list->list, list) { + if (num_registrations == 0) + goto out_update; + arr[i].adh = adapter->handle; arr[i].port_id = port->logical_port_id; arr[i].reg_type = EHEA_BCMC_SCOPE_ALL | @@ -287,6 +307,7 @@ static void ehea_update_bcmc_registrations(void) EHEA_BCMC_MULTICAST | EHEA_BCMC_VLANID_ALL; arr[i++].macaddr = mc_entry->macaddr; + num_registrations -= 2; } } } @@ -295,6 +316,8 @@ out_update: kfree(ehea_bcmc_regs.arr); ehea_bcmc_regs.arr = arr; ehea_bcmc_regs.num_entries = i; +out: + spin_unlock_irqrestore(&ehea_bcmc_regs.lock, flags); } static struct net_device_stats *ehea_get_stats(struct net_device *dev) @@ -1782,8 +1805,6 @@ static int ehea_set_mac_addr(struct net_device *dev, void *sa) memcpy(dev->dev_addr, mac_addr->sa_data, dev->addr_len); - spin_lock(&ehea_bcmc_regs.lock); - /* Deregister old MAC in pHYP */ if (port->state == EHEA_PORT_UP) { ret = ehea_broadcast_reg_helper(port, H_DEREG_BCMC); @@ -1804,7 +1825,6 @@ static int ehea_set_mac_addr(struct net_device *dev, void *sa) out_upregs: ehea_update_bcmc_registrations(); - spin_unlock(&ehea_bcmc_regs.lock); out_free: kfree(cb0); out: @@ -1966,8 +1986,6 @@ static void ehea_set_multicast_list(struct net_device *dev) } ehea_promiscuous(dev, 0); - spin_lock(&ehea_bcmc_regs.lock); - if (dev->flags & IFF_ALLMULTI) { ehea_allmulti(dev, 1); goto out; @@ -1997,7 +2015,6 @@ static void ehea_set_multicast_list(struct net_device *dev) } out: ehea_update_bcmc_registrations(); - spin_unlock(&ehea_bcmc_regs.lock); return; } @@ -2477,8 +2494,6 @@ static int ehea_up(struct net_device *dev) if (port->state == EHEA_PORT_UP) return 0; - mutex_lock(&ehea_fw_handles.lock); - ret = ehea_port_res_setup(port, port->num_def_qps, port->num_add_tx_qps); if (ret) { @@ -2515,8 +2530,6 @@ static int ehea_up(struct net_device *dev) } } - spin_lock(&ehea_bcmc_regs.lock); - ret = ehea_broadcast_reg_helper(port, H_REG_BCMC); if (ret) { ret = -EIO; @@ -2538,10 +2551,7 @@ out: ehea_info("Failed starting %s. ret=%i", dev->name, ret); ehea_update_bcmc_registrations(); - spin_unlock(&ehea_bcmc_regs.lock); - ehea_update_firmware_handles(); - mutex_unlock(&ehea_fw_handles.lock); return ret; } @@ -2593,9 +2603,6 @@ static int ehea_down(struct net_device *dev) if (port->state == EHEA_PORT_DOWN) return 0; - mutex_lock(&ehea_fw_handles.lock); - - spin_lock(&ehea_bcmc_regs.lock); ehea_drop_multicast_list(dev); ehea_broadcast_reg_helper(port, H_DEREG_BCMC); @@ -2604,7 +2611,6 @@ static int ehea_down(struct net_device *dev) port->state = EHEA_PORT_DOWN; ehea_update_bcmc_registrations(); - spin_unlock(&ehea_bcmc_regs.lock); ret = ehea_clean_all_portres(port); if (ret) @@ -2612,7 +2618,6 @@ static int ehea_down(struct net_device *dev) dev->name, ret); ehea_update_firmware_handles(); - mutex_unlock(&ehea_fw_handles.lock); return ret; } @@ -3438,7 +3443,6 @@ static int __devinit ehea_probe_adapter(struct ibmebus_dev *dev, ehea_error("Invalid ibmebus device probed"); return -EINVAL; } - mutex_lock(&ehea_fw_handles.lock); adapter = kzalloc(sizeof(*adapter), GFP_KERNEL); if (!adapter) { @@ -3523,7 +3527,7 @@ out_free_ad: out: ehea_update_firmware_handles(); - mutex_unlock(&ehea_fw_handles.lock); + return ret; } @@ -3542,8 +3546,6 @@ static int __devexit ehea_remove(struct ibmebus_dev *dev) flush_scheduled_work(); - mutex_lock(&ehea_fw_handles.lock); - ibmebus_free_irq(NULL, adapter->neq->attr.ist1, adapter); tasklet_kill(&adapter->neq_tasklet); @@ -3553,7 +3555,6 @@ static int __devexit ehea_remove(struct ibmebus_dev *dev) kfree(adapter); ehea_update_firmware_handles(); - mutex_unlock(&ehea_fw_handles.lock); return 0; }