Re: [PATCH V9 2/2] powerpc/nodes: Ensure enough nodes avail for operations
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 infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH V9 2/2] powerpc/nodes: Ensure enough nodes avail for operations
On 08/21/2017 04:44 PM, Michael Bringmann wrote: > To: linuxppc-...@lists.ozlabs.org > > From: Michael Bringmann > > To: linux-kernel@vger.kernel.org > Cc: Michael Ellerman > Cc: Michael Bringmann > Cc: John Allen > Cc: Nathan Fontenot > 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 > --- > 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; >
Re: [PATCH V9 2/2] powerpc/nodes: Ensure enough nodes avail for operations
Michael Bringmann writes: > To: linuxppc-...@lists.ozlabs.org > > From: Michael Bringmann > > To: linux-kernel@vger.kernel.org > Cc: Michael Ellerman > Cc: Michael Bringmann > Cc: John Allen > Cc: Nathan Fontenot > 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
[PATCH V9 2/2] powerpc/nodes: Ensure enough nodes avail for operations
To: linuxppc-...@lists.ozlabs.org From: Michael Bringmann To: linux-kernel@vger.kernel.org Cc: Michael Ellerman Cc: Michael Bringmann Cc: John Allen Cc: Nathan Fontenot 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 --- 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;