All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V9 2/2] powerpc/nodes: Ensure enough nodes avail for operations
@ 2017-08-21 21:44 Michael Bringmann
  2017-08-23 11:20 ` Michael Ellerman
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Michael Bringmann @ 2017-08-21 21:44 UTC (permalink / raw)
  To: linuxppc-dev, linux-kernel
  Cc: Michael Ellerman, Michael Bringmann, John Allen, Nathan Fontenot,
	9e5050e1-e0cc-0e0e-7b31-5dcb38b307f4

To: linuxppc-dev@lists.ozlabs.org

From: Michael Bringmann <mwb@linux.vnet.ibm.com>

To: linux-kernel@vger.kernel.org
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michael Bringmann <mwb@linux.vnet.ibm.com>
Cc: John Allen <jallen@linux.vnet.ibm.com>
Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Subject: [PATCH V9 2/2] powerpc/nodes: Ensure enough nodes avail for operations

powerpc/nodes: On systems like PowerPC which allow 'hot-add' of CPU
or memory resources, it may occur that the new resources are to be
inserted into nodes that were not used for these resources at bootup.
In the kernel, any node that is used must be defined and initialized
at boot.

This patch extracts the value of the lowest domain level (number of
allocable resources) from the "rtas" device tree property
"ibm,max-associativity-domains" to use as the maximum number of nodes
to setup as possibly available in the system.  This new setting will
override the instruction,

    nodes_and(node_possible_map, node_possible_map, node_online_map);

presently seen in the function arch/powerpc/mm/numa.c:initmem_init().

If the property is not present at boot, no operation will be performed
to define or enable additional nodes.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
 arch/powerpc/mm/numa.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 3fd4536..3ae6510 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -893,6 +893,48 @@ static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn)
 	NODE_DATA(nid)->node_spanned_pages = spanned_pages;
 }
 
+static void __init node_associativity_setup(void)
+{
+	struct device_node *rtas;
+	printk(KERN_INFO "%s:%d\n", __FUNCTION__, __LINE__);
+
+	rtas = of_find_node_by_path("/rtas");
+	if (rtas) {
+		const __be32 *prop;
+		u32 len, entries, levelval, i;
+	printk(KERN_INFO "%s:%d\n", __FUNCTION__, __LINE__);
+
+		prop = of_get_property(rtas, "ibm,max-associativity-domains", &len);
+		if (!prop || len < sizeof(unsigned int)) {
+	printk(KERN_INFO "%s:%d\n", __FUNCTION__, __LINE__);
+			goto endit;
+		}
+
+		entries = of_read_number(prop++, 1);
+
+		if (len < (entries * sizeof(unsigned int))) {
+	printk(KERN_INFO "%s:%d\n", __FUNCTION__, __LINE__);
+			goto endit;
+		}
+
+		for (i = 0; i < entries; i++)
+			levelval = of_read_number(prop++, 1);
+
+		printk(KERN_INFO "Numa nodes avail: %d (%d) \n", (int) levelval, (int) entries);
+
+		for (i = 0; i < levelval; i++) {
+			if (!node_possible(i)) {
+				setup_node_data(i, 0, 0);
+				node_set(i, node_possible_map);
+			}
+		}
+	}
+
+endit:
+	if (rtas)
+		of_node_put(rtas);
+}
+
 void __init initmem_init(void)
 {
 	int nid, cpu;
@@ -912,6 +954,8 @@ void __init initmem_init(void)
 	 */
 	nodes_and(node_possible_map, node_possible_map, node_online_map);
 
+	node_associativity_setup();
+
 	for_each_online_node(nid) {
 		unsigned long start_pfn, end_pfn;
 

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

* Re: [PATCH V9 2/2] powerpc/nodes: Ensure enough nodes avail for operations
  2017-08-21 21:44 [PATCH V9 2/2] powerpc/nodes: Ensure enough nodes avail for operations Michael Bringmann
@ 2017-08-23 11:20 ` Michael Ellerman
  2017-08-23 14:42 ` Nathan Fontenot
  2017-08-23 16:04 ` kbuild test robot
  2 siblings, 0 replies; 4+ messages in thread
From: Michael Ellerman @ 2017-08-23 11:20 UTC (permalink / raw)
  To: Michael Bringmann, linuxppc-dev, linux-kernel
  Cc: Michael Bringmann, John Allen, Nathan Fontenot,
	9e5050e1-e0cc-0e0e-7b31-5dcb38b307f4

Michael Bringmann <mwb@linux.vnet.ibm.com> writes:

> To: linuxppc-dev@lists.ozlabs.org
>
> From: Michael Bringmann <mwb@linux.vnet.ibm.com>
>
> To: linux-kernel@vger.kernel.org
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Michael Bringmann <mwb@linux.vnet.ibm.com>
> Cc: John Allen <jallen@linux.vnet.ibm.com>
> Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> Subject: [PATCH V9 2/2] powerpc/nodes: Ensure enough nodes avail for operations
>
> powerpc/nodes: On systems like PowerPC which allow 'hot-add' of CPU
> or memory resources, it may occur that the new resources are to be
> inserted into nodes that were not used for these resources at bootup.
> In the kernel, any node that is used must be defined and initialized
> at boot.
>
> This patch extracts the value of the lowest domain level (number of
> allocable resources) from the "rtas" device tree property
> "ibm,max-associativity-domains" to use as the maximum number of nodes
> to setup as possibly available in the system.  This new setting will
> override the instruction,

Thanks for implementing my suggestion of using "ibm,max-associativity-domains".

However I don't think it's correct to use the lowest domain level.

It's not very clearly specified in PAPR, but I think you need to treat
it like an associativity property and index into based on the
associativity reference points.

You should be able to use min_common_depth as the index.

cheers

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

* Re: [PATCH V9 2/2] powerpc/nodes: Ensure enough nodes avail for operations
  2017-08-21 21:44 [PATCH V9 2/2] powerpc/nodes: Ensure enough nodes avail for operations Michael Bringmann
  2017-08-23 11:20 ` Michael Ellerman
@ 2017-08-23 14:42 ` Nathan Fontenot
  2017-08-23 16:04 ` kbuild test robot
  2 siblings, 0 replies; 4+ messages in thread
From: Nathan Fontenot @ 2017-08-23 14:42 UTC (permalink / raw)
  To: Michael Bringmann, linuxppc-dev, linux-kernel
  Cc: Michael Ellerman, John Allen, 9e5050e1-e0cc-0e0e-7b31-5dcb38b307f4

On 08/21/2017 04:44 PM, Michael Bringmann wrote:
> To: linuxppc-dev@lists.ozlabs.org
> 
> From: Michael Bringmann <mwb@linux.vnet.ibm.com>
> 
> To: linux-kernel@vger.kernel.org
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Michael Bringmann <mwb@linux.vnet.ibm.com>
> Cc: John Allen <jallen@linux.vnet.ibm.com>
> Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> Subject: [PATCH V9 2/2] powerpc/nodes: Ensure enough nodes avail for operations
> 
> powerpc/nodes: On systems like PowerPC which allow 'hot-add' of CPU
> or memory resources, it may occur that the new resources are to be
> inserted into nodes that were not used for these resources at bootup.
> In the kernel, any node that is used must be defined and initialized
> at boot.
> 
> This patch extracts the value of the lowest domain level (number of
> allocable resources) from the "rtas" device tree property
> "ibm,max-associativity-domains" to use as the maximum number of nodes
> to setup as possibly available in the system.  This new setting will
> override the instruction,
> 
>     nodes_and(node_possible_map, node_possible_map, node_online_map);
> 
> presently seen in the function arch/powerpc/mm/numa.c:initmem_init().
> 
> If the property is not present at boot, no operation will be performed
> to define or enable additional nodes.
> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
>  arch/powerpc/mm/numa.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 3fd4536..3ae6510 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -893,6 +893,48 @@ static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn)
>  	NODE_DATA(nid)->node_spanned_pages = spanned_pages;
>  }
> 
> +static void __init node_associativity_setup(void)
> +{
> +	struct device_node *rtas;
> +	printk(KERN_INFO "%s:%d\n", __FUNCTION__, __LINE__);

Is there a reson we need to have all these KERN_INFO printk's?

This looks like debug statements that accidentally were left in.

> +
> +	rtas = of_find_node_by_path("/rtas");
> +	if (rtas) {
> +		const __be32 *prop;
> +		u32 len, entries, levelval, i;
> +	printk(KERN_INFO "%s:%d\n", __FUNCTION__, __LINE__);
> +
> +		prop = of_get_property(rtas, "ibm,max-associativity-domains", &len);

You could put the of_node_put() call here after getting the property and get
rid of all the goto's.

> +		if (!prop || len < sizeof(unsigned int)) {
> +	printk(KERN_INFO "%s:%d\n", __FUNCTION__, __LINE__);
> +			goto endit;
> +		}
> +
> +		entries = of_read_number(prop++, 1);
> +
> +		if (len < (entries * sizeof(unsigned int))) {
> +	printk(KERN_INFO "%s:%d\n", __FUNCTION__, __LINE__);
> +			goto endit;
> +		}
> +
> +		for (i = 0; i < entries; i++)
> +			levelval = of_read_number(prop++, 1);

Couldn't you just read the last enbtry instead of doing a loop reading each
entry until you get to the last one?

-Nathan

> +
> +		printk(KERN_INFO "Numa nodes avail: %d (%d) \n", (int) levelval, (int) entries);
> +
> +		for (i = 0; i < levelval; i++) {
> +			if (!node_possible(i)) {
> +				setup_node_data(i, 0, 0);
> +				node_set(i, node_possible_map);
> +			}
> +		}
> +	}
> +
> +endit:
> +	if (rtas)
> +		of_node_put(rtas)> +}
> +
>  void __init initmem_init(void)
>  {
>  	int nid, cpu;
> @@ -912,6 +954,8 @@ void __init initmem_init(void)
>  	 */
>  	nodes_and(node_possible_map, node_possible_map, node_online_map);
> 
> +	node_associativity_setup();
> +
>  	for_each_online_node(nid) {
>  		unsigned long start_pfn, end_pfn;
> 

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

* Re: [PATCH V9 2/2] powerpc/nodes: Ensure enough nodes avail for operations
  2017-08-21 21:44 [PATCH V9 2/2] powerpc/nodes: Ensure enough nodes avail for operations Michael Bringmann
  2017-08-23 11:20 ` Michael Ellerman
  2017-08-23 14:42 ` Nathan Fontenot
@ 2017-08-23 16:04 ` kbuild test robot
  2 siblings, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2017-08-23 16:04 UTC (permalink / raw)
  To: Michael Bringmann
  Cc: kbuild-all, linuxppc-dev, linux-kernel, Michael Ellerman,
	Michael Bringmann, John Allen, Nathan Fontenot,
	9e5050e1-e0cc-0e0e-7b31-5dcb38b307f4

[-- Attachment #1: Type: text/plain, Size: 2977 bytes --]

Hi Michael,

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.13-rc6 next-20170823]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Michael-Bringmann/powerpc-numa-Update-CPU-topology-when-VPHN-enabled/20170823-173526
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All errors (new ones prefixed by >>):

   arch/powerpc/mm/numa.c: In function 'initmem_init':
>> arch/powerpc/mm/numa.c:923:3: error: 'levelval' may be used uninitialized in this function [-Werror=maybe-uninitialized]
      printk(KERN_INFO "Numa nodes avail: %d (%d) \n", (int) levelval, (int) entries);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/mm/numa.c:904:21: note: 'levelval' was declared here
      u32 len, entries, levelval, i;
                        ^~~~~~~~
   cc1: all warnings being treated as errors

vim +/levelval +923 arch/powerpc/mm/numa.c

   895	
   896	static void __init node_associativity_setup(void)
   897	{
   898		struct device_node *rtas;
   899		printk(KERN_INFO "%s:%d\n", __FUNCTION__, __LINE__);
   900	
   901		rtas = of_find_node_by_path("/rtas");
   902		if (rtas) {
   903			const __be32 *prop;
   904			u32 len, entries, levelval, i;
   905		printk(KERN_INFO "%s:%d\n", __FUNCTION__, __LINE__);
   906	
   907			prop = of_get_property(rtas, "ibm,max-associativity-domains", &len);
   908			if (!prop || len < sizeof(unsigned int)) {
   909		printk(KERN_INFO "%s:%d\n", __FUNCTION__, __LINE__);
   910				goto endit;
   911			}
   912	
   913			entries = of_read_number(prop++, 1);
   914	
   915			if (len < (entries * sizeof(unsigned int))) {
   916		printk(KERN_INFO "%s:%d\n", __FUNCTION__, __LINE__);
   917				goto endit;
   918			}
   919	
   920			for (i = 0; i < entries; i++)
   921				levelval = of_read_number(prop++, 1);
   922	
 > 923			printk(KERN_INFO "Numa nodes avail: %d (%d) \n", (int) levelval, (int) entries);
   924	
   925			for (i = 0; i < levelval; i++) {
   926				if (!node_possible(i)) {
   927					setup_node_data(i, 0, 0);
   928					node_set(i, node_possible_map);
   929				}
   930			}
   931		}
   932	
   933	endit:
   934		if (rtas)
   935			of_node_put(rtas);
   936	}
   937	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23572 bytes --]

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

end of thread, other threads:[~2017-08-23 15:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-21 21:44 [PATCH V9 2/2] powerpc/nodes: Ensure enough nodes avail for operations Michael Bringmann
2017-08-23 11:20 ` Michael Ellerman
2017-08-23 14:42 ` Nathan Fontenot
2017-08-23 16:04 ` kbuild test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.