From: Jesse Gross <jesse@nicira.com>
Date: Tue, 27 Apr 2010 01:08:54 +0000 (-0700)
Subject: datapath: Don't hold dp_mutex when setting internal devs MTU.
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d8b5d43a04fba8ba09d5bb3c745808964f371d80;p=openvswitch

datapath: Don't hold dp_mutex when setting internal devs MTU.

We currently acquire dp_mutex when we are notified that the MTU
of a device attached to the datapath has changed so that we can
set the internal devices to the minimum MTU.  However, it is not
required to hold dp_mutex because we already have RTNL lock and it
causes a deadlock, so don't do it.

Specifically, the issue is that DP mutex is acquired twice: once in
dp_device_event() before calling set_internal_devs_mtu() and then
again in internal_dev_change_mtu() when it is actually being changed
(since the MTU can also be set directly).  Since it's not a recursive
mutex, deadlock.
---

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 83dd76fe..71a44e6f 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -428,8 +428,7 @@ got_port_no:
 	if (err)
 		goto out_unlock_dp;
 
-	if (!(port.flags & ODP_PORT_INTERNAL))
-		set_internal_devs_mtu(dp);
+	set_internal_devs_mtu(dp);
 	dp_sysfs_add_if(dp->ports[port_no]);
 
 	err = __put_user(port_no, &portp->port);
@@ -1358,7 +1357,7 @@ int dp_min_mtu(const struct datapath *dp)
 }
 
 /* Sets the MTU of all datapath devices to the minimum of the ports.  Must
- * be called with RTNL lock and dp_mutex. */
+ * be called with RTNL lock. */
 void set_internal_devs_mtu(const struct datapath *dp)
 {
 	struct dp_port *p;
diff --git a/datapath/dp_notify.c b/datapath/dp_notify.c
index 4a16a93f..e73a731a 100644
--- a/datapath/dp_notify.c
+++ b/datapath/dp_notify.c
@@ -54,11 +54,8 @@ static int dp_device_event(struct notifier_block *unused, unsigned long event,
 		break;
 
 	case NETDEV_CHANGEMTU:
-		if (!is_internal_dev(dev)) {
-			mutex_lock(&dp->mutex);
+		if (!is_internal_dev(dev))
 			set_internal_devs_mtu(dp);
-			mutex_unlock(&dp->mutex);
-		}
 		break;
 	}
 	return NOTIFY_DONE;
diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
index 39283e05..88b6cc19 100644
--- a/datapath/vport-internal_dev.c
+++ b/datapath/vport-internal_dev.c
@@ -144,13 +144,7 @@ static int internal_dev_change_mtu(struct net_device *netdev, int new_mtu)
 		return -EINVAL;
 
 	if (dp_port) {
-		int min_mtu;
-
-		mutex_lock(&dp_port->dp->mutex);
-		min_mtu = dp_min_mtu(dp_port->dp);
-		mutex_unlock(&dp_port->dp->mutex);
-
-		if (new_mtu > min_mtu)
+		if (new_mtu > dp_min_mtu(dp_port->dp))
 			return -EINVAL;
 	}