[PATCH V2] irqchip/loongson-eiointc: Use early_cpu_to_node() instead of cpu_to_node()

[PATCH V2] irqchip/loongson-eiointc: Use early_cpu_to_node() instead of cpu_to_node()

[Huacai Chen]

When we use "nr_cpus=n" to hard limit the CPU number, cpu_to_node() is
not usable because it can only applied on "possible" CPUs. On the other
hand, early_cpu_to_node() can be always used instead.

Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
V2: Fix build for !NUMA.

 drivers/irqchip/irq-loongson-eiointc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
index c7ddebf312ad..0b486fe6dc57 100644
--- a/drivers/irqchip/irq-loongson-eiointc.c
+++ b/drivers/irqchip/irq-loongson-eiointc.c
@@ -7,6 +7,7 @@
 
 #define pr_fmt(fmt) "eiointc: " fmt
 
+#include <asm/numa.h>
 #include <linux/cpuhotplug.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
@@ -339,7 +340,7 @@ static int __init pch_msi_parse_madt(union acpi_subtable_headers *header,
 	int node;
 
 	if (cpu_has_flatmode)
-		node = cpu_to_node(eiointc_priv[nr_pics - 1]->node * CORES_PER_EIO_NODE);
+		node = early_cpu_to_node(eiointc_priv[nr_pics - 1]->node * CORES_PER_EIO_NODE);
 	else
 		node = eiointc_priv[nr_pics - 1]->node;
 
@@ -431,7 +432,7 @@ int __init eiointc_acpi_init(struct irq_domain *parent,
 		goto out_free_handle;
 
 	if (cpu_has_flatmode)
-		node = cpu_to_node(acpi_eiointc->node * CORES_PER_EIO_NODE);
+		node = early_cpu_to_node(acpi_eiointc->node * CORES_PER_EIO_NODE);
 	else
 		node = acpi_eiointc->node;
 	acpi_set_vec_parent(node, priv->eiointc_domain, pch_group);
-- 
2.43.0

Re: [PATCH V2] irqchip/loongson-eiointc: Use early_cpu_to_node() instead of cpu_to_node()

[Thomas Gleixner]

On Thu, Jun 20 2024 at 10:06, Huacai Chen wrote:
> When we use "nr_cpus=n" to hard limit the CPU number, cpu_to_node() is
> not usable because it can only applied on "possible" CPUs. On the other
> hand, early_cpu_to_node() can be always used instead.

We use nothing...

Aside of that you fail to explain what the problem is (i.e. the
malfunction) and why using early_cpu_to_node() is fixing it.

>  #define pr_fmt(fmt) "eiointc: " fmt
>  
> +#include <asm/numa.h>

asm includes come after the linux includes.

>  #include <linux/cpuhotplug.h>

Thanks,

        tglx

Re: [PATCH V2] irqchip/loongson-eiointc: Use early_cpu_to_node() instead of cpu_to_node()

[Huacai Chen]

Hi, Thomas,

On Sat, Jun 22, 2024 at 4:42 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Thu, Jun 20 2024 at 10:06, Huacai Chen wrote:
> > When we use "nr_cpus=n" to hard limit the CPU number, cpu_to_node() is
> > not usable because it can only applied on "possible" CPUs. On the other
> > hand, early_cpu_to_node() can be always used instead.
cpu_to_node() depends on per-cpu area, and per-cpu area is only usable
for "possible" CPUs.

>
> We use nothing...
>
> Aside of that you fail to explain what the problem is (i.e. the
> malfunction) and why using early_cpu_to_node() is fixing it.
>
> >  #define pr_fmt(fmt) "eiointc: " fmt
> >
> > +#include <asm/numa.h>
>
> asm includes come after the linux includes.
>
> >  #include <linux/cpuhotplug.h>
OK, will modify in the next version.

Huacai

>
> Thanks,
>
>         tglx

Re: [PATCH V2] irqchip/loongson-eiointc: Use early_cpu_to_node() instead of cpu_to_node()

[Thomas Gleixner]

Huacai!

On Sat, Jun 22 2024 at 10:49, Huacai Chen wrote:
> On Sat, Jun 22, 2024 at 4:42 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>> On Thu, Jun 20 2024 at 10:06, Huacai Chen wrote:
>> > When we use "nr_cpus=n" to hard limit the CPU number, cpu_to_node() is
>> > not usable because it can only applied on "possible" CPUs. On the other
>> > hand, early_cpu_to_node() can be always used instead.
> cpu_to_node() depends on per-cpu area, and per-cpu area is only usable
> for "possible" CPUs.

When nr_cpus=n is on the command line then what needs to access
something for CPUs which are not possible to come ever online?

That does not make sense because it's exactly the same situation when
you compile a kernel with NR_CPU=8 and boot it on a system with 16
CPUs. Then early_cpu_to_node() does not give you anything either.

So what's the technical problem you are trying to solve?

Thanks,

        tglx

Re: [PATCH V2] irqchip/loongson-eiointc: Use early_cpu_to_node() instead of cpu_to_node()

[Huacai Chen]

Hi, Thomas,

On Sat, Jun 22, 2024 at 6:17 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Huacai!
>
> On Sat, Jun 22 2024 at 10:49, Huacai Chen wrote:
> > On Sat, Jun 22, 2024 at 4:42 AM Thomas Gleixner <tglx@linutronix.de> wrote:
> >>
> >> On Thu, Jun 20 2024 at 10:06, Huacai Chen wrote:
> >> > When we use "nr_cpus=n" to hard limit the CPU number, cpu_to_node() is
> >> > not usable because it can only applied on "possible" CPUs. On the other
> >> > hand, early_cpu_to_node() can be always used instead.
> > cpu_to_node() depends on per-cpu area, and per-cpu area is only usable
> > for "possible" CPUs.
>
> When nr_cpus=n is on the command line then what needs to access
> something for CPUs which are not possible to come ever online?
>
> That does not make sense because it's exactly the same situation when
> you compile a kernel with NR_CPU=8 and boot it on a system with 16
> CPUs. Then early_cpu_to_node() does not give you anything either.
>
> So what's the technical problem you are trying to solve?
Frankly, there are some drawbacks on our hardware. On a dual-bridge
machine, there are two eiointc instances. Even if nr_cpus limits the
"possible" CPUs, we still hope the eiointc driver can initialize
correctly, otherwise the machine cannot boot.

Huacai

>
> Thanks,
>
>         tglx

Re: [PATCH V2] irqchip/loongson-eiointc: Use early_cpu_to_node() instead of cpu_to_node()

[Thomas Gleixner]

Huacai!

On Sat, Jun 22 2024 at 19:23, Huacai Chen wrote:
> On Sat, Jun 22, 2024 at 6:17 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>> That does not make sense because it's exactly the same situation when
>> you compile a kernel with NR_CPU=8 and boot it on a system with 16
>> CPUs. Then early_cpu_to_node() does not give you anything either.
>>
>> So what's the technical problem you are trying to solve?
>
> Frankly, there are some drawbacks on our hardware. On a dual-bridge
> machine, there are two eiointc instances. Even if nr_cpus limits the
> "possible" CPUs, we still hope the eiointc driver can initialize
> correctly, otherwise the machine cannot boot.

Hope does not help with technical limitations :)

So if CONFIG_NR_CPUS is not big enough to map the second node, then the
machine can't boot.

If nr_cpus limits it then early_cpu_to_node() because that mapping is
only limited by CONFIG_NR_CPUS.

Right?

If so, then please explain all of this properly in the change log.

Thanks,

        tglx

Re: [PATCH V2] irqchip/loongson-eiointc: Use early_cpu_to_node() instead of cpu_to_node()

[Huacai Chen]

Hi, Thomas,

On Sun, Jun 23, 2024 at 5:21 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Huacai!
>
> On Sat, Jun 22 2024 at 19:23, Huacai Chen wrote:
> > On Sat, Jun 22, 2024 at 6:17 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> >> That does not make sense because it's exactly the same situation when
> >> you compile a kernel with NR_CPU=8 and boot it on a system with 16
> >> CPUs. Then early_cpu_to_node() does not give you anything either.
> >>
> >> So what's the technical problem you are trying to solve?
> >
> > Frankly, there are some drawbacks on our hardware. On a dual-bridge
> > machine, there are two eiointc instances. Even if nr_cpus limits the
> > "possible" CPUs, we still hope the eiointc driver can initialize
> > correctly, otherwise the machine cannot boot.
>
> Hope does not help with technical limitations :)
>
> So if CONFIG_NR_CPUS is not big enough to map the second node, then the
> machine can't boot.
>
> If nr_cpus limits it then early_cpu_to_node() because that mapping is
> only limited by CONFIG_NR_CPUS.
Yes, if NR_CPUS is not big enough, then we have no solution.

>
> Right?
>
> If so, then please explain all of this properly in the change log.
OK, I will explain in detail.

Huacai

>
> Thanks,
>
>         tglx
>
>