[ewg] Re: Continue of "defer skb_orphan() until irqs enabled"

[Moni Shoua]

Eli Cohen wrote:
> On Wed, Sep 24, 2008 at 12:16:23PM -0700, akepner at sgi.com wrote:
>> On Wed, Sep 24, 2008 at 10:22:11AM -0700, Roland Dreier wrote:
>>>  > -	spin_unlock_irqrestore(&priv->tx_lock, flags);
>>>  > +	local_irq_restore(flags);
>>>  > +	if (orphan)
>>>  > +		skb_orphan(skb);
>>>  > +
>>>  > +	spin_unlock(&priv->tx_lock);
> I think this one will work, after applying the changes I proposed
> here:
Isn't it a little bit dangerous to enable interrupts at this point. Especially when we don't know what the 
skb destructor might do.
> http://lists.openfabrics.org/pipermail/general/2008-September/054079.html
> 
> But I would consider the patch bellow too since it allows the
> desctuctor to run with tx_lock freed.
> 
Are you sure? Although the chances are low I still don't see how it is 100% safe to assume that 
no cq polling that will drain the queue completely will happen if we don't hold the lock.

> +
> +	return !(sent && priv->tx_outstanding &&
> +			priv->tx_outstanding != ipoib_sendq_size);
>  }
>  
The second condition in the if() is contained in the third. I think you can drop it.
Your earlier patch checked for !netif_queue_stopped(). Is there a reason 
why you changed it to priv->tx_outstanding != ipoib_sendq_size?
Aren't they the same?