Fix a deadlock between child interface creation/deletion and ipoib start/stop.
The former takes first vlan_mutex, and might take rtnl_lock via register_netdev
or unregister_netdev. The latter is executed with rtnl_lock held, and tries
to take vlan_mutex.
We take the vlan_mutex and bring child interface up/down on a scheduled task
instead of during stop/start, since ipoib_workqueue will not be flushed with
rtnl_lock held.
Signed-off-by: Yossi Etigin <[email protected]>
---
Changes from v1:
- Use u8 as the vlan task up/down flag type instead of atomic_t.
Fix bug #1198.
An alternative approach might be to fine-grain the locking (for example use
one mutex to sync child creation/deletion, and another one to sync accesses to
child_intfs list).
drivers/infiniband/ulp/ipoib/ipoib.h | 3 ++
drivers/infiniband/ulp/ipoib/ipoib_main.c | 33 ++++--------------------------
drivers/infiniband/ulp/ipoib/ipoib_vlan.c | 22 ++++++++++++++++++++
3 files changed, 30 insertions(+), 28 deletions(-)
Index: b/drivers/infiniband/ulp/ipoib/ipoib.h
===================================================================
--- a/drivers/infiniband/ulp/ipoib/ipoib.h 2008-12-15 20:28:21.000000000
+0200
+++ b/drivers/infiniband/ulp/ipoib/ipoib.h 2008-12-31 14:46:29.000000000
+0200
@@ -298,6 +298,8 @@ struct ipoib_dev_priv {
struct work_struct flush_heavy;
struct work_struct restart_task;
struct delayed_work ah_reap_task;
+ struct work_struct vlan_task;
+ u8 vlan_task_flag;
struct ib_device *ca;
u8 port;
@@ -501,6 +503,7 @@ void ipoib_event(struct ib_event_handler
int ipoib_vlan_add(struct net_device *pdev, unsigned short pkey);
int ipoib_vlan_delete(struct net_device *pdev, unsigned short pkey);
+void ipoib_vlan_task(struct work_struct *work);
void ipoib_pkey_poll(struct work_struct *work);
int ipoib_pkey_dev_delay_open(struct net_device *dev);
Index: b/drivers/infiniband/ulp/ipoib/ipoib_main.c
===================================================================
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c 2008-12-15 20:28:21.000000000
+0200
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c 2008-12-31 14:51:00.000000000
+0200
@@ -125,20 +125,8 @@ int ipoib_open(struct net_device *dev)
}
if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) {
- struct ipoib_dev_priv *cpriv;
-
- /* Bring up any child interfaces too */
- mutex_lock(&priv->vlan_mutex);
- list_for_each_entry(cpriv, &priv->child_intfs, list) {
- int flags;
-
- flags = cpriv->dev->flags;
- if (flags & IFF_UP)
- continue;
-
- dev_change_flags(cpriv->dev, flags | IFF_UP);
- }
- mutex_unlock(&priv->vlan_mutex);
+ priv->vlan_task_flag = 1;
+ queue_work(ipoib_workqueue, &priv->vlan_task);
}
netif_start_queue(dev);
@@ -161,20 +149,8 @@ static int ipoib_stop(struct net_device
ipoib_ib_dev_stop(dev, 0);
if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) {
- struct ipoib_dev_priv *cpriv;
-
- /* Bring down any child interfaces too */
- mutex_lock(&priv->vlan_mutex);
- list_for_each_entry(cpriv, &priv->child_intfs, list) {
- int flags;
-
- flags = cpriv->dev->flags;
- if (!(flags & IFF_UP))
- continue;
-
- dev_change_flags(cpriv->dev, flags & ~IFF_UP);
- }
- mutex_unlock(&priv->vlan_mutex);
+ priv->vlan_task_flag = 0;
+ queue_work(ipoib_workqueue, &priv->vlan_task);
}
return 0;
@@ -1071,6 +1047,7 @@ static void ipoib_setup(struct net_devic
INIT_WORK(&priv->flush_heavy, ipoib_ib_dev_flush_heavy);
INIT_WORK(&priv->restart_task, ipoib_mcast_restart_task);
INIT_DELAYED_WORK(&priv->ah_reap_task, ipoib_reap_ah);
+ INIT_WORK(&priv->vlan_task, ipoib_vlan_task);
}
struct ipoib_dev_priv *ipoib_intf_alloc(const char *name)
Index: b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c
===================================================================
--- a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c 2008-12-15 20:28:21.000000000
+0200
+++ b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c 2008-12-31 14:46:50.000000000
+0200
@@ -178,3 +178,25 @@ int ipoib_vlan_delete(struct net_device
return ret;
}
+
+void ipoib_vlan_task(struct work_struct *work)
+{
+ struct ipoib_dev_priv *priv =
+ container_of(work, struct ipoib_dev_priv, vlan_task);
+ struct ipoib_dev_priv *cpriv;
+ int flags, new_flags, iffup_value;
+
+ iffup_value = priv->vlan_task_flag ? IFF_UP : 0;
+
+ mutex_lock(&priv->vlan_mutex);
+ list_for_each_entry(cpriv, &priv->child_intfs, list) {
+ flags = cpriv->dev->flags;
+ new_flags = (flags & ~IFF_UP) | iffup_value;
+ if (flags != new_flags) {
+ rtnl_lock();
+ dev_change_flags(cpriv->dev, new_flags);
+ rtnl_unlock();
+ }
+ }
+ mutex_unlock(&priv->vlan_mutex);
+}
--
--Yossi
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
