Re: [RFC PATCH 1/4] vmstat: percpu: Rename HAVE_CMPXCHG_LOCAL to HAVE_CMPXCHG_PERCPU_BYTE

From: Guo Ren
Date: Mon Aug 08 2022 - 22:58:37 EST


On Mon, Aug 8, 2022 at 5:31 PM Christoph Lameter <cl@xxxxxxxxx> wrote:
>
> On Mon, 8 Aug 2022, guoren@xxxxxxxxxx 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