Re: current_thread_info() not respecting program order with gcc4.8.x

From: Mathieu Desnoyers
Date: Wed Nov 20 2013 - 10:11:04 EST


----- Original Message -----
> From: "Linus Torvalds" <torvalds@xxxxxxxxxxxxxxxxxxxx>
> To: "Mathieu Desnoyers" <mathieu.desnoyers@xxxxxxxxxxxx>, "Jakub Jelinek" <jakub@xxxxxxxxxx>, "Richard Henderson"
> <rth@xxxxxxxxxxx>
> Cc: "Linux Kernel Mailing List" <linux-kernel@xxxxxxxxxxxxxxx>, "Will Deacon" <will.deacon@xxxxxxx>, "Catalin
> Marinas" <catalin.marinas@xxxxxxx>, "Peter Zijlstra" <peterz@xxxxxxxxxxxxx>, lttng-dev@xxxxxxxxxxxxxxx, "Nathan
> Lynch" <Nathan_Lynch@xxxxxxxxxx>, "Paul E. McKenney" <paulmck@xxxxxxxxxxxxxxxxxx>, "Andrew Morton"
> <akpm@xxxxxxxxxxxxxxxxxxxx>
> Sent: Tuesday, November 19, 2013 7:41:17 PM
> Subject: Re: current_thread_info() not respecting program order with gcc 4.8.x
>
> On Tue, Nov 19, 2013 at 7:29 AM, Mathieu Desnoyers
> <mathieu.desnoyers@xxxxxxxxxxxx> wrote:
> >
> > Since each current_thread_info() is a different asm ("sp") without clobber
> > nor volatile, AFAIU, the compiler is within its right to reorder them.
>
> I don't understand why you say that.
>
> The ordering of the access to the asm("sp") register is totally
> irrelevant. You are "correct" in saying that the compiler is within
> its right to re-order them, but that is the worst kind of correct:
> it's totally immaterial. In fact, we *want* the compiler to not just
> re-order the accesses to %sp, but to notice that it can combine them,
> and do CSE on that whole expression when it is used multiple times
> within the same function (like it often is used).
>
> So the compiler can very much decide to re-read %sp all it wants, and
> re-order those reads all it wants, and that's not the bug at all.
> Putting a clobber or a volatile on it would disable the optimization
> we *want* to happen.
>
> So don't bark up the wrong tree.
>
> The bug seems to be that gcc re-orders the *memory* *accesses* through
> that point, which is not correct in any way, shape, or form. If we
> have a write to a memory location followed by a read of the same
> memory location, the compiler ABSOLUTELY MUST NOT RE-ORDER THEM. The
> write obviously changes the value of the read.
>
> It seems that some gcc alias analysis completely incorrectly thinks
> that they are not the same memory location, and do not alias. My guess
> would be that gcc sees that that they are based on the stack pointer
> with "different" offsets, and decides that the memory locations must
> be different - without noticing that the "& ~(THREAD_SIZE - 1)" will
> end up generating the same address for both of them.
>
> There may be some insane "two different objects on the stack cannot
> alias" logic, which is true for *objects* on the stack, but it sure as
> hell isn't true for random accesses through asm("sp").
>
> If I read this thread correctly, you're all talking about something
> else than the actual bug, and are trying to say that there is
> something wrong with re-ordering the access to %sp itself. Missing the
> _real_ bug entirely. See above.

Yes, exactly, your explanation clarifies the underlying issue I'm trying to
point at.

Thank you !

Mathieu


>
> Linus
>

--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
--
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/