From: James Hogan <james.hogan@imgtec.com> To: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>, <linux-mips@linux-mips.org>, <benh@kernel.crashing.org>, <will.deacon@arm.com>, <linux-kernel@vger.kernel.org>, <ralf@linux-mips.org>, <markos.chandras@imgtec.com>, <macro@linux-mips.org>, <Steven.Hill@imgtec.com>, <alexander.h.duyck@redhat.com>, <davem@davemloft.net> Subject: Re: [PATCH 1/3] MIPS: R6: Use lightweight SYNC instruction in smp_* memory barriers Date: Tue, 2 Jun 2015 11:48:35 +0100 [thread overview] Message-ID: <556D8A03.9080201@imgtec.com> (raw) In-Reply-To: <20150602000934.6668.43645.stgit@ubuntu-yegoshin> [-- Attachment #1: Type: text/plain, Size: 5044 bytes --] Hi Leonid, On 02/06/15 01:09, Leonid Yegoshin wrote: > This instructions were specifically designed to work for smp_*() sort of > memory barriers in MIPS R2/R3/R5 and R6. > > Unfortunately, it's description is very cryptic and is done in HW engineering > style which prevents use of it by SW. This instructions are not mandatory but > there is a mandatory requirement - if not implemented, then a regular MIPS > SYNC 0 must be used instead. > > The reason for this change is - SYNC 0 is a heavvy-weighted in many CPUs, it may heavy > disrupt other cores pipelines etc. > > Due to concern about verification of old MIPS R2 compatible cores of other > vendors it is enforced only for MIPS R6 and other MIPS32/64 R2/R5 processors > have it configurable. Is it worth inverting the logic to exclude the platforms you're concerned about (which also makes it explicit what hardware needs verifying). For new platforms we don't need to worry about kernel regressions so much, so it probably makes sense to have them used by default. > > Signed-off-by: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com> > --- > arch/mips/Kconfig | 22 ++++++++++++++++++++++ > arch/mips/include/asm/barrier.h | 6 ++++++ > 2 files changed, 28 insertions(+) > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig > index be384d6a58bb..c7d0cacece3d 100644 > --- a/arch/mips/Kconfig > +++ b/arch/mips/Kconfig > @@ -1347,6 +1347,7 @@ config CPU_MIPS32_R2 > select CPU_SUPPORTS_32BIT_KERNEL > select CPU_SUPPORTS_HIGHMEM > select CPU_SUPPORTS_MSA > + select CPU_SUPPORTS_MIPS_LIGHTWEIGHT_SYNC > select HAVE_KVM > help > Choose this option to build a kernel for release 2 or later of the > @@ -1365,6 +1366,8 @@ config CPU_MIPS32_R6 > select GENERIC_CSUM > select HAVE_KVM > select MIPS_O32_FP64_SUPPORT > + select CPU_SUPPORTS_MIPS_LIGHTWEIGHT_SYNC > + select WEAK_REORDERING_BEYOND_LLSC > help > Choose this option to build a kernel for release 6 or later of the > MIPS32 architecture. New MIPS processors, starting with the Warrior > @@ -1399,6 +1402,7 @@ config CPU_MIPS64_R2 > select CPU_SUPPORTS_HIGHMEM > select CPU_SUPPORTS_HUGEPAGES > select CPU_SUPPORTS_MSA > + select CPU_SUPPORTS_MIPS_LIGHTWEIGHT_SYNC > help > Choose this option to build a kernel for release 2 or later of the > MIPS64 architecture. Many modern embedded systems with a 64-bit > @@ -1415,6 +1419,8 @@ config CPU_MIPS64_R6 > select CPU_SUPPORTS_HIGHMEM > select CPU_SUPPORTS_MSA > select GENERIC_CSUM > + select CPU_SUPPORTS_MIPS_LIGHTWEIGHT_SYNC > + select WEAK_REORDERING_BEYOND_LLSC > help > Choose this option to build a kernel for release 6 or later of the > MIPS64 architecture. New MIPS processors, starting with the Warrior > @@ -1876,6 +1882,20 @@ config WEAK_ORDERING > # > config WEAK_REORDERING_BEYOND_LLSC > bool > + > +config MIPS_LIGHTWEIGHT_SYNC > + bool "CPU lightweight SYNC instruction for weak reordering" > + depends on CPU_SUPPORTS_MIPS_LIGHTWEIGHT_SYNC && WEAK_ORDERING Worth depending on SMP, so as not to give the user more options than they need? > + default y if CPU_MIPSR6 > + help > + This option enforces a use of "lightweight sync" instructions > + for SMP (multi-CPU) memory barriers. This instructions are much > + more faster than a traditional "SYNC 0". > + > + If that instructions are not implemented in processor then it is > + converted to generic "SYNC 0". > + > + If you unsure, say N here, it may slightly decrease your performance "it" is ambiguous. "Saying N" or "this option"? (I guess "saying N", but its not obvious to an uninformed kernel configurer). > endmenu > > # > @@ -1928,6 +1948,8 @@ config CPU_SUPPORTS_HUGEPAGES > bool > config CPU_SUPPORTS_UNCACHED_ACCELERATED > bool > +config CPU_SUPPORTS_MIPS_LIGHTWEIGHT_SYNC > + bool > config MIPS_PGD_C0_CONTEXT > bool > default y if 64BIT && CPU_MIPSR2 && !CPU_XLP > diff --git a/arch/mips/include/asm/barrier.h b/arch/mips/include/asm/barrier.h > index 2b8bbbcb9be0..d2a63abfc7c6 100644 > --- a/arch/mips/include/asm/barrier.h > +++ b/arch/mips/include/asm/barrier.h > @@ -96,9 +96,15 @@ > # define smp_rmb() barrier() > # define smp_wmb() __syncw() > # else > +# ifdef CONFIG_MIPS_LIGHTWEIGHT_SYNC > +# define smp_mb() __asm__ __volatile__("sync 0x10" : : :"memory") > +# define smp_rmb() __asm__ __volatile__("sync 0x13" : : :"memory") > +# define smp_wmb() __asm__ __volatile__("sync 0x4" : : :"memory") binutils appears to support the sync_mb, sync_rmb, sync_wmb aliases since version 2.21. Can we safely use them? Cheers James > +# else > # define smp_mb() __asm__ __volatile__("sync" : : :"memory") > # define smp_rmb() __asm__ __volatile__("sync" : : :"memory") > # define smp_wmb() __asm__ __volatile__("sync" : : :"memory") > +# endif > # endif > #else > #define smp_mb() barrier() > > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: James Hogan <james.hogan@imgtec.com> To: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>, linux-mips@linux-mips.org, benh@kernel.crashing.org, will.deacon@arm.com, linux-kernel@vger.kernel.org, ralf@linux-mips.org, markos.chandras@imgtec.com, macro@linux-mips.org, Steven.Hill@imgtec.com, alexander.h.duyck@redhat.com, davem@davemloft.net Subject: Re: [PATCH 1/3] MIPS: R6: Use lightweight SYNC instruction in smp_* memory barriers Date: Tue, 2 Jun 2015 11:48:35 +0100 [thread overview] Message-ID: <556D8A03.9080201@imgtec.com> (raw) Message-ID: <20150602104835.U8lAbX8r_cCzkByOxvsUtEaU1XOe7anbo1Q11d8QFcs@z> (raw) In-Reply-To: <20150602000934.6668.43645.stgit@ubuntu-yegoshin> [-- Attachment #1: Type: text/plain, Size: 5044 bytes --] Hi Leonid, On 02/06/15 01:09, Leonid Yegoshin wrote: > This instructions were specifically designed to work for smp_*() sort of > memory barriers in MIPS R2/R3/R5 and R6. > > Unfortunately, it's description is very cryptic and is done in HW engineering > style which prevents use of it by SW. This instructions are not mandatory but > there is a mandatory requirement - if not implemented, then a regular MIPS > SYNC 0 must be used instead. > > The reason for this change is - SYNC 0 is a heavvy-weighted in many CPUs, it may heavy > disrupt other cores pipelines etc. > > Due to concern about verification of old MIPS R2 compatible cores of other > vendors it is enforced only for MIPS R6 and other MIPS32/64 R2/R5 processors > have it configurable. Is it worth inverting the logic to exclude the platforms you're concerned about (which also makes it explicit what hardware needs verifying). For new platforms we don't need to worry about kernel regressions so much, so it probably makes sense to have them used by default. > > Signed-off-by: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com> > --- > arch/mips/Kconfig | 22 ++++++++++++++++++++++ > arch/mips/include/asm/barrier.h | 6 ++++++ > 2 files changed, 28 insertions(+) > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig > index be384d6a58bb..c7d0cacece3d 100644 > --- a/arch/mips/Kconfig > +++ b/arch/mips/Kconfig > @@ -1347,6 +1347,7 @@ config CPU_MIPS32_R2 > select CPU_SUPPORTS_32BIT_KERNEL > select CPU_SUPPORTS_HIGHMEM > select CPU_SUPPORTS_MSA > + select CPU_SUPPORTS_MIPS_LIGHTWEIGHT_SYNC > select HAVE_KVM > help > Choose this option to build a kernel for release 2 or later of the > @@ -1365,6 +1366,8 @@ config CPU_MIPS32_R6 > select GENERIC_CSUM > select HAVE_KVM > select MIPS_O32_FP64_SUPPORT > + select CPU_SUPPORTS_MIPS_LIGHTWEIGHT_SYNC > + select WEAK_REORDERING_BEYOND_LLSC > help > Choose this option to build a kernel for release 6 or later of the > MIPS32 architecture. New MIPS processors, starting with the Warrior > @@ -1399,6 +1402,7 @@ config CPU_MIPS64_R2 > select CPU_SUPPORTS_HIGHMEM > select CPU_SUPPORTS_HUGEPAGES > select CPU_SUPPORTS_MSA > + select CPU_SUPPORTS_MIPS_LIGHTWEIGHT_SYNC > help > Choose this option to build a kernel for release 2 or later of the > MIPS64 architecture. Many modern embedded systems with a 64-bit > @@ -1415,6 +1419,8 @@ config CPU_MIPS64_R6 > select CPU_SUPPORTS_HIGHMEM > select CPU_SUPPORTS_MSA > select GENERIC_CSUM > + select CPU_SUPPORTS_MIPS_LIGHTWEIGHT_SYNC > + select WEAK_REORDERING_BEYOND_LLSC > help > Choose this option to build a kernel for release 6 or later of the > MIPS64 architecture. New MIPS processors, starting with the Warrior > @@ -1876,6 +1882,20 @@ config WEAK_ORDERING > # > config WEAK_REORDERING_BEYOND_LLSC > bool > + > +config MIPS_LIGHTWEIGHT_SYNC > + bool "CPU lightweight SYNC instruction for weak reordering" > + depends on CPU_SUPPORTS_MIPS_LIGHTWEIGHT_SYNC && WEAK_ORDERING Worth depending on SMP, so as not to give the user more options than they need? > + default y if CPU_MIPSR6 > + help > + This option enforces a use of "lightweight sync" instructions > + for SMP (multi-CPU) memory barriers. This instructions are much > + more faster than a traditional "SYNC 0". > + > + If that instructions are not implemented in processor then it is > + converted to generic "SYNC 0". > + > + If you unsure, say N here, it may slightly decrease your performance "it" is ambiguous. "Saying N" or "this option"? (I guess "saying N", but its not obvious to an uninformed kernel configurer). > endmenu > > # > @@ -1928,6 +1948,8 @@ config CPU_SUPPORTS_HUGEPAGES > bool > config CPU_SUPPORTS_UNCACHED_ACCELERATED > bool > +config CPU_SUPPORTS_MIPS_LIGHTWEIGHT_SYNC > + bool > config MIPS_PGD_C0_CONTEXT > bool > default y if 64BIT && CPU_MIPSR2 && !CPU_XLP > diff --git a/arch/mips/include/asm/barrier.h b/arch/mips/include/asm/barrier.h > index 2b8bbbcb9be0..d2a63abfc7c6 100644 > --- a/arch/mips/include/asm/barrier.h > +++ b/arch/mips/include/asm/barrier.h > @@ -96,9 +96,15 @@ > # define smp_rmb() barrier() > # define smp_wmb() __syncw() > # else > +# ifdef CONFIG_MIPS_LIGHTWEIGHT_SYNC > +# define smp_mb() __asm__ __volatile__("sync 0x10" : : :"memory") > +# define smp_rmb() __asm__ __volatile__("sync 0x13" : : :"memory") > +# define smp_wmb() __asm__ __volatile__("sync 0x4" : : :"memory") binutils appears to support the sync_mb, sync_rmb, sync_wmb aliases since version 2.21. Can we safely use them? Cheers James > +# else > # define smp_mb() __asm__ __volatile__("sync" : : :"memory") > # define smp_rmb() __asm__ __volatile__("sync" : : :"memory") > # define smp_wmb() __asm__ __volatile__("sync" : : :"memory") > +# endif > # endif > #else > #define smp_mb() barrier() > > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --]
next prev parent reply other threads:[~2015-06-02 10:48 UTC|newest] Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-06-02 0:09 [PATCH 0/3] MIPS: SMP memory barriers: lightweight sync, acquire-release Leonid Yegoshin 2015-06-02 0:09 ` Leonid Yegoshin 2015-06-02 0:09 ` [PATCH 1/3] MIPS: R6: Use lightweight SYNC instruction in smp_* memory barriers Leonid Yegoshin 2015-06-02 0:09 ` Leonid Yegoshin 2015-06-02 10:08 ` Paul Burton 2015-06-02 10:08 ` Paul Burton 2015-06-02 12:12 ` Luc Van Oostenryck 2015-06-02 12:44 ` Ralf Baechle 2015-06-02 18:20 ` Leonid Yegoshin 2015-06-02 18:20 ` Leonid Yegoshin 2015-06-02 10:48 ` James Hogan [this message] 2015-06-02 10:48 ` James Hogan 2015-06-02 16:15 ` Maciej W. Rozycki 2015-06-02 23:56 ` David Daney 2015-06-03 1:56 ` Leonid Yegoshin 2015-06-03 1:56 ` Leonid Yegoshin 2015-06-05 13:10 ` Ralf Baechle 2015-06-05 21:18 ` Maciej W. Rozycki 2016-01-28 2:28 ` Joshua Kinard 2016-01-29 13:32 ` Maciej W. Rozycki 2016-01-29 13:32 ` Maciej W. Rozycki 2016-01-30 16:25 ` Joshua Kinard 2015-06-02 0:09 ` [PATCH 2/3] MIPS: enforce LL-SC loop enclosing with SYNC (ACQUIRE and RELEASE) Leonid Yegoshin 2015-06-02 0:09 ` Leonid Yegoshin 2015-06-02 11:39 ` James Hogan 2015-06-02 11:39 ` James Hogan 2015-06-02 18:43 ` Leonid Yegoshin 2015-06-02 18:43 ` Leonid Yegoshin 2015-06-02 18:53 ` Leonid Yegoshin 2015-06-02 0:09 ` [PATCH 3/3] MIPS: bugfix - replace smp_mb with release barrier function in unlocks Leonid Yegoshin 2015-06-02 0:09 ` Leonid Yegoshin 2015-06-02 11:42 ` James Hogan 2015-06-02 11:42 ` James Hogan 2015-06-02 13:22 ` Ralf Baechle 2015-06-02 8:41 ` [PATCH 0/3] MIPS: SMP memory barriers: lightweight sync, acquire-release Joshua Kinard 2015-06-02 9:59 ` Ralf Baechle 2015-06-02 18:59 ` Joshua Kinard 2015-06-02 19:19 ` Ralf Baechle
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=556D8A03.9080201@imgtec.com \ --to=james.hogan@imgtec.com \ --cc=Leonid.Yegoshin@imgtec.com \ --cc=Steven.Hill@imgtec.com \ --cc=alexander.h.duyck@redhat.com \ --cc=benh@kernel.crashing.org \ --cc=davem@davemloft.net \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mips@linux-mips.org \ --cc=macro@linux-mips.org \ --cc=markos.chandras@imgtec.com \ --cc=ralf@linux-mips.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: linkBe 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.