| Message ID | 20220808080600.3346843-2-guoren@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: Add basic percpu operations | expand |
On Mon, 8 Aug 2022, guoren@kernel.org wrote: > The name HAVE_CMPXCHG_LOCAL is confused with using cmpxchg_local, but > vmstat needs this_cpu_cmpxchg_1. Rename would clarify the meaning, and > maybe we could remove cmpxchg(64)_local API (Only drivers/iommu/intel > used) in the future. HAVE_CMPXCHG_LOCAL indicates that cmpxchg_local() is available. The term LOCAL is important because that has traditionally signified an operation that has an atomic nature that only works on the local core. cmpxchg local is used in slub too in the form of this_cpu_cmpxchg_double. But there is the other naming using this_cpu..... Maybe rename to HAVE_THIS_CPU_CMPXCHG ? and clean up all the other mentions of "local" in the source too? There is also a local.h header around somewhere
On Mon, Aug 8, 2022 at 5:31 PM Christoph Lameter <cl@gentwo.de> wrote: > > On Mon, 8 Aug 2022, guoren@kernel.org wrote: > > > The name HAVE_CMPXCHG_LOCAL is confused with using cmpxchg_local, but > > vmstat needs this_cpu_cmpxchg_1. Rename would clarify the meaning, and > > maybe we could remove cmpxchg(64)_local API (Only drivers/iommu/intel > > used) in the future. > > HAVE_CMPXCHG_LOCAL indicates that cmpxchg_local() is available. > > The term LOCAL is important because that has traditionally signified an > operation that has an atomic nature that only works on the local core. > > cmpxchg local is used in slub too in the form of this_cpu_cmpxchg_double. 1. raw_cpu_generic_cmpxchg_double don't use cmpxchg(64)_local. 2. x86 and s390 implement this_cpu_cmpxchg_double with direct asm code, no relationship to cmpxchg local. 3. Only arm64 using cmpxchg_double_local internal, but we could remove the relationship from generic cmpxchg_double_local. It's a fake usage. So maybe it's time to remove cmpxchg(64)_local in Linux and replace them by this_cpu_cmpxchg & cmpxchg_relaxed. > > But there is the other naming using this_cpu..... > > Maybe rename to > > HAVE_THIS_CPU_CMPXCHG ? I think we should keep 1/BYTE as a suffix because riscv only implements 4bytes & 8bytes size cmpxchg. But vmstat needs 1Byte. > > and clean up all the other mentions of "local" in the source too? Good point, I would try. How we deal with drivers/iommu/intel/iommu.c: tmp = cmpxchg64_local(&pte->val, 0ULL, pteval); Change "cmpxchg64_local -> cmpxchg64_relaxed" would make them happy? I think they are cmpxchg_local & cmpxchg_sync users. > > There is also a local.h header around somewhere Yes, thx for mentioning, I missed that. The alpha, loongarch, MIPS, PowerPC and x86 make local_cmpxchg -> cmpxchg_local. Most of them are copy-paste guys, not real users. -- Best Regards Guo Ren
diff --git a/Documentation/features/locking/cmpxchg-local/arch-support.txt b/Documentation/features/locking/cmpxchg-local/arch-support.txt index 8b1a8d9e1c79..4d4c5c2fa66d 100644 --- a/Documentation/features/locking/cmpxchg-local/arch-support.txt +++ b/Documentation/features/locking/cmpxchg-local/arch-support.txt @@ -1,7 +1,7 @@ # -# Feature name: cmpxchg-local -# Kconfig: HAVE_CMPXCHG_LOCAL -# description: arch supports the this_cpu_cmpxchg() API +# Feature name: cmpxchg-percpu-byte +# Kconfig: HAVE_CMPXCHG_PERCPU_BYTE +# description: arch supports the this_cpu_cmpxchg_1() API # ----------------------- | arch |status| diff --git a/arch/Kconfig b/arch/Kconfig index f330410da63a..81800cdfe161 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -471,7 +471,7 @@ config HAVE_ALIGNED_STRUCT_PAGE on a struct page for better performance. However selecting this might increase the size of a struct page by a word. -config HAVE_CMPXCHG_LOCAL +config HAVE_CMPXCHG_PERCPU_BYTE bool config HAVE_CMPXCHG_DOUBLE diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 571cc234d0b3..24a82bdc766a 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -175,7 +175,7 @@ config ARM64 select HAVE_EBPF_JIT select HAVE_C_RECORDMCOUNT select HAVE_CMPXCHG_DOUBLE - select HAVE_CMPXCHG_LOCAL + select HAVE_CMPXCHG_PERCPU_BYTE select HAVE_CONTEXT_TRACKING_USER select HAVE_DEBUG_KMEMLEAK select HAVE_DMA_CONTIGUOUS diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig index 318fce77601d..ac03af800bf7 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -151,7 +151,7 @@ config S390 select HAVE_ARCH_VMAP_STACK select HAVE_ASM_MODVERSIONS select HAVE_CMPXCHG_DOUBLE - select HAVE_CMPXCHG_LOCAL + select HAVE_CMPXCHG_PERCPU_BYTE select HAVE_DEBUG_KMEMLEAK select HAVE_DMA_CONTIGUOUS select HAVE_DYNAMIC_FTRACE diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index f9920f1341c8..5f4f6df7b89f 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -184,7 +184,7 @@ config X86 select HAVE_ARCH_WITHIN_STACK_FRAMES select HAVE_ASM_MODVERSIONS select HAVE_CMPXCHG_DOUBLE - select HAVE_CMPXCHG_LOCAL + select HAVE_CMPXCHG_PERCPU_BYTE select HAVE_CONTEXT_TRACKING_USER if X86_64 select HAVE_CONTEXT_TRACKING_USER_OFFSTACK if HAVE_CONTEXT_TRACKING_USER select HAVE_C_RECORDMCOUNT diff --git a/mm/vmstat.c b/mm/vmstat.c index 373d2730fcf2..b2fc6d28d3b2 100644 --- a/mm/vmstat.c +++ b/mm/vmstat.c @@ -554,9 +554,9 @@ void __dec_node_page_state(struct page *page, enum node_stat_item item) } EXPORT_SYMBOL(__dec_node_page_state); -#ifdef CONFIG_HAVE_CMPXCHG_LOCAL +#ifdef CONFIG_HAVE_CMPXCHG_PERCPU_BYTE /* - * If we have cmpxchg_local support then we do not need to incur the overhead + * If we have this_cpu_cmpxchg_1 arch support then we do not need to incur the overhead * that comes with local_irq_save/restore if we use this_cpu_cmpxchg. * * mod_state() modifies the zone counter state through atomic per cpu