2024-03-26 06:21:24

by Tiezhu Yang

[permalink] [raw]
Subject: [PATCH] LoongArch: Give chance to build under !CONFIG_SMP

In the current code, SMP is selected in Kconfig for LoongArch, the users
can not unset it, this is reasonable for a multiprocessor machine. But as
the help info of config SMP said, if you have a system with only one CPU,
say N. On a uniprocessor machine, the kernel will run faster if you say N
here.

The Loongson-2K0500 is a single-core CPU for applications like industrial
control, printing terminals, and BMC (Baseboard Management Controller),
there are many development boards, products and solutions on the market,
so it is better and necessary to give a chance to build under !CONFIG_SMP
for a uniprocessor machine.

First of all, do not select SMP for config LOONGARCH in Kconfig to make it
possible to unset CONFIG_SMP. Then, do some changes to fix the warnings and
errors if CONFIG_SMP is not set.

(1) Select GENERIC_IRQ_EFFECTIVE_AFF_MASK only if SMP for config
IRQ_LOONGARCH_CPU to fix the warning when make menuconfig:
WARNING: unmet direct dependencies detected for GENERIC_IRQ_EFFECTIVE_AFF_MASK

(2) Define cpu_logical_map(cpu) as 0 under !CONFIG_SMP in asm/smp.h and
include asm/smp.h in asm/acpi.h to fix the error:
/arch/loongarch/include/asm/acpi.h: In function ‘get_acpi_id_for_cpu’:
/arch/loongarch/include/asm/acpi.h:44:30: error: implicit declaration of function ‘cpu_logical_map’ [-Wimplicit-function-declaration]

(3) Define get_ipi_irq() only if CONFIG_SMP is set to fix the warning:
arch/loongarch/kernel/irq.c:90:19: warning: ‘get_ipi_irq’ defined but not used [-Wunused-function]

(4) Define machine_shutdown() as empty under !CONFIG_SMP to fix the error:
arch/loongarch/kernel/machine_kexec.c: In function ‘machine_shutdown’:
arch/loongarch/kernel/machine_kexec.c:233:25: error: implicit declaration of function ‘cpu_device_up’; did you mean ‘put_device’? [-Wimplicit-function-declaration]

(5) Make config SCHED_SMT depends on SMP to avoid many errors such as:
kernel/sched/core.c: In function ‘sched_core_find’:
kernel/sched/core.c:310:43: error: ‘struct rq’ has no member named ‘cpu’

(6) Call per_cpu_offset() only under CONFIG_HAVE_SETUP_PER_CPU_AREA to
fix the error:
arch/loongarch/power/suspend.c: In function ‘loongarch_common_resume’:
arch/loongarch/power/suspend.c:47:21: error: implicit declaration of function ‘per_cpu_offset’ [-Wimplicit-function-declaration]

(7) Define and call eiointc_set_irq_affinity() under CONFIG_SMP to set
the CPU affinity only on SMP machines.

When running the UnixBench tests with "-c 1" single-streamed pass,
the improvement in performance is about 9 percent with this patch.

By the way, it is helpful to debug and analysis the kernel issue
of multi-core system under !CONFIG_SMP.

Signed-off-by: Tiezhu Yang <[email protected]>
---

This patch is based on 6.9-rc1. Please let me know if it is better to
split it into two patches, the first one is for arch/loongarch and the
second one is for drivers/irqchip.

arch/loongarch/Kconfig | 2 +-
arch/loongarch/include/asm/acpi.h | 1 +
arch/loongarch/include/asm/smp.h | 5 +++++
arch/loongarch/kernel/irq.c | 2 ++
arch/loongarch/kernel/machine_kexec.c | 2 +-
arch/loongarch/power/suspend.c | 2 ++
drivers/irqchip/Kconfig | 2 +-
drivers/irqchip/irq-loongson-eiointc.c | 4 ++++
8 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index a5f300ec6f28..8d892de0b7a8 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -174,7 +174,6 @@ config LOONGARCH
select PCI_QUIRKS
select PERF_USE_VMALLOC
select RTC_LIB
- select SMP
select SPARSE_IRQ
select SYSCTL_ARCH_UNALIGN_ALLOW
select SYSCTL_ARCH_UNALIGN_NO_WARN
@@ -420,6 +419,7 @@ config EFI_STUB

config SCHED_SMT
bool "SMT scheduler support"
+ depends on SMP
default y
help
Improves scheduler's performance when there are multiple
diff --git a/arch/loongarch/include/asm/acpi.h b/arch/loongarch/include/asm/acpi.h
index 49e29b29996f..313f66f7913a 100644
--- a/arch/loongarch/include/asm/acpi.h
+++ b/arch/loongarch/include/asm/acpi.h
@@ -8,6 +8,7 @@
#ifndef _ASM_LOONGARCH_ACPI_H
#define _ASM_LOONGARCH_ACPI_H

+#include <asm/smp.h>
#include <asm/suspend.h>

#ifdef CONFIG_ACPI
diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
index f81e5f01d619..c14ddfcd4829 100644
--- a/arch/loongarch/include/asm/smp.h
+++ b/arch/loongarch/include/asm/smp.h
@@ -6,6 +6,7 @@
#ifndef __ASM_SMP_H
#define __ASM_SMP_H

+#ifdef CONFIG_SMP
#include <linux/atomic.h>
#include <linux/bitops.h>
#include <linux/linkage.h>
@@ -101,4 +102,8 @@ static inline void __cpu_die(unsigned int cpu)
}
#endif

+#else /* !CONFIG_SMP */
+#define cpu_logical_map(cpu) 0
+#endif
+
#endif /* __ASM_SMP_H */
diff --git a/arch/loongarch/kernel/irq.c b/arch/loongarch/kernel/irq.c
index 883e5066ae44..e791fa275ec5 100644
--- a/arch/loongarch/kernel/irq.c
+++ b/arch/loongarch/kernel/irq.c
@@ -87,6 +87,7 @@ static void __init init_vec_parent_group(void)
acpi_table_parse(ACPI_SIG_MCFG, early_pci_mcfg_parse);
}

+#ifdef CONFIG_SMP
static int __init get_ipi_irq(void)
{
struct irq_domain *d = irq_find_matching_fwnode(cpuintc_handle, DOMAIN_BUS_ANY);
@@ -96,6 +97,7 @@ static int __init get_ipi_irq(void)

return -EINVAL;
}
+#endif

void __init init_IRQ(void)
{
diff --git a/arch/loongarch/kernel/machine_kexec.c b/arch/loongarch/kernel/machine_kexec.c
index 2dcb9e003657..8ae641dc53bb 100644
--- a/arch/loongarch/kernel/machine_kexec.c
+++ b/arch/loongarch/kernel/machine_kexec.c
@@ -225,6 +225,7 @@ void crash_smp_send_stop(void)

void machine_shutdown(void)
{
+#ifdef CONFIG_SMP
int cpu;

/* All CPUs go to reboot_code_buffer */
@@ -232,7 +233,6 @@ void machine_shutdown(void)
if (!cpu_online(cpu))
cpu_device_up(get_cpu_device(cpu));

-#ifdef CONFIG_SMP
smp_call_function(kexec_shutdown_secondary, NULL, 0);
#endif
}
diff --git a/arch/loongarch/power/suspend.c b/arch/loongarch/power/suspend.c
index 166d9e06a64b..e8ca77eb3288 100644
--- a/arch/loongarch/power/suspend.c
+++ b/arch/loongarch/power/suspend.c
@@ -44,7 +44,9 @@ void loongarch_common_resume(void)
{
sync_counter();
local_flush_tlb_all();
+#ifdef CONFIG_HAVE_SETUP_PER_CPU_AREA
csr_write64(per_cpu_offset(0), PERCPU_BASE_KS);
+#endif
csr_write64(eentry, LOONGARCH_CSR_EENTRY);
csr_write64(eentry, LOONGARCH_CSR_MERRENTRY);
csr_write64(tlbrentry, LOONGARCH_CSR_TLBRENTRY);
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 72c07a12f5e1..bfa1d77749f3 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -568,7 +568,7 @@ config IRQ_LOONGARCH_CPU
bool
select GENERIC_IRQ_CHIP
select IRQ_DOMAIN
- select GENERIC_IRQ_EFFECTIVE_AFF_MASK
+ select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP
select LOONGSON_HTVEC
select LOONGSON_LIOINTC
select LOONGSON_EIOINTC
diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
index b64cbe3052e8..4f5e6d21d77d 100644
--- a/drivers/irqchip/irq-loongson-eiointc.c
+++ b/drivers/irqchip/irq-loongson-eiointc.c
@@ -59,6 +59,7 @@ static int cpu_to_eio_node(int cpu)
return cpu_logical_map(cpu) / CORES_PER_EIO_NODE;
}

+#ifdef CONFIG_SMP
static void eiointc_set_irq_route(int pos, unsigned int cpu, unsigned int mnode, nodemask_t *node_map)
{
int i, node, cpu_node, route_node;
@@ -126,6 +127,7 @@ static int eiointc_set_irq_affinity(struct irq_data *d, const struct cpumask *af

return IRQ_SET_MASK_OK;
}
+#endif

static int eiointc_index(int node)
{
@@ -238,7 +240,9 @@ static struct irq_chip eiointc_irq_chip = {
.irq_ack = eiointc_ack_irq,
.irq_mask = eiointc_mask_irq,
.irq_unmask = eiointc_unmask_irq,
+#ifdef CONFIG_SMP
.irq_set_affinity = eiointc_set_irq_affinity,
+#endif
};

static int eiointc_domain_alloc(struct irq_domain *domain, unsigned int virq,
--
2.42.0



2024-03-26 10:02:13

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: Give chance to build under !CONFIG_SMP

On Tue, Mar 26 2024 at 14:21, Tiezhu Yang wrote:
>
> This patch is based on 6.9-rc1. Please let me know if it is better to
> split it into two patches, the first one is for arch/loongarch and the
> second one is for drivers/irqchip.

Why are you asking? It's documented that patches should be split, no?

2024-03-26 11:33:15

by Tiezhu Yang

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: Give chance to build under !CONFIG_SMP

On 03/26/2024 06:02 PM, Thomas Gleixner wrote:
> On Tue, Mar 26 2024 at 14:21, Tiezhu Yang wrote:
>>
>> This patch is based on 6.9-rc1. Please let me know if it is better to
>> split it into two patches, the first one is for arch/loongarch and the
>> second one is for drivers/irqchip.
>
> Why are you asking? It's documented that patches should be split, no?

OK, thanks for your reply, I read the document [1],
but I am not sure whether it is a single logical change.
I will split it into two patches and send them ASAP.

[1]
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separate-your-changes

Thanks,
Tiezhu


2024-03-26 12:59:37

by maobibo

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: Give chance to build under !CONFIG_SMP



On 2024/3/26 下午2:21, Tiezhu Yang wrote:
> In the current code, SMP is selected in Kconfig for LoongArch, the users
> can not unset it, this is reasonable for a multiprocessor machine. But as
> the help info of config SMP said, if you have a system with only one CPU,
> say N. On a uniprocessor machine, the kernel will run faster if you say N
> here.
>
> The Loongson-2K0500 is a single-core CPU for applications like industrial
> control, printing terminals, and BMC (Baseboard Management Controller),
> there are many development boards, products and solutions on the market,
> so it is better and necessary to give a chance to build under !CONFIG_SMP
> for a uniprocessor machine.
>
> First of all, do not select SMP for config LOONGARCH in Kconfig to make it
> possible to unset CONFIG_SMP. Then, do some changes to fix the warnings and
> errors if CONFIG_SMP is not set.
>
> (1) Select GENERIC_IRQ_EFFECTIVE_AFF_MASK only if SMP for config
> IRQ_LOONGARCH_CPU to fix the warning when make menuconfig:
> WARNING: unmet direct dependencies detected for GENERIC_IRQ_EFFECTIVE_AFF_MASK
>
> (2) Define cpu_logical_map(cpu) as 0 under !CONFIG_SMP in asm/smp.h and
> include asm/smp.h in asm/acpi.h to fix the error:
> /arch/loongarch/include/asm/acpi.h: In function ‘get_acpi_id_for_cpu’:
> /arch/loongarch/include/asm/acpi.h:44:30: error: implicit declaration of function ‘cpu_logical_map’ [-Wimplicit-function-declaration]
>
> (3) Define get_ipi_irq() only if CONFIG_SMP is set to fix the warning:
> arch/loongarch/kernel/irq.c:90:19: warning: ‘get_ipi_irq’ defined but not used [-Wunused-function]
>
> (4) Define machine_shutdown() as empty under !CONFIG_SMP to fix the error:
> arch/loongarch/kernel/machine_kexec.c: In function ‘machine_shutdown’:
> arch/loongarch/kernel/machine_kexec.c:233:25: error: implicit declaration of function ‘cpu_device_up’; did you mean ‘put_device’? [-Wimplicit-function-declaration]
>
> (5) Make config SCHED_SMT depends on SMP to avoid many errors such as:
> kernel/sched/core.c: In function ‘sched_core_find’:
> kernel/sched/core.c:310:43: error: ‘struct rq’ has no member named ‘cpu’
>
> (6) Call per_cpu_offset() only under CONFIG_HAVE_SETUP_PER_CPU_AREA to
> fix the error:
> arch/loongarch/power/suspend.c: In function ‘loongarch_common_resume’:
> arch/loongarch/power/suspend.c:47:21: error: implicit declaration of function ‘per_cpu_offset’ [-Wimplicit-function-declaration]
>
> (7) Define and call eiointc_set_irq_affinity() under CONFIG_SMP to set
> the CPU affinity only on SMP machines.
>
> When running the UnixBench tests with "-c 1" single-streamed pass,
> the improvement in performance is about 9 percent with this patch.
>
> By the way, it is helpful to debug and analysis the kernel issue
> of multi-core system under !CONFIG_SMP.
>
> Signed-off-by: Tiezhu Yang <[email protected]>
> ---
>
> This patch is based on 6.9-rc1. Please let me know if it is better to
> split it into two patches, the first one is for arch/loongarch and the
> second one is for drivers/irqchip.
>
> arch/loongarch/Kconfig | 2 +-
> arch/loongarch/include/asm/acpi.h | 1 +
> arch/loongarch/include/asm/smp.h | 5 +++++
> arch/loongarch/kernel/irq.c | 2 ++
> arch/loongarch/kernel/machine_kexec.c | 2 +-
> arch/loongarch/power/suspend.c | 2 ++
> drivers/irqchip/Kconfig | 2 +-
> drivers/irqchip/irq-loongson-eiointc.c | 4 ++++
> 8 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index a5f300ec6f28..8d892de0b7a8 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -174,7 +174,6 @@ config LOONGARCH
> select PCI_QUIRKS
> select PERF_USE_VMALLOC
> select RTC_LIB
> - select SMP
> select SPARSE_IRQ
> select SYSCTL_ARCH_UNALIGN_ALLOW
> select SYSCTL_ARCH_UNALIGN_NO_WARN
> @@ -420,6 +419,7 @@ config EFI_STUB
>
> config SCHED_SMT
> bool "SMT scheduler support"
> + depends on SMP
> default y
> help
> Improves scheduler's performance when there are multiple
> diff --git a/arch/loongarch/include/asm/acpi.h b/arch/loongarch/include/asm/acpi.h
> index 49e29b29996f..313f66f7913a 100644
> --- a/arch/loongarch/include/asm/acpi.h
> +++ b/arch/loongarch/include/asm/acpi.h
> @@ -8,6 +8,7 @@
> #ifndef _ASM_LOONGARCH_ACPI_H
> #define _ASM_LOONGARCH_ACPI_H
>
> +#include <asm/smp.h>
> #include <asm/suspend.h>
>
> #ifdef CONFIG_ACPI
> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
> index f81e5f01d619..c14ddfcd4829 100644
> --- a/arch/loongarch/include/asm/smp.h
> +++ b/arch/loongarch/include/asm/smp.h
> @@ -6,6 +6,7 @@
> #ifndef __ASM_SMP_H
> #define __ASM_SMP_H
>
> +#ifdef CONFIG_SMP
> #include <linux/atomic.h>
> #include <linux/bitops.h>
> #include <linux/linkage.h>
> @@ -101,4 +102,8 @@ static inline void __cpu_die(unsigned int cpu)
> }
> #endif
>
> +#else /* !CONFIG_SMP */
> +#define cpu_logical_map(cpu) 0
It is unsafe here though physical cpuid of BSP is 0 always.
It is better to use __cpu_logical_map[cpu] or read_csr_cpuid()

> +#endif
> +
> #endif /* __ASM_SMP_H */
> diff --git a/arch/loongarch/kernel/irq.c b/arch/loongarch/kernel/irq.c
> index 883e5066ae44..e791fa275ec5 100644
> --- a/arch/loongarch/kernel/irq.c
> +++ b/arch/loongarch/kernel/irq.c
> @@ -87,6 +87,7 @@ static void __init init_vec_parent_group(void)
> acpi_table_parse(ACPI_SIG_MCFG, early_pci_mcfg_parse);
> }
>
> +#ifdef CONFIG_SMP
> static int __init get_ipi_irq(void)
> {
> struct irq_domain *d = irq_find_matching_fwnode(cpuintc_handle, DOMAIN_BUS_ANY);
> @@ -96,6 +97,7 @@ static int __init get_ipi_irq(void)
>
> return -EINVAL;
> }
> +#endif
>
> void __init init_IRQ(void)
> {
> diff --git a/arch/loongarch/kernel/machine_kexec.c b/arch/loongarch/kernel/machine_kexec.c
> index 2dcb9e003657..8ae641dc53bb 100644
> --- a/arch/loongarch/kernel/machine_kexec.c
> +++ b/arch/loongarch/kernel/machine_kexec.c
> @@ -225,6 +225,7 @@ void crash_smp_send_stop(void)
>
> void machine_shutdown(void)
> {
> +#ifdef CONFIG_SMP
> int cpu;
>
> /* All CPUs go to reboot_code_buffer */
> @@ -232,7 +233,6 @@ void machine_shutdown(void)
> if (!cpu_online(cpu))
> cpu_device_up(get_cpu_device(cpu));
>
> -#ifdef CONFIG_SMP
> smp_call_function(kexec_shutdown_secondary, NULL, 0);
> #endif
> }
> diff --git a/arch/loongarch/power/suspend.c b/arch/loongarch/power/suspend.c
> index 166d9e06a64b..e8ca77eb3288 100644
> --- a/arch/loongarch/power/suspend.c
> +++ b/arch/loongarch/power/suspend.c
> @@ -44,7 +44,9 @@ void loongarch_common_resume(void)
> {
> sync_counter();
> local_flush_tlb_all();
> +#ifdef CONFIG_HAVE_SETUP_PER_CPU_AREA
> csr_write64(per_cpu_offset(0), PERCPU_BASE_KS);
> +#endif
It is another issue, CONFIG_HAVE_SETUP_PER_CPU_AREA should not depend on
NUMA. PERCPU_BASE_KS should be set even for UP.

Regards
Bibo Mao

> csr_write64(eentry, LOONGARCH_CSR_EENTRY);
> csr_write64(eentry, LOONGARCH_CSR_MERRENTRY);
> csr_write64(tlbrentry, LOONGARCH_CSR_TLBRENTRY);
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 72c07a12f5e1..bfa1d77749f3 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -568,7 +568,7 @@ config IRQ_LOONGARCH_CPU
> bool
> select GENERIC_IRQ_CHIP
> select IRQ_DOMAIN
> - select GENERIC_IRQ_EFFECTIVE_AFF_MASK
> + select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP
> select LOONGSON_HTVEC
> select LOONGSON_LIOINTC
> select LOONGSON_EIOINTC
> diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
> index b64cbe3052e8..4f5e6d21d77d 100644
> --- a/drivers/irqchip/irq-loongson-eiointc.c
> +++ b/drivers/irqchip/irq-loongson-eiointc.c
> @@ -59,6 +59,7 @@ static int cpu_to_eio_node(int cpu)
> return cpu_logical_map(cpu) / CORES_PER_EIO_NODE;
> }
>
> +#ifdef CONFIG_SMP
> static void eiointc_set_irq_route(int pos, unsigned int cpu, unsigned int mnode, nodemask_t *node_map)
> {
> int i, node, cpu_node, route_node;
> @@ -126,6 +127,7 @@ static int eiointc_set_irq_affinity(struct irq_data *d, const struct cpumask *af
>
> return IRQ_SET_MASK_OK;
> }
> +#endif
>
> static int eiointc_index(int node)
> {
> @@ -238,7 +240,9 @@ static struct irq_chip eiointc_irq_chip = {
> .irq_ack = eiointc_ack_irq,
> .irq_mask = eiointc_mask_irq,
> .irq_unmask = eiointc_unmask_irq,
> +#ifdef CONFIG_SMP
> .irq_set_affinity = eiointc_set_irq_affinity,
> +#endif
> };
>
> static int eiointc_domain_alloc(struct irq_domain *domain, unsigned int virq,
>


2024-03-26 21:31:02

by Tiezhu Yang

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: Give chance to build under !CONFIG_SMP

Hi Bibo,

On 3/26/24 20:58, maobibo wrote:
>
>
> On 2024/3/26 下午2:21, Tiezhu Yang wrote:
>> In the current code, SMP is selected in Kconfig for LoongArch, the users
>> can not unset it, this is reasonable for a multiprocessor machine. But as
>> the help info of config SMP said, if you have a system with only one CPU,
>> say N. On a uniprocessor machine, the kernel will run faster if you say N
>> here.
>>
>> The Loongson-2K0500 is a single-core CPU for applications like industrial
>> control, printing terminals, and BMC (Baseboard Management Controller),
>> there are many development boards, products and solutions on the market,
>> so it is better and necessary to give a chance to build under !CONFIG_SMP
>> for a uniprocessor machine.

..

>> +++ b/arch/loongarch/include/asm/smp.h
>> @@ -6,6 +6,7 @@
>>   #ifndef __ASM_SMP_H
>>   #define __ASM_SMP_H
>> +#ifdef CONFIG_SMP
>>   #include <linux/atomic.h>
>>   #include <linux/bitops.h>
>>   #include <linux/linkage.h>
>> @@ -101,4 +102,8 @@ static inline void __cpu_die(unsigned int cpu)
>>   }
>>   #endif
>> +#else /* !CONFIG_SMP */
>> +#define cpu_logical_map(cpu)    0
> It is unsafe here though physical cpuid of BSP is 0 always.
> It is better to use __cpu_logical_map[cpu] or read_csr_cpuid()
>
>> +#endif

..

>> +++ b/arch/loongarch/power/suspend.c
>> @@ -44,7 +44,9 @@ void loongarch_common_resume(void)
>>   {
>>       sync_counter();
>>       local_flush_tlb_all();
>> +#ifdef CONFIG_HAVE_SETUP_PER_CPU_AREA
>>       csr_write64(per_cpu_offset(0), PERCPU_BASE_KS);
>> +#endif
> It is another issue, CONFIG_HAVE_SETUP_PER_CPU_AREA should not depend on
> NUMA. PERCPU_BASE_KS should be set even for UP.

Thanks for your review and suggestions.

Let me wait for some days, if no more comments, I will address in v3
since the v2 has been sent.

Thanks,
Tiezhu


2024-03-27 01:09:24

by maobibo

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: Give chance to build under !CONFIG_SMP



On 2024/3/27 上午5:30, Tiezhu Yang wrote:
> Hi Bibo,
>
> On 3/26/24 20:58, maobibo wrote:
>>
>>
>> On 2024/3/26 下午2:21, Tiezhu Yang wrote:
>>> In the current code, SMP is selected in Kconfig for LoongArch, the users
>>> can not unset it, this is reasonable for a multiprocessor machine.
>>> But as
>>> the help info of config SMP said, if you have a system with only one
>>> CPU,
>>> say N. On a uniprocessor machine, the kernel will run faster if you
>>> say N
>>> here.
>>>
>>> The Loongson-2K0500 is a single-core CPU for applications like
>>> industrial
>>> control, printing terminals, and BMC (Baseboard Management Controller),
>>> there are many development boards, products and solutions on the market,
>>> so it is better and necessary to give a chance to build under
>>> !CONFIG_SMP
>>> for a uniprocessor machine.
>
> ...
>
>>> +++ b/arch/loongarch/include/asm/smp.h
>>> @@ -6,6 +6,7 @@
>>>   #ifndef __ASM_SMP_H
>>>   #define __ASM_SMP_H
>>> +#ifdef CONFIG_SMP
>>>   #include <linux/atomic.h>
>>>   #include <linux/bitops.h>
>>>   #include <linux/linkage.h>
>>> @@ -101,4 +102,8 @@ static inline void __cpu_die(unsigned int cpu)
>>>   }
>>>   #endif
>>> +#else /* !CONFIG_SMP */
>>> +#define cpu_logical_map(cpu)    0
>> It is unsafe here though physical cpuid of BSP is 0 always.
>> It is better to use __cpu_logical_map[cpu] or read_csr_cpuid()
>>
>>> +#endif
If kernel run with AMP mode where each cpu has its separate kernel img,
physical cpuid of BSP will be not 0.
>
> ...
>
>>> +++ b/arch/loongarch/power/suspend.c
>>> @@ -44,7 +44,9 @@ void loongarch_common_resume(void)
>>>   {
>>>       sync_counter();
>>>       local_flush_tlb_all();
>>> +#ifdef CONFIG_HAVE_SETUP_PER_CPU_AREA
>>>       csr_write64(per_cpu_offset(0), PERCPU_BASE_KS);
>>> +#endif
>> It is another issue, CONFIG_HAVE_SETUP_PER_CPU_AREA should not depend
>> on NUMA. PERCPU_BASE_KS should be set even for UP.
>
> Thanks for your review and suggestions.
>
> Let me wait for some days, if no more comments, I will address in v3
> since the v2 has been sent.
It is fine to me. Do something is better than do nothing, only that we
should be careful and it will take much time.

Regards
Bibo Mao
>
> Thanks,
> Tiezhu
>
> _______________________________________________
> Loongson-kernel mailing list -- [email protected]
> To unsubscribe send an email to [email protected]