* [PATCH] arm64: Implement 1-,2- byte smp_load_acquire and smp_store_release
@ 2015-04-20 15:45 Andrey Ryabinin
2015-04-20 15:48 ` Will Deacon
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Andrey Ryabinin @ 2015-04-20 15:45 UTC (permalink / raw)
To: linux-arm-kernel
commit 47933ad41a86 ("arch: Introduce smp_load_acquire(), smp_store_release()")
allowed only 4- and 8-byte smp_load_acquire, smp_store_release.
So 1- and 2-byte cases weren't implemented in arm64.
Later commit 536fa402221f ("compiler: Allow 1- and 2-byte smp_load_acquire()
and smp_store_release()")
allowed to use 1 and 2 byte smp_load_acquire and smp_store_release
by adjusting the definition of __native_word().
However, 1-,2- byte cases in arm64 version left unimplemented.
Commit 8053871d0f7f ("smp: Fix smp_call_function_single_async() locking")
started to use smp_load_acquire() to load 2-bytes csd->flags.
That crashes arm64 kernel during the boot.
Implement 1,2 byte cases in arm64's smp_load_acquire()
and smp_store_release() to fix this.
Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com>
---
arch/arm64/include/asm/barrier.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
index a5abb00..71f19c4 100644
--- a/arch/arm64/include/asm/barrier.h
+++ b/arch/arm64/include/asm/barrier.h
@@ -65,6 +65,14 @@ do { \
do { \
compiletime_assert_atomic_type(*p); \
switch (sizeof(*p)) { \
+ case 1: \
+ asm volatile ("stlrb %w1, %0" \
+ : "=Q" (*p) : "r" (v) : "memory"); \
+ break; \
+ case 2: \
+ asm volatile ("stlrh %w1, %0" \
+ : "=Q" (*p) : "r" (v) : "memory"); \
+ break; \
case 4: \
asm volatile ("stlr %w1, %0" \
: "=Q" (*p) : "r" (v) : "memory"); \
@@ -81,6 +89,14 @@ do { \
typeof(*p) ___p1; \
compiletime_assert_atomic_type(*p); \
switch (sizeof(*p)) { \
+ case 1: \
+ asm volatile ("ldarb %w0, %1" \
+ : "=r" (___p1) : "Q" (*p) : "memory"); \
+ break; \
+ case 2: \
+ asm volatile ("ldarh %w0, %1" \
+ : "=r" (___p1) : "Q" (*p) : "memory"); \
+ break; \
case 4: \
asm volatile ("ldar %w0, %1" \
: "=r" (___p1) : "Q" (*p) : "memory"); \
--
2.3.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] arm64: Implement 1-,2- byte smp_load_acquire and smp_store_release
2015-04-20 15:45 [PATCH] arm64: Implement 1-,2- byte smp_load_acquire and smp_store_release Andrey Ryabinin
@ 2015-04-20 15:48 ` Will Deacon
2015-04-20 16:09 ` Paul E. McKenney
2015-04-20 16:08 ` [PATCH] arm64: Implement 1-, 2- " Linus Torvalds
2015-04-20 16:08 ` Paul E. McKenney
2 siblings, 1 reply; 6+ messages in thread
From: Will Deacon @ 2015-04-20 15:48 UTC (permalink / raw)
To: linux-arm-kernel
Hi Andrey,
On Mon, Apr 20, 2015 at 04:45:53PM +0100, Andrey Ryabinin wrote:
> commit 47933ad41a86 ("arch: Introduce smp_load_acquire(), smp_store_release()")
> allowed only 4- and 8-byte smp_load_acquire, smp_store_release.
> So 1- and 2-byte cases weren't implemented in arm64.
> Later commit 536fa402221f ("compiler: Allow 1- and 2-byte smp_load_acquire()
> and smp_store_release()")
> allowed to use 1 and 2 byte smp_load_acquire and smp_store_release
> by adjusting the definition of __native_word().
> However, 1-,2- byte cases in arm64 version left unimplemented.
>
> Commit 8053871d0f7f ("smp: Fix smp_call_function_single_async() locking")
> started to use smp_load_acquire() to load 2-bytes csd->flags.
> That crashes arm64 kernel during the boot.
>
> Implement 1,2 byte cases in arm64's smp_load_acquire()
> and smp_store_release() to fix this.
>
> Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com>
I already have an equivalent patch queued in the arm64/fixes branch[1]. I'll
send a pull shortly.
Will
[1]
https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=fixes/core
> ---
> arch/arm64/include/asm/barrier.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
> index a5abb00..71f19c4 100644
> --- a/arch/arm64/include/asm/barrier.h
> +++ b/arch/arm64/include/asm/barrier.h
> @@ -65,6 +65,14 @@ do { \
> do { \
> compiletime_assert_atomic_type(*p); \
> switch (sizeof(*p)) { \
> + case 1: \
> + asm volatile ("stlrb %w1, %0" \
> + : "=Q" (*p) : "r" (v) : "memory"); \
> + break; \
> + case 2: \
> + asm volatile ("stlrh %w1, %0" \
> + : "=Q" (*p) : "r" (v) : "memory"); \
> + break; \
> case 4: \
> asm volatile ("stlr %w1, %0" \
> : "=Q" (*p) : "r" (v) : "memory"); \
> @@ -81,6 +89,14 @@ do { \
> typeof(*p) ___p1; \
> compiletime_assert_atomic_type(*p); \
> switch (sizeof(*p)) { \
> + case 1: \
> + asm volatile ("ldarb %w0, %1" \
> + : "=r" (___p1) : "Q" (*p) : "memory"); \
> + break; \
> + case 2: \
> + asm volatile ("ldarh %w0, %1" \
> + : "=r" (___p1) : "Q" (*p) : "memory"); \
> + break; \
> case 4: \
> asm volatile ("ldar %w0, %1" \
> : "=r" (___p1) : "Q" (*p) : "memory"); \
> --
> 2.3.5
>
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] arm64: Implement 1-,2- byte smp_load_acquire and smp_store_release
2015-04-20 15:48 ` Will Deacon
@ 2015-04-20 16:09 ` Paul E. McKenney
0 siblings, 0 replies; 6+ messages in thread
From: Paul E. McKenney @ 2015-04-20 16:09 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Apr 20, 2015 at 04:48:24PM +0100, Will Deacon wrote:
> Hi Andrey,
>
> On Mon, Apr 20, 2015 at 04:45:53PM +0100, Andrey Ryabinin wrote:
> > commit 47933ad41a86 ("arch: Introduce smp_load_acquire(), smp_store_release()")
> > allowed only 4- and 8-byte smp_load_acquire, smp_store_release.
> > So 1- and 2-byte cases weren't implemented in arm64.
> > Later commit 536fa402221f ("compiler: Allow 1- and 2-byte smp_load_acquire()
> > and smp_store_release()")
> > allowed to use 1 and 2 byte smp_load_acquire and smp_store_release
> > by adjusting the definition of __native_word().
> > However, 1-,2- byte cases in arm64 version left unimplemented.
> >
> > Commit 8053871d0f7f ("smp: Fix smp_call_function_single_async() locking")
> > started to use smp_load_acquire() to load 2-bytes csd->flags.
> > That crashes arm64 kernel during the boot.
> >
> > Implement 1,2 byte cases in arm64's smp_load_acquire()
> > and smp_store_release() to fix this.
> >
> > Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com>
>
> I already have an equivalent patch queued in the arm64/fixes branch[1]. I'll
> send a pull shortly.
Even better! ;-)
Thanx, Paul
> Will
>
> [1]
> https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=fixes/core
>
> > ---
> > arch/arm64/include/asm/barrier.h | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
> > index a5abb00..71f19c4 100644
> > --- a/arch/arm64/include/asm/barrier.h
> > +++ b/arch/arm64/include/asm/barrier.h
> > @@ -65,6 +65,14 @@ do { \
> > do { \
> > compiletime_assert_atomic_type(*p); \
> > switch (sizeof(*p)) { \
> > + case 1: \
> > + asm volatile ("stlrb %w1, %0" \
> > + : "=Q" (*p) : "r" (v) : "memory"); \
> > + break; \
> > + case 2: \
> > + asm volatile ("stlrh %w1, %0" \
> > + : "=Q" (*p) : "r" (v) : "memory"); \
> > + break; \
> > case 4: \
> > asm volatile ("stlr %w1, %0" \
> > : "=Q" (*p) : "r" (v) : "memory"); \
> > @@ -81,6 +89,14 @@ do { \
> > typeof(*p) ___p1; \
> > compiletime_assert_atomic_type(*p); \
> > switch (sizeof(*p)) { \
> > + case 1: \
> > + asm volatile ("ldarb %w0, %1" \
> > + : "=r" (___p1) : "Q" (*p) : "memory"); \
> > + break; \
> > + case 2: \
> > + asm volatile ("ldarh %w0, %1" \
> > + : "=r" (___p1) : "Q" (*p) : "memory"); \
> > + break; \
> > case 4: \
> > asm volatile ("ldar %w0, %1" \
> > : "=r" (___p1) : "Q" (*p) : "memory"); \
> > --
> > 2.3.5
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] arm64: Implement 1-, 2- byte smp_load_acquire and smp_store_release
2015-04-20 15:45 [PATCH] arm64: Implement 1-,2- byte smp_load_acquire and smp_store_release Andrey Ryabinin
2015-04-20 15:48 ` Will Deacon
@ 2015-04-20 16:08 ` Linus Torvalds
2015-04-20 16:13 ` [PATCH] arm64: Implement 1-,2- " Will Deacon
2015-04-20 16:08 ` Paul E. McKenney
2 siblings, 1 reply; 6+ messages in thread
From: Linus Torvalds @ 2015-04-20 16:08 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Apr 20, 2015 at 8:45 AM, Andrey Ryabinin <a.ryabinin@samsung.com> wrote:
>
> Commit 8053871d0f7f ("smp: Fix smp_call_function_single_async() locking")
> started to use smp_load_acquire() to load 2-bytes csd->flags.
> That crashes arm64 kernel during the boot.
Ahh. I don't disagree with your patch, but we should just make
csd->flags be an "int" regardless of this issue.
There is absolutely no reason to try to use 16-bit words. It doesn't
save us any space (alignment), and it's just a *bad* idea to depend on
any kind of atomicity of 16-bit entities. Bytes, yes. Words, yes. But
16-bit half-words? There's no point.
So I'll make it "unsigned int" instead of "u16".
But as said, the ARM64 patch definitely is a good idea regardless.
Linus
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] arm64: Implement 1-,2- byte smp_load_acquire and smp_store_release
2015-04-20 16:08 ` [PATCH] arm64: Implement 1-, 2- " Linus Torvalds
@ 2015-04-20 16:13 ` Will Deacon
0 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2015-04-20 16:13 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Apr 20, 2015 at 05:08:37PM +0100, Linus Torvalds wrote:
> On Mon, Apr 20, 2015 at 8:45 AM, Andrey Ryabinin <a.ryabinin@samsung.com> wrote:
> >
> > Commit 8053871d0f7f ("smp: Fix smp_call_function_single_async() locking")
> > started to use smp_load_acquire() to load 2-bytes csd->flags.
> > That crashes arm64 kernel during the boot.
>
> Ahh. I don't disagree with your patch, but we should just make
> csd->flags be an "int" regardless of this issue.
>
> There is absolutely no reason to try to use 16-bit words. It doesn't
> save us any space (alignment), and it's just a *bad* idea to depend on
> any kind of atomicity of 16-bit entities. Bytes, yes. Words, yes. But
> 16-bit half-words? There's no point.
>
> So I'll make it "unsigned int" instead of "u16".
>
> But as said, the ARM64 patch definitely is a good idea regardless.
Sounds good to me and means I can leave the pull for a day or two to see
if any other arm64 fixes queue up.
Will
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] arm64: Implement 1-,2- byte smp_load_acquire and smp_store_release
2015-04-20 15:45 [PATCH] arm64: Implement 1-,2- byte smp_load_acquire and smp_store_release Andrey Ryabinin
2015-04-20 15:48 ` Will Deacon
2015-04-20 16:08 ` [PATCH] arm64: Implement 1-, 2- " Linus Torvalds
@ 2015-04-20 16:08 ` Paul E. McKenney
2 siblings, 0 replies; 6+ messages in thread
From: Paul E. McKenney @ 2015-04-20 16:08 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Apr 20, 2015 at 06:45:53PM +0300, Andrey Ryabinin wrote:
> commit 47933ad41a86 ("arch: Introduce smp_load_acquire(), smp_store_release()")
> allowed only 4- and 8-byte smp_load_acquire, smp_store_release.
> So 1- and 2-byte cases weren't implemented in arm64.
> Later commit 536fa402221f ("compiler: Allow 1- and 2-byte smp_load_acquire()
> and smp_store_release()")
> allowed to use 1 and 2 byte smp_load_acquire and smp_store_release
> by adjusting the definition of __native_word().
> However, 1-,2- byte cases in arm64 version left unimplemented.
>
> Commit 8053871d0f7f ("smp: Fix smp_call_function_single_async() locking")
> started to use smp_load_acquire() to load 2-bytes csd->flags.
> That crashes arm64 kernel during the boot.
>
> Implement 1,2 byte cases in arm64's smp_load_acquire()
> and smp_store_release() to fix this.
>
> Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com>
I am introducing a similar smp_load_acquire() case in rcutorture to
replace use of explicit memory barriers, so thank you! ;-)
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
> arch/arm64/include/asm/barrier.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
> index a5abb00..71f19c4 100644
> --- a/arch/arm64/include/asm/barrier.h
> +++ b/arch/arm64/include/asm/barrier.h
> @@ -65,6 +65,14 @@ do { \
> do { \
> compiletime_assert_atomic_type(*p); \
> switch (sizeof(*p)) { \
> + case 1: \
> + asm volatile ("stlrb %w1, %0" \
> + : "=Q" (*p) : "r" (v) : "memory"); \
> + break; \
> + case 2: \
> + asm volatile ("stlrh %w1, %0" \
> + : "=Q" (*p) : "r" (v) : "memory"); \
> + break; \
> case 4: \
> asm volatile ("stlr %w1, %0" \
> : "=Q" (*p) : "r" (v) : "memory"); \
> @@ -81,6 +89,14 @@ do { \
> typeof(*p) ___p1; \
> compiletime_assert_atomic_type(*p); \
> switch (sizeof(*p)) { \
> + case 1: \
> + asm volatile ("ldarb %w0, %1" \
> + : "=r" (___p1) : "Q" (*p) : "memory"); \
> + break; \
> + case 2: \
> + asm volatile ("ldarh %w0, %1" \
> + : "=r" (___p1) : "Q" (*p) : "memory"); \
> + break; \
> case 4: \
> asm volatile ("ldar %w0, %1" \
> : "=r" (___p1) : "Q" (*p) : "memory"); \
> --
> 2.3.5
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-04-20 16:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-20 15:45 [PATCH] arm64: Implement 1-,2- byte smp_load_acquire and smp_store_release Andrey Ryabinin
2015-04-20 15:48 ` Will Deacon
2015-04-20 16:09 ` Paul E. McKenney
2015-04-20 16:08 ` [PATCH] arm64: Implement 1-, 2- " Linus Torvalds
2015-04-20 16:13 ` [PATCH] arm64: Implement 1-,2- " Will Deacon
2015-04-20 16:08 ` Paul E. McKenney
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).