[PATCH -next] RISCV: Add some depends for NUMA

[PATCH -next] RISCV: Add some depends for NUMA

[Kefeng Wang]

The numa feature is useless for riscv32 platform(MAXPHYSMEM_1GB if 32bit),
and it should depends on SMP feature, this also fix the build error,

  riscv64-buildroot-linux-gnu-ld: mm/page_alloc.o: in function `.L0 ':
  page_alloc.c:(.text+0x4808): undefined reference to `node_reclaim_distance'

Fixes: 4f0e8eef772e ("riscv: Add numa support for riscv64 platform")
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 arch/riscv/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 22fa17898d29..ac7f5801bd82 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -312,6 +312,8 @@ endchoice
 # Common NUMA Features
 config NUMA
 	bool "NUMA Memory Allocation and Scheduler Support"
+	depends on SMP
+	depends on 64BIT
 	select GENERIC_ARCH_NUMA
 	select OF_NUMA
 	select ARCH_SUPPORTS_NUMA_BALANCING
-- 
2.26.2

Re: [PATCH -next] RISCV: Add some depends for NUMA

[Atish Patra]

On Wed, Feb 3, 2021 at 6:20 AM Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>
> The numa feature is useless for riscv32 platform(MAXPHYSMEM_1GB if 32bit),
> and it should depends on SMP feature, this also fix the build error,
>
>   riscv64-buildroot-linux-gnu-ld: mm/page_alloc.o: in function `.L0 ':
>   page_alloc.c:(.text+0x4808): undefined reference to `node_reclaim_distance'
>

Thanks for the fix. The commit text is a bit ambiguous. The
compilation issue error is due to SMP.
RV32 dependency is just another fix  as NUMA is not very useful for RV32.

> Fixes: 4f0e8eef772e ("riscv: Add numa support for riscv64 platform")
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  arch/riscv/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 22fa17898d29..ac7f5801bd82 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -312,6 +312,8 @@ endchoice
>  # Common NUMA Features
>  config NUMA
>         bool "NUMA Memory Allocation and Scheduler Support"
> +       depends on SMP
> +       depends on 64BIT
>         select GENERIC_ARCH_NUMA
>         select OF_NUMA
>         select ARCH_SUPPORTS_NUMA_BALANCING
> --
> 2.26.2
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv


Reviewed-by: Atish Patra <atish.patra@wdc.com>
-- 
Regards,
Atish

Re: [PATCH -next] RISCV: Add some depends for NUMA

[Kefeng Wang]

On 2021/2/4 3:59, Atish Patra wrote:
> On Wed, Feb 3, 2021 at 6:20 AM Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>> The numa feature is useless for riscv32 platform(MAXPHYSMEM_1GB if 32bit),
>> and it should depends on SMP feature, this also fix the build error,
>>
>>    riscv64-buildroot-linux-gnu-ld: mm/page_alloc.o: in function `.L0 ':
>>    page_alloc.c:(.text+0x4808): undefined reference to `node_reclaim_distance'
>>
> Thanks for the fix. The commit text is a bit ambiguous. The
> compilation issue error is due to SMP.
> RV32 dependency is just another fix  as NUMA is not very useful for RV32.

Yes, the two depends fix different issue，the lack of SMP leads to the 
compilation issue,  another

depend is to remove support on RV32.

Thanks for your review, if necessary, I could resend with changelog 
updated.

> Reviewed-by: Atish Patra <atish.patra@wdc.com>

Re: [PATCH -next] RISCV: Add some depends for NUMA

[Palmer Dabbelt]

On Wed, 03 Feb 2021 06:23:43 PST (-0800), wangkefeng.wang@huawei.com wrote:
> The numa feature is useless for riscv32 platform(MAXPHYSMEM_1GB if 32bit),

I'm not convinced of that.  There's no reason NUMA shouldn't work on 32-bit, it
doesn't depend on having a large amount of memory just having non-uniform
memory.  I could buy the argument that build a 32-bit NUMA system would be
wacky, but IIUC it works now and I don't see any reason to throw that away.

> and it should depends on SMP feature, this also fix the build error,

I can buy that CONFIG_NUMA doesn't really make sense without CONFIG_SMP, as
there's not a whole lot to do, but I also don't see any reason from disallowing
users from picking it.  arm64 allows !SMP && NUMA, and I don't see any reason
it wouldn't work just as well for us.

>   riscv64-buildroot-linux-gnu-ld: mm/page_alloc.o: in function `.L0 ':
>   page_alloc.c:(.text+0x4808): undefined reference to `node_reclaim_distance'

The only instance of node_reclaim_distance I see in mm/page_alloc.c is already
guarded with CONFIG_NUMA, but the definition of node_reclaim_distance isn't.
I'll send out some patches to add the guard which might make sorting this out
earlier, but I don't see it fixing any failures.

> Fixes: 4f0e8eef772e ("riscv: Add numa support for riscv64 platform")
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  arch/riscv/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 22fa17898d29..ac7f5801bd82 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -312,6 +312,8 @@ endchoice
>  # Common NUMA Features
>  config NUMA
>  	bool "NUMA Memory Allocation and Scheduler Support"
> +	depends on SMP
> +	depends on 64BIT
>  	select GENERIC_ARCH_NUMA
>  	select OF_NUMA
>  	select ARCH_SUPPORTS_NUMA_BALANCING

Re: [PATCH -next] RISCV: Add some depends for NUMA

[Kefeng Wang]

On 2021/2/5 14:58, Palmer Dabbelt wrote:
> On Wed, 03 Feb 2021 06:23:43 PST (-0800), wangkefeng.wang@huawei.com 
> wrote:
>> The numa feature is useless for riscv32 platform(MAXPHYSMEM_1GB if 
>> 32bit),
>
> I'm not convinced of that.  There's no reason NUMA shouldn't work on 
> 32-bit, it
> doesn't depend on having a large amount of memory just having non-uniform
> memory.  I could buy the argument that build a 32-bit NUMA system 
> would be
> wacky, but IIUC it works now and I don't see any reason to throw that 
> away.

I think the RV32 won't benefit from NUMA, there's no too much memory 
support on RV32,

and it may increase the Image size,  so I decided to drop support for RV32.

>
>> and it should depends on SMP feature, this also fix the build error,
>
> I can buy that CONFIG_NUMA doesn't really make sense without 
> CONFIG_SMP, as
> there's not a whole lot to do, but I also don't see any reason from 
> disallowing
> users from picking it.  arm64 allows !SMP && NUMA, and I don't see any 
> reason
> it wouldn't work just as well for us.

ARM64 won' support !SMP after following patch, and for most 
architecture,  the NUMA feature

depends or select SMP feature.


commit 4b3dc9679cf779339d9049800803dfc3c83433d1
Author: Will Deacon <will@kernel.org>
Date:   Fri May 29 18:28:44 2015 +0100

     arm64: force CONFIG_SMP=y and remove redundant #ifdefs



>
>>   riscv64-buildroot-linux-gnu-ld: mm/page_alloc.o: in function `.L0 ':
>>   page_alloc.c:(.text+0x4808): undefined reference to 
>> `node_reclaim_distance'
>
> The only instance of node_reclaim_distance I see in mm/page_alloc.c is 
> already
> guarded with CONFIG_NUMA, but the definition of node_reclaim_distance 
> isn't.
> I'll send out some patches to add the guard which might make sorting 
> this out
> earlier, but I don't see it fixing any failures.
>
>
>