Re: [Crash-utility] [PATCH 07/16] MIPS: Fix display memory size issue

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.

--- 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