Re: Possible eHEA performance issue

[Thomas Klein <osstklei@de.ibm.com>]

Michael Neuling wrote:
> From ehea_start_xmit in ehea_main.c we have:
> 
>     if (unlikely(atomic_read(&pr->swqe_avail) <= 1)) {
> 	    spin_lock_irqsave(&pr->netif_queue, flags);
> 	    if (unlikely(atomic_read(&pr->swqe_avail) <= 1)) {
> 		    pr->p_stats.queue_stopped++;
> 		    netif_stop_queue(dev);
> 		    pr->queue_stopped = 1;
> 	    }
> 	    spin_unlock_irqrestore(&pr->netif_queue, flags);
>     }
> 
> Since the conditions are the same, isn't it likely that the second 'if'
> is going to be taken.  Hence, shouldn't the second 'unlikely' hint be
> removed or even changed to likely?
> 
> Either way, some documentation here as to why it's done this way would
> be useful.  I assume the atomic_read is cheap compared to the
> spin_unlock_irqsave, so we quickly check swqe_avail before we check it
> again properly with the lock on so we can change some stuff.
> 
> Mikey

Hi Mike,

good point the second if could be a likely(). The impact isn't that big
because the if statement is true in the unlikely() case that the send queue
is full - which doesn't happen often. Anyway we will modify this in one of
the next driver versions. Thanks for the hint!

Thomas