Message-ID: <4F44934B.2000808@zytor.com>
Date:	Tue, 21 Feb 2012 23:03:39 -0800
From:	"H. Peter Anvin" <hpa@...or.com>
To:	Ingo Molnar <mingo@...e.hu>
CC:	Jason Baron <jbaron@...hat.com>, a.p.zijlstra@...llo.nl,
	rostedt@...dmis.org, mathieu.desnoyers@...icios.com,
	davem@...emloft.net, ddaney.cavm@...il.com,
	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 00/10] jump label: introduce very_[un]likely + cleanups
 + docs

On 02/21/2012 10:50 PM, Ingo Molnar wrote:
> 
> The pattern has spread beyond the niche of tracing internals and 
> nobody seemed to have any second thoughts about the actual 
> readability of these names. That is a major FAIL and it was my 
> primary concern.
> 
> For those *reading* the code, the similarity of 
> very_likely()/very_unlikely() to the likely()/unlikely() 
> patterns is *INTENTIONAL*, as functionally the branch site 
> itself is very similar to a real branch!
> 
> Secondly, for those *writing* such code, you cannot just 
> 'accidentally' change a unlikely() to a very_unlikely() and get 
> something you didn't ask for ...
> 
> It is the modification site that is dissimilar (and which is 
> slow in the jump label case) - and that is very apparently 
> dissimilar that it takes some effort to change these flags. If 
> you write such code you have to add the whole jump label 
> mechanism to it, which makes it very apparent what is happening 
> and makes the costs very apparent as well.
> 
> Thirdly, the fact that it's a 'jump label' is an 
> *implementational* detail. On some architectures very_unlikely() 
> indeed maps to unlikely(), because jump labels have not been 
> implemented yet. On some architectures very_unlikely() is 
> implemented via jump labels. On some future architectures it 
> might be implemented via JIT-ing the target function. (I made 
> the last one up)
> 
> So it makes sense to decouple 'jump labels' from the actual high 
> level naming of this API.
> 
> Anyway, I've Cc:-ed Linus and Andrew, I plan to take the renames 
> but they can NAK me if doing that is stupid.
> 

I have to VEHEMENTLY disagree with you here.

likely()/unlikely() are supposed to be used when it is clear at the time
the code is written which way the branch is biased or which path is
important (unlikely() is typically used for performance-unimportant
bailouts.)

Jump labels are not that way.  The key aspect for when the jump label
optimization is relevant is *how often is the direction changed*.  It is
perfectly sane for this to be done on a 50:50 branch, as long as the
50:50-ness is settled once for all dring a boot.  Consider, for example,
an Intel CPU versus an AMD CPU.  Wouldn't you agree that it would be
absolutely ridiculous and downright offensive to have:

	if (very_unlikely(cpu_vendor_amd)) {

Yet this is a *fantastic* use of the jump labels, because your CPU
vendor isn't going to change in the middle of execution (hot VM
migrations excepted ;)

So the key aspect of this is the staticness of the conditional, NOT the
degree of bias of the branch.  Hence my past insistence on the
"static_branch" name (rather than "jump_label")... the branch part can
be omitted, as an implementation detail, but the staticness of it is its
absolutely key defining characteristic.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists