[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <526D62D8.8050001@canonical.com>
Date: Sun, 27 Oct 2013 20:00:40 +0100
From: Maarten Lankhorst <maarten.lankhorst@...onical.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>,
Ingo Molnar <mingo@...nel.org>
CC: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Thomas Gleixner <tglx@...utronix.de>,
Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [GIT PULL] locking fix
op 27-10-13 18:28, Linus Torvalds schreef:
> On Sat, Oct 26, 2013 at 5:19 AM, Ingo Molnar <mingo@...nel.org> wrote:
>> This tree fixes a boot crash in CONFIG_DEBUG_MUTEXES=y kernels, on
>> kernels built with GCC 3.x. (There are still such distros.)
> Btw, it's really not just gcc 3.x. That code was (a) incomprehensible,
> (b) wrong and (c) caused problems for LLVM too.
>
> It was wrong because "__builtin_constant_p(ww_ctx == NULL)" simply
> makes no sense.
>
> Why?
>
> 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.
__builtin_constant_p(ww_ctx) was NOT equal to __builtin_constant_p(ww_ctx == NULL), iirc.
__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.
But yeah I guess it was too broken in gcc after all, so that's why it had to be killed altogether.
--
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