Re: [patch 05/61] lock validator: introduce WARN_ON_ONCE(cond)

[Arjan van de Ven]

On Sat, 2006-06-03 at 14:09 -0400, Steven Rostedt wrote:

> 
> As you can see, because the whole thing is unlikely, the first condition
> is expected to fail.  With the current WARN_ON logic, that means that
> the __warn_once is expected to fail, but that's not the case.  So on a
> normal system where the WARN_ON_ONCE condition would never happen, you
> are always branching. 

which is no cost since it's consistent for the branch predictor

>   So simply reversing the order to test the
> condition before testing the __warn_once variable should improve cache
> performance.
> -	if (unlikely(__warn_once && (condition))) {	\
> +	if (unlikely((condition) && __warn_once)) {	\
>  		__warn_once = 0;			\

I disagree with this; "condition" can be a relatively complex thing,
such as a function call. doing the cheaper (and consistent!) test first
will be better. __warn_once will be branch predicted correctly ALWAYS,
except the exact ONE time you turn hit the backtrace. So it's really
really cheap to test, and if the WARN_ON_ONCE is triggering a lot after
the first time, you now would have a flapping first condition (which
means lots of branch mispredicts) while the original code has a perfect
one-check-predicted-exit scenario.



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