diff mbox

[net-next-2.6,2/2] bonding: don't lock when copying/clearing VLAN list on slave

Message ID 1279750488-32611-2-git-send-email-fubar@us.ibm.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Jay Vosburgh July 21, 2010, 10:14 p.m. UTC
When copying VLAN information to or removing from a slave
during slave addition or removal, the bonding code currently holds
the bond->lock for write to prevent concurrent modification of the
vlan_list / vlgrp.

	This is unnecessary, as all of these operations occur under
RTNL.  Holding the bond->lock also caused might_sleep issues for
some drivers' ndo_vlan_* functions.  This patch removes the extra
locking.

	Problem reported by Michael Chan <mchan@broadcom.com>

Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
Cc: Michael Chan <mchan@broadcom.com>
---
 drivers/net/bonding/bond_main.c |   16 +++-------------
 1 files changed, 3 insertions(+), 13 deletions(-)

Comments

David Miller July 22, 2010, 9:13 p.m. UTC | #1
From: Jay Vosburgh <fubar@us.ibm.com>
Date: Wed, 21 Jul 2010 15:14:48 -0700

> 	When copying VLAN information to or removing from a slave
> during slave addition or removal, the bonding code currently holds
> the bond->lock for write to prevent concurrent modification of the
> vlan_list / vlgrp.
> 
> 	This is unnecessary, as all of these operations occur under
> RTNL.  Holding the bond->lock also caused might_sleep issues for
> some drivers' ndo_vlan_* functions.  This patch removes the extra
> locking.
> 
> 	Problem reported by Michael Chan <mchan@broadcom.com>
> 
> Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index f3b01ce..2cc4cfc 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -570,10 +570,8 @@  static void bond_add_vlans_on_slave(struct bonding *bond, struct net_device *sla
 	struct vlan_entry *vlan;
 	const struct net_device_ops *slave_ops = slave_dev->netdev_ops;
 
-	write_lock_bh(&bond->lock);
-
 	if (!bond->vlgrp)
-		goto out;
+		return;
 
 	if ((slave_dev->features & NETIF_F_HW_VLAN_RX) &&
 	    slave_ops->ndo_vlan_rx_register)
@@ -581,13 +579,10 @@  static void bond_add_vlans_on_slave(struct bonding *bond, struct net_device *sla
 
 	if (!(slave_dev->features & NETIF_F_HW_VLAN_FILTER) ||
 	    !(slave_ops->ndo_vlan_rx_add_vid))
-		goto out;
+		return;
 
 	list_for_each_entry(vlan, &bond->vlan_list, vlan_list)
 		slave_ops->ndo_vlan_rx_add_vid(slave_dev, vlan->vlan_id);
-
-out:
-	write_unlock_bh(&bond->lock);
 }
 
 static void bond_del_vlans_from_slave(struct bonding *bond,
@@ -597,10 +592,8 @@  static void bond_del_vlans_from_slave(struct bonding *bond,
 	struct vlan_entry *vlan;
 	struct net_device *vlan_dev;
 
-	write_lock_bh(&bond->lock);
-
 	if (!bond->vlgrp)
-		goto out;
+		return;
 
 	if (!(slave_dev->features & NETIF_F_HW_VLAN_FILTER) ||
 	    !(slave_ops->ndo_vlan_rx_kill_vid))
@@ -621,9 +614,6 @@  unreg:
 	if ((slave_dev->features & NETIF_F_HW_VLAN_RX) &&
 	    slave_ops->ndo_vlan_rx_register)
 		slave_ops->ndo_vlan_rx_register(slave_dev, NULL);
-
-out:
-	write_unlock_bh(&bond->lock);
 }
 
 /*------------------------------- Link status -------------------------------*/