Received-SPF: pass (google.com: domain of linux-kernel+bounces-141909-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240411010510.22135-1-yangtiezhu@loongson.cn> <20240411010510.22135-2-yangtiezhu@loongson.cn>
In-Reply-To: <20240411010510.22135-2-yangtiezhu@loongson.cn>
From: Huacai Chen <chenhuacai@kernel.org>
Date: Fri, 12 Apr 2024 12:12:30 +0800
Message-ID: <CAAhV-H6TVEEB=xdr_ymuUnDqPDXwtM7R7yJBj03YhWpCwqjAiA@mail.gmail.com>
Subject: Re: [PATCH v3 1/4] LoongArch: Move CONFIG_HAVE_SETUP_PER_CPU_AREA
 related code to smp.c
To: Tiezhu Yang <yangtiezhu@loongson.cn>
Cc: Thomas Gleixner <tglx@linutronix.de>, Arnd Bergmann <arnd@arndb.de>, Marc Zyngier <maz@kernel.org>, 
	loongarch@lists.linux.dev, linux-kernel@vger.kernel.org, 
	loongson-kernel@lists.loongnix.cn
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Apr 11, 2024 at 9:05=E2=80=AFAM Tiezhu Yang <yangtiezhu@loongson.cn=
> wrote:
>
> Currently, if CONFIG_NUMA is not set but CONFIG_SMP is set, the arch
> specified setup_per_cpu_areas() in arch/loongarch/kernel/numa.c will
> not be built and the generic setup_per_cpu_areas() in mm/percpu.c is
> actually used, this is not reasonable and does not work as intended.
Why is the generic version not reasonable? We need a custom version
just because it can put the percpu variable in the best node. If NUMA
disabled, software can only see one node, how to optimize?

Huacai

>
> Let us select HAVE_SETUP_PER_CPU_AREA unconditionally and then move
> CONFIG_HAVE_SETUP_PER_CPU_AREA related code from numa.c to smp.c to
> avoid this problem.
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  arch/loongarch/Kconfig       |  2 +-
>  arch/loongarch/kernel/numa.c | 58 -----------------------------------
>  arch/loongarch/kernel/smp.c  | 59 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 60 insertions(+), 59 deletions(-)
>
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index a5f300ec6f28..64052ae2c2af 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -151,7 +151,7 @@ config LOONGARCH
>         select HAVE_RUST
>         select HAVE_SAMPLE_FTRACE_DIRECT
>         select HAVE_SAMPLE_FTRACE_DIRECT_MULTI
> -       select HAVE_SETUP_PER_CPU_AREA if NUMA
> +       select HAVE_SETUP_PER_CPU_AREA
>         select HAVE_STACK_VALIDATION if HAVE_OBJTOOL
>         select HAVE_STACKPROTECTOR
>         select HAVE_SYSCALL_TRACEPOINTS
> diff --git a/arch/loongarch/kernel/numa.c b/arch/loongarch/kernel/numa.c
> index 8fe21f868f72..49dc1d932ce2 100644
> --- a/arch/loongarch/kernel/numa.c
> +++ b/arch/loongarch/kernel/numa.c
> @@ -48,64 +48,6 @@ EXPORT_SYMBOL(__cpuid_to_node);
>
>  nodemask_t numa_nodes_parsed __initdata;
>
> -#ifdef CONFIG_HAVE_SETUP_PER_CPU_AREA
> -unsigned long __per_cpu_offset[NR_CPUS] __read_mostly;
> -EXPORT_SYMBOL(__per_cpu_offset);
> -
> -static int __init pcpu_cpu_to_node(int cpu)
> -{
> -       return early_cpu_to_node(cpu);
> -}
> -
> -static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
> -{
> -       if (early_cpu_to_node(from) =3D=3D early_cpu_to_node(to))
> -               return LOCAL_DISTANCE;
> -       else
> -               return REMOTE_DISTANCE;
> -}
> -
> -void __init pcpu_populate_pte(unsigned long addr)
> -{
> -       populate_kernel_pte(addr);
> -}
> -
> -void __init setup_per_cpu_areas(void)
> -{
> -       unsigned long delta;
> -       unsigned int cpu;
> -       int rc =3D -EINVAL;
> -
> -       if (pcpu_chosen_fc =3D=3D PCPU_FC_AUTO) {
> -               if (nr_node_ids >=3D 8)
> -                       pcpu_chosen_fc =3D PCPU_FC_PAGE;
> -               else
> -                       pcpu_chosen_fc =3D PCPU_FC_EMBED;
> -       }
> -
> -       /*
> -        * Always reserve area for module percpu variables.  That's
> -        * what the legacy allocator did.
> -        */
> -       if (pcpu_chosen_fc !=3D PCPU_FC_PAGE) {
> -               rc =3D pcpu_embed_first_chunk(PERCPU_MODULE_RESERVE,
> -                                           PERCPU_DYNAMIC_RESERVE, PMD_S=
IZE,
> -                                           pcpu_cpu_distance, pcpu_cpu_t=
o_node);
> -               if (rc < 0)
> -                       pr_warn("%s allocator failed (%d), falling back t=
o page size\n",
> -                               pcpu_fc_names[pcpu_chosen_fc], rc);
> -       }
> -       if (rc < 0)
> -               rc =3D pcpu_page_first_chunk(PERCPU_MODULE_RESERVE, pcpu_=
cpu_to_node);
> -       if (rc < 0)
> -               panic("cannot initialize percpu area (err=3D%d)", rc);
> -
> -       delta =3D (unsigned long)pcpu_base_addr - (unsigned long)__per_cp=
u_start;
> -       for_each_possible_cpu(cpu)
> -               __per_cpu_offset[cpu] =3D delta + pcpu_unit_offsets[cpu];
> -}
> -#endif
> -
>  /*
>   * Get nodeid by logical cpu number.
>   * __cpuid_to_node maps phyical cpu id to node, so we
> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> index aabee0b280fe..88b9c6b68d1e 100644
> --- a/arch/loongarch/kernel/smp.c
> +++ b/arch/loongarch/kernel/smp.c
> @@ -29,6 +29,7 @@
>  #include <asm/loongson.h>
>  #include <asm/mmu_context.h>
>  #include <asm/numa.h>
> +#include <asm/pgalloc.h>
>  #include <asm/processor.h>
>  #include <asm/setup.h>
>  #include <asm/time.h>
> @@ -717,3 +718,61 @@ void flush_tlb_one(unsigned long vaddr)
>         on_each_cpu(flush_tlb_one_ipi, (void *)vaddr, 1);
>  }
>  EXPORT_SYMBOL(flush_tlb_one);
> +
> +#ifdef CONFIG_HAVE_SETUP_PER_CPU_AREA
> +unsigned long __per_cpu_offset[NR_CPUS] __read_mostly;
> +EXPORT_SYMBOL(__per_cpu_offset);
> +
> +static int __init pcpu_cpu_to_node(int cpu)
> +{
> +       return early_cpu_to_node(cpu);
> +}
> +
> +static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
> +{
> +       if (early_cpu_to_node(from) =3D=3D early_cpu_to_node(to))
> +               return LOCAL_DISTANCE;
> +       else
> +               return REMOTE_DISTANCE;
> +}
> +
> +void __init pcpu_populate_pte(unsigned long addr)
> +{
> +       populate_kernel_pte(addr);
> +}
> +
> +void __init setup_per_cpu_areas(void)
> +{
> +       unsigned long delta;
> +       unsigned int cpu;
> +       int rc =3D -EINVAL;
> +
> +       if (pcpu_chosen_fc =3D=3D PCPU_FC_AUTO) {
> +               if (nr_node_ids >=3D 8)
> +                       pcpu_chosen_fc =3D PCPU_FC_PAGE;
> +               else
> +                       pcpu_chosen_fc =3D PCPU_FC_EMBED;
> +       }
> +
> +       /*
> +        * Always reserve area for module percpu variables.  That's
> +        * what the legacy allocator did.
> +        */
> +       if (pcpu_chosen_fc !=3D PCPU_FC_PAGE) {
> +               rc =3D pcpu_embed_first_chunk(PERCPU_MODULE_RESERVE,
> +                                           PERCPU_DYNAMIC_RESERVE, PMD_S=
IZE,
> +                                           pcpu_cpu_distance, pcpu_cpu_t=
o_node);
> +               if (rc < 0)
> +                       pr_warn("%s allocator failed (%d), falling back t=
o page size\n",
> +                               pcpu_fc_names[pcpu_chosen_fc], rc);
> +       }
> +       if (rc < 0)
> +               rc =3D pcpu_page_first_chunk(PERCPU_MODULE_RESERVE, pcpu_=
cpu_to_node);
> +       if (rc < 0)
> +               panic("cannot initialize percpu area (err=3D%d)", rc);
> +
> +       delta =3D (unsigned long)pcpu_base_addr - (unsigned long)__per_cp=
u_start;
> +       for_each_possible_cpu(cpu)
> +               __per_cpu_offset[cpu] =3D delta + pcpu_unit_offsets[cpu];
> +}
> +#endif
> --
> 2.42.0
>
>