arch/x86/mm/srat: Skip NUMA_NO_NODE while parsing SLIT
tip-bot for Toshi Kani
merge botCommit-ID: a85eba8814631d0d48361c8b9a7ee0984e80c03c Gitweb: http://git.kernel.org/tip/a85eba8814631d0d48361c8b9a7ee0984e80c03c Author: Toshi Kani <[email protected]> AuthorDate: Tue, 21 Jan 2014 14:33:15 -0800 Committer: Ingo Molnar <[email protected]> CommitDate: Sat, 25 Jan 2014 09:13:35 +0100
arch/x86/mm/srat: Skip NUMA_NO_NODE while parsing SLIT
When ACPI SLIT table has an I/O locality (i.e. a locality unique to an I/O device), numa_set_distance() emits this warning message:
NUMA: Warning: node ids are out of bound, from=-1 to=-1 distance=10
acpi_numa_slit_init() calls numa_set_distance() with pxm_to_node(), which assumes that all localities have been parsed with SRAT previously. SRAT does not list I/O localities, where as SLIT lists all localities including I/Os. Hence, pxm_to_node() returns NUMA_NO_NODE (-1) for an I/O locality.
I/O localities are not supported and are ignored today, but emitting such warning message leads to unnecessary confusion.
Change acpi_numa_slit_init() to avoid callingnuma_set_distance() with NUMA_NO_NODE.
--- arch/x86/mm/srat.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
arch/x86/mm/srat.c +13 -3
@@ -42,15 +42,25 @@ static __init inline int srat_disabled(void)return acpi_numa <0 ; } -/* Callback for SLIT parsing */ +/* +* Callback for SLIT parsing. pxm_to_node() returns NUMA_NO_NODE for +* I/O localities since SRAT does not list them. I/O localities are +* not supported at this point. +*/ void __init acpi_numa_slit_init (struct acpi_table_slit *slit) {int i, j; -for (i =0 ; i < slit->locality_count; i++) -for (j =0 ; j < slit->locality_count; j++) +for (i =0 ; i < slit->locality_count; i++) { +if (pxm_to_node (i) ==NUMA_NO_NODE ) +continue ; +for (j =0 ; j < slit->locality_count; j++) { +if (pxm_to_node (j) ==NUMA_NO_NODE ) +continue ;numa_set_distance (pxm_to_node (i),pxm_to_node (j), slit->entry[slit->locality_count * i + j]); + } + } }/* Callback for Proximity Domain -> x2APIC mapping */
Quoted 60 lines · tip-bot for Toshi Kani
Commit-ID: a85eba8814631d0d48361c8b9a7ee0984e80c03c Gitweb: http://git.kernel.org/tip/a85eba8814631d0d48361c8b9a7ee0984e80c03c Author: Toshi Kani <[email protected]> AuthorDate: Tue, 21 Jan 2014 14:33:15 -0800 Committer: Ingo Molnar <[email protected]> CommitDate: Sat, 25 Jan 2014 09:13:35 +0100
arch/x86/mm/srat: SkipNUMA_NO_NODEwhile parsing SLITWhen ACPI SLIT table has an I/O locality (i.e. a locality unique to an I/O device),
numa_set_distance()emits this warning message:NUMA: Warning: node ids are out of bound, from=-1 to=-1 distance=10
acpi_numa_slit_init()callsnuma_set_distance()withpxm_to_node(), which assumes that all localities have been parsed with SRAT previously. SRAT does not list I/O localities, where as SLIT lists all localities including I/Os. Hence,pxm_to_node()returnsNUMA_NO_NODE(-1) for an I/O locality.I/O localities are not supported and are ignored today, but emitting such warning message leads to unnecessary confusion.
Change
acpi_numa_slit_init()to avoid callingnuma_set_distance()withNUMA_NO_NODE.Signed-off-by Toshi Kani <[email protected]>Acked-by David Rientjes <[email protected]>Signed-off-by Andrew Morton <[email protected]>Cc Yinghai Lu <[email protected]>Signed-off-by Ingo Molnar <[email protected]>--- arch/x86/mm/srat.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)arch/x86/mm/srat.c +8
-3@@ -42,15 +42,25 @@ static __init inline int srat_disabled(void)return acpi_numa <0 ; } -/* Callback for SLIT parsing */ +/* +* Callback for SLIT parsing. pxm_to_node() returns NUMA_NO_NODE for +* I/O localities since SRAT does not list them. I/O localities are +* not supported at this point. +*/ void __init acpi_numa_slit_init (struct acpi_table_slit *slit) {int i, j; -for (i =0 ; i < slit->locality_count; i++) -for (j =0 ; j < slit->locality_count; j++) +for (i =0 ; i < slit->locality_count; i++) { +if (pxm_to_node (i) ==NUMA_NO_NODE ) +continue ;
arch/x86/mm/srat.c static __init inline int srat_disabled(void) · line 66
Quoted 10 lines
arch/x86/mm/srat.c +5
-0+for (j =0 ; j < slit->locality_count; j++) { +if (pxm_to_node (j) ==NUMA_NO_NODE ) +continue ;numa_set_distance (pxm_to_node (i),pxm_to_node (j), slit->entry[slit->locality_count * i + j]); + } + } }/* Callback for Proximity Domain -> x2APIC mapping */
wonder if the patch that i sent one year ago is better.
https://lkml.org/lkml/2013/1/21/559
as it avoid calling extra calling of pxm_to_node(i).
From: Yinghai Lu <[email protected]>
Subject: [PATCH] x86, mm: Skip unknown PXM early in SLIT parsing
Some systems one bios could support 2 sockets and 4 sockets, and SLIT is the same, aka 4x4.
For 2 sockets configuration, SRAT will only have PXM0 and PXM2.
So we will get warning:
NUMA: Warning: node ids are out of bound, from=0 to=-1 distance=15
Need to skip PXM1 and PXM2 as there is no responding node, To avoid uncorrect warning.
--- arch/x86/mm/srat.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
arch/x86/mm/srat.c +14 -3
@@ -46,11 +46,22 @@ static __init inline int srat_disabled(vvoid __init acpi_numa_slit_init (struct acpi_table_slit *slit) {int i, j; +int from_node, to_node; -for (i =0 ; i < slit->locality_count; i++) -for (j =0 ; j < slit->locality_count; j++) -numa_set_distance (pxm_to_node (i),pxm_to_node (j), +for (i =0 ; i < slit->locality_count; i++) { + from_node =pxm_to_node (i); +if (from_node <0 ) +continue ;/* skip unknown PXM */ + +for (j =0 ; j < slit->locality_count; j++) { + to_node =pxm_to_node (j); +if (to_node <0 ) +continue ;/* skip unknown PXM */ + +numa_set_distance (from_node, to_node, slit->entry[slit->locality_count * i + j]); + } + } }/* Callback for Proximity Domain -> x2APIC mapping */
Quoted 71 lines · Yinghai Lu
Quoted 69 lines · tip-bot for Toshi Kani
Commit-ID: a85eba8814631d0d48361c8b9a7ee0984e80c03c Gitweb: http://git.kernel.org/tip/a85eba8814631d0d48361c8b9a7ee0984e80c03c Author: Toshi Kani <[email protected]> AuthorDate: Tue, 21 Jan 2014 14:33:15 -0800 Committer: Ingo Molnar <[email protected]> CommitDate: Sat, 25 Jan 2014 09:13:35 +0100
arch/x86/mm/srat: SkipNUMA_NO_NODEwhile parsing SLITWhen ACPI SLIT table has an I/O locality (i.e. a locality unique to an I/O device),
numa_set_distance()emits this warning message:NUMA: Warning: node ids are out of bound, from=-1 to=-1 distance=10
acpi_numa_slit_init()callsnuma_set_distance()withpxm_to_node(), which assumes that all localities have been parsed with SRAT previously. SRAT does not list I/O localities, where as SLIT lists all localities including I/Os. Hence,pxm_to_node()returnsNUMA_NO_NODE(-1) for an I/O locality.I/O localities are not supported and are ignored today, but emitting such warning message leads to unnecessary confusion.
Change
acpi_numa_slit_init()to avoid callingnuma_set_distance()withNUMA_NO_NODE.Signed-off-by Toshi Kani <[email protected]>Acked-by David Rientjes <[email protected]>Signed-off-by Andrew Morton <[email protected]>Cc Yinghai Lu <[email protected]>Signed-off-by Ingo Molnar <[email protected]>--- arch/x86/mm/srat.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)arch/x86/mm/srat.c +13
-3@@ -42,15 +42,25 @@ static __init inline int srat_disabled(void)return acpi_numa <0 ; } -/* Callback for SLIT parsing */ +/* +* Callback for SLIT parsing. pxm_to_node() returns NUMA_NO_NODE for +* I/O localities since SRAT does not list them. I/O localities are +* not supported at this point. +*/ void __init acpi_numa_slit_init (struct acpi_table_slit *slit) {int i, j; -for (i =0 ; i < slit->locality_count; i++) -for (j =0 ; j < slit->locality_count; j++) +for (i =0 ; i < slit->locality_count; i++) { +if (pxm_to_node (i) ==NUMA_NO_NODE ) +continue ; +for (j =0 ; j < slit->locality_count; j++) { +if (pxm_to_node (j) ==NUMA_NO_NODE ) +continue ;numa_set_distance (pxm_to_node (i),pxm_to_node (j), slit->entry[slit->locality_count * i + j]); + } + } }
Quoted 7 lines
arch/x86/mm/srat.c static __init inline int srat_disabled(v · line 67
Quoted 1 line
arch/x86/mm/srat.c +0
-0/* Callback for Proximity Domain -> x2APIC mapping */ wonder if the patch that i sent one year ago is better.
https://lkml.org/lkml/2013/1/21/559
as it avoid calling extra calling of
pxm_to_node(i).
If it's "better" in the sense of being faster (and is otherwise equivalent functionally - assuming the original patch is bug free) then please submit it as a delta patch. (I assume the one you sent here wasn't.)
Thanks,
Ingo
PATCH 1/1 x86, mm: Avoid extra pxm_to_node()
In slit init code, more pxm_to_node() calling are added.
We can cache return with from_node/to_node to avoid them.
--- arch/x86/mm/srat.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
arch/x86/mm/srat.c +8 -3
@@ -50,14 +50,19 @@ static __init inline int srat_disabled(vvoid __init acpi_numa_slit_init (struct acpi_table_slit *slit) {int i, j; +int from_node, to_node;for (i =0 ; i < slit->locality_count; i++) { -if (pxm_to_node (i) ==NUMA_NO_NODE ) + from_node =pxm_to_node (i); +if (from_node ==NUMA_NO_NODE )continue ; +for (j =0 ; j < slit->locality_count; j++) { -if (pxm_to_node (j) ==NUMA_NO_NODE ) + to_node =pxm_to_node (j); +if (to_node ==NUMA_NO_NODE )continue ; -numa_set_distance (pxm_to_node (i),pxm_to_node (j), + +numa_set_distance (from_node, to_node, slit->entry[slit->locality_count * i + j]); } }
Re: [PATCH] x86, mm: Avoid extra pxm_to_node()
Quoted 3 lines · Yinghai Lu
In slit init code, more
pxm_to_node()calling are added.We can cache return with from_node/to_node to avoid them.
-ENOPARSE. Please formulate the title and the changelog in an understandable form.
Thanks,
Ingo
Re: [PATCH] x86, mm: Avoid extra pxm_to_node()
In reply to Yinghai LuQuoted 17 lines · Yinghai Lu
Index:
linux-2.6/arch/x86/mm/srat.c
===================================================================arch/x86/mm/srat.c +4
-1@@ -50,14 +50,19 @@ static __init inline int srat_disabled(vvoid __init acpi_numa_slit_init (struct acpi_table_slit *slit) {int i, j; +int from_node, to_node;for (i =0 ; i < slit->locality_count; i++) { -if (pxm_to_node (i) ==NUMA_NO_NODE ) + from_node =pxm_to_node (i); +if (from_node ==NUMA_NO_NODE )continue ; +for (j =0 ; j < slit->locality_count; j++) {
arch/x86/mm/srat.c static __init inline int srat_disabled(v · line 68
Quoted 10 lines
arch/x86/mm/srat.c +4
-2-if (pxm_to_node (j) ==NUMA_NO_NODE ) + to_node =pxm_to_node (j); +if (to_node ==NUMA_NO_NODE )continue ; -numa_set_distance (pxm_to_node (i),pxm_to_node (j), + +numa_set_distance (from_node, to_node, slit->entry[slit->locality_count * i + j]); } }
Might as well make them "const" while you're at it by moving the definitions inside the iteration blocks.
PATCH v2 1/1 x86, mm: Avoid duplicated pxm_to_node() calling.
In reply to Ingo MolnarIn slit init code, more pxm_to_node() calling are added.
We can store from_node/to_node instead of keep calling pxm_to_node().
After this patch: pxm_to_node() is called n*(1+n) Before this patch: pxm_to_node() is called n*(1+n*3)
for 8 socket, it will be 72 instead of 200.
for 32 socket, it will be 1056 instead of 3104.
-v2: update title and change log according to Ingo.
move from_node/to_node in loop and change to const according to
David Rientjes.
--- arch/x86/mm/srat.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
arch/x86/mm/srat.c +9 -3
@@ -52,12 +52,18 @@ void __init acpi_numa_slit_init(struct aint i, j;for (i =0 ; i < slit->locality_count; i++) { -if (pxm_to_node (i) ==NUMA_NO_NODE ) +const int from_node =pxm_to_node (i); + +if (from_node ==NUMA_NO_NODE )continue ; +for (j =0 ; j < slit->locality_count; j++) { -if (pxm_to_node (j) ==NUMA_NO_NODE ) +const int to_node =pxm_to_node (j); + +if (to_node ==NUMA_NO_NODE )continue ; -numa_set_distance (pxm_to_node (i),pxm_to_node (j), + +numa_set_distance (from_node, to_node, slit->entry[slit->locality_count * i + j]); } }
Re: [PATCH v2] x86, mm: Avoid duplicated pxm_to_node() calling.
Quoted 7 lines · Yinghai Lu
In slit init code, more
pxm_to_node()calling are added.We can store from_node/to_node instead of keep calling
pxm_to_node().After this patch:
pxm_to_node()is called n*(1+n) Before this patch:pxm_to_node()is called n*(1+n*3)
Quoted 8 lines
for 8 socket, it will be 72 instead of 200.
for 32 socket, it will be 1056 instead of 3104.-v2: update title and change log according to Ingo.
move from_node/to_node in loop and change to const according to
David Rientjes.Signed-off-by Yinghai Lu <[email protected]>
Re: [PATCH v2] x86, mm: Avoid duplicated pxm_to_node() calling.
In reply to Yinghai LuQuoted 7 lines · Yinghai Lu
In slit init code, more
pxm_to_node()calling are added.We can store from_node/to_node instead of keep calling
pxm_to_node().After this patch:
pxm_to_node()is called n*(1+n) Before this patch:pxm_to_node()is called n*(1+n*3)
Quoted 8 lines
for 8 socket, it will be 72 instead of 200.
for 32 socket, it will be 1056 instead of 3104.-v2: update title and change log according to Ingo.
move from_node/to_node in loop and change to const according to
David Rientjes.Signed-off-by Yinghai Lu <[email protected]>
In my original patch, I added the following comment to the function to address David's comment (which he acked). So, can you add this comment?
-/* Callback for SLIT parsing */
+/*
+ * Callback for SLIT parsing. pxm_to_node() returns NUMA_NO_NODE for
+ * I/O localities since SRAT does not list them. I/O localities are
+ * not supported at this point.
+ */
Otherwise, the change looks good to me.
Thanks,
-Toshi
Re: [PATCH v2] x86, mm: Avoid duplicated pxm_to_node() calling.
Quoted 27 lines · Toshi Kani
Quoted 7 lines · Yinghai Lu
In slit init code, more
pxm_to_node()calling are added.We can store from_node/to_node instead of keep calling
pxm_to_node().After this patch:
pxm_to_node()is called n*(1+n) Before this patch:pxm_to_node()is called n*(1+n*3)Quoted 8 lines
for 8 socket, it will be 72 instead of 200.
for 32 socket, it will be 1056 instead of 3104.-v2: update title and change log according to Ingo.
move from_node/to_node in loop and change to const according to
David Rientjes.Signed-off-by Yinghai Lu <[email protected]>In my original patch, I added the following comment to the function to address David's comment (which he acked). So, can you add this comment?
(quoted diff) +5
-1-/* Callback for SLIT parsing */ +/* + * Callback for SLIT parsing. pxm_to_node() returns NUMA_NO_NODE for + * I/O localities since SRAT does not list them. I/O localities are + * not supported at this point. + */
Quoted 3 lines
Otherwise, the change looks good to me.
Acked-by Toshi Kani <[email protected]>
Hi, Toshi,
This patch is delta patch to your patch that is in tip already.
So you comments change is still there and not touched.
Thanks
Yinghai
Re: [PATCH v2] x86, mm: Avoid duplicated pxm_to_node() calling.
Quoted 30 lines · Yinghai Lu
Quoted 29 lines · Toshi Kani
Quoted 7 lines · Yinghai Lu
In slit init code, more
pxm_to_node()calling are added.We can store from_node/to_node instead of keep calling
pxm_to_node().After this patch:
pxm_to_node()is called n*(1+n) Before this patch:pxm_to_node()is called n*(1+n*3)Quoted 8 lines
for 8 socket, it will be 72 instead of 200.
for 32 socket, it will be 1056 instead of 3104.-v2: update title and change log according to Ingo.
move from_node/to_node in loop and change to const according to
David Rientjes.Signed-off-by Yinghai Lu <[email protected]>In my original patch, I added the following comment to the function to address David's comment (which he acked). So, can you add this comment?
(quoted diff) +5
-1-/* Callback for SLIT parsing */ +/* + * Callback for SLIT parsing. pxm_to_node() returns NUMA_NO_NODE for + * I/O localities since SRAT does not list them. I/O localities are + * not supported at this point. + */Otherwise, the change looks good to me.
Quoted 7 lines
Quoted 1 line
Acked-by Toshi Kani <[email protected]>Hi, Toshi,
This patch is delta patch to your patch that is in tip already.
So you comments change is still there and not touched.
Hi Yinghai,
Sounds great! Thanks for the clarification!
-Toshi
tip-bot for Yinghai Lu
merge bot[tip:x86/mm] x86/mm: Avoid duplicated pxm_to_node() calls
In reply to Yinghai Lux86/mm: Avoid duplicated pxm_to_node() calls
Commit-ID: ba6a328f7dfc95b20df5e0eb33c698187e997190 Gitweb: http://git.kernel.org/tip/ba6a328f7dfc95b20df5e0eb33c698187e997190 Author: Yinghai Lu <[email protected]> AuthorDate: Sun, 26 Jan 2014 13:01:42 -0800 Committer: Ingo Molnar <[email protected]> CommitDate: Sun, 9 Feb 2014 15:32:31 +0100
x86/mm: Avoid duplicated pxm_to_node() calls
In slit init code, too many pxm_to_node() function calls are done.
We can store from_node/to_node instead of keep calling pxm_to_node().
- Before this patch: pxm_to_node() is called n*(1+n*3) times.
- After this patch: pxm_to_node() is called n*(1+n) times.
for 8 sockets, it will be 72 instead of 200.
for 32 sockets, it will be 1056 instead of 3104.
--- arch/x86/mm/srat.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
arch/x86/mm/srat.c +9 -3
@@ -52,12 +52,18 @@ void __init acpi_numa_slit_init(struct acpi_table_slit *slit)int i, j;for (i =0 ; i < slit->locality_count; i++) { -if (pxm_to_node (i) ==NUMA_NO_NODE ) +const int from_node =pxm_to_node (i); + +if (from_node ==NUMA_NO_NODE )continue ; +for (j =0 ; j < slit->locality_count; j++) { -if (pxm_to_node (j) ==NUMA_NO_NODE ) +const int to_node =pxm_to_node (j); + +if (to_node ==NUMA_NO_NODE )continue ; -numa_set_distance (pxm_to_node (i),pxm_to_node (j), + +numa_set_distance (from_node, to_node, slit->entry[slit->locality_count * i + j]); } }
Quoted 71 lines · Yinghai Lu
Quoted 69 lines · tip-bot for Toshi Kani
Commit-ID: a85eba8814631d0d48361c8b9a7ee0984e80c03c Gitweb: http://git.kernel.org/tip/a85eba8814631d0d48361c8b9a7ee0984e80c03c Author: Toshi Kani <[email protected]> AuthorDate: Tue, 21 Jan 2014 14:33:15 -0800 Committer: Ingo Molnar <[email protected]> CommitDate: Sat, 25 Jan 2014 09:13:35 +0100
arch/x86/mm/srat: SkipNUMA_NO_NODEwhile parsing SLITWhen ACPI SLIT table has an I/O locality (i.e. a locality unique to an I/O device),
numa_set_distance()emits this warning message:NUMA: Warning: node ids are out of bound, from=-1 to=-1 distance=10
acpi_numa_slit_init()callsnuma_set_distance()withpxm_to_node(), which assumes that all localities have been parsed with SRAT previously. SRAT does not list I/O localities, where as SLIT lists all localities including I/Os. Hence,pxm_to_node()returnsNUMA_NO_NODE(-1) for an I/O locality.I/O localities are not supported and are ignored today, but emitting such warning message leads to unnecessary confusion.
Change
acpi_numa_slit_init()to avoid callingnuma_set_distance()withNUMA_NO_NODE.Signed-off-by Toshi Kani <[email protected]>Acked-by David Rientjes <[email protected]>Signed-off-by Andrew Morton <[email protected]>Cc Yinghai Lu <[email protected]>Signed-off-by Ingo Molnar <[email protected]>--- arch/x86/mm/srat.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)arch/x86/mm/srat.c +13
-3@@ -42,15 +42,25 @@ static __init inline int srat_disabled(void)return acpi_numa <0 ; } -/* Callback for SLIT parsing */ +/* +* Callback for SLIT parsing. pxm_to_node() returns NUMA_NO_NODE for +* I/O localities since SRAT does not list them. I/O localities are +* not supported at this point. +*/ void __init acpi_numa_slit_init (struct acpi_table_slit *slit) {int i, j; -for (i =0 ; i < slit->locality_count; i++) -for (j =0 ; j < slit->locality_count; j++) +for (i =0 ; i < slit->locality_count; i++) { +if (pxm_to_node (i) ==NUMA_NO_NODE ) +continue ; +for (j =0 ; j < slit->locality_count; j++) { +if (pxm_to_node (j) ==NUMA_NO_NODE ) +continue ;numa_set_distance (pxm_to_node (i),pxm_to_node (j), slit->entry[slit->locality_count * i + j]); + } + } }
Quoted 7 lines
arch/x86/mm/srat.c static __init inline int srat_disabled(v · line 67
Quoted 1 line
arch/x86/mm/srat.c +0
-0/* Callback for Proximity Domain -> x2APIC mapping */ wonder if the patch that i sent one year ago is better.
https://lkml.org/lkml/2013/1/21/559
as it avoid calling extra calling of
pxm_to_node(i).
Your version looks good to me, although I do not believe the difference is visible. I will reply to your patch with one comment and ack.
Thanks,
-Toshi