datapath: Don't hold dp_mutex when setting internal devs MTU.
authorJesse Gross <jesse@nicira.com>
Tue, 27 Apr 2010 01:08:54 +0000 (18:08 -0700)
committerJesse Gross <jesse@nicira.com>
Sat, 1 May 2010 00:23:44 +0000 (17:23 -0700)
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.

datapath/datapath.c
datapath/dp_notify.c
datapath/vport-internal_dev.c

index 83dd76fefcbd4d6d072c98d72b12b7cc5285cd2d..71a44e6fcb4cd0201a5e753c33bf9b0d4153c09b 100644 (file)
@@ -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;
index 4a16a93fdc5356d24620b94690b8476a16b03774..e73a731a3fdaffc60d0b6705ea99eac052e02951 100644 (file)
@@ -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;
index 39283e052cc7d0b75e8795d0c32dd5d534998881..88b6cc1923bfa31f5807eab0e4382f581f3e4298 100644 (file)
@@ -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;
        }