> -----Original Message-----
> On 04/29/2020 10:27 PM, HAGIO KAZUHITO wrote:
> > Hi Pingfan,
> >
> >> -----Original Message-----
> >> Hi Kazu and Cascardo,
> >>
> >> I encounter a weird problem when running makedumpfile on a s390 machine.
> >>
> >> Our production kernel uses extreme sparse memory model, and has the
> >> following:
> >>
> >> in mm/sparse.c
> >>
> >> #ifdef CONFIG_SPARSEMEM_EXTREME
> >> struct mem_section **mem_section;
> >> #else
> >> struct mem_section mem_section[NR_SECTION_ROOTS][SECTIONS_PER_ROOT]
> >> ____cacheline_internodealigned_in_smp;
> >> #endif
> >>
> >> So in makedumpfile.c, get_mem_section(), it got a failed result when the
> >> first call site to validate_mem_section(), then it should success at the
> >> second call site to validate_mem_section(), which is inside if
> >> (is_sparsemem_extreme()) condition.
> >
> > I think your production kernel should have kernel commit a0b1280368d1
> > ("kdump: write correct address of mem_section into vmcoreinfo"), so the
> > first call should return TRUE and the second one should return FALSE.
> Yes, it is.
> >
> >>
> >> But the actual result is not like expected.
> >>
> >> After introducing
> >> commit e113f1c974c820f9633dc0073eda525d7575f365 [PATCH] cope with
> >> not-present mem section
> >>
> >> I got two successful calls to validate_mem_section(), and finally failed
> >> at the condition
> >> ret = symbol_valid ^ pointer_valid;
> >> if (!ret) {
> >> ERRMSG("Could not validate mem_section.\n");
> >> }
> >>
> >>
> >> Do you have any idea?
> >
> > Presumably this will be what I expected that it might be possible.
> > I can apply the patch below this time, what about this?
> > https://github.com/k-hagio/makedumpfile-old/commit/ce883df3864a5744ac0f1eff47de06b5074edb5f.patch
> looks good.
Thanks.
> >
> > or, we can also investigate why the second call returns TRUE, and
> > fix the conditions in the validate_mem_section()..
> This is due to the relaxed condition check after applying my commit
> commit e113f1c974("[PATCH] cope with not-present mem section")
>
> diff --git a/makedumpfile.c b/makedumpfile.c
> index ae7336a..607e07f 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -3406,8 +3406,6 @@ section_mem_map_addr(unsigned long addr, unsigned
> long *map_mask)
> map = ULONG(mem_section + OFFSET(mem_section.section_mem_map));
> mask = SECTION_MAP_MASK;
> *map_mask = map & ~mask;
> - if (map == 0x0)
> - *map_mask |= SECTION_MARKED_PRESENT;
> map &= mask;
> free(mem_section);
>
> @@ -3453,10 +3451,8 @@ validate_mem_section(unsigned long *mem_sec,
> mem_map = NOT_MEMMAP_ADDR;
> } else {
> mem_map = section_mem_map_addr(section, &map_mask);
> + /* for either no mem_map or hot-removed */
> if (!(map_mask & SECTION_MARKED_PRESENT)) {
> - return FALSE; ------> a strict check
> - }
> - if (mem_map == 0) {
> mem_map = NOT_MEMMAP_ADDR;
> } else {
> mem_map = sparse_decode_mem_map(mem_map,
>
>
> Before my patch, it return FALSE for any non NULL value without
> SECTION_MARKED_PRESENT. But my patch relaxes the restriction and
> consider it as hot-removed mem_section and keeps the parsing on.
Yes, so I meant that we might add some conditions so that the second call
could return FALSE for your vmcore as expected. But I decided to apply
the patch I wrote before.. and applied:
https://github.com/makedumpfile/makedumpfile/commit/81b79c514ff6fc881f1df4cb04ecb2d7cb22badc
I deferred merging this at that time because it might not be needed
actually and I didn't want to change the behavior if possible.
But it happened.
Thank you for the report.
Kazu
_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
