x86: Remove wrong error message in x86_default_fixup_cpu_id
PATCH 1/1 x86: Remove wrong error message in x86_default_fixup_cpu_id
It's only called from amd.c:srat_detect_node(). The introduced condition for calling the fixup code is true for all AMD multi-node processors, e.g. Magny-Cours and Interlagos. There we have 2 NUMA nodes on one socket. And thus there are cores having different numa-node-id but with equal phys_proc_id. For example on such a system we now get
[ 0.228109] Booting Node 0, Processors #1 [ 0.232337] smpboot cpu 1: start_ip = 83000 [ 0.252088] #2 [ 0.253746] smpboot cpu 2: start_ip = 83000 [ 0.272086] #3 [ 0.276018] smpboot cpu 3: start_ip = 83000 [ 0.296088] #4 [ 0.297745] smpboot cpu 4: start_ip = 83000 [ 0.316088] #5 [ 0.320021] smpboot cpu 5: start_ip = 83000 [ 0.340113] Ok. [ 0.342324] Booting Node 1, Processors #6 [ 0.344344] smpboot cpu 6: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0 [ 0.372110] #7 [ 0.373771] smpboot cpu 7: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0 [ 0.396104] #8 [ 0.397764] smpboot cpu 8: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0 [ 0.420109] #9 [ 0.421773] smpboot cpu 9: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0 [ 0.444113] #10 [ 0.445865] smpboot cpu 10: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0 [ 0.468111] #11 [ 0.472030] smpboot cpu 11: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0
These NUMA core numbering error messages are plain wrong.
The confusing/misleading error message was introduced with commit 64be4c1c2428e148de6081af235e2418e6a66dda (x86: Add x86_init platform override to fix up NUMA core numbering) and should be removed.
--- arch/x86/kernel/cpu/common.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
BTW, I wonder why the fixup code isn't called from the Intel path. At least the mentioned patch suggests that something more generic was introduced here.
And I am curious why there is this specific condition to decide whether a call to x86_cpuinit.fixup_cpu_id is required.
Regards,
Andreas
arch/x86/kernel/cpu/common.c +0 -1
@@ -1163,7 +1163,6 @@ static void dbg_restore_debug_regs(void)*/ void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node) { -pr_err ("NUMA core number %d differs from configured core number %d\n" , node, c->phys_proc_id); }/*
Signature · 1 line
1.7.8.4
Quoted 46 lines · Andreas Herrmann
It's only called from amd.c:srat_detect_node(). The introduced condition for calling the fixup code is true for all AMD multi-node processors, e.g. Magny-Cours and Interlagos. There we have 2 NUMA nodes on one socket. And thus there are cores having different numa-node-id but with equal
phys_proc_id. For example on such a system we now get[ 0.228109] Booting Node 0, Processors #1 [ 0.232337] smpboot cpu 1: start_ip = 83000 [ 0.252088] #2 [ 0.253746] smpboot cpu 2: start_ip = 83000 [ 0.272086] #3 [ 0.276018] smpboot cpu 3: start_ip = 83000 [ 0.296088] #4 [ 0.297745] smpboot cpu 4: start_ip = 83000 [ 0.316088] #5 [ 0.320021] smpboot cpu 5: start_ip = 83000 [ 0.340113] Ok. [ 0.342324] Booting Node 1, Processors #6 [ 0.344344] smpboot cpu 6: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0 [ 0.372110] #7 [ 0.373771] smpboot cpu 7: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0 [ 0.396104] #8 [ 0.397764] smpboot cpu 8: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0 [ 0.420109] #9 [ 0.421773] smpboot cpu 9: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0 [ 0.444113] #10 [ 0.445865] smpboot cpu 10: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0 [ 0.468111] #11 [ 0.472030] smpboot cpu 11: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0These NUMA core numbering error messages are plain wrong.
The confusing/misleading error message was introduced with commit
64be4c1c2428e148de6081af235e2418e6a66dda(x86: Addx86_initplatform override to fix up NUMA core numbering) and should be removed.Reported-by Borislav Petkov <[email protected]>Signed-off-by Andreas Herrmann <[email protected]>
Quoted 7 lines
--- arch/x86/kernel/cpu/common.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)BTW, I wonder why the fixup code isn't called from the Intel path. At least the mentioned patch suggests that something more generic was introduced here.
Right, and I would remove the check in amd.c:srat_detect_node() instead of removing the printk statement in the default implementation.
IOW, we need more info on why the check was added only to the AMD path? Daniel?
Thanks.
Signature · 8 lines
Regards/Gruss, Boris. Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach GM: Alberto Bozzo Reg: Dornach, Landkreis Muenchen HRB Nr. 43632 WEEE Registernr: 129 19551
Quoted 51 lines · Borislav Petkov
On Mon, Feb 20, 2012 at 06:17:05PM +0100, Andreas Herrmann wrote:
It's only called from amd.c:srat_detect_node(). The introduced condition for calling the fixup code is true for all AMD multi-node processors, e.g. Magny-Cours and Interlagos. There we have 2 NUMA nodes on one socket. And thus there are cores having different numa-node-id but with equal
phys_proc_id. For example on such a system we now get[ 0.228109] Booting Node 0, Processors #1 [ 0.232337] smpboot cpu 1: start_ip = 83000 [ 0.252088] #2 [ 0.253746] smpboot cpu 2: start_ip = 83000 [ 0.272086] #3 [ 0.276018] smpboot cpu 3: start_ip = 83000 [ 0.296088] #4 [ 0.297745] smpboot cpu 4: start_ip = 83000 [ 0.316088] #5 [ 0.320021] smpboot cpu 5: start_ip = 83000 [ 0.340113] Ok. [ 0.342324] Booting Node 1, Processors #6 [ 0.344344] smpboot cpu 6: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0 [ 0.372110] #7 [ 0.373771] smpboot cpu 7: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0 [ 0.396104] #8 [ 0.397764] smpboot cpu 8: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0 [ 0.420109] #9 [ 0.421773] smpboot cpu 9: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0 [ 0.444113] #10 [ 0.445865] smpboot cpu 10: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0 [ 0.468111] #11 [ 0.472030] smpboot cpu 11: start_ip = 83000 [ 0.016000] NUMA core number 1 differs from configured core number 0These NUMA core numbering error messages are plain wrong.
The confusing/misleading error message was introduced with commit
64be4c1c2428e148de6081af235e2418e6a66dda(x86: Addx86_initplatform override to fix up NUMA core numbering) and should be removed.Reported-by Borislav Petkov<[email protected]>Signed-off-by Andreas Herrmann<[email protected]>--- arch/x86/kernel/cpu/common.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
Quoted 8 lines
BTW, I wonder why the fixup code isn't called from the Intel path. At least the mentioned patch suggests that something more generic was introduced here.
Right, and I would remove the check in amd.c:srat_detect_node() instead of removing the printk statement in the default implementation.IOW, we need more info on why the check was added only to the AMD path? Daniel?
The check and fixup wasn't needed in the Intel path thus far, so wasn't added.
We could specialise the 'if (c->phys_proc_id != node)' test to check for x86_cpuinit.fixup_cpu_id being NULL and drop the default override, if that is preferred?
Thanks,
Daniel
Signature · 2 lines
Daniel J Blueman Principal Software Engineer, Numascale Asia
Quoted 6 lines · Daniel J Blueman
The check and fixup wasn't needed in the Intel path thus far, so wasn't added.
We could specialise the 'if (
c->phys_proc_id!= node)' test to check for x86_cpuinit.fixup_cpu_id being NULL and drop the default override, if that is preferred?
Before that, why do you need that check in the AMD path at all? Please give a more detailed explanation as to why is it needed on the AMD path at all.
Thanks.
Signature · 8 lines
Regards/Gruss, Boris. Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach GM: Alberto Bozzo Reg: Dornach, Landkreis Muenchen HRB Nr. 43632 WEEE Registernr: 129 19551
Quoted 10 lines · Borislav Petkov
Quoted 6 lines · Daniel J Blueman
The check and fixup wasn't needed in the Intel path thus far, so wasn't added.
We could specialise the 'if (
c->phys_proc_id!= node)' test to check for x86_cpuinit.fixup_cpu_id being NULL and drop the default override, if that is preferred?Before that, why do you need that check in the AMD path at all? Please give a more detailed explanation as to why is it needed on the AMD path at all.
Since Numascale's NumaConnect bridges multiple separate HyperTransport fabrics across multiple servers together, the HT IDs written in the hardware thus don't match the information in the SRAT table constructed in the bootloader, thus we need to set this to the logical value [1, 'fixup_cpu_id'].
[1] https://lkml.org/lkml/2011/12/5/292
Signature · 2 lines
Daniel J Blueman Principal Software Engineer, Numascale Asia
Quoted 2 lines · Daniel J Blueman
Quoted 1 line · Borislav Petkov
On Mon, Feb 20, 2012 at 06:17:05PM +0100, Andreas Herrmann wrote:
[snip]
Quoted 9 lines
Quoted 8 lines
BTW, I wonder why the fixup code isn't called from the Intel path. At least the mentioned patch suggests that something more generic was introduced here.
Right, and I would remove the check in amd.c:srat_detect_node() instead of removing the printk statement in the default implementation.IOW, we need more info on why the check was added only to the AMD path? Daniel?
Quoted 6 lines
The check and fixup wasn't needed in the Intel path thus far, so wasn't added.
We could specialise the 'if (
c->phys_proc_id!= node)' test to check for x86_cpuinit.fixup_cpu_id being NULL and drop the default override, if that is preferred?
It seems that all the stuff in x86_init.[ch] is using default/noop functions instead of NULL pointer checks. So we shouldn't deviate from this for x86_cpuinit.fixup_cpu_id.
I think attached patch is more suitable to avoid the wrong warning message.
Please review.
Thanks,
Andreas
--
x86: Remove wrong error message in x86_default_fixup_cpu_id
It's only called from amd.c:srat_detect_node(). The introduced condition for calling the fixup code is true for all AMD multi-node processors, e.g. Magny-Cours and Interlagos. There we have 2 NUMA nodes on one socket. Thus there are cores having different numa-node-id but with equal phys_proc_id.
There is no point to print error messages in such a situation.
The confusing/misleading error message was introduced with commit 64be4c1c2428e148de6081af235e2418e6a66dda (x86: Add x86_init platform override to fix up NUMA core numbering).
Change the default fixup function (remove the error message), move the Numascale-specific condition for calling the fixup into the fixup-function itself and slightly adapt the comment.
--- arch/x86/include/asm/x86_init.h | 2 +- arch/x86/kernel/apic/apic_numachip.c | 7 +++++-- arch/x86/kernel/cpu/amd.c | 8 ++++---- arch/x86/kernel/cpu/common.c | 9 --------- arch/x86/kernel/x86_init.c | 1 + 5 files changed, 11 insertions(+), 16 deletions(-)
arch/x86/include/asm/x86_init.h +1 -1
@@ -189,6 +189,6 @@ extern struct x86_msi_ops x86_msi;extern void x86_init_noop (void );extern void x86_init_uint_noop (unsigned int unused); -extern void x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node); +extern void x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int n);#endif
arch/x86/kernel/apic/apic_numachip.c +5 -2
@@ -201,8 +201,11 @@ static void __init map_csrs(void)static void fixup_cpu_id (struct cpuinfo_x86 *c,int node) { - c->phys_proc_id = node; -per_cpu (cpu_llc_id,smp_processor_id ()) = node; + +if (c->phys_proc_id != node) { + c->phys_proc_id = node; +per_cpu (cpu_llc_id,smp_processor_id ()) = node; + } }static int __init numachip_system_init (void )
arch/x86/kernel/cpu/amd.c +4 -4
@@ -352,11 +352,11 @@ static void __cpuinit srat_detect_node(struct cpuinfo_x86 *c) node =per_cpu (cpu_llc_id, cpu);/* -* If core numbers are inconsistent, it's likely a multi-fabric platform, -* so invoke platform-specific handler +* On multi-fabric platform (e.g. Numascale NumaChip) a +* platform-specific handler needs to be called to fixup some +* IDs of the CPU. */ -if (c->phys_proc_id != node) - x86_cpuinit.fixup_cpu_id (c, node); + x86_cpuinit.fixup_cpu_id (c, node);if (!node_online (node)) {/*
arch/x86/kernel/cpu/common.c +0 -9
@@ -1158,15 +1158,6 @@ static void dbg_restore_debug_regs(void)#endif /* ! CONFIG_KGDB */ /* -* Prints an error where the NUMA and configured core-number mismatch and the -* platform didn't override this to fix it up -*/ -void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node) -{ -pr_err ("NUMA core number %d differs from configured core number %d\n" , node, c->phys_proc_id); -} - -/* * cpu_init() initializes state that is per-CPU. Some data is already * initialized (naturally) in the bootstrap process, such as the GDT * and IDT. We reload them nevertheless, this function acts as a
arch/x86/kernel/x86_init.c +1 -0
@@ -90,6 +90,7 @@ struct x86_init_ops x86_init __initdata = { }, }; +void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int n) { }struct x86_cpuinit_ops x86_cpuinit__cpuinitdata = { .setup_percpu_clockev = setup_secondary_APIC_clock, .fixup_cpu_id = x86_default_fixup_cpu_id,
Signature · 1 line
1.7.8.4
Quoted 19 lines · Andreas Herrmann
Quoted 2 lines · Daniel J Blueman
Quoted 1 line · Borislav Petkov
On Mon, Feb 20, 2012 at 06:17:05PM +0100, Andreas Herrmann wrote:
[snip]
Quoted 6 lines
Quoted 6 lines
BTW, I wonder why the fixup code isn't called from the Intel path. At least the mentioned patch suggests that something more generic was introduced here.
Right, and I would remove the check in amd.c:srat_detect_node() instead of removing the printk statement in the default implementation.Quoted 8 lines
Quoted 2 lines
IOW, we need more info on why the check was added only to the AMD path? Daniel?
The check and fixup wasn't needed in the Intel path thus far, so wasn't added.
We could specialise the 'if (
c->phys_proc_id!= node)' test to check for x86_cpuinit.fixup_cpu_id being NULL and drop the default override, if that is preferred?
Quoted 8 lines
It seems that all the stuff in x86_init.[ch] is using default/noop functions instead of NULL pointer checks. So we shouldn't deviate from this for x86_cpuinit.fixup_cpu_id.
I think attached patch is more suitable to avoid the wrong warning message.
Please review.
Yes, this looks reasonable and tests out successfully on systems with and without NumaConnect.
Thanks,
Daniel
Quoted 116 lines
Thanks,
Andreas
--
x86: Remove wrong error message inx86_default_fixup_cpu_idIt's only called from amd.c:srat_detect_node(). The introduced condition for calling the fixup code is true for all AMD multi-node processors, e.g. Magny-Cours and Interlagos. There we have 2 NUMA nodes on one socket. Thus there are cores having different numa-node-id but with equal
phys_proc_id.There is no point to print error messages in such a situation.
The confusing/misleading error message was introduced with commit
64be4c1c2428e148de6081af235e2418e6a66dda(x86: Addx86_initplatform override to fix up NUMA core numbering).Change the default fixup function (remove the error message), move the Numascale-specific condition for calling the fixup into the fixup-function itself and slightly adapt the comment.
Signed-off-by Andreas Herrmann<[email protected]>--- arch/x86/include/asm/x86_init.h | 2 +- arch/x86/kernel/apic/apic_numachip.c | 7 +++++-- arch/x86/kernel/cpu/amd.c | 8 ++++---- arch/x86/kernel/cpu/common.c | 9 --------- arch/x86/kernel/x86_init.c | 1 + 5 files changed, 11 insertions(+), 16 deletions(-)arch/x86/include/asm/x86_init.h +1
-1@@ -189,6 +189,6 @@ extern struct x86_msi_ops x86_msi;extern void x86_init_noop (void );extern void x86_init_uint_noop (unsigned int unused); -extern void x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node); +extern void x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int n);#endif arch/x86/kernel/apic/apic_numachip.c +5
-2@@ -201,8 +201,11 @@ static void __init map_csrs(void)static void fixup_cpu_id (struct cpuinfo_x86 *c,int node) { - c->phys_proc_id = node; -per_cpu (cpu_llc_id,smp_processor_id ()) = node; + +if (c->phys_proc_id != node) { + c->phys_proc_id = node; +per_cpu (cpu_llc_id,smp_processor_id ()) = node; + } }static int __init numachip_system_init (void )arch/x86/kernel/cpu/amd.c +4
-4@@ -352,11 +352,11 @@ static void __cpuinit srat_detect_node(struct cpuinfo_x86 *c) node =per_cpu (cpu_llc_id, cpu);/* -* If core numbers are inconsistent, it's likely a multi-fabric platform, -* so invoke platform-specific handler +* On multi-fabric platform (e.g. Numascale NumaChip) a +* platform-specific handler needs to be called to fixup some +* IDs of the CPU. */ -if (c->phys_proc_id != node) - x86_cpuinit.fixup_cpu_id (c, node); + x86_cpuinit.fixup_cpu_id (c, node);if (!node_online (node)) {/* arch/x86/kernel/cpu/common.c +0
-9@@ -1158,15 +1158,6 @@ static void dbg_restore_debug_regs(void)#endif /* ! CONFIG_KGDB */ /* -* Prints an error where the NUMA and configured core-number mismatch and the -* platform didn't override this to fix it up -*/ -void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node) -{ -pr_err ("NUMA core number %d differs from configured core number %d\n" , node, c->phys_proc_id); -} - -/* * cpu_init() initializes state that is per-CPU. Some data is already * initialized (naturally) in the bootstrap process, such as the GDT * and IDT. We reload them nevertheless, this function acts as a arch/x86/kernel/x86_init.c +1
-0@@ -90,6 +90,7 @@ struct x86_init_ops x86_init __initdata = { }, }; +void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int n) { }struct x86_cpuinit_ops x86_cpuinit__cpuinitdata = { .setup_percpu_clockev = setup_secondary_APIC_clock, .fixup_cpu_id = x86_default_fixup_cpu_id,
Signature · 2 lines
Daniel J Blueman Principal Software Engineer, Numascale Asia
PATCH RESEND 1/1 x86: Remove wrong error message in x86_default_fixup_cpu_id
It's only called from amd.c:srat_detect_node(). The introduced condition for calling the fixup code is true for all AMD multi-node processors, e.g. Magny-Cours and Interlagos. There we have 2 NUMA nodes on one socket. Thus there are cores having different numa-node-id but with equal phys_proc_id.
There is no point to print error messages in such a situation.
The confusing/misleading error message was introduced with commit 64be4c1c2428e148de6081af235e2418e6a66dda (x86: Add x86_init platform override to fix up NUMA core numbering).
Change the default fixup function (remove the error message), move the Numascale-specific condition for calling the fixup into the fixup-function itself and slightly adapt the comment.
--- arch/x86/include/asm/x86_init.h | 2 +- arch/x86/kernel/apic/apic_numachip.c | 7 +++++-- arch/x86/kernel/cpu/amd.c | 8 ++++---- arch/x86/kernel/cpu/common.c | 9 --------- arch/x86/kernel/x86_init.c | 1 + 5 files changed, 11 insertions(+), 16 deletions(-)
Please apply.
Thanks,
Andreas
arch/x86/include/asm/x86_init.h +1 -1
@@ -189,6 +189,6 @@ extern struct x86_msi_ops x86_msi;extern void x86_init_noop (void );extern void x86_init_uint_noop (unsigned int unused); -extern void x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node); +extern void x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int n);#endif
arch/x86/kernel/apic/apic_numachip.c +5 -2
@@ -201,8 +201,11 @@ static void __init map_csrs(void)static void fixup_cpu_id (struct cpuinfo_x86 *c,int node) { - c->phys_proc_id = node; -per_cpu (cpu_llc_id,smp_processor_id ()) = node; + +if (c->phys_proc_id != node) { + c->phys_proc_id = node; +per_cpu (cpu_llc_id,smp_processor_id ()) = node; + } }static int __init numachip_system_init (void )
arch/x86/kernel/cpu/amd.c +4 -4
@@ -352,11 +352,11 @@ static void __cpuinit srat_detect_node(struct cpuinfo_x86 *c) node =per_cpu (cpu_llc_id, cpu);/* -* If core numbers are inconsistent, it's likely a multi-fabric platform, -* so invoke platform-specific handler +* On multi-fabric platform (e.g. Numascale NumaChip) a +* platform-specific handler needs to be called to fixup some +* IDs of the CPU. */ -if (c->phys_proc_id != node) - x86_cpuinit.fixup_cpu_id (c, node); + x86_cpuinit.fixup_cpu_id (c, node);if (!node_online (node)) {/*
arch/x86/kernel/cpu/common.c +0 -9
@@ -1158,15 +1158,6 @@ static void dbg_restore_debug_regs(void)#endif /* ! CONFIG_KGDB */ /* -* Prints an error where the NUMA and configured core-number mismatch and the -* platform didn't override this to fix it up -*/ -void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node) -{ -pr_err ("NUMA core number %d differs from configured core number %d\n" , node, c->phys_proc_id); -} - -/* * cpu_init() initializes state that is per-CPU. Some data is already * initialized (naturally) in the bootstrap process, such as the GDT * and IDT. We reload them nevertheless, this function acts as a
arch/x86/kernel/x86_init.c +1 -0
@@ -90,6 +90,7 @@ struct x86_init_ops x86_init __initdata = { }, }; +void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int n) { }struct x86_cpuinit_ops x86_cpuinit__cpuinitdata = { .setup_percpu_clockev = setup_secondary_APIC_clock, .fixup_cpu_id = x86_default_fixup_cpu_id,
Signature · 1 line
1.7.8.4
[tip:x86/platform] x86/platform: Remove incorrect error message in x86_default_fixup_cpu_id()
Commit-ID: d7dedbe3694bbf97b4e3409e1fa52d97a3b9f028 Gitweb: http://git.kernel.org/tip/d7dedbe3694bbf97b4e3409e1fa52d97a3b9f028 Author: Andreas Herrmann <[email protected]> AuthorDate: Fri, 24 Feb 2012 16:31:27 +0100 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 27 Feb 2012 10:35:47 +0100
x86/platform: Remove incorrect error message in x86_default_fixup_cpu_id()
It's only called from amd.c:srat_detect_node(). The introduced condition for calling the fixup code is true for all AMD multi-node processors, e.g. Magny-Cours and Interlagos. There we have 2 NUMA nodes on one socket. Thus there are cores having different numa-node-id but with equal phys_proc_id.
There is no point to print error messages in such a situation.
The confusing/misleading error message was introduced with commit 64be4c1c2428e148de6081af235e2418e6a66dda (x86: Add x86_init platform override to fix up NUMA core numbering).
Change the default fixup function (remove the error message), move the Numascale-specific condition for calling the fixup into the fixup-function itself and slightly adapt the comment.
--- arch/x86/include/asm/x86_init.h | 2 +- arch/x86/kernel/apic/apic_numachip.c | 7 +++++-- arch/x86/kernel/cpu/amd.c | 8 ++++---- arch/x86/kernel/cpu/common.c | 9 --------- arch/x86/kernel/x86_init.c | 1 + 5 files changed, 11 insertions(+), 16 deletions(-)
arch/x86/include/asm/x86_init.h +1 -1
@@ -189,6 +189,6 @@ extern struct x86_msi_ops x86_msi;extern void x86_init_noop (void );extern void x86_init_uint_noop (unsigned int unused); -extern void x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node); +extern void x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int n);#endif
arch/x86/kernel/apic/apic_numachip.c +5 -2
@@ -201,8 +201,11 @@ static void __init map_csrs(void)static void fixup_cpu_id (struct cpuinfo_x86 *c,int node) { - c->phys_proc_id = node; -per_cpu (cpu_llc_id,smp_processor_id ()) = node; + +if (c->phys_proc_id != node) { + c->phys_proc_id = node; +per_cpu (cpu_llc_id,smp_processor_id ()) = node; + } }static int __init numachip_system_init (void )
arch/x86/kernel/cpu/amd.c +4 -4
@@ -352,11 +352,11 @@ static void __cpuinit srat_detect_node(struct cpuinfo_x86 *c) node =per_cpu (cpu_llc_id, cpu);/* -* If core numbers are inconsistent, it's likely a multi-fabric platform, -* so invoke platform-specific handler +* On multi-fabric platform (e.g. Numascale NumaChip) a +* platform-specific handler needs to be called to fixup some +* IDs of the CPU. */ -if (c->phys_proc_id != node) - x86_cpuinit.fixup_cpu_id (c, node); + x86_cpuinit.fixup_cpu_id (c, node);if (!node_online (node)) {/*
arch/x86/kernel/cpu/common.c +0 -9
@@ -1158,15 +1158,6 @@ static void dbg_restore_debug_regs(void)#endif /* ! CONFIG_KGDB */ /* -* Prints an error where the NUMA and configured core-number mismatch and the -* platform didn't override this to fix it up -*/ -void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node) -{ -pr_err ("NUMA core number %d differs from configured core number %d\n" , node, c->phys_proc_id); -} - -/* * cpu_init() initializes state that is per-CPU. Some data is already * initialized (naturally) in the bootstrap process, such as the GDT * and IDT. We reload them nevertheless, this function acts as a
arch/x86/kernel/x86_init.c +1 -0
@@ -90,6 +90,7 @@ struct x86_init_ops x86_init __initdata = { }, }; +void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int n) { }struct x86_cpuinit_ops x86_cpuinit__cpuinitdata = { .setup_percpu_clockev = setup_secondary_APIC_clock, .fixup_cpu_id = x86_default_fixup_cpu_id,
Re: [tip:x86/platform] x86/platform: Remove incorrect error message in x86_default_fixup_cpu_id()
Quoted 25 lines · tip-bot for Andreas Herrmann
Commit-ID: d7dedbe3694bbf97b4e3409e1fa52d97a3b9f028 Gitweb: http://git.kernel.org/tip/d7dedbe3694bbf97b4e3409e1fa52d97a3b9f028 Author: Andreas Herrmann <[email protected]> AuthorDate: Fri, 24 Feb 2012 16:31:27 +0100 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 27 Feb 2012 10:35:47 +0100x86/platform: Remove incorrect error message in
x86_default_fixup_cpu_id()It's only called from amd.c:srat_detect_node(). The introduced condition for calling the fixup code is true for all AMD multi-node processors, e.g. Magny-Cours and Interlagos. There we have 2 NUMA nodes on one socket. Thus there are cores having different numa-node-id but with equal
phys_proc_id.There is no point to print error messages in such a situation.
The confusing/misleading error message was introduced with commit
64be4c1c2428e148de6081af235e2418e6a66dda(x86: Addx86_initplatform override to fix up NUMA core numbering).Change the default fixup function (remove the error message), move the Numascale-specific condition for calling the fixup into the fixup-function itself and slightly adapt the comment.
Quoted 7 lines
Signed-off-by Andreas Herrmann <[email protected]>Reviewed-by Daniel J Blueman <[email protected]>Cc Borislav Petkov <[email protected]>Cc Borislav Petkov <[email protected]>Cc Steffen Persvold <[email protected]>Signed-off-by Ingo Molnar <[email protected]>
Maybe this one should be queued for x86/urgent instead and should go to Linus soonish since it cures an issue which came in during the 3.3 merge window with 64be4c1c2428e148de6081af235e2418e6a66dda?
Thanks.
Signature · 8 lines
Regards/Gruss, Boris. Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach GM: Alberto Bozzo Reg: Dornach, Landkreis Muenchen HRB Nr. 43632 WEEE Registernr: 129 19551
[tip:x86/urgent] x86/platform: Remove incorrect error message in x86_default_fixup_cpu_id()
In reply to Andreas HerrmannCommit-ID: 20b25f03fbd17b4e32b786024a5f9c6306795411 Gitweb: http://git.kernel.org/tip/20b25f03fbd17b4e32b786024a5f9c6306795411 Author: Andreas Herrmann <[email protected]> AuthorDate: Fri, 24 Feb 2012 16:31:27 +0100 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 28 Feb 2012 17:30:00 +0100
x86/platform: Remove incorrect error message in x86_default_fixup_cpu_id()
It's only called from amd.c:srat_detect_node(). The introduced condition for calling the fixup code is true for all AMD multi-node processors, e.g. Magny-Cours and Interlagos. There we have 2 NUMA nodes on one socket. Thus there are cores having different numa-node-id but with equal phys_proc_id.
There is no point to print error messages in such a situation.
The confusing/misleading error message was introduced with commit 64be4c1c2428e148de6081af235e2418e6a66dda (x86: Add x86_init platform override to fix up NUMA core numbering).
Change the default fixup function (remove the error message), move the Numascale-specific condition for calling the fixup into the fixup-function itself and slightly adapt the comment.
--- arch/x86/include/asm/x86_init.h | 2 +- arch/x86/kernel/apic/apic_numachip.c | 7 +++++-- arch/x86/kernel/cpu/amd.c | 8 ++++---- arch/x86/kernel/cpu/common.c | 9 --------- arch/x86/kernel/x86_init.c | 1 + 5 files changed, 11 insertions(+), 16 deletions(-)
arch/x86/include/asm/x86_init.h +1 -1
@@ -189,6 +189,6 @@ extern struct x86_msi_ops x86_msi;extern void x86_init_noop (void );extern void x86_init_uint_noop (unsigned int unused); -extern void x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node); +extern void x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int n);#endif
arch/x86/kernel/apic/apic_numachip.c +5 -2
@@ -201,8 +201,11 @@ static void __init map_csrs(void)static void fixup_cpu_id (struct cpuinfo_x86 *c,int node) { - c->phys_proc_id = node; -per_cpu (cpu_llc_id,smp_processor_id ()) = node; + +if (c->phys_proc_id != node) { + c->phys_proc_id = node; +per_cpu (cpu_llc_id,smp_processor_id ()) = node; + } }static int __init numachip_system_init (void )
arch/x86/kernel/cpu/amd.c +4 -4
@@ -352,11 +352,11 @@ static void __cpuinit srat_detect_node(struct cpuinfo_x86 *c) node =per_cpu (cpu_llc_id, cpu);/* -* If core numbers are inconsistent, it's likely a multi-fabric platform, -* so invoke platform-specific handler +* On multi-fabric platform (e.g. Numascale NumaChip) a +* platform-specific handler needs to be called to fixup some +* IDs of the CPU. */ -if (c->phys_proc_id != node) - x86_cpuinit.fixup_cpu_id (c, node); + x86_cpuinit.fixup_cpu_id (c, node);if (!node_online (node)) {/*
arch/x86/kernel/cpu/common.c +0 -9
@@ -1163,15 +1163,6 @@ static void dbg_restore_debug_regs(void)#endif /* ! CONFIG_KGDB */ /* -* Prints an error where the NUMA and configured core-number mismatch and the -* platform didn't override this to fix it up -*/ -void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node) -{ -pr_err ("NUMA core number %d differs from configured core number %d\n" , node, c->phys_proc_id); -} - -/* * cpu_init() initializes state that is per-CPU. Some data is already * initialized (naturally) in the bootstrap process, such as the GDT * and IDT. We reload them nevertheless, this function acts as a
arch/x86/kernel/x86_init.c +1 -0
@@ -90,6 +90,7 @@ struct x86_init_ops x86_init __initdata = { }, }; +void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int n) { }struct x86_cpuinit_ops x86_cpuinit__cpuinitdata = { .setup_percpu_clockev = setup_secondary_APIC_clock, .fixup_cpu_id = x86_default_fixup_cpu_id,
Re: [tip:x86/urgent] x86/platform: Remove incorrect error message in x86_default_fixup_cpu_id()
arch/x86/kernel/x86_init.c struct x86_init_ops x86_init __initdata = { · line 94
Quoted 11 lines · tip-bot for Andreas Herrmann
+++
b/arch/x86/kernel/cpu/common.carch/x86/kernel/x86_init.c +1
-0@@ -90,6 +90,7 @@ struct x86_init_ops x86_init __initdata = { }, }; +void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int n) { }struct x86_cpuinit_ops x86_cpuinit__cpuinitdata = {
Hm, I missed this first time around:
- why is this function global? It's not used by anything else.
- why is it squeezed before a structure without any vertical separation?
- why does it have the body as { }, as if it were an inline function?
Really, this should either be a short static function, with a proper body, or we should accept a NULL pointer there and check for it before calling it - there's a single usage site right now.
The latter looks like the cleanest solution to me.
Thanks,
Ingo
Re: [tip:x86/urgent] x86/platform: Remove incorrect error message in x86_default_fixup_cpu_id()
Quoted 12 lines · Ingo Molnar
Quoted 9 lines · tip-bot for Andreas Herrmann
+++
b/arch/x86/kernel/cpu/common.carch/x86/kernel/x86_init.c +0
-0@@ -90,6 +90,7 @@ struct x86_init_ops x86_init __initdata = { }, };
Quoted 6 lines
arch/x86/kernel/x86_init.c struct x86_init_ops x86_init __initdata = { · line 94
Quoted 2 lines
arch/x86/kernel/x86_init.c +1
-0+void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int n) { }struct x86_cpuinit_ops x86_cpuinit__cpuinitdata = {Hm, I missed this first time around:
- why is this function global? It's not used by anything else.
No reason. Should be static.
Quoted 2 lines
- why is it squeezed before a structure without any vertical separation?
Should be corrected too.
Quoted 2 lines
- why does it have the body as { }, as if it were an inline function?
To make it fit into one line, similar to other default init functions, e.g. default_nmi_init().
Quoted 6 lines
Really, this should either be a short static function, with a proper body, or we should accept a NULL pointer there and check for it before calling it - there's a single usage site right now.
The latter looks like the cleanest solution to me.
Yes, the NULL pointer check is probably the better alternative. How about below patch version?
Regards,
Andreas
--
x86: Remove wrong error message in x86_default_fixup_cpu_id
It's only called from amd.c:srat_detect_node(). The introduced condition for calling the fixup code is true for all AMD multi-node processors, e.g. Magny-Cours and Interlagos. There we have 2 NUMA nodes on one socket. Thus there are cores having different numa-node-id but with equal phys_proc_id.
There is no point to print error messages in such a situation.
The confusing/misleading error message was introduced with commit 64be4c1c2428e148de6081af235e2418e6a66dda (x86: Add x86_init platform override to fix up NUMA core numbering).
Remove the default fixup function (especially the error message) and replace it by a NULL pointer check, move the Numascale-specific condition for calling the fixup into the fixup-function itself and slightly adapt the comment.
--- arch/x86/include/asm/x86_init.h | 1 - arch/x86/kernel/apic/apic_numachip.c | 7 +++++-- arch/x86/kernel/cpu/amd.c | 7 ++++--- arch/x86/kernel/cpu/common.c | 9 --------- arch/x86/kernel/x86_init.c | 1 - 5 files changed, 9 insertions(+), 16 deletions(-)
arch/x86/include/asm/x86_init.h +0 -1
@@ -189,6 +189,5 @@ extern struct x86_msi_ops x86_msi;extern void x86_init_noop (void );extern void x86_init_uint_noop (unsigned int unused); -extern void x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node);#endif
arch/x86/kernel/apic/apic_numachip.c +5 -2
@@ -201,8 +201,11 @@ static void __init map_csrs(void)static void fixup_cpu_id (struct cpuinfo_x86 *c,int node) { - c->phys_proc_id = node; -per_cpu (cpu_llc_id,smp_processor_id ()) = node; + +if (c->phys_proc_id != node) { + c->phys_proc_id = node; +per_cpu (cpu_llc_id,smp_processor_id ()) = node; + } }static int __init numachip_system_init (void )
arch/x86/kernel/cpu/amd.c +4 -3
@@ -353,10 +353,11 @@ static void __cpuinit srat_detect_node(struct cpuinfo_x86 *c) node =per_cpu (cpu_llc_id, cpu);/* -* If core numbers are inconsistent, it's likely a multi-fabric platform, -* so invoke platform-specific handler +* On multi-fabric platform (e.g. Numascale NumaChip) a +* platform-specific handler needs to be called to fixup some +* IDs of the CPU. */ -if (c->phys_proc_id != node) +if (x86_cpuinit.fixup_cpu_id) x86_cpuinit.fixup_cpu_id (c, node);if (!node_online (node)) {
arch/x86/kernel/cpu/common.c +0 -9
@@ -1162,15 +1162,6 @@ static void dbg_restore_debug_regs(void)#endif /* ! CONFIG_KGDB */ /* -* Prints an error where the NUMA and configured core-number mismatch and the -* platform didn't override this to fix it up -*/ -void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node) -{ -pr_err ("NUMA core number %d differs from configured core number %d\n" , node, c->phys_proc_id); -} - -/* * cpu_init() initializes state that is per-CPU. Some data is already * initialized (naturally) in the bootstrap process, such as the GDT * and IDT. We reload them nevertheless, this function acts as a
arch/x86/kernel/x86_init.c +0 -1
@@ -92,7 +92,6 @@ struct x86_init_ops x86_init __initdata = {struct x86_cpuinit_ops x86_cpuinit__cpuinitdata = { .setup_percpu_clockev = setup_secondary_APIC_clock, - .fixup_cpu_id = x86_default_fixup_cpu_id, };static void default_nmi_init (void ) { };
Signature · 1 line
1.7.8.4
PATCH RESEND 1/1 x86/platform: Remove incorrect error message in x86_default_fixup_cpu_id()
It's only called from amd.c:srat_detect_node(). The introduced condition for calling the fixup code is true for all AMD multi-node processors, e.g. Magny-Cours and Interlagos. There we have 2 NUMA nodes on one socket. Thus there are cores having different numa-node-id but with equal phys_proc_id.
There is no point to print error messages in such a situation.
The confusing/misleading error message was introduced with commit 64be4c1c2428e148de6081af235e2418e6a66dda (x86: Add x86_init platform override to fix up NUMA core numbering).
Remove the default fixup function (especially the error message) and replace it by a NULL pointer check, move the Numascale-specific condition for calling the fixup into the fixup-function itself and slightly adapt the comment.
--- arch/x86/include/asm/x86_init.h | 1 - arch/x86/kernel/apic/apic_numachip.c | 7 +++++-- arch/x86/kernel/cpu/amd.c | 7 ++++--- arch/x86/kernel/cpu/common.c | 9 --------- arch/x86/kernel/x86_init.c | 1 - 5 files changed, 9 insertions(+), 16 deletions(-)
This patch didn't make it into v3.3.
But the misleading error message introduced with numachip support was merged.
Please apply this patch.
Thanks,
Andreas
arch/x86/include/asm/x86_init.h +0 -1
@@ -195,6 +195,5 @@ extern struct x86_msi_ops x86_msi;extern void x86_init_noop (void );extern void x86_init_uint_noop (unsigned int unused); -extern void x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node);#endif
arch/x86/kernel/apic/apic_numachip.c +5 -2
@@ -207,8 +207,11 @@ static void __init map_csrs(void)static void fixup_cpu_id (struct cpuinfo_x86 *c,int node) { - c->phys_proc_id = node; -per_cpu (cpu_llc_id,smp_processor_id ()) = node; + +if (c->phys_proc_id != node) { + c->phys_proc_id = node; +per_cpu (cpu_llc_id,smp_processor_id ()) = node; + } }static int __init numachip_system_init (void )
arch/x86/kernel/cpu/amd.c +4 -3
@@ -353,10 +353,11 @@ static void __cpuinit srat_detect_node(struct cpuinfo_x86 *c) node =per_cpu (cpu_llc_id, cpu);/* -* If core numbers are inconsistent, it's likely a multi-fabric platform, -* so invoke platform-specific handler +* On multi-fabric platform (e.g. Numascale NumaChip) a +* platform-specific handler needs to be called to fixup some +* IDs of the CPU. */ -if (c->phys_proc_id != node) +if (x86_cpuinit.fixup_cpu_id) x86_cpuinit.fixup_cpu_id (c, node);if (!node_online (node)) {
arch/x86/kernel/cpu/common.c +0 -9
@@ -1163,15 +1163,6 @@ static void dbg_restore_debug_regs(void)#endif /* ! CONFIG_KGDB */ /* -* Prints an error where the NUMA and configured core-number mismatch and the -* platform didn't override this to fix it up -*/ -void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node) -{ -pr_err ("NUMA core number %d differs from configured core number %d\n" , node, c->phys_proc_id); -} - -/* * cpu_init() initializes state that is per-CPU. Some data is already * initialized (naturally) in the bootstrap process, such as the GDT * and IDT. We reload them nevertheless, this function acts as a
arch/x86/kernel/x86_init.c +0 -1
@@ -93,7 +93,6 @@ struct x86_init_ops x86_init __initdata = {struct x86_cpuinit_ops x86_cpuinit__cpuinitdata = { .early_percpu_clock_init = x86_init_noop, .setup_percpu_clockev = setup_secondary_APIC_clock, - .fixup_cpu_id = x86_default_fixup_cpu_id, };static void default_nmi_init (void ) { };
Signature · 1 line
1.7.8.5
Borislav Petkov
maintainerRe: [PATCH resend] x86/platform: Remove incorrect error message in x86_default_fixup_cpu_id()
Quoted 28 lines · Andreas Herrmann
It's only called from amd.c:srat_detect_node(). The introduced condition for calling the fixup code is true for all AMD multi-node processors, e.g. Magny-Cours and Interlagos. There we have 2 NUMA nodes on one socket. Thus there are cores having different numa-node-id but with equal
phys_proc_id.There is no point to print error messages in such a situation.
The confusing/misleading error message was introduced with commit
64be4c1c2428e148de6081af235e2418e6a66dda(x86: Addx86_initplatform override to fix up NUMA core numbering).Remove the default fixup function (especially the error message) and replace it by a NULL pointer check, move the Numascale-specific condition for calling the fixup into the fixup-function itself and slightly adapt the comment.
Cc <[email protected]>Signed-off-by Andreas Herrmann <[email protected]>--- arch/x86/include/asm/x86_init.h | 1 - arch/x86/kernel/apic/apic_numachip.c | 7 +++++-- arch/x86/kernel/cpu/amd.c | 7 ++++--- arch/x86/kernel/cpu/common.c | 9 --------- arch/x86/kernel/x86_init.c | 1 - 5 files changed, 9 insertions(+), 16 deletions(-)
Quoted 3 lines
This patch didn't make it into v3.3.
But the misleading error message introduced with numachip support was merged.
Yes, please apply this one, I still get the following on my box with 3.4-rc1+:
[ 0.382396] Booting Node 0, Processors #1 #2 #3 #4 #5 Ok. [ 0.454471] Booting Node 1, Processors #6 [ 0.469600] NUMA core number 1 differs from configured core number 0 [ 0.478361] #7 [ 0.490949] NUMA core number 1 differs from configured core number 0 [ 0.499695] #8 [ 0.512577] NUMA core number 1 differs from configured core number 0 [ 0.521331] #9 [ 0.533921] NUMA core number 1 differs from configured core number 0 [ 0.542679] #10 [ 0.555340] NUMA core number 1 differs from configured core number 0 [ 0.564088] #11 [ 0.576730] NUMA core number 1 differs from configured core number 0 [ 0.585497] Ok. [ 0.587698] Booting Node 3, Processors #12 [ 0.602916] NUMA core number 3 differs from configured core number 1 [ 0.709432] #13 [ 0.722106] NUMA core number 3 differs from configured core number 1 [ 0.730856] #14 [ 0.743823] NUMA core number 3 differs from configured core number 1 [ 0.752577] #15 [ 0.765253] NUMA core number 3 differs from configured core number 1 [ 0.774002] #16 [ 0.786684] NUMA core number 3 differs from configured core number 1 [ 0.795436] #17 [ 0.808113] NUMA core number 3 differs from configured core number 1 [ 0.816878] Ok. [ 0.819377] Booting Node 2, Processors #18 [ 0.834597] NUMA core number 2 differs from configured core number 1 [ 0.843348] #19 [ 0.856025] NUMA core number 2 differs from configured core number 1 [ 0.864777] #20 [ 0.877750] NUMA core number 2 differs from configured core number 1 [ 0.886505] #21 [ 0.899180] NUMA core number 2 differs from configured core number 1 [ 0.907939] #22 [ 0.920583] NUMA core number 2 differs from configured core number 1 [ 0.929335] #23 Ok. [ 0.942785] NUMA core number 2 differs from configured core number 1 [ 0.951262] Brought up 24 CPUs
Thanks.
Signature · 2 lines
Regards/Gruss, Boris.
Re: [PATCH resend] x86/platform: Remove incorrect error message in x86_default_fixup_cpu_id()
Quoted 36 lines · Borislav Petkov
Quoted 28 lines · Andreas Herrmann
It's only called from amd.c:srat_detect_node(). The introduced condition for calling the fixup code is true for all AMD multi-node processors, e.g. Magny-Cours and Interlagos. There we have 2 NUMA nodes on one socket. Thus there are cores having different numa-node-id but with equal
phys_proc_id.There is no point to print error messages in such a situation.
The confusing/misleading error message was introduced with commit
64be4c1c2428e148de6081af235e2418e6a66dda(x86: Addx86_initplatform override to fix up NUMA core numbering).Remove the default fixup function (especially the error message) and replace it by a NULL pointer check, move the Numascale-specific condition for calling the fixup into the fixup-function itself and slightly adapt the comment.
Cc <[email protected]>Signed-off-by Andreas Herrmann <[email protected]>--- arch/x86/include/asm/x86_init.h | 1 - arch/x86/kernel/apic/apic_numachip.c | 7 +++++-- arch/x86/kernel/cpu/amd.c | 7 ++++--- arch/x86/kernel/cpu/common.c | 9 --------- arch/x86/kernel/x86_init.c | 1 - 5 files changed, 9 insertions(+), 16 deletions(-)Quoted 3 lines
This patch didn't make it into v3.3.
But the misleading error message introduced with numachip support was merged.Yes, please apply this one, I still get the following on my box with 3.4-rc1+:
Quoted 13 lines
[ 0.382396] Booting Node 0, Processors #1 #2 #3 #4 #5 Ok. [ 0.454471] Booting Node 1, Processors #6 [ 0.469600] NUMA core number 1 differs from configured core number 0 [ 0.478361] #7 [ 0.490949] NUMA core number 1 differs from configured core number 0 [ 0.499695] #8 [ 0.512577] NUMA core number 1 differs from configured core number 0 [ 0.521331] #9 [ 0.533921] NUMA core number 1 differs from configured core number 0 [ 0.542679] #10 [ 0.555340] NUMA core number 1 differs from configured core number 0 [ 0.564088] #11 [ 0.576730] NUMA core number 1 differs from configured core number 0
Still happens on -rc3. Ingo, can we please merge the above fix before 3.4 is out?
Thanks.
Signature · 8 lines
Regards/Gruss, Boris. Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach GM: Alberto Bozzo Reg: Dornach, Landkreis Muenchen HRB Nr. 43632 WEEE Registernr: 129 19551
[tip:x86/urgent] x86/platform: Remove incorrect error message in x86_default_fixup_cpu_id()
In reply to Andreas HerrmannCommit-ID: 68894632afb2729a1d8785c877840953894c7283 Gitweb: http://git.kernel.org/tip/68894632afb2729a1d8785c877840953894c7283 Author: Andreas Herrmann <[email protected]> AuthorDate: Mon, 2 Apr 2012 18:06:48 +0200 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 16 Apr 2012 20:43:43 +0200
x86/platform: Remove incorrect error message in x86_default_fixup_cpu_id()
It's only called from amd.c:srat_detect_node(). The introduced condition for calling the fixup code is true for all AMD multi-node processors, e.g. Magny-Cours and Interlagos. There we have 2 NUMA nodes on one socket. Thus there are cores having different numa-node-id but with equal phys_proc_id.
There is no point to print error messages in such a situation.
The confusing/misleading error message was introduced with commit 64be4c1c2428e148de6081af235e2418e6a66dda ("x86: Add x86_init platform override to fix up NUMA core numbering").
Remove the default fixup function (especially the error message) and replace it by a NULL pointer check, move the Numascale-specific condition for calling the fixup into the fixup-function itself and slightly adapt the comment.
--- arch/x86/include/asm/x86_init.h | 1 - arch/x86/kernel/apic/apic_numachip.c | 7 +++++-- arch/x86/kernel/cpu/amd.c | 7 ++++--- arch/x86/kernel/cpu/common.c | 9 --------- arch/x86/kernel/x86_init.c | 1 - 5 files changed, 9 insertions(+), 16 deletions(-)
arch/x86/include/asm/x86_init.h +0 -1
@@ -195,6 +195,5 @@ extern struct x86_msi_ops x86_msi;extern void x86_init_noop (void );extern void x86_init_uint_noop (unsigned int unused); -extern void x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node);#endif
arch/x86/kernel/apic/apic_numachip.c +5 -2
@@ -207,8 +207,11 @@ static void __init map_csrs(void)static void fixup_cpu_id (struct cpuinfo_x86 *c,int node) { - c->phys_proc_id = node; -per_cpu (cpu_llc_id,smp_processor_id ()) = node; + +if (c->phys_proc_id != node) { + c->phys_proc_id = node; +per_cpu (cpu_llc_id,smp_processor_id ()) = node; + } }static int __init numachip_system_init (void )
arch/x86/kernel/cpu/amd.c +4 -3
@@ -353,10 +353,11 @@ static void __cpuinit srat_detect_node(struct cpuinfo_x86 *c) node =per_cpu (cpu_llc_id, cpu);/* -* If core numbers are inconsistent, it's likely a multi-fabric platform, -* so invoke platform-specific handler +* On multi-fabric platform (e.g. Numascale NumaChip) a +* platform-specific handler needs to be called to fixup some +* IDs of the CPU. */ -if (c->phys_proc_id != node) +if (x86_cpuinit.fixup_cpu_id) x86_cpuinit.fixup_cpu_id (c, node);if (!node_online (node)) {
arch/x86/kernel/cpu/common.c +0 -9
@@ -1163,15 +1163,6 @@ static void dbg_restore_debug_regs(void)#endif /* ! CONFIG_KGDB */ /* -* Prints an error where the NUMA and configured core-number mismatch and the -* platform didn't override this to fix it up -*/ -void __cpuinit x86_default_fixup_cpu_id (struct cpuinfo_x86 *c,int node) -{ -pr_err ("NUMA core number %d differs from configured core number %d\n" , node, c->phys_proc_id); -} - -/* * cpu_init() initializes state that is per-CPU. Some data is already * initialized (naturally) in the bootstrap process, such as the GDT * and IDT. We reload them nevertheless, this function acts as a
arch/x86/kernel/x86_init.c +0 -1
@@ -93,7 +93,6 @@ struct x86_init_ops x86_init __initdata = {struct x86_cpuinit_ops x86_cpuinit__cpuinitdata = { .early_percpu_clock_init = x86_init_noop, .setup_percpu_clockev = setup_secondary_APIC_clock, - .fixup_cpu_id = x86_default_fixup_cpu_id, };static void default_nmi_init (void ) { };