Messages in this thread | | | Date | Wed, 22 Feb 2012 09:18:55 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH 00/10] jump label: introduce very_[un]likely + cleanups + docs |
| |
* H. Peter Anvin <hpa@zytor.com> wrote:
> Stupid thought... do we have cases that matter where the bias > and default don't agree?
Yeah, that was one of my worries about the proposed original tongue twisters (see Jason's original series: "jump label: introduce default true branch").
For example could you tell *at a glance* what this does:
+ if (!static_branch_def_false(&perf_sched_events.key))
?
I certainly couldn't, I'd have to consider the '!', that it's a 'static branch' and that it's either 'defined to false' or 'default to false'.
Linguistic and visual barriers all around, and that's for code that I am intimately familar with ...
The problem with static_branch_def_false/def_true was that the very intuitively visible bias that we see with likely()/unlikely() is confused in jump label constructs through two layers of modifiers. And the fix is so easy, a simple rename in most cases ;-)
So instead of that, in this series we have:
+ if (very_unlikely(&perf_sched_events.key))
which is a heck of an improvement IMO. I'd still up its readability a notch, by also signalling the overhead of the update path by making it:
+ if (very_unlikely(&perf_sched_events.slow_flag))
... but I don't want to be that much of a readability nazi ;-)
Thanks,
Ingo
| |