[openib-general] Re: [PATCH] ipoib: pkt_queue

[Michael S. Tsirkin]

Quoting r. Roland Dreier <rdreier at cisco.com>:
> Subject: Re: [PATCH] ipoib: pkt_queue
> 
>  > -	while (!skb_queue_empty(&mcast->pkt_queue))
>  > +	while (!skb_queue_empty(&mcast->pkt_queue)) {
>  > +		spin_lock_irqsave(&priv->tx_lock, flags);
>  > +		++priv->stats.tx_dropped;
>  > +		spin_unlock_irqrestore(&priv->tx_lock, flags);
>  >  		dev_kfree_skb_any(skb_dequeue(&mcast->pkt_queue));
>  > +	}
> 
> Any reason to drop the lock every time around this loop?  Would it
> make more sense to count the number of packets and then just add it in
> after the loop?

Makes sense.

>  > +	spin_lock_irq(&priv->tx_lock);
>  >  	while (!skb_queue_empty(&mcast->pkt_queue)) {
>  >  		struct sk_buff *skb = skb_dequeue(&mcast->pkt_queue);
>  > +		spin_unlock_irq(&priv->tx_lock);
> 
> Again, why are we dropping the lock every time through this loop?  Is
> it just to reduce the lock hold time here?

We seem to be doing operations that cant be called under
tx_lock a few lines below.

-- 
MST