* [PATCH 2/2] x86/mm/numa: remove the numa_nodemask_from_meminfo()
2017-02-06 15:35 [PATCH 1/2] x86/mm/numa: trivial fix on typo and error message Wei Yang
@ 2017-02-06 15:35 ` Wei Yang
2017-02-17 14:18 ` Wei Yang
2017-03-13 13:30 ` Borislav Petkov
2017-02-28 7:02 ` [PATCH 1/2] x86/mm/numa: trivial fix on typo and error message Wei Yang
` (2 subsequent siblings)
3 siblings, 2 replies; 9+ messages in thread
From: Wei Yang @ 2017-02-06 15:35 UTC (permalink / raw)
To: tglx, mingo, hpa, tj; +Cc: linux-kernel, Wei Yang
numa_nodemask_from_meminfo() is called to set bit according to
numa_meminfo. While the only two places for this call is used to set proper
bit to a copy of numa_nodes_parsed from numa_meminfo. With current code
path, those numa node information in numa_meminfo is a subset of
numa_nodes_parsed. So it is not necessary to set the bits again.
The following is a code path analysis to prove the numa node information in
numa_meminfo is a subset of numa_nodes_parsed.
x86_numa_init()
numa_init()
Case 1
acpi_numa_init()
acpi_parse_memory_affinity()
numa_add_memblk()
node_set(numa_nodes_parsed)
acpi_parse_slit()
numa_nodemask_from_meminfo()
Case 2
amd_numa_init()
numa_add_memblk()
node_set(numa_nodes_parsed)
Case 3
dummy_numa_init()
node_set(numa_nodes_parsed)
numa_add_memblk()
numa_register_memblks()
numa_nodemask_from_meminfo()
>From the code path analysis, we can see each time a memblk is added, the
proper bit is set in numa_nodes_parsed, which means it is not necessary to
set it again in numa_nodemask_from_meminfo() for a copy of
numa_nodes_parsed.
This patch removes numa_nodemask_from_meminfo().
Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
arch/x86/mm/numa.c | 21 +--------------------
1 file changed, 1 insertion(+), 20 deletions(-)
diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index 3e9110b34147..4c9070507a59 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -314,20 +314,6 @@ int __init numa_cleanup_meminfo(struct numa_meminfo *mi)
return 0;
}
-/*
- * Set nodes, which have memory in @mi, in *@nodemask.
- */
-static void __init numa_nodemask_from_meminfo(nodemask_t *nodemask,
- const struct numa_meminfo *mi)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(mi->blk); i++)
- if (mi->blk[i].start != mi->blk[i].end &&
- mi->blk[i].nid != NUMA_NO_NODE)
- node_set(mi->blk[i].nid, *nodemask);
-}
-
/**
* numa_reset_distance - Reset NUMA distance table
*
@@ -347,16 +333,12 @@ void __init numa_reset_distance(void)
static int __init numa_alloc_distance(void)
{
- nodemask_t nodes_parsed;
size_t size;
int i, j, cnt = 0;
u64 phys;
/* size the new table and allocate it */
- nodes_parsed = numa_nodes_parsed;
- numa_nodemask_from_meminfo(&nodes_parsed, &numa_meminfo);
-
- for_each_node_mask(i, nodes_parsed)
+ for_each_node_mask(i, numa_nodes_parsed)
cnt = i;
cnt++;
size = cnt * cnt * sizeof(numa_distance[0]);
@@ -535,7 +517,6 @@ static int __init numa_register_memblks(struct numa_meminfo *mi)
/* Account for nodes with cpus and no memory */
node_possible_map = numa_nodes_parsed;
- numa_nodemask_from_meminfo(&node_possible_map, mi);
if (WARN_ON(nodes_empty(node_possible_map)))
return -EINVAL;
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/2] x86/mm/numa: remove the numa_nodemask_from_meminfo()
2017-02-06 15:35 ` [PATCH 2/2] x86/mm/numa: remove the numa_nodemask_from_meminfo() Wei Yang
@ 2017-02-17 14:18 ` Wei Yang
2017-03-13 13:30 ` Borislav Petkov
1 sibling, 0 replies; 9+ messages in thread
From: Wei Yang @ 2017-02-17 14:18 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Tejun Heo
Cc: linux-kernel, Wei Yang
Hi,
Willing to hear from all of you :-)
On Mon, Feb 6, 2017 at 11:35 PM, Wei Yang <richard.weiyang@gmail.com> wrote:
> numa_nodemask_from_meminfo() is called to set bit according to
> numa_meminfo. While the only two places for this call is used to set proper
> bit to a copy of numa_nodes_parsed from numa_meminfo. With current code
> path, those numa node information in numa_meminfo is a subset of
> numa_nodes_parsed. So it is not necessary to set the bits again.
>
> The following is a code path analysis to prove the numa node information in
> numa_meminfo is a subset of numa_nodes_parsed.
>
> x86_numa_init()
> numa_init()
> Case 1
> acpi_numa_init()
> acpi_parse_memory_affinity()
> numa_add_memblk()
> node_set(numa_nodes_parsed)
> acpi_parse_slit()
> numa_nodemask_from_meminfo()
>
> Case 2
> amd_numa_init()
> numa_add_memblk()
> node_set(numa_nodes_parsed)
>
> Case 3
> dummy_numa_init()
> node_set(numa_nodes_parsed)
> numa_add_memblk()
>
> numa_register_memblks()
> numa_nodemask_from_meminfo()
>
> From the code path analysis, we can see each time a memblk is added, the
> proper bit is set in numa_nodes_parsed, which means it is not necessary to
> set it again in numa_nodemask_from_meminfo() for a copy of
> numa_nodes_parsed.
>
> This patch removes numa_nodemask_from_meminfo().
>
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> ---
> arch/x86/mm/numa.c | 21 +--------------------
> 1 file changed, 1 insertion(+), 20 deletions(-)
>
> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> index 3e9110b34147..4c9070507a59 100644
> --- a/arch/x86/mm/numa.c
> +++ b/arch/x86/mm/numa.c
> @@ -314,20 +314,6 @@ int __init numa_cleanup_meminfo(struct numa_meminfo *mi)
> return 0;
> }
>
> -/*
> - * Set nodes, which have memory in @mi, in *@nodemask.
> - */
> -static void __init numa_nodemask_from_meminfo(nodemask_t *nodemask,
> - const struct numa_meminfo *mi)
> -{
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(mi->blk); i++)
> - if (mi->blk[i].start != mi->blk[i].end &&
> - mi->blk[i].nid != NUMA_NO_NODE)
> - node_set(mi->blk[i].nid, *nodemask);
> -}
> -
> /**
> * numa_reset_distance - Reset NUMA distance table
> *
> @@ -347,16 +333,12 @@ void __init numa_reset_distance(void)
>
> static int __init numa_alloc_distance(void)
> {
> - nodemask_t nodes_parsed;
> size_t size;
> int i, j, cnt = 0;
> u64 phys;
>
> /* size the new table and allocate it */
> - nodes_parsed = numa_nodes_parsed;
> - numa_nodemask_from_meminfo(&nodes_parsed, &numa_meminfo);
> -
> - for_each_node_mask(i, nodes_parsed)
> + for_each_node_mask(i, numa_nodes_parsed)
> cnt = i;
> cnt++;
> size = cnt * cnt * sizeof(numa_distance[0]);
> @@ -535,7 +517,6 @@ static int __init numa_register_memblks(struct numa_meminfo *mi)
>
> /* Account for nodes with cpus and no memory */
> node_possible_map = numa_nodes_parsed;
> - numa_nodemask_from_meminfo(&node_possible_map, mi);
> if (WARN_ON(nodes_empty(node_possible_map)))
> return -EINVAL;
>
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 2/2] x86/mm/numa: remove the numa_nodemask_from_meminfo()
2017-02-06 15:35 ` [PATCH 2/2] x86/mm/numa: remove the numa_nodemask_from_meminfo() Wei Yang
2017-02-17 14:18 ` Wei Yang
@ 2017-03-13 13:30 ` Borislav Petkov
2017-03-14 2:50 ` Wei Yang
1 sibling, 1 reply; 9+ messages in thread
From: Borislav Petkov @ 2017-03-13 13:30 UTC (permalink / raw)
To: Wei Yang; +Cc: tglx, mingo, hpa, tj, linux-kernel
On Mon, Feb 06, 2017 at 11:35:29PM +0800, Wei Yang wrote:
> numa_nodemask_from_meminfo() is called to set bit according to
> numa_meminfo. While the only two places for this call is used to set proper
> bit to a copy of numa_nodes_parsed from numa_meminfo. With current code
> path, those numa node information in numa_meminfo is a subset of
> numa_nodes_parsed. So it is not necessary to set the bits again.
>
> The following is a code path analysis to prove the numa node information in
> numa_meminfo is a subset of numa_nodes_parsed.
>
> x86_numa_init()
> numa_init()
> Case 1
> acpi_numa_init()
> acpi_parse_memory_affinity()
> numa_add_memblk()
> node_set(numa_nodes_parsed)
> acpi_parse_slit()
> numa_nodemask_from_meminfo()
>
> Case 2
> amd_numa_init()
> numa_add_memblk()
> node_set(numa_nodes_parsed)
>
> Case 3
> dummy_numa_init()
> node_set(numa_nodes_parsed)
> numa_add_memblk()
>
> numa_register_memblks()
> numa_nodemask_from_meminfo()
What about of_numa_parse_memory_nodes() ?
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 2/2] x86/mm/numa: remove the numa_nodemask_from_meminfo()
2017-03-13 13:30 ` Borislav Petkov
@ 2017-03-14 2:50 ` Wei Yang
0 siblings, 0 replies; 9+ messages in thread
From: Wei Yang @ 2017-03-14 2:50 UTC (permalink / raw)
To: Borislav Petkov; +Cc: Wei Yang, tglx, mingo, hpa, tj, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1864 bytes --]
On Mon, Mar 13, 2017 at 02:30:46PM +0100, Borislav Petkov wrote:
>On Mon, Feb 06, 2017 at 11:35:29PM +0800, Wei Yang wrote:
>> numa_nodemask_from_meminfo() is called to set bit according to
>> numa_meminfo. While the only two places for this call is used to set proper
>> bit to a copy of numa_nodes_parsed from numa_meminfo. With current code
>> path, those numa node information in numa_meminfo is a subset of
>> numa_nodes_parsed. So it is not necessary to set the bits again.
>>
>> The following is a code path analysis to prove the numa node information in
>> numa_meminfo is a subset of numa_nodes_parsed.
>>
>> x86_numa_init()
>> numa_init()
>> Case 1
>> acpi_numa_init()
>> acpi_parse_memory_affinity()
>> numa_add_memblk()
>> node_set(numa_nodes_parsed)
>> acpi_parse_slit()
>> numa_nodemask_from_meminfo()
>>
>> Case 2
>> amd_numa_init()
>> numa_add_memblk()
>> node_set(numa_nodes_parsed)
>>
>> Case 3
>> dummy_numa_init()
>> node_set(numa_nodes_parsed)
>> numa_add_memblk()
>>
>> numa_register_memblks()
>> numa_nodemask_from_meminfo()
>
>What about of_numa_parse_memory_nodes() ?
>
Hi, Borislav
Glad to see your comment.
Here is the only place who will invoke of_numa_parse_memory_nodes()
arm64_numa_init()
of_numa_init()
of_numa_parse_memory_nodes()
So this is an arm64 specific function.
In this patch, these numa_nodemask_from_meminfo only affects x86.
>--
>Regards/Gruss,
> Boris.
>
>Good mailing practices for 400: avoid top-posting and trim the reply.
--
Wei Yang
Help you, Help me
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] x86/mm/numa: trivial fix on typo and error message
2017-02-06 15:35 [PATCH 1/2] x86/mm/numa: trivial fix on typo and error message Wei Yang
2017-02-06 15:35 ` [PATCH 2/2] x86/mm/numa: remove the numa_nodemask_from_meminfo() Wei Yang
@ 2017-02-28 7:02 ` Wei Yang
2017-03-09 2:52 ` Wei Yang
2017-03-13 13:00 ` Borislav Petkov
3 siblings, 0 replies; 9+ messages in thread
From: Wei Yang @ 2017-02-28 7:02 UTC (permalink / raw)
To: Wei Yang; +Cc: tglx, mingo, hpa, tj, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]
Hope someone like these two patches.
On Mon, Feb 06, 2017 at 11:35:28PM +0800, Wei Yang wrote:
>When allocating pg_data in alloc_node_data(), it will try to allocate from
>local node first and then from any node. If it fails at the second trial,
>it means there is not available memory on any node.
>
>This patch fixes the error message and correct one typo.
>
>Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>---
> arch/x86/mm/numa.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
>index 4366242356c5..3e9110b34147 100644
>--- a/arch/x86/mm/numa.c
>+++ b/arch/x86/mm/numa.c
>@@ -201,8 +201,8 @@ static void __init alloc_node_data(int nid)
> nd_pa = __memblock_alloc_base(nd_size, SMP_CACHE_BYTES,
> MEMBLOCK_ALLOC_ACCESSIBLE);
> if (!nd_pa) {
>- pr_err("Cannot find %zu bytes in node %d\n",
>- nd_size, nid);
>+ pr_err("Cannot find %zu bytes in any node\n",
>+ nd_size);
> return;
> }
> }
>@@ -225,7 +225,7 @@ static void __init alloc_node_data(int nid)
> * numa_cleanup_meminfo - Cleanup a numa_meminfo
> * @mi: numa_meminfo to clean up
> *
>- * Sanitize @mi by merging and removing unncessary memblks. Also check for
>+ * Sanitize @mi by merging and removing unnecessary memblks. Also check for
> * conflicts and clear unused memblks.
> *
> * RETURNS:
>--
>2.11.0
--
Wei Yang
Help you, Help me
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/2] x86/mm/numa: trivial fix on typo and error message
2017-02-06 15:35 [PATCH 1/2] x86/mm/numa: trivial fix on typo and error message Wei Yang
2017-02-06 15:35 ` [PATCH 2/2] x86/mm/numa: remove the numa_nodemask_from_meminfo() Wei Yang
2017-02-28 7:02 ` [PATCH 1/2] x86/mm/numa: trivial fix on typo and error message Wei Yang
@ 2017-03-09 2:52 ` Wei Yang
2017-03-13 13:00 ` Borislav Petkov
3 siblings, 0 replies; 9+ messages in thread
From: Wei Yang @ 2017-03-09 2:52 UTC (permalink / raw)
To: Wei Yang; +Cc: tglx, mingo, hpa, tj, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1503 bytes --]
Dear masters~
Would you like to share some comments on these two?
On Mon, Feb 06, 2017 at 11:35:28PM +0800, Wei Yang wrote:
>When allocating pg_data in alloc_node_data(), it will try to allocate from
>local node first and then from any node. If it fails at the second trial,
>it means there is not available memory on any node.
>
>This patch fixes the error message and correct one typo.
>
>Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>---
> arch/x86/mm/numa.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
>index 4366242356c5..3e9110b34147 100644
>--- a/arch/x86/mm/numa.c
>+++ b/arch/x86/mm/numa.c
>@@ -201,8 +201,8 @@ static void __init alloc_node_data(int nid)
> nd_pa = __memblock_alloc_base(nd_size, SMP_CACHE_BYTES,
> MEMBLOCK_ALLOC_ACCESSIBLE);
> if (!nd_pa) {
>- pr_err("Cannot find %zu bytes in node %d\n",
>- nd_size, nid);
>+ pr_err("Cannot find %zu bytes in any node\n",
>+ nd_size);
> return;
> }
> }
>@@ -225,7 +225,7 @@ static void __init alloc_node_data(int nid)
> * numa_cleanup_meminfo - Cleanup a numa_meminfo
> * @mi: numa_meminfo to clean up
> *
>- * Sanitize @mi by merging and removing unncessary memblks. Also check for
>+ * Sanitize @mi by merging and removing unnecessary memblks. Also check for
> * conflicts and clear unused memblks.
> *
> * RETURNS:
>--
>2.11.0
--
Wei Yang
Help you, Help me
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/2] x86/mm/numa: trivial fix on typo and error message
2017-02-06 15:35 [PATCH 1/2] x86/mm/numa: trivial fix on typo and error message Wei Yang
` (2 preceding siblings ...)
2017-03-09 2:52 ` Wei Yang
@ 2017-03-13 13:00 ` Borislav Petkov
2017-03-14 2:51 ` Wei Yang
3 siblings, 1 reply; 9+ messages in thread
From: Borislav Petkov @ 2017-03-13 13:00 UTC (permalink / raw)
To: Wei Yang; +Cc: tglx, mingo, hpa, tj, linux-kernel
On Mon, Feb 06, 2017 at 11:35:28PM +0800, Wei Yang wrote:
> When allocating pg_data in alloc_node_data(), it will try to allocate from
> local node first and then from any node. If it fails at the second trial,
> it means there is not available memory on any node.
>
> This patch fixes the error message and correct one typo.
>
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> ---
> arch/x86/mm/numa.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> index 4366242356c5..3e9110b34147 100644
> --- a/arch/x86/mm/numa.c
> +++ b/arch/x86/mm/numa.c
> @@ -201,8 +201,8 @@ static void __init alloc_node_data(int nid)
> nd_pa = __memblock_alloc_base(nd_size, SMP_CACHE_BYTES,
> MEMBLOCK_ALLOC_ACCESSIBLE);
> if (!nd_pa) {
> - pr_err("Cannot find %zu bytes in node %d\n",
> - nd_size, nid);
> + pr_err("Cannot find %zu bytes in any node\n",
> + nd_size);
> return;
> }
Actually, the best would be:
pr_err("Cannot find %zu bytes in any node (initial node: %d)\n", nd_size, nid);
or something like that. This way you say
a) the initial node %nid failed
b) the fallback to any node failed too
so that people trying to debug code can have the complete info in the
logs.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/2] x86/mm/numa: trivial fix on typo and error message
2017-03-13 13:00 ` Borislav Petkov
@ 2017-03-14 2:51 ` Wei Yang
0 siblings, 0 replies; 9+ messages in thread
From: Wei Yang @ 2017-03-14 2:51 UTC (permalink / raw)
To: Borislav Petkov; +Cc: Wei Yang, tglx, mingo, hpa, tj, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1689 bytes --]
On Mon, Mar 13, 2017 at 02:00:10PM +0100, Borislav Petkov wrote:
>On Mon, Feb 06, 2017 at 11:35:28PM +0800, Wei Yang wrote:
>> When allocating pg_data in alloc_node_data(), it will try to allocate from
>> local node first and then from any node. If it fails at the second trial,
>> it means there is not available memory on any node.
>>
>> This patch fixes the error message and correct one typo.
>>
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> ---
>> arch/x86/mm/numa.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
>> index 4366242356c5..3e9110b34147 100644
>> --- a/arch/x86/mm/numa.c
>> +++ b/arch/x86/mm/numa.c
>> @@ -201,8 +201,8 @@ static void __init alloc_node_data(int nid)
>> nd_pa = __memblock_alloc_base(nd_size, SMP_CACHE_BYTES,
>> MEMBLOCK_ALLOC_ACCESSIBLE);
>> if (!nd_pa) {
>> - pr_err("Cannot find %zu bytes in node %d\n",
>> - nd_size, nid);
>> + pr_err("Cannot find %zu bytes in any node\n",
>> + nd_size);
>> return;
>> }
>
>Actually, the best would be:
>
> pr_err("Cannot find %zu bytes in any node (initial node: %d)\n", nd_size, nid);
>
>or something like that. This way you say
>
>a) the initial node %nid failed
>b) the fallback to any node failed too
>
>so that people trying to debug code can have the complete info in the
>logs.
>
You are right, your message is more clear.
I would send out v2 with this change.
Thanks :-)
>--
>Regards/Gruss,
> Boris.
>
>Good mailing practices for 400: avoid top-posting and trim the reply.
--
Wei Yang
Help you, Help me
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread