[ofa-general] [PATCH] ipoib: defer skb_orphan() until irqs enabled

[Eli Cohen]

On Thu, 2008-09-11 at 14:19 -0700, Roland Dreier wrote:
> Maybe I'm missing something but where is the logic that stops draining
> the CQ?  I just see
> 
Well, there is a hole after all... you're right.

> static int poll_tx(struct ipoib_dev_priv *priv)
> {
> 	int n, i;
> 
> 	n = ib_poll_cq(priv->send_cq, MAX_SEND_CQE, priv->send_wc);
> 	for (i = 0; i < n; ++i)
> 		ipoib_ib_handle_tx_wc(priv->dev, priv->send_wc + i);
> 
> 	return n == MAX_SEND_CQE;
> }
> 
> and
> 
> 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);
> 	while (poll_tx(priv))
> 		; /* nothing */
> 
> which seem like they could easily poll that last completion.
There is no problem to poll the last completion. It's a problem if this
code polls the last completion before the transmit function calls
skb_orphan() on it, and that I think does not have much chances to
happen.

But, if we agree that the SKB posted just before the queue is stopped,
is the problematic one, we can extend the following condition to be:


       if (unlikely(priv->tx_outstanding > MAX_SEND_CQE))
               while (poll_tx(priv))
                       ; /* nothing */

-       return ret;

+       return !(sent && priv->tx_outstanding && !netif_queue_stopped(dev));
 }

what do you think?