Re: [PATCH] Fix WARN_ON / WARN_ON_ONCE regression

[Jeremy Fitzhardinge <jeremy@goop.org>]

Tim Chen wrote:
> I think if the condition changes between two evaluations, we do have a
> problem with my fix.

That's not the problem; one hopes that the WARN_ON predicate has no 
side-effects (though I know there are places which have side effects in 
BUG_ON).  The point is that the vast majority of WARN_ONs *don't* have 
their values used, so in the current code the variable reference is dead 
code and will be removed.  But if gcc can't prove the predicate is 
side-effect free (call to an external function, for example), then gcc 
will have to generate two calls to it, regardless of whether the second 
value is used.

And since the condition variable will - at worst - be stored on the 
stack on a hot cache line, I don't see how there could be any extra 
cache misses.

>   I don't have a better idea to avoid using a local
> variable to store the condition.  I think we should at least reverse the
> WARN_ON/WARN_ON_ONCE patch if a better way cannot be found.

I don't think you've proved your case here.  Do you *know* there are 
extra cache misses (ie, measuring them), or is it just your theory to 
explain a performance regression?

The other question is whether WARN_ON should return a value.  Where does 
it get used?  It doesn't seem very valuable.

    J