Re: [GIT PULL] locking fix

[Linus Torvalds <torvalds@linux-foundation.org>]

On Sun, Oct 27, 2013 at 12:00 PM, Maarten Lankhorst
<maarten.lankhorst@canonical.com> wrote:
> op 27-10-13 18:28, Linus Torvalds schreef:
>>
>> That expression is largely equivalent to
>> "__builtin_constant_p(ww_ctx)" (because iff ww_ctx is constant, then
>> the comparison to NULL is constant), which is actually much easier to
>> read, while carrying a totally different semantic meaning. Making
>> things worse, the comparison to NULL *may* be marked constant under
>> some very random situations (ie the compiler could turn a "taking an
>> address of a variable is never NULL" kind of knowledge and combining
>> it with other knowledge, and turn a complicated "ctx" expression into
>> a "I know this cannot be NULL" thing, and thus the "== NULL" is a
>> constant, even though ctx itself is some dynamic calculation).
>>
>> Whoever wrote the original should be shot. And this commit shouldn't
>> have been marked as being somehow about gcc-version dependence, but
>> about removing completely crap code.
>>
> Unfortunately gcc disagreed there, which was another compiler bug.

Stop this idiotic "blame gcc bug" crap. Which part of my explanation
for why it was *NOT* a compiler bug did you not understand?

> __builtin_constant_p(ww_ctx) was NOT equal to __builtin_constant_p(ww_ctx == NULL), iirc.

See my "largely equivalent" comment, with the *EXTRA* logic that gcc
may actually find cases where the comparison is a constant even if the
ww_ctx thing itself isn't a constant.

> __builtin_constant_p(ww_ctx == NULL) is equal to __builtin_constant_p(ww_ctx != NULL), but
> the former is more readable, since it shows we expect ww_ctx to be null.

Stop the f*cking around already! The  whole "we expect ww_ctx to be
null" thing shows that YOU DO NOT SEEM TO UNDERSTAND WHAT THE TEST
ACTUALLY IS!

The expression

   __builtin_constant_p(ww_ctx == NULL)

has ABSOLUTELY NOTHING to do with whether ww_ctx is NULL or not!
Christ, can you really not understand that?

For example, ww_ctx could be "&static_variable", and the compiler can
- and some compiles _will_ - say that ww_ctx clearly cannot be NULL,
so "ww_ctx == NULL" is 0, which is a constant, so the
__builtin_constant_p() expression returns true. See? That expression
has absolutely NOTHING to do with whether you passed in NULL or not.
NOTHING.

That __builtin_constant_p() tests whether the comparison is
*CONSTANT*. And "0" is just as much a constant as "1" is. Really. So
the whole f*cking expression is total and utter crap, because it is
entirely and utterly senseless. It lacks all meaning. It's not
actually testing for NULL at all. Never was, never will.

The *ONLY* thing it is testing for is "how much can the compiler
optimize this", and as such the *ONLY* thing it tests for is compiler
differences.

Really. Seriously. If you start blaming the compiler for different
compilers giving different results, the only thing *that* shows is
that you didn't understand the expression to begin with.

> But yeah I guess it was too broken in gcc after all, so that's why it had to be killed altogether.

NO NO NO NO. No a f*cking thousand times. It's not "too broken in
gcc". It's too broken in the source code, and the fact that you don't
even understand that is sad. You wrote the code, and you seem to be
unable to admit that *your* code was buggy.

It's not a compiler bug. It's your bug. Stand up like a man, instead
of trying to flail around and blame anything else but yourself.

So guys, get your act together, and stop blaming the compiler already.

               Linus