From 4e0ab200fa4594d16bb7fc697ec4d9edfb5a43f0 Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Fri, 30 Jan 2015 15:38:46 -0800 Subject: Revert "IPoIB: Make ipoib_mcast_stop_thread flush the workqueue" This reverts commit bb42a6dd02fb2901a69dbec2358810735b14b186. The series of IPoIB bug fixes that went into 3.19-rc1 introduce regressions, and after trying to sort things out, we decided to revert to 3.18's IPoIB driver and get things right for 3.20. Signed-off-by: Roland Dreier --- drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'drivers/infiniband/ulp/ipoib/ipoib_multicast.c') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c index bc50dd0d0e4..845f910eb21 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c @@ -648,7 +648,7 @@ int ipoib_mcast_start_thread(struct net_device *dev) return 0; } -int ipoib_mcast_stop_thread(struct net_device *dev) +int ipoib_mcast_stop_thread(struct net_device *dev, int flush) { struct ipoib_dev_priv *priv = netdev_priv(dev); @@ -659,7 +659,8 @@ int ipoib_mcast_stop_thread(struct net_device *dev) cancel_delayed_work(&priv->mcast_task); mutex_unlock(&mcast_mutex); - flush_workqueue(priv->wq); + if (flush) + flush_workqueue(priv->wq); return 0; } @@ -837,6 +838,8 @@ void ipoib_mcast_restart_task(struct work_struct *work) ipoib_dbg_mcast(priv, "restarting multicast task\n"); + ipoib_mcast_stop_thread(dev, 0); + local_irq_save(flags); netif_addr_lock(dev); spin_lock(&priv->lock); @@ -933,10 +936,13 @@ void ipoib_mcast_restart_task(struct work_struct *work) * We have to cancel outside of the spinlock, but we have to * take the rtnl lock or else we race with the removal of * entries from the remove list in mcast_dev_flush as part - * of ipoib_stop(). We detect the drop of the ADMIN_UP flag - * to signal that we have hit this particular race, and we - * return since we know we don't need to do anything else - * anyway. + * of ipoib_stop() which will call mcast_stop_thread with + * flush == 1 while holding the rtnl lock, and the + * flush_workqueue won't complete until this restart_mcast_task + * completes. So do like the carrier on task and attempt to + * take the rtnl lock, but if we can't before the ADMIN_UP flag + * goes away, then just return and know that the remove list will + * get flushed later by mcast_stop_thread. */ while (!rtnl_trylock()) { if (!test_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags)) @@ -948,9 +954,6 @@ void ipoib_mcast_restart_task(struct work_struct *work) ipoib_mcast_leave(mcast->dev, mcast); ipoib_mcast_free(mcast); } - /* - * Restart our join task if needed - */ ipoib_mcast_start_thread(dev); rtnl_unlock(); } -- cgit v1.2.3-70-g09d2 From 0306eda2269f961d77c09b1badc0031438c21f44 Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Fri, 30 Jan 2015 15:38:55 -0800 Subject: Revert "IPoIB: Use dedicated workqueues per interface" This reverts commit 5141861cd5e17eac9676ff49c5abfafbea2b0e98. The series of IPoIB bug fixes that went into 3.19-rc1 introduce regressions, and after trying to sort things out, we decided to revert to 3.18's IPoIB driver and get things right for 3.20. Signed-off-by: Roland Dreier --- drivers/infiniband/ulp/ipoib/ipoib.h | 1 - drivers/infiniband/ulp/ipoib/ipoib_cm.c | 18 +++++++++--------- drivers/infiniband/ulp/ipoib/ipoib_ib.c | 6 +++--- drivers/infiniband/ulp/ipoib/ipoib_main.c | 19 +++++++------------ drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 26 ++++++++++++++------------ drivers/infiniband/ulp/ipoib/ipoib_verbs.c | 22 +--------------------- 6 files changed, 34 insertions(+), 58 deletions(-) (limited to 'drivers/infiniband/ulp/ipoib/ipoib_multicast.c') diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h index 45fd10a72ec..f4c1b20b23b 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib.h +++ b/drivers/infiniband/ulp/ipoib/ipoib.h @@ -323,7 +323,6 @@ struct ipoib_dev_priv { struct list_head multicast_list; struct rb_root multicast_tree; - struct workqueue_struct *wq; struct delayed_work mcast_task; struct work_struct carrier_on_task; struct work_struct flush_light; diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c index 56959adb6c7..933efcea0d0 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c @@ -474,7 +474,7 @@ static int ipoib_cm_req_handler(struct ib_cm_id *cm_id, struct ib_cm_event *even } spin_lock_irq(&priv->lock); - queue_delayed_work(priv->wq, + queue_delayed_work(ipoib_workqueue, &priv->cm.stale_task, IPOIB_CM_RX_DELAY); /* Add this entry to passive ids list head, but do not re-add it * if IB_EVENT_QP_LAST_WQE_REACHED has moved it to flush list. */ @@ -576,7 +576,7 @@ void ipoib_cm_handle_rx_wc(struct net_device *dev, struct ib_wc *wc) spin_lock_irqsave(&priv->lock, flags); list_splice_init(&priv->cm.rx_drain_list, &priv->cm.rx_reap_list); ipoib_cm_start_rx_drain(priv); - queue_work(priv->wq, &priv->cm.rx_reap_task); + queue_work(ipoib_workqueue, &priv->cm.rx_reap_task); spin_unlock_irqrestore(&priv->lock, flags); } else ipoib_warn(priv, "cm recv completion event with wrid %d (> %d)\n", @@ -603,7 +603,7 @@ void ipoib_cm_handle_rx_wc(struct net_device *dev, struct ib_wc *wc) spin_lock_irqsave(&priv->lock, flags); list_move(&p->list, &priv->cm.rx_reap_list); spin_unlock_irqrestore(&priv->lock, flags); - queue_work(priv->wq, &priv->cm.rx_reap_task); + queue_work(ipoib_workqueue, &priv->cm.rx_reap_task); } return; } @@ -827,7 +827,7 @@ void ipoib_cm_handle_tx_wc(struct net_device *dev, struct ib_wc *wc) if (test_and_clear_bit(IPOIB_FLAG_INITIALIZED, &tx->flags)) { list_move(&tx->list, &priv->cm.reap_list); - queue_work(priv->wq, &priv->cm.reap_task); + queue_work(ipoib_workqueue, &priv->cm.reap_task); } clear_bit(IPOIB_FLAG_OPER_UP, &tx->flags); @@ -1255,7 +1255,7 @@ static int ipoib_cm_tx_handler(struct ib_cm_id *cm_id, if (test_and_clear_bit(IPOIB_FLAG_INITIALIZED, &tx->flags)) { list_move(&tx->list, &priv->cm.reap_list); - queue_work(priv->wq, &priv->cm.reap_task); + queue_work(ipoib_workqueue, &priv->cm.reap_task); } spin_unlock_irqrestore(&priv->lock, flags); @@ -1284,7 +1284,7 @@ struct ipoib_cm_tx *ipoib_cm_create_tx(struct net_device *dev, struct ipoib_path tx->dev = dev; list_add(&tx->list, &priv->cm.start_list); set_bit(IPOIB_FLAG_INITIALIZED, &tx->flags); - queue_work(priv->wq, &priv->cm.start_task); + queue_work(ipoib_workqueue, &priv->cm.start_task); return tx; } @@ -1295,7 +1295,7 @@ void ipoib_cm_destroy_tx(struct ipoib_cm_tx *tx) if (test_and_clear_bit(IPOIB_FLAG_INITIALIZED, &tx->flags)) { spin_lock_irqsave(&priv->lock, flags); list_move(&tx->list, &priv->cm.reap_list); - queue_work(priv->wq, &priv->cm.reap_task); + queue_work(ipoib_workqueue, &priv->cm.reap_task); ipoib_dbg(priv, "Reap connection for gid %pI6\n", tx->neigh->daddr + 4); tx->neigh = NULL; @@ -1417,7 +1417,7 @@ void ipoib_cm_skb_too_long(struct net_device *dev, struct sk_buff *skb, skb_queue_tail(&priv->cm.skb_queue, skb); if (e) - queue_work(priv->wq, &priv->cm.skb_task); + queue_work(ipoib_workqueue, &priv->cm.skb_task); } static void ipoib_cm_rx_reap(struct work_struct *work) @@ -1450,7 +1450,7 @@ static void ipoib_cm_stale_task(struct work_struct *work) } if (!list_empty(&priv->cm.passive_ids)) - queue_delayed_work(priv->wq, + queue_delayed_work(ipoib_workqueue, &priv->cm.stale_task, IPOIB_CM_RX_DELAY); spin_unlock_irq(&priv->lock); } diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c index bfd17d41b5f..72626c34817 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c @@ -655,7 +655,7 @@ void ipoib_reap_ah(struct work_struct *work) __ipoib_reap_ah(dev); if (!test_bit(IPOIB_STOP_REAPER, &priv->flags)) - queue_delayed_work(priv->wq, &priv->ah_reap_task, + queue_delayed_work(ipoib_workqueue, &priv->ah_reap_task, round_jiffies_relative(HZ)); } @@ -696,7 +696,7 @@ int ipoib_ib_dev_open(struct net_device *dev, int flush) } clear_bit(IPOIB_STOP_REAPER, &priv->flags); - queue_delayed_work(priv->wq, &priv->ah_reap_task, + queue_delayed_work(ipoib_workqueue, &priv->ah_reap_task, round_jiffies_relative(HZ)); if (!test_and_set_bit(IPOIB_FLAG_INITIALIZED, &priv->flags)) @@ -881,7 +881,7 @@ timeout: set_bit(IPOIB_STOP_REAPER, &priv->flags); cancel_delayed_work(&priv->ah_reap_task); if (flush) - flush_workqueue(priv->wq); + flush_workqueue(ipoib_workqueue); begin = jiffies; diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index 42e5c278f48..2cf81ef5141 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -839,7 +839,7 @@ static void ipoib_set_mcast_list(struct net_device *dev) return; } - queue_work(priv->wq, &priv->restart_task); + queue_work(ipoib_workqueue, &priv->restart_task); } static u32 ipoib_addr_hash(struct ipoib_neigh_hash *htbl, u8 *daddr) @@ -954,7 +954,7 @@ static void ipoib_reap_neigh(struct work_struct *work) __ipoib_reap_neigh(priv); if (!test_bit(IPOIB_STOP_NEIGH_GC, &priv->flags)) - queue_delayed_work(priv->wq, &priv->neigh_reap_task, + queue_delayed_work(ipoib_workqueue, &priv->neigh_reap_task, arp_tbl.gc_interval); } @@ -1133,7 +1133,7 @@ static int ipoib_neigh_hash_init(struct ipoib_dev_priv *priv) /* start garbage collection */ clear_bit(IPOIB_STOP_NEIGH_GC, &priv->flags); - queue_delayed_work(priv->wq, &priv->neigh_reap_task, + queue_delayed_work(ipoib_workqueue, &priv->neigh_reap_task, arp_tbl.gc_interval); return 0; @@ -1293,7 +1293,7 @@ int ipoib_dev_init(struct net_device *dev, struct ib_device *ca, int port) return 0; out_dev_uninit: - ipoib_ib_dev_cleanup(dev); + ipoib_ib_dev_cleanup(); out_tx_ring_cleanup: vfree(priv->tx_ring); @@ -1646,7 +1646,7 @@ register_failed: /* Stop GC if started before flush */ set_bit(IPOIB_STOP_NEIGH_GC, &priv->flags); cancel_delayed_work(&priv->neigh_reap_task); - flush_workqueue(priv->wq); + flush_workqueue(ipoib_workqueue); event_failed: ipoib_dev_cleanup(priv->dev); @@ -1717,7 +1717,7 @@ static void ipoib_remove_one(struct ib_device *device) /* Stop GC */ set_bit(IPOIB_STOP_NEIGH_GC, &priv->flags); cancel_delayed_work(&priv->neigh_reap_task); - flush_workqueue(priv->wq); + flush_workqueue(ipoib_workqueue); unregister_netdev(priv->dev); free_netdev(priv->dev); @@ -1758,13 +1758,8 @@ static int __init ipoib_init_module(void) * unregister_netdev() and linkwatch_event take the rtnl lock, * so flush_scheduled_work() can deadlock during device * removal. - * - * In addition, bringing one device up and another down at the - * same time can deadlock a single workqueue, so we have this - * global fallback workqueue, but we also attempt to open a - * per device workqueue each time we bring an interface up */ - ipoib_workqueue = create_singlethread_workqueue("ipoib_flush"); + ipoib_workqueue = create_singlethread_workqueue("ipoib"); if (!ipoib_workqueue) { ret = -ENOMEM; goto err_fs; diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c index 845f910eb21..41325960e4e 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c @@ -388,7 +388,7 @@ void ipoib_mcast_carrier_on_task(struct work_struct *work) * the workqueue while holding the rtnl lock, so loop * on trylock until either we get the lock or we see * FLAG_ADMIN_UP go away as that signals that we are bailing - * and can safely ignore the carrier on work. + * and can safely ignore the carrier on work */ while (!rtnl_trylock()) { if (!test_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags)) @@ -432,14 +432,15 @@ static int ipoib_mcast_join_complete(int status, if (!status) { mcast->backoff = 1; if (test_bit(IPOIB_MCAST_RUN, &priv->flags)) - queue_delayed_work(priv->wq, &priv->mcast_task, 0); + queue_delayed_work(ipoib_workqueue, + &priv->mcast_task, 0); /* - * Defer carrier on work to priv->wq to avoid a + * Defer carrier on work to ipoib_workqueue to avoid a * deadlock on rtnl_lock here. */ if (mcast == priv->broadcast) - queue_work(priv->wq, &priv->carrier_on_task); + queue_work(ipoib_workqueue, &priv->carrier_on_task); } else { if (mcast->logcount++ < 20) { if (status == -ETIMEDOUT || status == -EAGAIN) { @@ -464,7 +465,7 @@ out: if (status == -ENETRESET) status = 0; if (status && test_bit(IPOIB_MCAST_RUN, &priv->flags)) - queue_delayed_work(priv->wq, &priv->mcast_task, + queue_delayed_work(ipoib_workqueue, &priv->mcast_task, mcast->backoff * HZ); spin_unlock_irq(&priv->lock); mutex_unlock(&mcast_mutex); @@ -534,7 +535,8 @@ static void ipoib_mcast_join(struct net_device *dev, struct ipoib_mcast *mcast, mcast->backoff = IPOIB_MAX_BACKOFF_SECONDS; if (test_bit(IPOIB_MCAST_RUN, &priv->flags)) - queue_delayed_work(priv->wq, &priv->mcast_task, + queue_delayed_work(ipoib_workqueue, + &priv->mcast_task, mcast->backoff * HZ); } mutex_unlock(&mcast_mutex); @@ -574,8 +576,8 @@ void ipoib_mcast_join_task(struct work_struct *work) ipoib_warn(priv, "failed to allocate broadcast group\n"); mutex_lock(&mcast_mutex); if (test_bit(IPOIB_MCAST_RUN, &priv->flags)) - queue_delayed_work(priv->wq, &priv->mcast_task, - HZ); + queue_delayed_work(ipoib_workqueue, + &priv->mcast_task, HZ); mutex_unlock(&mcast_mutex); return; } @@ -642,7 +644,7 @@ int ipoib_mcast_start_thread(struct net_device *dev) mutex_lock(&mcast_mutex); if (!test_and_set_bit(IPOIB_MCAST_RUN, &priv->flags)) - queue_delayed_work(priv->wq, &priv->mcast_task, 0); + queue_delayed_work(ipoib_workqueue, &priv->mcast_task, 0); mutex_unlock(&mcast_mutex); return 0; @@ -660,7 +662,7 @@ int ipoib_mcast_stop_thread(struct net_device *dev, int flush) mutex_unlock(&mcast_mutex); if (flush) - flush_workqueue(priv->wq); + flush_workqueue(ipoib_workqueue); return 0; } @@ -727,7 +729,7 @@ void ipoib_mcast_send(struct net_device *dev, u8 *daddr, struct sk_buff *skb) __ipoib_mcast_add(dev, mcast); list_add_tail(&mcast->list, &priv->multicast_list); if (!test_and_set_bit(IPOIB_MCAST_RUN, &priv->flags)) - queue_delayed_work(priv->wq, &priv->mcast_task, 0); + queue_delayed_work(ipoib_workqueue, &priv->mcast_task, 0); } if (!mcast->ah) { @@ -942,7 +944,7 @@ void ipoib_mcast_restart_task(struct work_struct *work) * completes. So do like the carrier on task and attempt to * take the rtnl lock, but if we can't before the ADMIN_UP flag * goes away, then just return and know that the remove list will - * get flushed later by mcast_stop_thread. + * get flushed later by mcast_dev_flush. */ while (!rtnl_trylock()) { if (!test_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags)) diff --git a/drivers/infiniband/ulp/ipoib/ipoib_verbs.c b/drivers/infiniband/ulp/ipoib/ipoib_verbs.c index b72a753eb41..c56d5d44c53 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_verbs.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_verbs.c @@ -145,20 +145,10 @@ int ipoib_transport_dev_init(struct net_device *dev, struct ib_device *ca) int ret, size; int i; - /* - * the various IPoIB tasks assume they will never race against - * themselves, so always use a single thread workqueue - */ - priv->wq = create_singlethread_workqueue("ipoib_wq"); - if (!priv->wq) { - printk(KERN_WARNING "ipoib: failed to allocate device WQ\n"); - return -ENODEV; - } - priv->pd = ib_alloc_pd(priv->ca); if (IS_ERR(priv->pd)) { printk(KERN_WARNING "%s: failed to allocate PD\n", ca->name); - goto out_free_wq; + return -ENODEV; } priv->mr = ib_get_dma_mr(priv->pd, IB_ACCESS_LOCAL_WRITE); @@ -252,10 +242,6 @@ out_free_mr: out_free_pd: ib_dealloc_pd(priv->pd); - -out_free_wq: - destroy_workqueue(priv->wq); - priv->wq = NULL; return -ENODEV; } @@ -284,12 +270,6 @@ void ipoib_transport_dev_cleanup(struct net_device *dev) if (ib_dealloc_pd(priv->pd)) ipoib_warn(priv, "ib_dealloc_pd failed\n"); - - if (priv->wq) { - flush_workqueue(priv->wq); - destroy_workqueue(priv->wq); - priv->wq = NULL; - } } void ipoib_event(struct ib_event_handler *handler, -- cgit v1.2.3-70-g09d2 From 962121b4fcd3e022c276104aec50b56d7ed1f71b Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Fri, 30 Jan 2015 15:39:11 -0800 Subject: Revert "IPoIB: fix mcast_dev_flush/mcast_restart_task race" This reverts commit e5d1dcf1b0951f4ba00d93653942dda6196109d8. The series of IPoIB bug fixes that went into 3.19-rc1 introduce regressions, and after trying to sort things out, we decided to revert to 3.18's IPoIB driver and get things right for 3.20. Signed-off-by: Roland Dreier --- drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 37 ++++---------------------- 1 file changed, 5 insertions(+), 32 deletions(-) (limited to 'drivers/infiniband/ulp/ipoib/ipoib_multicast.c') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c index 41325960e4e..a52c9f3f7e4 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c @@ -802,10 +802,7 @@ void ipoib_mcast_dev_flush(struct net_device *dev) spin_unlock_irqrestore(&priv->lock, flags); - /* - * make sure the in-flight joins have finished before we attempt - * to leave - */ + /* seperate between the wait to the leave*/ list_for_each_entry_safe(mcast, tmcast, &remove_list, list) if (test_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags)) wait_for_completion(&mcast->done); @@ -926,38 +923,14 @@ void ipoib_mcast_restart_task(struct work_struct *work) netif_addr_unlock(dev); local_irq_restore(flags); - /* - * make sure the in-flight joins have finished before we attempt - * to leave - */ - list_for_each_entry_safe(mcast, tmcast, &remove_list, list) - if (test_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags)) - wait_for_completion(&mcast->done); - - /* - * We have to cancel outside of the spinlock, but we have to - * take the rtnl lock or else we race with the removal of - * entries from the remove list in mcast_dev_flush as part - * of ipoib_stop() which will call mcast_stop_thread with - * flush == 1 while holding the rtnl lock, and the - * flush_workqueue won't complete until this restart_mcast_task - * completes. So do like the carrier on task and attempt to - * take the rtnl lock, but if we can't before the ADMIN_UP flag - * goes away, then just return and know that the remove list will - * get flushed later by mcast_dev_flush. - */ - while (!rtnl_trylock()) { - if (!test_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags)) - return; - else - msleep(20); - } + /* We have to cancel outside of the spinlock */ list_for_each_entry_safe(mcast, tmcast, &remove_list, list) { ipoib_mcast_leave(mcast->dev, mcast); ipoib_mcast_free(mcast); } - ipoib_mcast_start_thread(dev); - rtnl_unlock(); + + if (test_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags)) + ipoib_mcast_start_thread(dev); } #ifdef CONFIG_INFINIBAND_IPOIB_DEBUG -- cgit v1.2.3-70-g09d2 From e7a623d2df28a477efb6cf60471a4e2225e2e8e4 Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Fri, 30 Jan 2015 15:39:20 -0800 Subject: Revert "IPoIB: fix MCAST_FLAG_BUSY usage" This reverts commit 016d9fb25cd9817ea9c723f4f7ecd978636b4489. The series of IPoIB bug fixes that went into 3.19-rc1 introduce regressions, and after trying to sort things out, we decided to revert to 3.18's IPoIB driver and get things right for 3.20. Signed-off-by: Roland Dreier --- drivers/infiniband/ulp/ipoib/ipoib.h | 10 +- drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 148 +++++++++---------------- 2 files changed, 57 insertions(+), 101 deletions(-) (limited to 'drivers/infiniband/ulp/ipoib/ipoib_multicast.c') diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h index f4c1b20b23b..d7562beb542 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib.h +++ b/drivers/infiniband/ulp/ipoib/ipoib.h @@ -98,15 +98,9 @@ enum { IPOIB_MCAST_FLAG_FOUND = 0, /* used in set_multicast_list */ IPOIB_MCAST_FLAG_SENDONLY = 1, - /* - * For IPOIB_MCAST_FLAG_BUSY - * When set, in flight join and mcast->mc is unreliable - * When clear and mcast->mc IS_ERR_OR_NULL, need to restart or - * haven't started yet - * When clear and mcast->mc is valid pointer, join was successful - */ - IPOIB_MCAST_FLAG_BUSY = 2, + IPOIB_MCAST_FLAG_BUSY = 2, /* joining or already joined */ IPOIB_MCAST_FLAG_ATTACHED = 3, + IPOIB_MCAST_JOIN_STARTED = 4, MAX_SEND_CQE = 16, IPOIB_CM_COPYBREAK = 256, diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c index a52c9f3f7e4..9862c76a83f 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c @@ -271,27 +271,16 @@ ipoib_mcast_sendonly_join_complete(int status, struct ipoib_mcast *mcast = multicast->context; struct net_device *dev = mcast->dev; - /* - * We have to take the mutex to force mcast_sendonly_join to - * return from ib_sa_multicast_join and set mcast->mc to a - * valid value. Otherwise we were racing with ourselves in - * that we might fail here, but get a valid return from - * ib_sa_multicast_join after we had cleared mcast->mc here, - * resulting in mis-matched joins and leaves and a deadlock - */ - mutex_lock(&mcast_mutex); - /* We trap for port events ourselves. */ if (status == -ENETRESET) - goto out; + return 0; if (!status) status = ipoib_mcast_join_finish(mcast, &multicast->rec); if (status) { if (mcast->logcount++ < 20) - ipoib_dbg_mcast(netdev_priv(dev), "sendonly multicast " - "join failed for %pI6, status %d\n", + ipoib_dbg_mcast(netdev_priv(dev), "multicast join failed for %pI6, status %d\n", mcast->mcmember.mgid.raw, status); /* Flush out any queued packets */ @@ -301,15 +290,11 @@ ipoib_mcast_sendonly_join_complete(int status, dev_kfree_skb_any(skb_dequeue(&mcast->pkt_queue)); } netif_tx_unlock_bh(dev); + + /* Clear the busy flag so we try again */ + status = test_and_clear_bit(IPOIB_MCAST_FLAG_BUSY, + &mcast->flags); } -out: - clear_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags); - if (status) - mcast->mc = NULL; - complete(&mcast->done); - if (status == -ENETRESET) - status = 0; - mutex_unlock(&mcast_mutex); return status; } @@ -327,14 +312,12 @@ static int ipoib_mcast_sendonly_join(struct ipoib_mcast *mcast) int ret = 0; if (!test_bit(IPOIB_FLAG_OPER_UP, &priv->flags)) { - ipoib_dbg_mcast(priv, "device shutting down, no sendonly " - "multicast joins\n"); + ipoib_dbg_mcast(priv, "device shutting down, no multicast joins\n"); return -ENODEV; } - if (test_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags)) { - ipoib_dbg_mcast(priv, "multicast entry busy, skipping " - "sendonly join\n"); + if (test_and_set_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags)) { + ipoib_dbg_mcast(priv, "multicast entry busy, skipping\n"); return -EBUSY; } @@ -342,9 +325,6 @@ static int ipoib_mcast_sendonly_join(struct ipoib_mcast *mcast) rec.port_gid = priv->local_gid; rec.pkey = cpu_to_be16(priv->pkey); - mutex_lock(&mcast_mutex); - init_completion(&mcast->done); - set_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags); mcast->mc = ib_sa_join_multicast(&ipoib_sa_client, priv->ca, priv->port, &rec, IB_SA_MCMEMBER_REC_MGID | @@ -357,14 +337,12 @@ static int ipoib_mcast_sendonly_join(struct ipoib_mcast *mcast) if (IS_ERR(mcast->mc)) { ret = PTR_ERR(mcast->mc); clear_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags); - complete(&mcast->done); - ipoib_warn(priv, "ib_sa_join_multicast for sendonly join " - "failed (ret = %d)\n", ret); + ipoib_warn(priv, "ib_sa_join_multicast failed (ret = %d)\n", + ret); } else { - ipoib_dbg_mcast(priv, "no multicast record for %pI6, starting " - "sendonly join\n", mcast->mcmember.mgid.raw); + ipoib_dbg_mcast(priv, "no multicast record for %pI6, starting join\n", + mcast->mcmember.mgid.raw); } - mutex_unlock(&mcast_mutex); return ret; } @@ -412,28 +390,22 @@ static int ipoib_mcast_join_complete(int status, ipoib_dbg_mcast(priv, "join completion for %pI6 (status %d)\n", mcast->mcmember.mgid.raw, status); - /* - * We have to take the mutex to force mcast_join to - * return from ib_sa_multicast_join and set mcast->mc to a - * valid value. Otherwise we were racing with ourselves in - * that we might fail here, but get a valid return from - * ib_sa_multicast_join after we had cleared mcast->mc here, - * resulting in mis-matched joins and leaves and a deadlock - */ - mutex_lock(&mcast_mutex); - /* We trap for port events ourselves. */ - if (status == -ENETRESET) + if (status == -ENETRESET) { + status = 0; goto out; + } if (!status) status = ipoib_mcast_join_finish(mcast, &multicast->rec); if (!status) { mcast->backoff = 1; + mutex_lock(&mcast_mutex); if (test_bit(IPOIB_MCAST_RUN, &priv->flags)) queue_delayed_work(ipoib_workqueue, &priv->mcast_task, 0); + mutex_unlock(&mcast_mutex); /* * Defer carrier on work to ipoib_workqueue to avoid a @@ -441,35 +413,37 @@ static int ipoib_mcast_join_complete(int status, */ if (mcast == priv->broadcast) queue_work(ipoib_workqueue, &priv->carrier_on_task); - } else { - if (mcast->logcount++ < 20) { - if (status == -ETIMEDOUT || status == -EAGAIN) { - ipoib_dbg_mcast(priv, "multicast join failed for %pI6, status %d\n", - mcast->mcmember.mgid.raw, status); - } else { - ipoib_warn(priv, "multicast join failed for %pI6, status %d\n", - mcast->mcmember.mgid.raw, status); - } - } - mcast->backoff *= 2; - if (mcast->backoff > IPOIB_MAX_BACKOFF_SECONDS) - mcast->backoff = IPOIB_MAX_BACKOFF_SECONDS; + status = 0; + goto out; } -out: + + if (mcast->logcount++ < 20) { + if (status == -ETIMEDOUT || status == -EAGAIN) { + ipoib_dbg_mcast(priv, "multicast join failed for %pI6, status %d\n", + mcast->mcmember.mgid.raw, status); + } else { + ipoib_warn(priv, "multicast join failed for %pI6, status %d\n", + mcast->mcmember.mgid.raw, status); + } + } + + mcast->backoff *= 2; + if (mcast->backoff > IPOIB_MAX_BACKOFF_SECONDS) + mcast->backoff = IPOIB_MAX_BACKOFF_SECONDS; + + /* Clear the busy flag so we try again */ + status = test_and_clear_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags); + + mutex_lock(&mcast_mutex); spin_lock_irq(&priv->lock); - clear_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags); - if (status) - mcast->mc = NULL; - complete(&mcast->done); - if (status == -ENETRESET) - status = 0; - if (status && test_bit(IPOIB_MCAST_RUN, &priv->flags)) + if (test_bit(IPOIB_MCAST_RUN, &priv->flags)) queue_delayed_work(ipoib_workqueue, &priv->mcast_task, mcast->backoff * HZ); spin_unlock_irq(&priv->lock); mutex_unlock(&mcast_mutex); - +out: + complete(&mcast->done); return status; } @@ -518,9 +492,10 @@ static void ipoib_mcast_join(struct net_device *dev, struct ipoib_mcast *mcast, rec.hop_limit = priv->broadcast->mcmember.hop_limit; } - mutex_lock(&mcast_mutex); - init_completion(&mcast->done); set_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags); + init_completion(&mcast->done); + set_bit(IPOIB_MCAST_JOIN_STARTED, &mcast->flags); + mcast->mc = ib_sa_join_multicast(&ipoib_sa_client, priv->ca, priv->port, &rec, comp_mask, GFP_KERNEL, ipoib_mcast_join_complete, mcast); @@ -534,12 +509,13 @@ static void ipoib_mcast_join(struct net_device *dev, struct ipoib_mcast *mcast, if (mcast->backoff > IPOIB_MAX_BACKOFF_SECONDS) mcast->backoff = IPOIB_MAX_BACKOFF_SECONDS; + mutex_lock(&mcast_mutex); if (test_bit(IPOIB_MCAST_RUN, &priv->flags)) queue_delayed_work(ipoib_workqueue, &priv->mcast_task, mcast->backoff * HZ); + mutex_unlock(&mcast_mutex); } - mutex_unlock(&mcast_mutex); } void ipoib_mcast_join_task(struct work_struct *work) @@ -592,8 +568,7 @@ void ipoib_mcast_join_task(struct work_struct *work) } if (!test_bit(IPOIB_MCAST_FLAG_ATTACHED, &priv->broadcast->flags)) { - if (IS_ERR_OR_NULL(priv->broadcast->mc) && - !test_bit(IPOIB_MCAST_FLAG_BUSY, &priv->broadcast->flags)) + if (!test_bit(IPOIB_MCAST_FLAG_BUSY, &priv->broadcast->flags)) ipoib_mcast_join(dev, priv->broadcast, 0); return; } @@ -601,33 +576,23 @@ void ipoib_mcast_join_task(struct work_struct *work) while (1) { struct ipoib_mcast *mcast = NULL; - /* - * Need the mutex so our flags are consistent, need the - * priv->lock so we don't race with list removals in either - * mcast_dev_flush or mcast_restart_task - */ - mutex_lock(&mcast_mutex); spin_lock_irq(&priv->lock); list_for_each_entry(mcast, &priv->multicast_list, list) { - if (IS_ERR_OR_NULL(mcast->mc) && - !test_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags) && - !test_bit(IPOIB_MCAST_FLAG_ATTACHED, &mcast->flags)) { + if (!test_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags) + && !test_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags) + && !test_bit(IPOIB_MCAST_FLAG_ATTACHED, &mcast->flags)) { /* Found the next unjoined group */ break; } } spin_unlock_irq(&priv->lock); - mutex_unlock(&mcast_mutex); if (&mcast->list == &priv->multicast_list) { /* All done */ break; } - if (test_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags)) - ipoib_mcast_sendonly_join(mcast); - else - ipoib_mcast_join(dev, mcast, 1); + ipoib_mcast_join(dev, mcast, 1); return; } @@ -673,9 +638,6 @@ static int ipoib_mcast_leave(struct net_device *dev, struct ipoib_mcast *mcast) int ret = 0; if (test_and_clear_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags)) - ipoib_warn(priv, "ipoib_mcast_leave on an in-flight join\n"); - - if (!IS_ERR_OR_NULL(mcast->mc)) ib_sa_free_multicast(mcast->mc); if (test_and_clear_bit(IPOIB_MCAST_FLAG_ATTACHED, &mcast->flags)) { @@ -728,8 +690,6 @@ void ipoib_mcast_send(struct net_device *dev, u8 *daddr, struct sk_buff *skb) memcpy(mcast->mcmember.mgid.raw, mgid, sizeof (union ib_gid)); __ipoib_mcast_add(dev, mcast); list_add_tail(&mcast->list, &priv->multicast_list); - if (!test_and_set_bit(IPOIB_MCAST_RUN, &priv->flags)) - queue_delayed_work(ipoib_workqueue, &priv->mcast_task, 0); } if (!mcast->ah) { @@ -743,6 +703,8 @@ void ipoib_mcast_send(struct net_device *dev, u8 *daddr, struct sk_buff *skb) if (test_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags)) ipoib_dbg_mcast(priv, "no address vector, " "but multicast join already started\n"); + else if (test_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags)) + ipoib_mcast_sendonly_join(mcast); /* * If lookup completes between here and out:, don't @@ -804,7 +766,7 @@ void ipoib_mcast_dev_flush(struct net_device *dev) /* seperate between the wait to the leave*/ list_for_each_entry_safe(mcast, tmcast, &remove_list, list) - if (test_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags)) + if (test_bit(IPOIB_MCAST_JOIN_STARTED, &mcast->flags)) wait_for_completion(&mcast->done); list_for_each_entry_safe(mcast, tmcast, &remove_list, list) { -- cgit v1.2.3-70-g09d2 From c6a7ec7a0f3b7817ac1710c41fdeb84a3c33a47a Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Fri, 30 Jan 2015 15:39:29 -0800 Subject: Revert "IPoIB: Make the carrier_on_task race aware" This reverts commit 67d7209e1f481cbaed37f9a224a328a3f83d0482. The series of IPoIB bug fixes that went into 3.19-rc1 introduce regressions, and after trying to sort things out, we decided to revert to 3.18's IPoIB driver and get things right for 3.20. Signed-off-by: Roland Dreier --- drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) (limited to 'drivers/infiniband/ulp/ipoib/ipoib_multicast.c') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c index 9862c76a83f..eee66d13e5b 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c @@ -353,27 +353,18 @@ void ipoib_mcast_carrier_on_task(struct work_struct *work) carrier_on_task); struct ib_port_attr attr; + /* + * Take rtnl_lock to avoid racing with ipoib_stop() and + * turning the carrier back on while a device is being + * removed. + */ if (ib_query_port(priv->ca, priv->port, &attr) || attr.state != IB_PORT_ACTIVE) { ipoib_dbg(priv, "Keeping carrier off until IB port is active\n"); return; } - /* - * Take rtnl_lock to avoid racing with ipoib_stop() and - * turning the carrier back on while a device is being - * removed. However, ipoib_stop() will attempt to flush - * the workqueue while holding the rtnl lock, so loop - * on trylock until either we get the lock or we see - * FLAG_ADMIN_UP go away as that signals that we are bailing - * and can safely ignore the carrier on work - */ - while (!rtnl_trylock()) { - if (!test_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags)) - return; - else - msleep(20); - } + rtnl_lock(); if (!ipoib_cm_admin_enabled(priv->dev)) dev_set_mtu(priv->dev, min(priv->mcast_mtu, priv->admin_mtu)); netif_carrier_on(priv->dev); -- cgit v1.2.3-70-g09d2 From 4143a9515d355857aeaa151a63cf21dd061151d9 Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Fri, 30 Jan 2015 15:39:36 -0800 Subject: Revert "IPoIB: Consolidate rtnl_lock tasks in workqueue" This reverts commit afe1de664ef3cb756e70938d99417dcbc6b1379a. The series of IPoIB bug fixes that went into 3.19-rc1 introduce regressions, and after trying to sort things out, we decided to revert to 3.18's IPoIB driver and get things right for 3.20. Signed-off-by: Roland Dreier --- drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'drivers/infiniband/ulp/ipoib/ipoib_multicast.c') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c index eee66d13e5b..ffb83b5f7e8 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c @@ -190,6 +190,12 @@ static int ipoib_mcast_join_finish(struct ipoib_mcast *mcast, spin_unlock_irq(&priv->lock); priv->tx_wr.wr.ud.remote_qkey = priv->qkey; set_qkey = 1; + + if (!ipoib_cm_admin_enabled(dev)) { + rtnl_lock(); + dev_set_mtu(dev, min(priv->mcast_mtu, priv->admin_mtu)); + rtnl_unlock(); + } } if (!test_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags)) { @@ -365,8 +371,6 @@ void ipoib_mcast_carrier_on_task(struct work_struct *work) } rtnl_lock(); - if (!ipoib_cm_admin_enabled(priv->dev)) - dev_set_mtu(priv->dev, min(priv->mcast_mtu, priv->admin_mtu)); netif_carrier_on(priv->dev); rtnl_unlock(); } -- cgit v1.2.3-70-g09d2