IPoIB: Use netif_tx_lock() and get rid of private tx_lock, LLTX

Currently, IPoIB is an LLTX driver that uses its own IRQ-disabling
tx_lock.  Not only do we want to get rid of LLTX, this actually causes
problems because of the skb_orphan() done with this tx_lock held: some
skb destructors expect to be run with interrupts enabled.

The simplest fix for this is to get rid of the driver-private tx_lock
and stop using LLTX.  We kill off priv->tx_lock and use
netif_tx_lock[_bh]() instead; the patch to do this is a tiny bit
tricky because we need to update places that take priv->lock inside
the tx_lock to disable IRQs, rather than relying on tx_lock having
already disabled IRQs.

Also, there are a couple of places where we need to disable BHs to
make sure we have a consistent context to call netif_tx_lock() (since
we no longer can use _irqsave() variants), and we also have to change
ipoib_send_comp_handler() to call drain_tx_cq() through a timer rather
than directly, because ipoib_send_comp_handler() runs in interrupt
context and drain_tx_cq() must run in BH context so it can call
netif_tx_lock().

Signed-off-by: Roland Dreier <rolandd@cisco.com>

1 files changed, 22 insertions, 8 deletions

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
index 66cafa20c246..0e748aeeae99 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
@@ -468,21 +468,22 @@ void ipoib_ib_completion(struct ib_cq *cq, void *dev_ptr)
 static void drain_tx_cq(struct net_device *dev)
 {
         struct ipoib_dev_priv *priv = netdev_priv(dev);
-        unsigned long flags;
 
-        spin_lock_irqsave(&priv->tx_lock, flags);
+        netif_tx_lock(dev);
         while (poll_tx(priv))
                 ; /* nothing */
 
         if (netif_queue_stopped(dev))
                 mod_timer(&priv->poll_timer, jiffies + 1);
 
-        spin_unlock_irqrestore(&priv->tx_lock, flags);
+        netif_tx_unlock(dev);
 }
 
 void ipoib_send_comp_handler(struct ib_cq *cq, void *dev_ptr)
 {
-        drain_tx_cq((struct net_device *)dev_ptr);
+        struct ipoib_dev_priv *priv = netdev_priv(dev_ptr);
+
+        mod_timer(&priv->poll_timer, jiffies);
 }
 
 static inline int post_send(struct ipoib_dev_priv *priv,
@@ -614,17 +615,20 @@ static void __ipoib_reap_ah(struct net_device *dev)
         struct ipoib_dev_priv *priv = netdev_priv(dev);
         struct ipoib_ah *ah, *tah;
         LIST_HEAD(remove_list);
+        unsigned long flags;
+
+        netif_tx_lock_bh(dev);
+        spin_lock_irqsave(&priv->lock, flags);
 
-        spin_lock_irq(&priv->tx_lock);
-        spin_lock(&priv->lock);
         list_for_each_entry_safe(ah, tah, &priv->dead_ahs, list)
                 if ((int) priv->tx_tail - (int) ah->last_send >= 0) {
                         list_del(&ah->list);
                         ib_destroy_ah(ah->ah);
                         kfree(ah);
                 }
-        spin_unlock(&priv->lock);
-        spin_unlock_irq(&priv->tx_lock);
+
+        spin_unlock_irqrestore(&priv->lock, flags);
+        netif_tx_unlock_bh(dev);
 }
 
 void ipoib_reap_ah(struct work_struct *work)
@@ -761,6 +765,14 @@ void ipoib_drain_cq(struct net_device *dev)
 {
         struct ipoib_dev_priv *priv = netdev_priv(dev);
         int i, n;
+
+        /*
+         * We call completion handling routines that expect to be
+         * called from the BH-disabled NAPI poll context, so disable
+         * BHs here too.
+         */
+        local_bh_disable();
+
         do {
                 n = ib_poll_cq(priv->recv_cq, IPOIB_NUM_WC, priv->ibwc);
                 for (i = 0; i < n; ++i) {
@@ -784,6 +796,8 @@ void ipoib_drain_cq(struct net_device *dev)
 
         while (poll_tx(priv))
                 ; /* nothing */
+
+        local_bh_enable();
 }
 
 int ipoib_ib_dev_stop(struct net_device *dev, int flush)