* [PATCH] MIPS: Loongson: remove unnecessary loongson_llsc_mb()
@ 2019-09-09 3:42 Huacai Chen
2019-09-22 2:21 ` Huacai Chen
0 siblings, 1 reply; 2+ messages in thread
From: Huacai Chen @ 2019-09-09 3:42 UTC (permalink / raw)
To: Paul Burton, Ralf Baechle, James Hogan
Cc: linux-mips, linux-mips, Fuxin Zhang, Zhangjin Wu, Huacai Chen,
Huacai Chen, Huang Pei, Peter Zijlstra
Commit 1c6c1ca318585f1 ("mips/atomic: Fix loongson_llsc_mb() wreckage")
fix the description of Loongson-3's llsc bug and try to add all missing
loongson_llsc_mb(). This is a good job but there are some unnecessary
memory barries:
loongson_llsc_mb() is a Loongson-specific problem. smp_llsc_mb(),
smp_mb__before_llsc(), loongson_llsc_mb() and and other memory barriers
are essentially the same thing on Loongson-3. So we don't need to add
loongson_llsc_mb() if there is already a smp_mb__before_llsc() or other
types of memory barriers.
So, most of loongson_llsc_mb() in Peter's patch is superfluous and can
be removed, except the one in test_and_set_bit_lock().
mips_atomic_set() is not used on Loongson-3, and if in some cases we use
it, the user-to-kernel context switching probably has the same effect of
a memory barrier. But anyway, add a memory barrier in mips_atomic_set()
is harmless, so I keep this one.
For cmpxchg.h, the 32-bit version of cmpxchg64() doesn't need to be care
about because Loongson64 can support 64-bit kernel only, cmpxchg() need
memory barriers and it already has, cmpxchg_local() doesn't need memory
barriers because only the local cpu can write, which is the same as all
other local_t ops. So I remove all loongson_llsc_mb() in cmpxchg.h from
Peter's patch.
To summarize:
I keep Peter's comments and also keep necessary loongson_llsc_mb() in
test_and_set_bit_lock()/mips_atomic_set() from Peter's patch, but all
superfluous memory barries are removed.
Cc: Huang Pei <huangpei@loongson.cn>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Huacai Chen <chenhc@lemote.com>
---
arch/mips/include/asm/atomic.h | 5 ++---
arch/mips/include/asm/bitops.h | 4 ----
arch/mips/include/asm/cmpxchg.h | 5 -----
3 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/arch/mips/include/asm/atomic.h b/arch/mips/include/asm/atomic.h
index bb8658cc..4f6e538 100644
--- a/arch/mips/include/asm/atomic.h
+++ b/arch/mips/include/asm/atomic.h
@@ -193,7 +193,6 @@ static __inline__ int atomic_sub_if_positive(int i, atomic_t * v)
if (kernel_uses_llsc) {
int temp;
- loongson_llsc_mb();
__asm__ __volatile__(
" .set push \n"
" .set "MIPS_ISA_LEVEL" \n"
@@ -201,12 +200,12 @@ static __inline__ int atomic_sub_if_positive(int i, atomic_t * v)
" .set pop \n"
" subu %0, %1, %3 \n"
" move %1, %0 \n"
- " bltz %0, 2f \n"
+ " bltz %0, 1f \n"
" .set push \n"
" .set "MIPS_ISA_LEVEL" \n"
" sc %1, %2 \n"
"\t" __scbeqz " %1, 1b \n"
- "2: \n"
+ "1: \n"
" .set pop \n"
: "=&r" (result), "=&r" (temp),
"+" GCC_OFF_SMALL_ASM() (v->counter)
diff --git a/arch/mips/include/asm/bitops.h b/arch/mips/include/asm/bitops.h
index 985d6a0..5016b96 100644
--- a/arch/mips/include/asm/bitops.h
+++ b/arch/mips/include/asm/bitops.h
@@ -257,7 +257,6 @@ static inline int test_and_set_bit(unsigned long nr,
unsigned long *m = ((unsigned long *) addr) + (nr >> SZLONG_LOG);
unsigned long temp;
- loongson_llsc_mb();
do {
__asm__ __volatile__(
" .set push \n"
@@ -374,7 +373,6 @@ static inline int test_and_clear_bit(unsigned long nr,
unsigned long *m = ((unsigned long *) addr) + (nr >> SZLONG_LOG);
unsigned long temp;
- loongson_llsc_mb();
do {
__asm__ __volatile__(
" " __LL "%0, %1 # test_and_clear_bit \n"
@@ -390,7 +388,6 @@ static inline int test_and_clear_bit(unsigned long nr,
unsigned long *m = ((unsigned long *) addr) + (nr >> SZLONG_LOG);
unsigned long temp;
- loongson_llsc_mb();
do {
__asm__ __volatile__(
" .set push \n"
@@ -450,7 +447,6 @@ static inline int test_and_change_bit(unsigned long nr,
unsigned long *m = ((unsigned long *) addr) + (nr >> SZLONG_LOG);
unsigned long temp;
- loongson_llsc_mb();
do {
__asm__ __volatile__(
" .set push \n"
diff --git a/arch/mips/include/asm/cmpxchg.h b/arch/mips/include/asm/cmpxchg.h
index 79bf34e..dd39bae 100644
--- a/arch/mips/include/asm/cmpxchg.h
+++ b/arch/mips/include/asm/cmpxchg.h
@@ -46,7 +46,6 @@ extern unsigned long __xchg_called_with_bad_pointer(void)
__typeof(*(m)) __ret; \
\
if (kernel_uses_llsc) { \
- loongson_llsc_mb(); \
__asm__ __volatile__( \
" .set push \n" \
" .set noat \n" \
@@ -118,7 +117,6 @@ static inline unsigned long __xchg(volatile void *ptr, unsigned long x,
__typeof(*(m)) __ret; \
\
if (kernel_uses_llsc) { \
- loongson_llsc_mb(); \
__asm__ __volatile__( \
" .set push \n" \
" .set noat \n" \
@@ -136,7 +134,6 @@ static inline unsigned long __xchg(volatile void *ptr, unsigned long x,
: "=&r" (__ret), "=" GCC_OFF_SMALL_ASM() (*m) \
: GCC_OFF_SMALL_ASM() (*m), "Jr" (old), "Jr" (new) \
: __LLSC_CLOBBER); \
- loongson_llsc_mb(); \
} else { \
unsigned long __flags; \
\
@@ -232,7 +229,6 @@ static inline unsigned long __cmpxchg64(volatile void *ptr,
*/
local_irq_save(flags);
- loongson_llsc_mb();
asm volatile(
" .set push \n"
" .set " MIPS_ISA_ARCH_LEVEL " \n"
@@ -278,7 +274,6 @@ static inline unsigned long __cmpxchg64(volatile void *ptr,
"r" (old),
"r" (new)
: "memory");
- loongson_llsc_mb();
local_irq_restore(flags);
return ret;
--
2.7.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] MIPS: Loongson: remove unnecessary loongson_llsc_mb()
2019-09-09 3:42 [PATCH] MIPS: Loongson: remove unnecessary loongson_llsc_mb() Huacai Chen
@ 2019-09-22 2:21 ` Huacai Chen
0 siblings, 0 replies; 2+ messages in thread
From: Huacai Chen @ 2019-09-22 2:21 UTC (permalink / raw)
To: Paul Burton, Ralf Baechle, James Hogan
Cc: Linux MIPS Mailing List, open list:MIPS, Fuxin Zhang,
Zhangjin Wu, Huang Pei, Peter Zijlstra
Ping?
On Mon, Sep 9, 2019 at 11:41 AM Huacai Chen <chenhc@lemote.com> wrote:
>
> Commit 1c6c1ca318585f1 ("mips/atomic: Fix loongson_llsc_mb() wreckage")
> fix the description of Loongson-3's llsc bug and try to add all missing
> loongson_llsc_mb(). This is a good job but there are some unnecessary
> memory barries:
>
> loongson_llsc_mb() is a Loongson-specific problem. smp_llsc_mb(),
> smp_mb__before_llsc(), loongson_llsc_mb() and and other memory barriers
> are essentially the same thing on Loongson-3. So we don't need to add
> loongson_llsc_mb() if there is already a smp_mb__before_llsc() or other
> types of memory barriers.
>
> So, most of loongson_llsc_mb() in Peter's patch is superfluous and can
> be removed, except the one in test_and_set_bit_lock().
>
> mips_atomic_set() is not used on Loongson-3, and if in some cases we use
> it, the user-to-kernel context switching probably has the same effect of
> a memory barrier. But anyway, add a memory barrier in mips_atomic_set()
> is harmless, so I keep this one.
>
> For cmpxchg.h, the 32-bit version of cmpxchg64() doesn't need to be care
> about because Loongson64 can support 64-bit kernel only, cmpxchg() need
> memory barriers and it already has, cmpxchg_local() doesn't need memory
> barriers because only the local cpu can write, which is the same as all
> other local_t ops. So I remove all loongson_llsc_mb() in cmpxchg.h from
> Peter's patch.
>
> To summarize:
> I keep Peter's comments and also keep necessary loongson_llsc_mb() in
> test_and_set_bit_lock()/mips_atomic_set() from Peter's patch, but all
> superfluous memory barries are removed.
>
> Cc: Huang Pei <huangpei@loongson.cn>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Huacai Chen <chenhc@lemote.com>
> ---
> arch/mips/include/asm/atomic.h | 5 ++---
> arch/mips/include/asm/bitops.h | 4 ----
> arch/mips/include/asm/cmpxchg.h | 5 -----
> 3 files changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/arch/mips/include/asm/atomic.h b/arch/mips/include/asm/atomic.h
> index bb8658cc..4f6e538 100644
> --- a/arch/mips/include/asm/atomic.h
> +++ b/arch/mips/include/asm/atomic.h
> @@ -193,7 +193,6 @@ static __inline__ int atomic_sub_if_positive(int i, atomic_t * v)
> if (kernel_uses_llsc) {
> int temp;
>
> - loongson_llsc_mb();
> __asm__ __volatile__(
> " .set push \n"
> " .set "MIPS_ISA_LEVEL" \n"
> @@ -201,12 +200,12 @@ static __inline__ int atomic_sub_if_positive(int i, atomic_t * v)
> " .set pop \n"
> " subu %0, %1, %3 \n"
> " move %1, %0 \n"
> - " bltz %0, 2f \n"
> + " bltz %0, 1f \n"
> " .set push \n"
> " .set "MIPS_ISA_LEVEL" \n"
> " sc %1, %2 \n"
> "\t" __scbeqz " %1, 1b \n"
> - "2: \n"
> + "1: \n"
> " .set pop \n"
> : "=&r" (result), "=&r" (temp),
> "+" GCC_OFF_SMALL_ASM() (v->counter)
> diff --git a/arch/mips/include/asm/bitops.h b/arch/mips/include/asm/bitops.h
> index 985d6a0..5016b96 100644
> --- a/arch/mips/include/asm/bitops.h
> +++ b/arch/mips/include/asm/bitops.h
> @@ -257,7 +257,6 @@ static inline int test_and_set_bit(unsigned long nr,
> unsigned long *m = ((unsigned long *) addr) + (nr >> SZLONG_LOG);
> unsigned long temp;
>
> - loongson_llsc_mb();
> do {
> __asm__ __volatile__(
> " .set push \n"
> @@ -374,7 +373,6 @@ static inline int test_and_clear_bit(unsigned long nr,
> unsigned long *m = ((unsigned long *) addr) + (nr >> SZLONG_LOG);
> unsigned long temp;
>
> - loongson_llsc_mb();
> do {
> __asm__ __volatile__(
> " " __LL "%0, %1 # test_and_clear_bit \n"
> @@ -390,7 +388,6 @@ static inline int test_and_clear_bit(unsigned long nr,
> unsigned long *m = ((unsigned long *) addr) + (nr >> SZLONG_LOG);
> unsigned long temp;
>
> - loongson_llsc_mb();
> do {
> __asm__ __volatile__(
> " .set push \n"
> @@ -450,7 +447,6 @@ static inline int test_and_change_bit(unsigned long nr,
> unsigned long *m = ((unsigned long *) addr) + (nr >> SZLONG_LOG);
> unsigned long temp;
>
> - loongson_llsc_mb();
> do {
> __asm__ __volatile__(
> " .set push \n"
> diff --git a/arch/mips/include/asm/cmpxchg.h b/arch/mips/include/asm/cmpxchg.h
> index 79bf34e..dd39bae 100644
> --- a/arch/mips/include/asm/cmpxchg.h
> +++ b/arch/mips/include/asm/cmpxchg.h
> @@ -46,7 +46,6 @@ extern unsigned long __xchg_called_with_bad_pointer(void)
> __typeof(*(m)) __ret; \
> \
> if (kernel_uses_llsc) { \
> - loongson_llsc_mb(); \
> __asm__ __volatile__( \
> " .set push \n" \
> " .set noat \n" \
> @@ -118,7 +117,6 @@ static inline unsigned long __xchg(volatile void *ptr, unsigned long x,
> __typeof(*(m)) __ret; \
> \
> if (kernel_uses_llsc) { \
> - loongson_llsc_mb(); \
> __asm__ __volatile__( \
> " .set push \n" \
> " .set noat \n" \
> @@ -136,7 +134,6 @@ static inline unsigned long __xchg(volatile void *ptr, unsigned long x,
> : "=&r" (__ret), "=" GCC_OFF_SMALL_ASM() (*m) \
> : GCC_OFF_SMALL_ASM() (*m), "Jr" (old), "Jr" (new) \
> : __LLSC_CLOBBER); \
> - loongson_llsc_mb(); \
> } else { \
> unsigned long __flags; \
> \
> @@ -232,7 +229,6 @@ static inline unsigned long __cmpxchg64(volatile void *ptr,
> */
> local_irq_save(flags);
>
> - loongson_llsc_mb();
> asm volatile(
> " .set push \n"
> " .set " MIPS_ISA_ARCH_LEVEL " \n"
> @@ -278,7 +274,6 @@ static inline unsigned long __cmpxchg64(volatile void *ptr,
> "r" (old),
> "r" (new)
> : "memory");
> - loongson_llsc_mb();
>
> local_irq_restore(flags);
> return ret;
> --
> 2.7.0
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-09-22 2:16 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-09 3:42 [PATCH] MIPS: Loongson: remove unnecessary loongson_llsc_mb() Huacai Chen
2019-09-22 2:21 ` Huacai Chen
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).