Commit 059fe7a5 authored by Jay Vosburgh's avatar Jay Vosburgh Committed by Jeff Garzik
Browse files

bonding: Convert locks to _bh, rework alb locking for new locking 



	Convert locking-related activity to new & improved system.
Convert some lock acquisitions to _bh and rework parts of ALB mode, both
to avoid deadlocks with workqueue activity.

Signed-off-by: default avatarAndy Gospodarek <andy@greyhouse.net>
Signed-off-by: default avatarJay Vosburgh <fubar@us.ibm.com>
Signed-off-by: default avatarJeff Garzik <jeff@garzik.org>
parent 0b0eef66

drivers/net/bonding/bond_alb.c

+63 −6
Original line number Diff line number Diff line
@@ -959,19 +959,34 @@ static int alb_set_slave_mac_addr(struct slave *slave, u8 addr[], int hw) 
	return 0;

}



/* Caller must hold bond lock for write or curr_slave_lock for write*/

/*

 * Swap MAC addresses between two slaves.

 *

 * Called with RTNL held, and no other locks.

 *

 */



static void alb_swap_mac_addr(struct bonding *bond, struct slave *slave1, struct slave *slave2)

{

	struct slave *disabled_slave = NULL;

	u8 tmp_mac_addr[ETH_ALEN];

	int slaves_state_differ;



	slaves_state_differ = (SLAVE_IS_OK(slave1) != SLAVE_IS_OK(slave2));



	memcpy(tmp_mac_addr, slave1->dev->dev_addr, ETH_ALEN);

	alb_set_slave_mac_addr(slave1, slave2->dev->dev_addr, bond->alb_info.rlb_enabled);

	alb_set_slave_mac_addr(slave2, tmp_mac_addr, bond->alb_info.rlb_enabled);



}



/*

 * Send learning packets after MAC address swap.

 *

 * Called with RTNL and bond->lock held for read.

 */

static void alb_fasten_mac_swap(struct bonding *bond, struct slave *slave1,

				struct slave *slave2)

{

	int slaves_state_differ = (SLAVE_IS_OK(slave1) != SLAVE_IS_OK(slave2));

	struct slave *disabled_slave = NULL;



	/* fasten the change in the switch */

	if (SLAVE_IS_OK(slave1)) {

		alb_send_learning_packets(slave1, slave1->dev->dev_addr);
@@ -1044,7 +1059,9 @@ static void alb_change_hw_addr_on_detach(struct bonding *bond, struct slave *sla 
		}



		if (found) {

			/* locking: needs RTNL and nothing else */

			alb_swap_mac_addr(bond, slave, tmp_slave);

			alb_fasten_mac_swap(bond, slave, tmp_slave);

		}

	}

}
@@ -1571,13 +1588,21 @@ void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char 
 * Set the bond->curr_active_slave to @new_slave and handle

 * mac address swapping and promiscuity changes as needed.

 *

 * Caller must hold bond curr_slave_lock for write (or bond lock for write)

 * If new_slave is NULL, caller must hold curr_slave_lock or

 * bond->lock for write.

 *

 * If new_slave is not NULL, caller must hold RTNL, bond->lock for

 * read and curr_slave_lock for write.  Processing here may sleep, so

 * no other locks may be held.

 */

void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave)

{

	struct slave *swap_slave;

	int i;



	if (new_slave)

		ASSERT_RTNL();



	if (bond->curr_active_slave == new_slave) {

		return;

	}
@@ -1610,6 +1635,19 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave 
		}

	}



	/*

	 * Arrange for swap_slave and new_slave to temporarily be

	 * ignored so we can mess with their MAC addresses without

	 * fear of interference from transmit activity.

	 */

	if (swap_slave) {

		tlb_clear_slave(bond, swap_slave, 1);

	}

	tlb_clear_slave(bond, new_slave, 1);



	write_unlock_bh(&bond->curr_slave_lock);

	read_unlock(&bond->lock);



	/* curr_active_slave must be set before calling alb_swap_mac_addr */

	if (swap_slave) {

		/* swap mac address */
@@ -1618,11 +1656,23 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave 
		/* set the new_slave to the bond mac address */

		alb_set_slave_mac_addr(new_slave, bond->dev->dev_addr,

				       bond->alb_info.rlb_enabled);

	}



	read_lock(&bond->lock);



	if (swap_slave) {

		alb_fasten_mac_swap(bond, swap_slave, new_slave);

	} else {

		/* fasten bond mac on new current slave */

		alb_send_learning_packets(new_slave, bond->dev->dev_addr);

	}



	write_lock_bh(&bond->curr_slave_lock);

}



/*

 * Called with RTNL

 */

int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr)

{

	struct bonding *bond = bond_dev->priv;
@@ -1659,8 +1709,12 @@ int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr) 
		}

	}



	write_unlock_bh(&bond->curr_slave_lock);

	read_unlock(&bond->lock);



	if (swap_slave) {

		alb_swap_mac_addr(bond, swap_slave, bond->curr_active_slave);

		alb_fasten_mac_swap(bond, swap_slave, bond->curr_active_slave);

	} else {

		alb_set_slave_mac_addr(bond->curr_active_slave, bond_dev->dev_addr,

				       bond->alb_info.rlb_enabled);
@@ -1672,6 +1726,9 @@ int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr) 
		}

	}



	read_lock(&bond->lock);

	write_lock_bh(&bond->curr_slave_lock);



	return 0;

}

drivers/net/bonding/bond_main.c

+54 −24
Original line number Diff line number Diff line
@@ -1590,15 +1590,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) 
	case BOND_MODE_TLB:

	case BOND_MODE_ALB:

		new_slave->state = BOND_STATE_ACTIVE;

		if ((!bond->curr_active_slave) &&

		    (new_slave->link != BOND_LINK_DOWN)) {

			/* first slave or no active slave yet, and this link

			 * is OK, so make this interface the active one

			 */

			bond_change_active_slave(bond, new_slave);

		} else {

		bond_set_slave_inactive_flags(new_slave);

		}

		break;

	default:

		dprintk("This slave is always active in trunk mode\n");
@@ -1754,9 +1746,23 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev) 
		bond_alb_deinit_slave(bond, slave);

	}



	if (oldcurrent == slave)

	if (oldcurrent == slave) {

		/*

		 * Note that we hold RTNL over this sequence, so there

		 * is no concern that another slave add/remove event

		 * will interfere.

		 */

		write_unlock_bh(&bond->lock);

		read_lock(&bond->lock);

		write_lock_bh(&bond->curr_slave_lock);



		bond_select_active_slave(bond);



		write_unlock_bh(&bond->curr_slave_lock);

		read_unlock(&bond->lock);

		write_lock_bh(&bond->lock);

	}



	if (bond->slave_cnt == 0) {

		bond_set_carrier(bond);
@@ -2012,16 +2018,19 @@ static int bond_ioctl_change_active(struct net_device *bond_dev, struct net_devi 
		return -EINVAL;

	}



	write_lock_bh(&bond->lock);

	read_lock(&bond->lock);



	read_lock(&bond->curr_slave_lock);

	old_active = bond->curr_active_slave;

	read_unlock(&bond->curr_slave_lock);



	new_active = bond_get_slave_by_dev(bond, slave_dev);



	/*

	 * Changing to the current active: do nothing; return success.

	 */

	if (new_active && (new_active == old_active)) {

		write_unlock_bh(&bond->lock);

		read_unlock(&bond->lock);

		return 0;

	}
@@ -2029,12 +2038,14 @@ static int bond_ioctl_change_active(struct net_device *bond_dev, struct net_devi 
	    (old_active) &&

	    (new_active->link == BOND_LINK_UP) &&

	    IS_UP(new_active->dev)) {

		write_lock_bh(&bond->curr_slave_lock);

		bond_change_active_slave(bond, new_active);

		write_unlock_bh(&bond->curr_slave_lock);

	} else {

		res = -EINVAL;

	}



	write_unlock_bh(&bond->lock);

	read_unlock(&bond->lock);



	return res;

}
@@ -2140,7 +2151,11 @@ static int __bond_mii_monitor(struct bonding *bond, int have_locks) 
		switch (slave->link) {

		case BOND_LINK_UP:	/* the link was up */

			if (link_state == BMSR_LSTATUS) {

				/* link stays up, nothing more to do */

				if (!oldcurrent) {

					if (!have_locks)

						return 1;

					do_failover = 1;

				}

				break;

			} else { /* link going down */

				slave->link  = BOND_LINK_FAIL;
@@ -2327,11 +2342,14 @@ static int __bond_mii_monitor(struct bonding *bond, int have_locks) 
	} /* end of for */



	if (do_failover) {

		write_lock(&bond->curr_slave_lock);

		ASSERT_RTNL();



		write_lock_bh(&bond->curr_slave_lock);



		bond_select_active_slave(bond);



		write_unlock(&bond->curr_slave_lock);

		write_unlock_bh(&bond->curr_slave_lock);



	} else

		bond_set_carrier(bond);
@@ -2770,11 +2788,14 @@ void bond_loadbalance_arp_mon(struct work_struct *work) 
	}



	if (do_failover) {

		write_lock(&bond->curr_slave_lock);

		rtnl_lock();

		write_lock_bh(&bond->curr_slave_lock);



		bond_select_active_slave(bond);



		write_unlock(&bond->curr_slave_lock);

		write_unlock_bh(&bond->curr_slave_lock);

		rtnl_unlock();



	}



re_arm:
@@ -2831,7 +2852,9 @@ void bond_activebackup_arp_mon(struct work_struct *work) 


				slave->link = BOND_LINK_UP;



				write_lock(&bond->curr_slave_lock);

				rtnl_lock();



				write_lock_bh(&bond->curr_slave_lock);



				if ((!bond->curr_active_slave) &&

				    ((jiffies - slave->dev->trans_start) <= delta_in_ticks)) {
@@ -2865,7 +2888,8 @@ void bond_activebackup_arp_mon(struct work_struct *work) 
					       slave->dev->name);

				}



				write_unlock(&bond->curr_slave_lock);

				write_unlock_bh(&bond->curr_slave_lock);

				rtnl_unlock();

			}

		} else {

			read_lock(&bond->curr_slave_lock);
@@ -2935,12 +2959,15 @@ void bond_activebackup_arp_mon(struct work_struct *work) 
			       bond->dev->name,

			       slave->dev->name);



			write_lock(&bond->curr_slave_lock);

			rtnl_lock();

			write_lock_bh(&bond->curr_slave_lock);



			bond_select_active_slave(bond);

			slave = bond->curr_active_slave;



			write_unlock(&bond->curr_slave_lock);

			write_unlock_bh(&bond->curr_slave_lock);



			rtnl_unlock();



			bond->current_arp_slave = slave;
@@ -2959,9 +2986,12 @@ void bond_activebackup_arp_mon(struct work_struct *work) 
			       bond->primary_slave->dev->name);



			/* primary is up so switch to it */

			write_lock(&bond->curr_slave_lock);

			rtnl_lock();

			write_lock_bh(&bond->curr_slave_lock);

			bond_change_active_slave(bond, bond->primary_slave);

			write_unlock(&bond->curr_slave_lock);

			write_unlock_bh(&bond->curr_slave_lock);



			rtnl_unlock();



			slave = bond->primary_slave;

			slave->jiffies = jiffies;

CRA Git | Maintained and supported by SUSTech CRA and CCSE