On Tue, Jan 26, 2021 at 12:13:31PM +0100, Youling Tang wrote: > "__node_data" instead of "node_data" is used in the MIPS architecture, > so "__node_data" is used to replace "node_data" to improve the use of > next_online_pgdat() functions in the MIPS architecture. On my 32-bit MIPS dumps, MEMORY: works properly without this patch, but they have contig_page_data instead of __node_data/node_data so they

shouldn't be affected by this code. AFAICS only mach-loongson64 and mach-ip27 have __node_data.

> E.g. Without this patch: > ... > MEMORY: 0 > ... > > > With this patch: > ... > MEMORY: 7.5 GB > ... > > Signed-off-by: Huacai Chen <chenhuacai(a)loongson.cn&gt; > Signed-off-by: Youling Tang <tangyouling(a)loongson.cn&gt; > --- > memory.c | 28 ++++++++++++++++++---------- > 1 file changed, 18 insertions(+), 10 deletions(-) > > diff --git a/memory.c b/memory.c > index 33b0ca7..5347958 100644 > --- a/memory.c > +++ b/memory.c > @@ -17820,22 +17820,28 @@ next_online_pgdat(int node) > char buf[BUFSIZE]; > ulong pgdat; > > +#ifndef __mips__ > +#define NODE_DATA_VAR "node_data" > +#else > +#define NODE_DATA_VAR "__node_data" > +#endif Is this really correct? Doesn't __mips__ check the host architecture when what we want to check here is the target?

-----Original Message----- > Hi, Vincent > Thank you for your reply and suggestion. > > > On 02/10/2021 09:59 PM, Vincent Whitchurch wrote: > > > On Tue, Jan 26, 2021 at 12:13:31PM +0100, Youling Tang wrote: > > "__node_data" instead of "node_data" is used in the MIPS architecture, > so "__node_data" is used to replace "node_data" to improve the use of > next_online_pgdat() functions in the MIPS architecture. > > > On my 32-bit MIPS dumps, MEMORY: works properly without this patch, but > they have contig_page_data instead of __node_data/node_data so they > > When the node_data/__node_data and pgdat_list symbols cannot be successfully > read from the kernel symbols, the contig_page_data symbol will be read. In > your 32-bit machine, there should be no node_data/__node_data, pgdat_list > symbols, and a single node, so the memory size can be displayed normally > without this patch. > > > shouldn't be affected by this code. AFAICS only mach-loongson64 and > mach-ip27 have __node_data. > > > Yes, only mach-loongson64 and mach-ip27 use __node_data in the MIPS > architecture. > > > > E.g. Without this patch: > ... > MEMORY: 0 > ... > > > With this patch: > ... > MEMORY: 7.5 GB > ... > > Signed-off-by: Huacai Chen <chenhuacai(a)loongson.cn&gt; <mailto:chenhuacai@loongson.cn> > Signed-off-by: Youling Tang <tangyouling(a)loongson.cn&gt; <mailto:tangyouling@loongson.cn> > --- > memory.c | 28 ++++++++++++++++++---------- > 1 file changed, 18 insertions(+), 10 deletions(-) > > diff --git a/memory.c b/memory.c > index 33b0ca7..5347958 100644 > --- a/memory.c > +++ b/memory.c > @@ -17820,22 +17820,28 @@ next_online_pgdat(int node) > char buf[BUFSIZE]; > ulong pgdat; > > +#ifndef __mips__ > +#define NODE_DATA_VAR "node_data" > +#else > +#define NODE_DATA_VAR "__node_data" > +#endif > > > Is this really correct? Doesn't __mips__ check the host architecture > when what we want to check here is the target? > > The results of filtering in the kernel code are as follows : > # grep -wnr "EXPORT_SYMBOL(node_data)" > arch/alpha/mm/numa.c:23:EXPORT_SYMBOL(node_data); > arch/arc/mm/init.c:37:EXPORT_SYMBOL(node_data); > arch/sparc/mm/init_64.c:1106:EXPORT_SYMBOL(node_data); > arch/powerpc/mm/numa.c:52:EXPORT_SYMBOL(node_data); > arch/s390/kernel/numa.c:18:EXPORT_SYMBOL(node_data); > arch/arm64/mm/numa.c:20:EXPORT_SYMBOL(node_data); > arch/x86/mm/numa.c:26:EXPORT_SYMBOL(node_data); > > # grep -wnr "EXPORT_SYMBOL(__node_data)" > arch/mips/sgi-ip27/ip27-memory.c:38:EXPORT_SYMBOL(__node_data); > arch/mips/loongson64/numa.c:33:EXPORT_SYMBOL(__node_data); > > > In the MIPS architecture, no platform uses node_data, only the __node_data > symbol. All other architectures use node_data symbols, so it should be > feasible to use __mips__ to determine the target (as long as it is a MIPS > architecture, use __node_data instead of node_data). OK. Could you add a code comment about this to avoid confusion?

Thanks, Kazu

-----Original Message----- > On 03/10/2021 09:17 AM, HAGIO KAZUHITO(萩尾　一仁) wrote: >> -----Original Message----- >>> Hi, Vincent >>> Thank you for your reply and suggestion. >>> >>> >>> On 02/10/2021 09:59 PM, Vincent Whitchurch wrote: >>> >>> >>> On Tue, Jan 26, 2021 at 12:13:31PM +0100, Youling Tang wrote: >>> >>> "__node_data" instead of "node_data" is used in the MIPS architecture, >>> so "__node_data" is used to replace "node_data" to improve the use of >>> next_online_pgdat() functions in the MIPS architecture. >>> >>> >>> On my 32-bit MIPS dumps, MEMORY: works properly without this patch, but >>> they have contig_page_data instead of __node_data/node_data so they >>> >>> When the node_data/__node_data and pgdat_list symbols cannot be successfully >>> read from the kernel symbols, the contig_page_data symbol will be read. In >>> your 32-bit machine, there should be no node_data/__node_data, pgdat_list >>> symbols, and a single node, so the memory size can be displayed normally >>> without this patch. >>> >>> >>> shouldn't be affected by this code. AFAICS only mach-loongson64 and >>> mach-ip27 have __node_data. >>> >>> >>> Yes, only mach-loongson64 and mach-ip27 use __node_data in the MIPS >>> architecture. >>> >>> >>> >>> E.g. Without this patch: >>> ... >>> MEMORY: 0 >>> ... >>> >>> >>> With this patch: >>> ... >>> MEMORY: 7.5 GB >>> ... >>> >>> Signed-off-by: Huacai Chen <chenhuacai(a)loongson.cn&gt; <mailto:chenhuacai@loongson.cn> >>> Signed-off-by: Youling Tang <tangyouling(a)loongson.cn&gt; <mailto:tangyouling@loongson.cn> >>> --- >>> memory.c | 28 ++++++++++++++++++---------- >>> 1 file changed, 18 insertions(+), 10 deletions(-) >>> >>> diff --git a/memory.c b/memory.c >>> index 33b0ca7..5347958 100644 >>> --- a/memory.c >>> +++ b/memory.c >>> @@ -17820,22 +17820,28 @@ next_online_pgdat(int node) >>> char buf[BUFSIZE]; >>> ulong pgdat; >>> >>> +#ifndef __mips__ >>> +#define NODE_DATA_VAR "node_data" >>> +#else >>> +#define NODE_DATA_VAR "__node_data" >>> +#endif >>> >>> >>> Is this really correct? Doesn't __mips__ check the host architecture >>> when what we want to check here is the target? >>> >>> The results of filtering in the kernel code are as follows : >>> # grep -wnr "EXPORT_SYMBOL(node_data)" >>> arch/alpha/mm/numa.c:23:EXPORT_SYMBOL(node_data); >>> arch/arc/mm/init.c:37:EXPORT_SYMBOL(node_data); >>> arch/sparc/mm/init_64.c:1106:EXPORT_SYMBOL(node_data); >>> arch/powerpc/mm/numa.c:52:EXPORT_SYMBOL(node_data); >>> arch/s390/kernel/numa.c:18:EXPORT_SYMBOL(node_data); >>> arch/arm64/mm/numa.c:20:EXPORT_SYMBOL(node_data); >>> arch/x86/mm/numa.c:26:EXPORT_SYMBOL(node_data); >>> >>> # grep -wnr "EXPORT_SYMBOL(__node_data)" >>> arch/mips/sgi-ip27/ip27-memory.c:38:EXPORT_SYMBOL(__node_data); >>> arch/mips/loongson64/numa.c:33:EXPORT_SYMBOL(__node_data); >>> >>> >>> In the MIPS architecture, no platform uses node_data, only the __node_data >>> symbol. All other architectures use node_data symbols, so it should be >>> feasible to use __mips__ to determine the target (as long as it is a MIPS >>> architecture, use __node_data instead of node_data). >> OK. Could you add a code comment about this to avoid confusion? > Ok. I will add the following comment, you see if it is feasible. The sgi-ip27 is mips32, isn't it? then please add "the mips32 and mips64 architecutres".

Thanks, Kazu

> --- a/memory.c > +++ b/memory.c > @@ -17821,7 +17821,10 @@ next_online_pgdat(int node) > { > char buf[BUFSIZE]; > ulong pgdat; > > +/* > + * "__node_data" is used in the mips architecture, > + * and "node_data" is used in other architectures. > + */ > #ifndef __mips__ > #define NODE_DATA_VAR "node_data" > #else > > Thanks, > Youling >> Thanks, >> Kazu