On 01/10/18 at 02:16pm, Kirill A. Shutemov wrote:
On Wed, Jan 10, 2018 at 03:08:04AM +0000, Dave Young wrote:
On Tue, Jan 09, 2018 at 12:05:52PM +0300, Kirill A. Shutemov wrote:
On Tue, Jan 09, 2018 at 03:24:40PM +0800, Dave Young wrote:
On 01/09/18 at 01:41pm, Baoquan He wrote:
On 01/09/18 at 09:09am, Dave Young wrote:
As for the macro name, VMCOREINFO_SYMBOL_ARRAY sounds better.
Yep, that's better.
I still think using vmcoreinfo_append_str is better. Unless we replace all array variables with the newly added macro.
vmcoreinfo_append_str("SYMBOL(mem_section)=%lx\n", (unsigned long)mem_section);
I have no strong opinion, either change all array uses or just introduce the macro and start to use it from now on if we have similar array symbols.
Do you need some action on my side or will you folks take care about this?
I think Baoquan was suggesting to update all array users in current code, if you can check every VMCOREINFO_SYMBOL and update all the arrays he will be happy. But if can not do it easily I'm fine with a VMCOREINFO_SYMBOL_ARRAY changes only now, we kdump people can do it later as well.
It seems it's the only array we have there. swapper_pg_dir is a potential candidate, but it's 'unsigned long' on arm.
Below it patch with corrected macro name.
Please, consider applying.
From 70f3a84b97f2de98d1364f7b10b7a42a1d8e9968 Mon Sep 17 00:00:00 2001 From: "Kirill A. Shutemov" kirill.shutemov@linux.intel.com Date: Tue, 9 Jan 2018 02:55:47 +0300 Subject: [PATCH] kdump: Write a correct address of mem_section into vmcoreinfo
Depending on configuration mem_section can now be an array or a pointer to an array allocated dynamically. In most cases, we can continue to refer to it as 'mem_section' regardless of what it is.
But there's one exception: '&mem_section' means "address of the array" if mem_section is an array, but if mem_section is a pointer, it would mean "address of the pointer".
We've stepped onto this in kdump code. VMCOREINFO_SYMBOL(mem_section) writes down address of pointer into vmcoreinfo, not array as we wanted.
Let's introduce VMCOREINFO_SYMBOL_ARRAY() that would handle the situation correctly for both cases.
Signed-off-by: Kirill A. Shutemov kirill.shutemov@linux.intel.com Fixes: 83e3c48729d9 ("mm/sparsemem: Allocate mem_section at runtime for CONFIG_SPARSEMEM_EXTREME=y")
include/linux/crash_core.h | 2 ++ kernel/crash_core.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h index 06097ef30449..b511f6d24b42 100644 --- a/include/linux/crash_core.h +++ b/include/linux/crash_core.h @@ -42,6 +42,8 @@ phys_addr_t paddr_vmcoreinfo_note(void); vmcoreinfo_append_str("PAGESIZE=%ld\n", value) #define VMCOREINFO_SYMBOL(name) \ vmcoreinfo_append_str("SYMBOL(%s)=%lx\n", #name, (unsigned long)&name) +#define VMCOREINFO_SYMBOL_ARRAY(name) \
- vmcoreinfo_append_str("SYMBOL(%s)=%lx\n", #name, (unsigned long)name)
#define VMCOREINFO_SIZE(name) \ vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \ (unsigned long)sizeof(name)) diff --git a/kernel/crash_core.c b/kernel/crash_core.c index b3663896278e..4f63597c824d 100644 --- a/kernel/crash_core.c +++ b/kernel/crash_core.c @@ -410,7 +410,7 @@ static int __init crash_save_vmcoreinfo_init(void) VMCOREINFO_SYMBOL(contig_page_data); #endif #ifdef CONFIG_SPARSEMEM
- VMCOREINFO_SYMBOL(mem_section);
- VMCOREINFO_SYMBOL_ARRAY(mem_section); VMCOREINFO_LENGTH(mem_section, NR_SECTION_ROOTS); VMCOREINFO_STRUCT_SIZE(mem_section); VMCOREINFO_OFFSET(mem_section, section_mem_map);
-- Kirill A. Shutemov
Acked-by: Dave Young dyoung@redhat.com
If stable kernel took the mem section commits, then should also cc stable. Andrew, can you help to make this in 4.15?
Thanks Dave