From: Will Deacon <will@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>,
Linus Torvalds <torvalds@linux-foundation.org>,
dja@axtens.net, linux-kernel@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, christophe.leroy@c-s.fr,
linux-arch@vger.kernel.org, Mark Rutland <mark.rutland@arm.com>,
Segher Boessenkool <segher@kernel.crashing.org>,
Arnd Bergmann <arnd@arndb.de>,
Christian Borntraeger <borntraeger@de.ibm.com>
Subject: Re: READ_ONCE() + STACKPROTECTOR_STRONG == :/ (was Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.5-2 tag (topic/kasan-bitops))
Date: Thu, 12 Dec 2019 17:04:28 +0000 [thread overview]
Message-ID: <20191212170427.GA16364@willie-the-truck> (raw)
In-Reply-To: <20191212104610.GW2827@hirez.programming.kicks-ass.net>
On Thu, Dec 12, 2019 at 11:46:10AM +0100, Peter Zijlstra wrote:
> On Thu, Dec 12, 2019 at 10:07:56AM +0000, Will Deacon wrote:
>
> > > So your proposed change _should_ be fine. Will, I'm assuming you never
> > > saw this on your ARGH64 builds when you did this code ?
> >
> > I did see it, but (a) looking at the code out-of-line makes it look a lot
> > worse than it actually is (so the ext4 example is really helpful -- thanks
> > Michael!) and (b) I chalked it up to a crappy compiler.
> >
> > However, see this comment from Arnd on my READ_ONCE series from the other
> > day:
> >
> > https://lore.kernel.org/lkml/CAK8P3a0f=WvSQSBQ4t0FmEkcFE_mC3oARxaeTviTSkSa-D2qhg@mail.gmail.com
> >
> > In which case, I'm thinking that we should be doing better in READ_ONCE()
> > for non-buggy compilers which would also keep the KCSAN folks happy for this
> > code (and would help with [1] too).
>
> So something like this then? Although I suppose that should be moved
> into compiler-gcc.h and then guarded by #ifndef READ_ONCE or so.
Ah wait, I think we've been looking at this wrong. The volatile pointer
argument is actually the problem here, not READ_ONCE()! The use of typeof()
means that the temporary variable to which __READ_ONCE_SIZE writes ends up
being a volatile store, so it can't be optimised away. This is why we get
a stack access and why stack protector then wrecks the codegen for us.
I'll cook a patch getting rid of those volatiles.
Will
WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-arch@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
Christian Borntraeger <borntraeger@de.ibm.com>,
linux-kernel@vger.kernel.org,
Linus Torvalds <torvalds@linux-foundation.org>,
Mark Rutland <mark.rutland@arm.com>,
linuxppc-dev@lists.ozlabs.org, dja@axtens.net
Subject: Re: READ_ONCE() + STACKPROTECTOR_STRONG == :/ (was Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.5-2 tag (topic/kasan-bitops))
Date: Thu, 12 Dec 2019 17:04:28 +0000 [thread overview]
Message-ID: <20191212170427.GA16364@willie-the-truck> (raw)
In-Reply-To: <20191212104610.GW2827@hirez.programming.kicks-ass.net>
On Thu, Dec 12, 2019 at 11:46:10AM +0100, Peter Zijlstra wrote:
> On Thu, Dec 12, 2019 at 10:07:56AM +0000, Will Deacon wrote:
>
> > > So your proposed change _should_ be fine. Will, I'm assuming you never
> > > saw this on your ARGH64 builds when you did this code ?
> >
> > I did see it, but (a) looking at the code out-of-line makes it look a lot
> > worse than it actually is (so the ext4 example is really helpful -- thanks
> > Michael!) and (b) I chalked it up to a crappy compiler.
> >
> > However, see this comment from Arnd on my READ_ONCE series from the other
> > day:
> >
> > https://lore.kernel.org/lkml/CAK8P3a0f=WvSQSBQ4t0FmEkcFE_mC3oARxaeTviTSkSa-D2qhg@mail.gmail.com
> >
> > In which case, I'm thinking that we should be doing better in READ_ONCE()
> > for non-buggy compilers which would also keep the KCSAN folks happy for this
> > code (and would help with [1] too).
>
> So something like this then? Although I suppose that should be moved
> into compiler-gcc.h and then guarded by #ifndef READ_ONCE or so.
Ah wait, I think we've been looking at this wrong. The volatile pointer
argument is actually the problem here, not READ_ONCE()! The use of typeof()
means that the temporary variable to which __READ_ONCE_SIZE writes ends up
being a volatile store, so it can't be optimised away. This is why we get
a stack access and why stack protector then wrecks the codegen for us.
I'll cook a patch getting rid of those volatiles.
Will
next prev parent reply other threads:[~2019-12-12 17:04 UTC|newest]
Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-06 12:46 [GIT PULL] Please pull powerpc/linux.git powerpc-5.5-2 tag (topic/kasan-bitops) Michael Ellerman
2019-12-06 12:46 ` Michael Ellerman
2019-12-06 13:16 ` Peter Zijlstra
2019-12-06 13:16 ` Peter Zijlstra
2019-12-10 5:38 ` Michael Ellerman
2019-12-10 5:38 ` Michael Ellerman
2019-12-10 10:15 ` Peter Zijlstra
2019-12-10 10:15 ` Peter Zijlstra
2019-12-11 0:29 ` Michael Ellerman
2019-12-11 0:29 ` Michael Ellerman
2019-12-12 5:42 ` READ_ONCE() + STACKPROTECTOR_STRONG == :/ (was Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.5-2 tag (topic/kasan-bitops)) Michael Ellerman
2019-12-12 5:42 ` Michael Ellerman
2019-12-12 8:01 ` Peter Zijlstra
2019-12-12 8:01 ` Peter Zijlstra
2019-12-12 10:07 ` Will Deacon
2019-12-12 10:07 ` Will Deacon
2019-12-12 10:46 ` Peter Zijlstra
2019-12-12 10:46 ` Peter Zijlstra
2019-12-12 17:04 ` Will Deacon [this message]
2019-12-12 17:04 ` Will Deacon
2019-12-12 17:16 ` Will Deacon
2019-12-12 17:16 ` Will Deacon
2019-12-12 17:41 ` Linus Torvalds
2019-12-12 17:41 ` Linus Torvalds
2019-12-12 17:50 ` Segher Boessenkool
2019-12-12 17:50 ` Segher Boessenkool
2019-12-12 18:06 ` Will Deacon
2019-12-12 18:06 ` Will Deacon
2019-12-12 18:29 ` Christian Borntraeger
2019-12-12 18:29 ` Christian Borntraeger
2019-12-12 18:43 ` Linus Torvalds
2019-12-12 18:43 ` Linus Torvalds
2019-12-12 19:34 ` Will Deacon
2019-12-12 19:34 ` Will Deacon
2019-12-12 20:21 ` Peter Zijlstra
2019-12-12 20:21 ` Peter Zijlstra
2019-12-12 20:53 ` Peter Zijlstra
2019-12-12 20:53 ` Peter Zijlstra
2019-12-13 10:47 ` Luc Van Oostenryck
2019-12-13 10:47 ` Luc Van Oostenryck
2019-12-13 12:56 ` Peter Zijlstra
2019-12-13 12:56 ` Peter Zijlstra
2019-12-13 14:28 ` Luc Van Oostenryck
2019-12-13 14:28 ` Luc Van Oostenryck
2019-12-12 20:49 ` Linus Torvalds
2019-12-12 20:49 ` Linus Torvalds
2019-12-13 13:17 ` Arnd Bergmann
2019-12-13 13:17 ` Arnd Bergmann
2019-12-13 21:32 ` Arnd Bergmann
2019-12-13 21:32 ` Arnd Bergmann
2019-12-13 22:01 ` Linus Torvalds
2019-12-13 22:01 ` Linus Torvalds
2019-12-16 10:28 ` Will Deacon
2019-12-16 10:28 ` Will Deacon
2019-12-16 11:47 ` Peter Zijlstra
2019-12-16 11:47 ` Peter Zijlstra
2019-12-16 12:06 ` Arnd Bergmann
2019-12-16 12:06 ` Arnd Bergmann
2019-12-17 17:07 ` Will Deacon
2019-12-17 17:07 ` Will Deacon
2019-12-17 18:04 ` Linus Torvalds
2019-12-17 18:04 ` Linus Torvalds
2019-12-17 18:05 ` Linus Torvalds
2019-12-17 18:05 ` Linus Torvalds
2019-12-17 18:31 ` Will Deacon
2019-12-17 18:31 ` Will Deacon
2019-12-17 18:32 ` Linus Torvalds
2019-12-17 18:32 ` Linus Torvalds
2019-12-18 12:17 ` Michael Ellerman
2019-12-18 12:17 ` Michael Ellerman
2019-12-19 12:11 ` Will Deacon
2019-12-19 12:11 ` Will Deacon
2019-12-18 10:22 ` Christian Borntraeger
2019-12-18 10:22 ` Christian Borntraeger
2019-12-18 10:35 ` Will Deacon
2019-12-18 10:35 ` Will Deacon
2019-12-13 12:07 ` Michael Ellerman
2019-12-13 12:07 ` Michael Ellerman
2019-12-13 13:53 ` Segher Boessenkool
2019-12-13 13:53 ` Segher Boessenkool
2019-12-13 21:06 ` Michael Ellerman
2019-12-13 21:06 ` Michael Ellerman
2019-12-12 15:10 ` Segher Boessenkool
2019-12-12 15:10 ` Segher Boessenkool
2019-12-06 22:15 ` [GIT PULL] Please pull powerpc/linux.git powerpc-5.5-2 tag (topic/kasan-bitops) pr-tracker-bot
2019-12-06 22:15 ` pr-tracker-bot
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=20191212170427.GA16364@willie-the-truck \
--to=will@kernel.org \
--cc=arnd@arndb.de \
--cc=borntraeger@de.ibm.com \
--cc=christophe.leroy@c-s.fr \
--cc=dja@axtens.net \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mark.rutland@arm.com \
--cc=mpe@ellerman.id.au \
--cc=peterz@infradead.org \
--cc=segher@kernel.crashing.org \
--cc=torvalds@linux-foundation.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.