From: Boqun Feng <boqun.feng@gmail.com>
To: guoren@kernel.org
Cc: arnd@arndb.de, mark.rutland@arm.com, peterz@infradead.org,
will@kernel.org, linux-arch@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-csky@vger.kernel.org,
Guo Ren <guoren@linux.alibaba.com>
Subject: Re: [PATCH V3 1/3] csky: cmpxchg: Optimize with acquire & release
Date: Fri, 22 Apr 2022 11:20:38 +0800 [thread overview]
Message-ID: <YmIfBmisFVBu8Z5R@tardis> (raw)
In-Reply-To: <20220417083204.2048364-2-guoren@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 5672 bytes --]
On Sun, Apr 17, 2022 at 04:32:02PM +0800, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Optimize arch_xchg|cmpxchg|cmpxchg_local with ASM acquire|release
> instructions instead of previous C based.
>
> Important reference comment by Rutland:
> 8e86f0b409a4 ("arm64: atomics: fix use of acquire + release for
> full barrier semantics")
>
> Link: https://lore.kernel.org/linux-riscv/CAJF2gTSAxpAi=LbAdu7jntZRUa=-dJwL0VfmDfBV5MHB=rcZ-w@mail.gmail.com/T/#m27a0f1342995deae49ce1d0e1f2683f8a181d6c3
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
> arch/csky/include/asm/barrier.h | 11 +++---
> arch/csky/include/asm/cmpxchg.h | 64 ++++++++++++++++++++++++++++++---
> 2 files changed, 67 insertions(+), 8 deletions(-)
>
> diff --git a/arch/csky/include/asm/barrier.h b/arch/csky/include/asm/barrier.h
> index f4045dd53e17..fb63335ffa33 100644
> --- a/arch/csky/include/asm/barrier.h
> +++ b/arch/csky/include/asm/barrier.h
> @@ -37,17 +37,21 @@
> * bar.brar
> * bar.bwaw
> */
> +#define ACQUIRE_FENCE ".long 0x8427c000\n"
> +#define RELEASE_FENCE ".long 0x842ec000\n"
> +#define FULL_FENCE ".long 0x842fc000\n"
> +
> #define __bar_brw() asm volatile (".long 0x842cc000\n":::"memory")
> #define __bar_br() asm volatile (".long 0x8424c000\n":::"memory")
> #define __bar_bw() asm volatile (".long 0x8428c000\n":::"memory")
> #define __bar_arw() asm volatile (".long 0x8423c000\n":::"memory")
> #define __bar_ar() asm volatile (".long 0x8421c000\n":::"memory")
> #define __bar_aw() asm volatile (".long 0x8422c000\n":::"memory")
> -#define __bar_brwarw() asm volatile (".long 0x842fc000\n":::"memory")
> -#define __bar_brarw() asm volatile (".long 0x8427c000\n":::"memory")
> +#define __bar_brwarw() asm volatile (FULL_FENCE:::"memory")
> +#define __bar_brarw() asm volatile (ACQUIRE_FENCE:::"memory")
> #define __bar_bwarw() asm volatile (".long 0x842bc000\n":::"memory")
> #define __bar_brwar() asm volatile (".long 0x842dc000\n":::"memory")
> -#define __bar_brwaw() asm volatile (".long 0x842ec000\n":::"memory")
> +#define __bar_brwaw() asm volatile (RELEASE_FENCE:::"memory")
> #define __bar_brar() asm volatile (".long 0x8425c000\n":::"memory")
> #define __bar_brar() asm volatile (".long 0x8425c000\n":::"memory")
> #define __bar_bwaw() asm volatile (".long 0x842ac000\n":::"memory")
> @@ -56,7 +60,6 @@
> #define __smp_rmb() __bar_brar()
> #define __smp_wmb() __bar_bwaw()
>
> -#define ACQUIRE_FENCE ".long 0x8427c000\n"
> #define __smp_acquire_fence() __bar_brarw()
> #define __smp_release_fence() __bar_brwaw()
>
> diff --git a/arch/csky/include/asm/cmpxchg.h b/arch/csky/include/asm/cmpxchg.h
> index d1bef11f8dc9..06c550448bf1 100644
> --- a/arch/csky/include/asm/cmpxchg.h
> +++ b/arch/csky/include/asm/cmpxchg.h
> @@ -64,15 +64,71 @@ extern void __bad_xchg(void);
> #define arch_cmpxchg_relaxed(ptr, o, n) \
> (__cmpxchg_relaxed((ptr), (o), (n), sizeof(*(ptr))))
>
> -#define arch_cmpxchg(ptr, o, n) \
> +#define __cmpxchg_acquire(ptr, old, new, size) \
> ({ \
> + __typeof__(ptr) __ptr = (ptr); \
> + __typeof__(new) __new = (new); \
> + __typeof__(new) __tmp; \
> + __typeof__(old) __old = (old); \
> + __typeof__(*(ptr)) __ret; \
> + switch (size) { \
> + case 4: \
> + asm volatile ( \
> + "1: ldex.w %0, (%3) \n" \
> + " cmpne %0, %4 \n" \
> + " bt 2f \n" \
> + " mov %1, %2 \n" \
> + " stex.w %1, (%3) \n" \
> + " bez %1, 1b \n" \
> + ACQUIRE_FENCE \
> + "2: \n" \
> + : "=&r" (__ret), "=&r" (__tmp) \
> + : "r" (__new), "r"(__ptr), "r"(__old) \
> + :); \
> + break; \
> + default: \
> + __bad_xchg(); \
> + } \
> + __ret; \
> +})
> +
> +#define arch_cmpxchg_acquire(ptr, o, n) \
> + (__cmpxchg_acquire((ptr), (o), (n), sizeof(*(ptr))))
> +
> +#define __cmpxchg(ptr, old, new, size) \
> +({ \
> + __typeof__(ptr) __ptr = (ptr); \
> + __typeof__(new) __new = (new); \
> + __typeof__(new) __tmp; \
> + __typeof__(old) __old = (old); \
> __typeof__(*(ptr)) __ret; \
> - __smp_release_fence(); \
> - __ret = arch_cmpxchg_relaxed(ptr, o, n); \
> - __smp_acquire_fence(); \
> + switch (size) { \
> + case 4: \
> + asm volatile ( \
> + "1: ldex.w %0, (%3) \n" \
> + " cmpne %0, %4 \n" \
> + " bt 2f \n" \
> + " mov %1, %2 \n" \
> + RELEASE_FENCE \
FWIW, you probably need to make sure that a barrier instruction inside
an lr/sc loop is a good thing. IIUC, the execution time of a barrier
instruction is determined by the status of store buffers and invalidate
queues (and probably other stuffs), so it may increase the execution
time of the lr/sc loop, and make it unlikely to succeed. But this really
depends on how the arch executes these instructions.
Regards,
Boqun
> + " stex.w %1, (%3) \n" \
> + " bez %1, 1b \n" \
> + FULL_FENCE \
> + "2: \n" \
> + : "=&r" (__ret), "=&r" (__tmp) \
> + : "r" (__new), "r"(__ptr), "r"(__old) \
> + :); \
> + break; \
> + default: \
> + __bad_xchg(); \
> + } \
> __ret; \
> })
>
> +#define arch_cmpxchg(ptr, o, n) \
> + (__cmpxchg((ptr), (o), (n), sizeof(*(ptr))))
> +
> +#define arch_cmpxchg_local(ptr, o, n) \
> + (__cmpxchg_relaxed((ptr), (o), (n), sizeof(*(ptr))))
> #else
> #include <asm-generic/cmpxchg.h>
> #endif
> --
> 2.25.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2022-04-22 3:20 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-17 8:32 [PATCH V3 0/3] csky: Optimize with acquire & release for atomic & cmpxchg guoren
2022-04-17 8:32 ` [PATCH V3 1/3] csky: cmpxchg: Optimize with acquire & release guoren
2022-04-22 3:20 ` Boqun Feng [this message]
2022-04-22 3:45 ` Guo Ren
2022-04-17 8:32 ` [PATCH V3 2/3] csky: atomic: Add custom atomic.h implementation guoren
2022-04-17 8:32 ` [PATCH V3 3/3] csky: atomic: Add conditional atomic operations' optimization guoren
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=YmIfBmisFVBu8Z5R@tardis \
--to=boqun.feng@gmail.com \
--cc=arnd@arndb.de \
--cc=guoren@kernel.org \
--cc=guoren@linux.alibaba.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-csky@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=peterz@infradead.org \
--cc=will@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox