public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jakub Jelinek <jakub@redhat.com>,
	Richard Henderson <rth@twiddle.net>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Will Deacon <will.deacon@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	lttng-dev@lists.lttng.org, Nathan Lynch <Nathan_Lynch@mentor.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: current_thread_info() not respecting program order with gcc 4.8.x
Date: Wed, 20 Nov 2013 15:10:48 +0000 (UTC)	[thread overview]
Message-ID: <1067303101.71379.1384960248303.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <CA+55aFw923sA0E1RQNxFF0OWFtAfcaG-dBUc-0YYZGDSAsaQyg@mail.gmail.com>

----- Original Message -----
> From: "Linus Torvalds" <torvalds@linux-foundation.org>
> To: "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>, "Jakub Jelinek" <jakub@redhat.com>, "Richard Henderson"
> <rth@twiddle.net>
> Cc: "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>, "Will Deacon" <will.deacon@arm.com>, "Catalin
> Marinas" <catalin.marinas@arm.com>, "Peter Zijlstra" <peterz@infradead.org>, lttng-dev@lists.lttng.org, "Nathan
> Lynch" <Nathan_Lynch@mentor.com>, "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>, "Andrew Morton"
> <akpm@linux-foundation.org>
> 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@efficios.com> 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

  reply	other threads:[~2013-11-20 15:10 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <52803E5D.3050109@mentor.com>
     [not found] ` <52851395.3010306@mentor.com>
     [not found]   ` <67652521.68027.1384482849638.JavaMail.zimbra@efficios.com>
2013-11-19 15:29     ` current_thread_info() not respecting program order with gcc 4.8.x Mathieu Desnoyers
2013-11-19 15:57       ` Peter Zijlstra
2013-11-19 16:13         ` Jakub Jelinek
2013-11-19 16:21           ` Peter Zijlstra
2013-11-19 16:05       ` Will Deacon
2013-11-19 17:02         ` Mathieu Desnoyers
2013-11-19 17:33           ` Peter Zijlstra
2013-11-19 21:56             ` Multiple local register variables w/ same register Richard Henderson
2013-11-19 22:08               ` Jakub Jelinek
2013-11-19 22:13               ` Måns Rullgård
2013-11-19 22:25               ` Mathieu Desnoyers
2013-11-19 22:34                 ` [lttng-dev] " Mathieu Desnoyers
2013-11-20  0:41       ` current_thread_info() not respecting program order with gcc 4.8.x Linus Torvalds
2013-11-20 15:10         ` Mathieu Desnoyers [this message]
2013-11-21 16:02         ` Alexander Holler
2013-11-21 22:12           ` Luis Lozano
2013-11-21 22:32           ` Linus Torvalds
2013-11-21 23:18             ` Alexander Holler
2013-11-21 23:45               ` Luis Lozano
2013-11-22  0:39                 ` Jakub Jelinek
2013-11-22  1:57                   ` Mathieu Desnoyers
2013-11-22  2:36                     ` Luis Lozano
2013-11-22  3:38                       ` Mathieu Desnoyers
2013-11-22  8:18                         ` Luis Lozano
2013-11-22  8:33                           ` Luis Lozano
2013-11-22 13:06                           ` Mathieu Desnoyers
2013-11-22 20:33                             ` [lttng-dev] " Mathieu Desnoyers
2013-11-22  0:17               ` Linus Torvalds
2013-11-22  0:34                 ` Alexander Holler
2013-11-21  1:52 Luis Lozano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1067303101.71379.1384960248303.JavaMail.zimbra@efficios.com \
    --to=mathieu.desnoyers@efficios.com \
    --cc=Nathan_Lynch@mentor.com \
    --cc=akpm@linux-foundation.org \
    --cc=catalin.marinas@arm.com \
    --cc=jakub@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lttng-dev@lists.lttng.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=rth@twiddle.net \
    --cc=torvalds@linux-foundation.org \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox