netdev - Re: Possible eHEA performance issue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <46A06F7E.901@de.ibm.com>
Date:	Fri, 20 Jul 2007 10:17:02 +0200
From:	Thomas Klein <osstklei@...ibm.com>
To:	Michael Neuling <mikey@...ling.org>
CC:	Thomas Klein <tklein@...ibm.com>,
	Jan-Bernd Themann <themann@...ibm.com>,
	netdev <netdev@...r.kernel.org>,
	Christoph Raisch <raisch@...ibm.com>,
	Stefan Roscher <ossrosch@...ux.vnet.ibm.com>,
	linux-ppc <linuxppc-dev@...abs.org>, anton@...ba.org
Subject: Re: Possible eHEA performance issue

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

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html