| Message ID | 1387432860-14021-1-git-send-email-jiang.liu@linux.intel.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On 12/18/2013 10:00 PM, Jiang Liu wrote: > According to documentation for netdev_for_each_all_upper_dev_rcu(), > caller should take RCU read lock, otherwise it will trigger following > warnings. The simplest fix is to protect with rcu_read_lock()/ > rcu_read_unlock(). And this solutions passes basic tests. > > Otherwise we may need to introduce netdev_for_each_all_upper_dev() > as netdev_lower_get_next_private(), which assumes the caller has > gained the lock to protect the list. > > Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com> > --- Jiang, The following patch addresses the same issue, http://patchwork.ozlabs.org/patch/302711/ [net,1/5] net: allow netdev_all_upper_get_next_dev_rcu with rtnl lock held It is under review and solves the problem by having the call check for the rtnl lock as well. Take a look. I think it is a slightly better fix in that it doesn't clutter the driver with unnecessary rcu_read_lock() and rcu_read_unlock() calls. Thanks, John -- 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 --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index cc06854..da72a5f 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -4336,6 +4336,7 @@ static void ixgbe_configure_dfwd(struct ixgbe_adapter *adapter) struct list_head *iter; int err; + rcu_read_lock(); netdev_for_each_all_upper_dev_rcu(adapter->netdev, upper, iter) { if (netif_is_macvlan(upper)) { struct macvlan_dev *dfwd = netdev_priv(upper); @@ -4348,6 +4349,7 @@ static void ixgbe_configure_dfwd(struct ixgbe_adapter *adapter) } } } + rcu_read_unlock(); } static void ixgbe_configure(struct ixgbe_adapter *adapter) @@ -4601,6 +4603,7 @@ static void ixgbe_up_complete(struct ixgbe_adapter *adapter) netif_tx_start_all_queues(adapter->netdev); /* enable any upper devices */ + rcu_read_lock(); netdev_for_each_all_upper_dev_rcu(adapter->netdev, upper, iter) { if (netif_is_macvlan(upper)) { struct macvlan_dev *vlan = netdev_priv(upper); @@ -4609,6 +4612,7 @@ static void ixgbe_up_complete(struct ixgbe_adapter *adapter) netif_tx_start_all_queues(upper); } } + rcu_read_unlock(); /* bring the link up in the watchdog, this could race with our first * link up interrupt but shouldn't be a problem */ @@ -4803,6 +4807,7 @@ void ixgbe_down(struct ixgbe_adapter *adapter) netif_tx_disable(netdev); /* disable any upper devices */ + rcu_read_lock(); netdev_for_each_all_upper_dev_rcu(adapter->netdev, upper, iter) { if (netif_is_macvlan(upper)) { struct macvlan_dev *vlan = netdev_priv(upper); @@ -4814,6 +4819,7 @@ void ixgbe_down(struct ixgbe_adapter *adapter) } } } + rcu_read_unlock(); ixgbe_irq_disable(adapter);
According to documentation for netdev_for_each_all_upper_dev_rcu(), caller should take RCU read lock, otherwise it will trigger following warnings. The simplest fix is to protect with rcu_read_lock()/ rcu_read_unlock(). And this solutions passes basic tests. Otherwise we may need to introduce netdev_for_each_all_upper_dev() as netdev_lower_get_next_private(), which assumes the caller has gained the lock to protect the list. Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ++++++ 1 file changed, 6 insertions(+)