Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] arm64: numa: rightsize the distance array
@ 2020-07-08 11:38 Jonathan Cameron
  2020-07-24 11:01 ` Jonathan Cameron
  2020-07-24 11:45 ` Song Bao Hua (Barry Song)
  0 siblings, 2 replies; 3+ messages in thread
From: Jonathan Cameron @ 2020-07-08 11:38 UTC (permalink / raw)
  To: linux-acpi, linux-arm-kernel
  Cc: Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla, linuxarm, Tejun Heo,
	Barry Song, Dan Williams, Jonathan Cameron

Unfortunately we are currently calling numa_alloc_distance well before we call
setup_node_to_cpu_mask_map means that nr_node_ids is set to MAX_NUMNODES.
This wastes a bit of memory and is confusing to the reader.

Note we could just decide to hardcode it as MAX_NUMNODES but if so we should
do so explicitly.

Looking at what x86 does, they do a walk of nodes_parsed and locally
establish the maximum node count seen.  We can't actually do that where we
were previously calling it in numa_init because nodes_parsed isn't set up
either yet.  So let us take a leaf entirely out of x86's book and make
the true assumption that nodes_parsed will definitely be set up before
we try to put a real value in this array.  Hence just do it on demand.

In order to avoid trying and failing to allocate the array multiple times
we do the same thing as x86 and set numa_distance = 1. This requires a
few small modifications elsewhere.

Worth noting, that with one exception (which it appears can be removed [1])
the x86 and arm numa distance code is now identical.  Worth factoring it
out to some common location?

[1] https://lkml.kernel.org/r/20170406124459.dwn5zhpr2xqg3lqm@node.shutemov.name

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
arch/arm64/mm/numa.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
index aafcee3e3f7e..a2f549ef0a36 100644
--- a/arch/arm64/mm/numa.c
+++ b/arch/arm64/mm/numa.c
@@ -255,13 +255,11 @@ void __init numa_free_distance(void)
 {
 	size_t size;
 
-	if (!numa_distance)
-		return;
-
 	size = numa_distance_cnt * numa_distance_cnt *
 		sizeof(numa_distance[0]);
-
-	memblock_free(__pa(numa_distance), size);
+	/* numa_distance could be 1LU marking allocation failure, test cnt */
+	if (numa_distance_cnt)
+		memblock_free(__pa(numa_distance), size);
 	numa_distance_cnt = 0;
 	numa_distance = NULL;
 }
@@ -271,20 +269,29 @@ void __init numa_free_distance(void)
  */
 static int __init numa_alloc_distance(void)
 {
+	nodemask_t nodes_parsed;
 	size_t size;
+	int i, j, cnt = 0;
 	u64 phys;
-	int i, j;
 
-	size = nr_node_ids * nr_node_ids * sizeof(numa_distance[0]);
+	/* size the new table and allocate it */
+	nodes_parsed = numa_nodes_parsed;
+	for_each_node_mask(i, nodes_parsed)
+		cnt = i;
+	cnt++;
+	size = cnt * cnt * sizeof(numa_distance[0]);
 	phys = memblock_find_in_range(0, PFN_PHYS(max_pfn),
 				      size, PAGE_SIZE);
-	if (WARN_ON(!phys))
+	if (!phys) {
+		pr_warn("Warning: can't allocate distance table!\n");
+		/* don't retry until explicitly reset */
+		numa_distance = (void *)1LU;
 		return -ENOMEM;
-
+	}
 	memblock_reserve(phys, size);
 
 	numa_distance = __va(phys);
-	numa_distance_cnt = nr_node_ids;
+	numa_distance_cnt = cnt;
 
 	/* fill with the default distances */
 	for (i = 0; i < numa_distance_cnt; i++)
@@ -311,10 +318,8 @@ static int __init numa_alloc_distance(void)
  */
 void __init numa_set_distance(int from, int to, int distance)
 {
-	if (!numa_distance) {
-		pr_warn_once("Warning: distance table not allocated yet\n");
+	if (!numa_distance && numa_alloc_distance() < 0)
 		return;
-	}
 
 	if (from >= numa_distance_cnt || to >= numa_distance_cnt ||
 			from < 0 || to < 0) {
@@ -384,10 +389,6 @@ static int __init numa_init(int (*init_func)(void))
 	nodes_clear(node_possible_map);
 	nodes_clear(node_online_map);
 
-	ret = numa_alloc_distance();
-	if (ret < 0)
-		return ret;
-
 	ret = init_func();
 	if (ret < 0)
 		goto out_free_distance;
-- 
2.19.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] arm64: numa: rightsize the distance array
  2020-07-08 11:38 [PATCH] arm64: numa: rightsize the distance array Jonathan Cameron
@ 2020-07-24 11:01 ` Jonathan Cameron
  2020-07-24 11:45 ` Song Bao Hua (Barry Song)
  1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2020-07-24 11:01 UTC (permalink / raw)
  To: linux-acpi, linux-arm-kernel
  Cc: Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla, linuxarm, Tejun Heo,
	Barry Song, Dan Williams

On Wed, 8 Jul 2020 19:38:25 +0800
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> Unfortunately we are currently calling numa_alloc_distance well before we call
> setup_node_to_cpu_mask_map means that nr_node_ids is set to MAX_NUMNODES.
> This wastes a bit of memory and is confusing to the reader.
> 
> Note we could just decide to hardcode it as MAX_NUMNODES but if so we should
> do so explicitly.
> 
> Looking at what x86 does, they do a walk of nodes_parsed and locally
> establish the maximum node count seen.  We can't actually do that where we
> were previously calling it in numa_init because nodes_parsed isn't set up
> either yet.  So let us take a leaf entirely out of x86's book and make
> the true assumption that nodes_parsed will definitely be set up before
> we try to put a real value in this array.  Hence just do it on demand.
> 
> In order to avoid trying and failing to allocate the array multiple times
> we do the same thing as x86 and set numa_distance = 1. This requires a
> few small modifications elsewhere.
> 
> Worth noting, that with one exception (which it appears can be removed [1])
> the x86 and arm numa distance code is now identical.  Worth factoring it
> out to some common location?
> 
> [1] https://lkml.kernel.org/r/20170406124459.dwn5zhpr2xqg3lqm@node.shutemov.name
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Polite nudge.  Anyone?  No particular urgency on this one but I'm thinking
of taking a stab at factoring out this code into a common location for arm64
and x86 and this change needs to proceed that.

Thanks,

Jonathan


> ---
> arch/arm64/mm/numa.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
> index aafcee3e3f7e..a2f549ef0a36 100644
> --- a/arch/arm64/mm/numa.c
> +++ b/arch/arm64/mm/numa.c
> @@ -255,13 +255,11 @@ void __init numa_free_distance(void)
>  {
>  	size_t size;
>  
> -	if (!numa_distance)
> -		return;
> -
>  	size = numa_distance_cnt * numa_distance_cnt *
>  		sizeof(numa_distance[0]);
> -
> -	memblock_free(__pa(numa_distance), size);
> +	/* numa_distance could be 1LU marking allocation failure, test cnt */
> +	if (numa_distance_cnt)
> +		memblock_free(__pa(numa_distance), size);
>  	numa_distance_cnt = 0;
>  	numa_distance = NULL;
>  }
> @@ -271,20 +269,29 @@ void __init numa_free_distance(void)
>   */
>  static int __init numa_alloc_distance(void)
>  {
> +	nodemask_t nodes_parsed;
>  	size_t size;
> +	int i, j, cnt = 0;
>  	u64 phys;
> -	int i, j;
>  
> -	size = nr_node_ids * nr_node_ids * sizeof(numa_distance[0]);
> +	/* size the new table and allocate it */
> +	nodes_parsed = numa_nodes_parsed;
> +	for_each_node_mask(i, nodes_parsed)
> +		cnt = i;
> +	cnt++;
> +	size = cnt * cnt * sizeof(numa_distance[0]);
>  	phys = memblock_find_in_range(0, PFN_PHYS(max_pfn),
>  				      size, PAGE_SIZE);
> -	if (WARN_ON(!phys))
> +	if (!phys) {
> +		pr_warn("Warning: can't allocate distance table!\n");
> +		/* don't retry until explicitly reset */
> +		numa_distance = (void *)1LU;
>  		return -ENOMEM;
> -
> +	}
>  	memblock_reserve(phys, size);
>  
>  	numa_distance = __va(phys);
> -	numa_distance_cnt = nr_node_ids;
> +	numa_distance_cnt = cnt;
>  
>  	/* fill with the default distances */
>  	for (i = 0; i < numa_distance_cnt; i++)
> @@ -311,10 +318,8 @@ static int __init numa_alloc_distance(void)
>   */
>  void __init numa_set_distance(int from, int to, int distance)
>  {
> -	if (!numa_distance) {
> -		pr_warn_once("Warning: distance table not allocated yet\n");
> +	if (!numa_distance && numa_alloc_distance() < 0)
>  		return;
> -	}
>  
>  	if (from >= numa_distance_cnt || to >= numa_distance_cnt ||
>  			from < 0 || to < 0) {
> @@ -384,10 +389,6 @@ static int __init numa_init(int (*init_func)(void))
>  	nodes_clear(node_possible_map);
>  	nodes_clear(node_online_map);
>  
> -	ret = numa_alloc_distance();
> -	if (ret < 0)
> -		return ret;
> -
>  	ret = init_func();
>  	if (ret < 0)
>  		goto out_free_distance;



^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH] arm64: numa: rightsize the distance array
  2020-07-08 11:38 [PATCH] arm64: numa: rightsize the distance array Jonathan Cameron
  2020-07-24 11:01 ` Jonathan Cameron
@ 2020-07-24 11:45 ` Song Bao Hua (Barry Song)
  1 sibling, 0 replies; 3+ messages in thread
From: Song Bao Hua (Barry Song) @ 2020-07-24 11:45 UTC (permalink / raw)
  To: Jonathan Cameron, linux-acpi, linux-arm-kernel
  Cc: Lorenzo Pieralisi, Guohanjun (Hanjun Guo),
	Sudeep Holla, Linuxarm, Tejun Heo, Dan Williams



> -----Original Message-----
> From: Jonathan Cameron
> Sent: Wednesday, July 8, 2020 11:38 PM
> To: linux-acpi@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>; Guohanjun (Hanjun Guo)
> <guohanjun@huawei.com>; Sudeep Holla <sudeep.holla@arm.com>;
> Linuxarm <linuxarm@huawei.com>; Tejun Heo <tj@kernel.org>; Song Bao Hua
> (Barry Song) <song.bao.hua@hisilicon.com>; Dan Williams
> <dan.j.williams@intel.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>
> Subject: [PATCH] arm64: numa: rightsize the distance array
> 
> Unfortunately we are currently calling numa_alloc_distance well before we call
> setup_node_to_cpu_mask_map means that nr_node_ids is set to
> MAX_NUMNODES.
> This wastes a bit of memory and is confusing to the reader.
> 
> Note we could just decide to hardcode it as MAX_NUMNODES but if so we
> should
> do so explicitly.
> 
> Looking at what x86 does, they do a walk of nodes_parsed and locally
> establish the maximum node count seen.  We can't actually do that where we
> were previously calling it in numa_init because nodes_parsed isn't set up
> either yet.  So let us take a leaf entirely out of x86's book and make
> the true assumption that nodes_parsed will definitely be set up before
> we try to put a real value in this array.  Hence just do it on demand.
> 
> In order to avoid trying and failing to allocate the array multiple times
> we do the same thing as x86 and set numa_distance = 1. This requires a
> few small modifications elsewhere.
> 
> Worth noting, that with one exception (which it appears can be removed [1])
> the x86 and arm numa distance code is now identical.  Worth factoring it
> out to some common location?
> 
> [1]
> https://lkml.kernel.org/r/20170406124459.dwn5zhpr2xqg3lqm@node.shute
> mov.name
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Looks sensible to me.
Reviewed-by: Barry Song <song.bao.hua@hisilicon.com>


> ---
> arch/arm64/mm/numa.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
> index aafcee3e3f7e..a2f549ef0a36 100644
> --- a/arch/arm64/mm/numa.c
> +++ b/arch/arm64/mm/numa.c
> @@ -255,13 +255,11 @@ void __init numa_free_distance(void)
>  {
>  	size_t size;
> 
> -	if (!numa_distance)
> -		return;
> -
>  	size = numa_distance_cnt * numa_distance_cnt *
>  		sizeof(numa_distance[0]);
> -
> -	memblock_free(__pa(numa_distance), size);
> +	/* numa_distance could be 1LU marking allocation failure, test cnt */
> +	if (numa_distance_cnt)
> +		memblock_free(__pa(numa_distance), size);
>  	numa_distance_cnt = 0;
>  	numa_distance = NULL;
>  }
> @@ -271,20 +269,29 @@ void __init numa_free_distance(void)
>   */
>  static int __init numa_alloc_distance(void)
>  {
> +	nodemask_t nodes_parsed;
>  	size_t size;
> +	int i, j, cnt = 0;
>  	u64 phys;
> -	int i, j;
> 
> -	size = nr_node_ids * nr_node_ids * sizeof(numa_distance[0]);
> +	/* size the new table and allocate it */
> +	nodes_parsed = numa_nodes_parsed;
> +	for_each_node_mask(i, nodes_parsed)
> +		cnt = i;
> +	cnt++;
> +	size = cnt * cnt * sizeof(numa_distance[0]);
>  	phys = memblock_find_in_range(0, PFN_PHYS(max_pfn),
>  				      size, PAGE_SIZE);
> -	if (WARN_ON(!phys))
> +	if (!phys) {
> +		pr_warn("Warning: can't allocate distance table!\n");
> +		/* don't retry until explicitly reset */
> +		numa_distance = (void *)1LU;
>  		return -ENOMEM;
> -
> +	}
>  	memblock_reserve(phys, size);
> 
>  	numa_distance = __va(phys);
> -	numa_distance_cnt = nr_node_ids;
> +	numa_distance_cnt = cnt;
> 
>  	/* fill with the default distances */
>  	for (i = 0; i < numa_distance_cnt; i++)
> @@ -311,10 +318,8 @@ static int __init numa_alloc_distance(void)
>   */
>  void __init numa_set_distance(int from, int to, int distance)
>  {
> -	if (!numa_distance) {
> -		pr_warn_once("Warning: distance table not allocated yet\n");
> +	if (!numa_distance && numa_alloc_distance() < 0)
>  		return;
> -	}
> 
>  	if (from >= numa_distance_cnt || to >= numa_distance_cnt ||
>  			from < 0 || to < 0) {
> @@ -384,10 +389,6 @@ static int __init numa_init(int (*init_func)(void))
>  	nodes_clear(node_possible_map);
>  	nodes_clear(node_online_map);
> 
> -	ret = numa_alloc_distance();
> -	if (ret < 0)
> -		return ret;
> -
>  	ret = init_func();
>  	if (ret < 0)
>  		goto out_free_distance;
> --
> 2.19.1

Thanks
Barry


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-08 11:38 [PATCH] arm64: numa: rightsize the distance array Jonathan Cameron
2020-07-24 11:01 ` Jonathan Cameron
2020-07-24 11:45 ` Song Bao Hua (Barry Song)

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git