Re: [PATCH] makedumpfile: cope with not-present mem section

Re: [PATCH] makedumpfile: cope with not-present mem section

[piliu]

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.

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?

Thanks,
Pingfan


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

RE: [PATCH] makedumpfile: cope with not-present mem section

[HAGIO KAZUHITO(萩尾 一仁)]

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.

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

or, we can also investigate why the second call returns TRUE, and
fix the conditions in the validate_mem_section()..

Thanks,
Kazu
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] makedumpfile: cope with not-present mem section

[piliu]

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

Thanks,
Pingfan
> 
> Thanks,
> Kazu
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
> 


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

RE: [PATCH] makedumpfile: cope with not-present mem section

[HAGIO KAZUHITO(萩尾 一仁)]

> -----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@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec