Kexec Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] makedumpfile: call initial before use cache
@ 2024-06-25  1:57 Lichen Liu
  2024-07-17  7:44 ` HAGIO KAZUHITO(萩尾 一仁)
  0 siblings, 1 reply; 6+ messages in thread
From: Lichen Liu @ 2024-06-25  1:57 UTC (permalink / raw)
  To: kexec, k-hagio-ab; +Cc: Lichen Liu

Run 'makedumpfile --mem-usage /proc/kcore' will coredump on ppc64, it is
because show_mem_usage()->get_page_offset()->get_versiondep_info_ppc64()
->readmem() use cache before it is inited by initial().

Currently only ppc64 has this issue because only
get_versiondep_info_ppc64() call readmem().

Signed-off-by: Lichen Liu <lichliu@redhat.com>
---
 makedumpfile.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index 5b34712..6a42264 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -12019,6 +12019,9 @@ int show_mem_usage(void)
 		DEBUG_MSG("Read vmcoreinfo from NOTE segment: %d\n", vmcoreinfo);
 	}
 
+	if (!initial())
+		return FALSE;
+
 	if (!get_page_offset())
 		return FALSE;
 
@@ -12034,9 +12037,6 @@ int show_mem_usage(void)
 			return FALSE;
 	}
 
-	if (!initial())
-		return FALSE;
-
 	if (!open_dump_bitmap())
 		return FALSE;
 
-- 
2.44.0


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] makedumpfile: call initial before use cache
  2024-06-25  1:57 [PATCH] makedumpfile: call initial before use cache Lichen Liu
@ 2024-07-17  7:44 ` HAGIO KAZUHITO(萩尾 一仁)
  2024-07-20  7:38   ` Lichen Liu
  0 siblings, 1 reply; 6+ messages in thread
From: HAGIO KAZUHITO(萩尾 一仁) @ 2024-07-17  7:44 UTC (permalink / raw)
  To: Lichen Liu, kexec@lists.infradead.org

Hi Lichen,

sorry for the long delay.

On 2024/06/25 10:57, Lichen Liu wrote:
> Run 'makedumpfile --mem-usage /proc/kcore' will coredump on ppc64, it is
> because show_mem_usage()->get_page_offset()->get_versiondep_info_ppc64()
> ->readmem() use cache before it is inited by initial().
> 
> Currently only ppc64 has this issue because only
> get_versiondep_info_ppc64() call readmem().
> 
> Signed-off-by: Lichen Liu <lichliu@redhat.com>
> ---
>   makedumpfile.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 5b34712..6a42264 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -12019,6 +12019,9 @@ int show_mem_usage(void)
>   		DEBUG_MSG("Read vmcoreinfo from NOTE segment: %d\n", vmcoreinfo);
>   	}
>   
> +	if (!initial())
> +		return FALSE;
> +
>   	if (!get_page_offset())
>   		return FALSE;
>   
> @@ -12034,9 +12037,6 @@ int show_mem_usage(void)
>   			return FALSE;
>   	}
>   
> -	if (!initial())
> -		return FALSE;
> -
>   	if (!open_dump_bitmap())
>   		return FALSE;
>   

initial() needs to be called after set_kcore_vmcoreinfo(), when there is 
no vmcoreinfo in /proc/kcore ELF note.

So with the patch, "makedumpfile --mem-usage" fails on kernels that do 
not have a vmcoreinfo in ELF note, e.g. RHEL7 kernel:

   # makedumpfile-dev -f --mem-usage /proc/kcore
   exclude_free_page: Can't get necessary symbols for excluding free pages.

   makedumpfile Failed.


Probably readmem() should not be called before initial() in the first 
place.  I think it's the root cause, but I'm not sure how we can fix it.

A workaround I thought of is that moving get_page_offset() and 
get_phys_base() into the !vmcoreinfo block.  These are needed by 
set_kcore_vmcoreinfo(), so we can avoid calling them if there is a 
vmcoreinfo in ELF note:

--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -12019,14 +12019,14 @@ int show_mem_usage(void)
                 DEBUG_MSG("Read vmcoreinfo from NOTE segment: %d\n", 
vmcoreinfo);
         }

-       if (!get_page_offset())
-               return FALSE;
+       if (!vmcoreinfo) {
+               if (!get_page_offset())
+                       return FALSE;

-       /* paddr_to_vaddr() on arm64 needs phys_base. */
-       if (!get_phys_base())
-               return FALSE;
+               /* paddr_to_vaddr() on arm64 needs phys_base. */
+               if (!get_phys_base())
+                       return FALSE;

-       if (!vmcoreinfo) {
                 if (!get_sys_kernel_vmcoreinfo(&vmcoreinfo_addr, 
&vmcoreinfo_len))
                         return FALSE;


This will work only for 4.19 and later kernels, but might reduce users 
that hit the issue.  Does this work for you?

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] makedumpfile: call initial before use cache
  2024-07-17  7:44 ` HAGIO KAZUHITO(萩尾 一仁)
@ 2024-07-20  7:38   ` Lichen Liu
  2024-07-22  6:46     ` HAGIO KAZUHITO(萩尾 一仁)
  0 siblings, 1 reply; 6+ messages in thread
From: Lichen Liu @ 2024-07-20  7:38 UTC (permalink / raw)
  To: HAGIO KAZUHITO(萩尾 一仁)
  Cc: kexec@lists.infradead.org

Thanks for your explanation!


On Wed, Jul 17, 2024 at 3:44 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab@nec.com> wrote:
>
> Hi Lichen,
>
> sorry for the long delay.
>
> On 2024/06/25 10:57, Lichen Liu wrote:
> > Run 'makedumpfile --mem-usage /proc/kcore' will coredump on ppc64, it is
> > because show_mem_usage()->get_page_offset()->get_versiondep_info_ppc64()
> > ->readmem() use cache before it is inited by initial().
> >
> > Currently only ppc64 has this issue because only
> > get_versiondep_info_ppc64() call readmem().
> >
> > Signed-off-by: Lichen Liu <lichliu@redhat.com>
> > ---
> >   makedumpfile.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/makedumpfile.c b/makedumpfile.c
> > index 5b34712..6a42264 100644
> > --- a/makedumpfile.c
> > +++ b/makedumpfile.c
> > @@ -12019,6 +12019,9 @@ int show_mem_usage(void)
> >               DEBUG_MSG("Read vmcoreinfo from NOTE segment: %d\n", vmcoreinfo);
> >       }
> >
> > +     if (!initial())
> > +             return FALSE;
> > +
> >       if (!get_page_offset())
> >               return FALSE;
> >
> > @@ -12034,9 +12037,6 @@ int show_mem_usage(void)
> >                       return FALSE;
> >       }
> >
> > -     if (!initial())
> > -             return FALSE;
> > -
> >       if (!open_dump_bitmap())
> >               return FALSE;
> >
>
> initial() needs to be called after set_kcore_vmcoreinfo(), when there is
> no vmcoreinfo in /proc/kcore ELF note.
>
> So with the patch, "makedumpfile --mem-usage" fails on kernels that do
> not have a vmcoreinfo in ELF note, e.g. RHEL7 kernel:
>
>    # makedumpfile-dev -f --mem-usage /proc/kcore
>    exclude_free_page: Can't get necessary symbols for excluding free pages.
>
>    makedumpfile Failed.
>
>
> Probably readmem() should not be called before initial() in the first
> place.  I think it's the root cause, but I'm not sure how we can fix it.
>
> A workaround I thought of is that moving get_page_offset() and
> get_phys_base() into the !vmcoreinfo block.  These are needed by
> set_kcore_vmcoreinfo(), so we can avoid calling them if there is a
> vmcoreinfo in ELF note:
>
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -12019,14 +12019,14 @@ int show_mem_usage(void)
>                  DEBUG_MSG("Read vmcoreinfo from NOTE segment: %d\n",
> vmcoreinfo);
>          }
>
> -       if (!get_page_offset())
> -               return FALSE;
> +       if (!vmcoreinfo) {
> +               if (!get_page_offset())
> +                       return FALSE;
>
> -       /* paddr_to_vaddr() on arm64 needs phys_base. */
> -       if (!get_phys_base())
> -               return FALSE;
> +               /* paddr_to_vaddr() on arm64 needs phys_base. */
> +               if (!get_phys_base())
> +                       return FALSE;
>
> -       if (!vmcoreinfo) {
>                  if (!get_sys_kernel_vmcoreinfo(&vmcoreinfo_addr,
> &vmcoreinfo_len))
>                          return FALSE;
>
>
> This will work only for 4.19 and later kernels, but might reduce users
> that hit the issue.  Does this work for you?
That works for me because I'm testing for the 6.x kernel.

Thanks,
Lichen
>
> Thanks,
> Kazu


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] makedumpfile: call initial before use cache
  2024-07-20  7:38   ` Lichen Liu
@ 2024-07-22  6:46     ` HAGIO KAZUHITO(萩尾 一仁)
  2024-07-23  7:31       ` Lichen Liu
  0 siblings, 1 reply; 6+ messages in thread
From: HAGIO KAZUHITO(萩尾 一仁) @ 2024-07-22  6:46 UTC (permalink / raw)
  To: Lichen Liu; +Cc: kexec@lists.infradead.org

On 2024/07/20 16:38, Lichen Liu wrote:

>> This will work only for 4.19 and later kernels, but might reduce users
>> that hit the issue.  Does this work for you?
> That works for me because I'm testing for the 6.x kernel.

Thanks for the check.  Is the issue a segmentation fault?
I've made a patch below, is this OK?


 From 084742cba5b81da563074454ab8c879e8e411cb0 Mon Sep 17 00:00:00 2001
From: Kazuhito Hagio <k-hagio-ab@nec.com>
Date: Mon, 22 Jul 2024 14:31:43 +0900
Subject: [PATCH] Workaround for segfault by "makedumpfile --mem-usage" on PPC64

"makedumpfile --mem-usage /proc/kcore" can cause a segmentation fault on
PPC64, because the readmem() of the following code path uses cache
before it's initialized in initial().

   show_mem_usage
     get_page_offset
       get_versiondep_info_ppc64
         readmem
     ...
     initial
         cache_init

The get_page_offset() is needed to get vmcoreinfo from /proc/kcore data,
so we can avoid calling it when a vmcoreinfo exists in the ELF NOTE
segment of /proc/kcore, i.e. on Linux 4.19 and later.

(Note: for older kernels, we will need another way to fix it.)

Reported-by: Lichen Liu <lichliu@redhat.com>
Signed-off-by: Kazuhito Hagio <k-hagio-ab@nec.com>
---
  makedumpfile.c | 12 ++++++------
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index 5b347126db76..7d1dfcca50d8 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -12019,14 +12019,14 @@ int show_mem_usage(void)
  		DEBUG_MSG("Read vmcoreinfo from NOTE segment: %d\n", vmcoreinfo);
  	}
  
-	if (!get_page_offset())
-		return FALSE;
+	if (!vmcoreinfo) {
+		if (!get_page_offset())
+			return FALSE;
  
-	/* paddr_to_vaddr() on arm64 needs phys_base. */
-	if (!get_phys_base())
-		return FALSE;
+		/* paddr_to_vaddr() on arm64 needs phys_base. */
+		if (!get_phys_base())
+			return FALSE;
  
-	if (!vmcoreinfo) {
  		if (!get_sys_kernel_vmcoreinfo(&vmcoreinfo_addr, &vmcoreinfo_len))
  			return FALSE;
  
-- 
2.31.1
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] makedumpfile: call initial before use cache
  2024-07-22  6:46     ` HAGIO KAZUHITO(萩尾 一仁)
@ 2024-07-23  7:31       ` Lichen Liu
  2024-07-26  0:05         ` HAGIO KAZUHITO(萩尾 一仁)
  0 siblings, 1 reply; 6+ messages in thread
From: Lichen Liu @ 2024-07-23  7:31 UTC (permalink / raw)
  To: HAGIO KAZUHITO(萩尾 一仁)
  Cc: kexec@lists.infradead.org

Yes, it works!
Thank you so much!

On Mon, Jul 22, 2024 at 2:46 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab@nec.com> wrote:
>
> On 2024/07/20 16:38, Lichen Liu wrote:
>
> >> This will work only for 4.19 and later kernels, but might reduce users
> >> that hit the issue.  Does this work for you?
> > That works for me because I'm testing for the 6.x kernel.
>
> Thanks for the check.  Is the issue a segmentation fault?
> I've made a patch below, is this OK?
>
>
>  From 084742cba5b81da563074454ab8c879e8e411cb0 Mon Sep 17 00:00:00 2001
> From: Kazuhito Hagio <k-hagio-ab@nec.com>
> Date: Mon, 22 Jul 2024 14:31:43 +0900
> Subject: [PATCH] Workaround for segfault by "makedumpfile --mem-usage" on PPC64
>
> "makedumpfile --mem-usage /proc/kcore" can cause a segmentation fault on
> PPC64, because the readmem() of the following code path uses cache
> before it's initialized in initial().
>
>    show_mem_usage
>      get_page_offset
>        get_versiondep_info_ppc64
>          readmem
>      ...
>      initial
>          cache_init
>
> The get_page_offset() is needed to get vmcoreinfo from /proc/kcore data,
> so we can avoid calling it when a vmcoreinfo exists in the ELF NOTE
> segment of /proc/kcore, i.e. on Linux 4.19 and later.
>
> (Note: for older kernels, we will need another way to fix it.)
>
> Reported-by: Lichen Liu <lichliu@redhat.com>
> Signed-off-by: Kazuhito Hagio <k-hagio-ab@nec.com>
> ---
>   makedumpfile.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 5b347126db76..7d1dfcca50d8 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -12019,14 +12019,14 @@ int show_mem_usage(void)
>                 DEBUG_MSG("Read vmcoreinfo from NOTE segment: %d\n", vmcoreinfo);
>         }
>
> -       if (!get_page_offset())
> -               return FALSE;
> +       if (!vmcoreinfo) {
> +               if (!get_page_offset())
> +                       return FALSE;
>
> -       /* paddr_to_vaddr() on arm64 needs phys_base. */
> -       if (!get_phys_base())
> -               return FALSE;
> +               /* paddr_to_vaddr() on arm64 needs phys_base. */
> +               if (!get_phys_base())
> +                       return FALSE;
>
> -       if (!vmcoreinfo) {
>                 if (!get_sys_kernel_vmcoreinfo(&vmcoreinfo_addr, &vmcoreinfo_len))
>                         return FALSE;
>
> --
> 2.31.1


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] makedumpfile: call initial before use cache
  2024-07-23  7:31       ` Lichen Liu
@ 2024-07-26  0:05         ` HAGIO KAZUHITO(萩尾 一仁)
  0 siblings, 0 replies; 6+ messages in thread
From: HAGIO KAZUHITO(萩尾 一仁) @ 2024-07-26  0:05 UTC (permalink / raw)
  To: Lichen Liu; +Cc: kexec@lists.infradead.org


On 2024/07/23 16:31, Lichen Liu wrote:
> Yes, it works!
> Thank you so much!

Thanks, applied.
https://github.com/makedumpfile/makedumpfile/commit/900190de6b67b2de410cfc8023c1b198a416ceb3

Kazu

> 
> On Mon, Jul 22, 2024 at 2:46 PM HAGIO KAZUHITO(萩尾 一仁)
> <k-hagio-ab@nec.com> wrote:
>>
>> On 2024/07/20 16:38, Lichen Liu wrote:
>>
>>>> This will work only for 4.19 and later kernels, but might reduce users
>>>> that hit the issue.  Does this work for you?
>>> That works for me because I'm testing for the 6.x kernel.
>>
>> Thanks for the check.  Is the issue a segmentation fault?
>> I've made a patch below, is this OK?
>>
>>
>>   From 084742cba5b81da563074454ab8c879e8e411cb0 Mon Sep 17 00:00:00 2001
>> From: Kazuhito Hagio <k-hagio-ab@nec.com>
>> Date: Mon, 22 Jul 2024 14:31:43 +0900
>> Subject: [PATCH] Workaround for segfault by "makedumpfile --mem-usage" on PPC64
>>
>> "makedumpfile --mem-usage /proc/kcore" can cause a segmentation fault on
>> PPC64, because the readmem() of the following code path uses cache
>> before it's initialized in initial().
>>
>>     show_mem_usage
>>       get_page_offset
>>         get_versiondep_info_ppc64
>>           readmem
>>       ...
>>       initial
>>           cache_init
>>
>> The get_page_offset() is needed to get vmcoreinfo from /proc/kcore data,
>> so we can avoid calling it when a vmcoreinfo exists in the ELF NOTE
>> segment of /proc/kcore, i.e. on Linux 4.19 and later.
>>
>> (Note: for older kernels, we will need another way to fix it.)
>>
>> Reported-by: Lichen Liu <lichliu@redhat.com>
>> Signed-off-by: Kazuhito Hagio <k-hagio-ab@nec.com>
>> ---
>>    makedumpfile.c | 12 ++++++------
>>    1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/makedumpfile.c b/makedumpfile.c
>> index 5b347126db76..7d1dfcca50d8 100644
>> --- a/makedumpfile.c
>> +++ b/makedumpfile.c
>> @@ -12019,14 +12019,14 @@ int show_mem_usage(void)
>>                  DEBUG_MSG("Read vmcoreinfo from NOTE segment: %d\n", vmcoreinfo);
>>          }
>>
>> -       if (!get_page_offset())
>> -               return FALSE;
>> +       if (!vmcoreinfo) {
>> +               if (!get_page_offset())
>> +                       return FALSE;
>>
>> -       /* paddr_to_vaddr() on arm64 needs phys_base. */
>> -       if (!get_phys_base())
>> -               return FALSE;
>> +               /* paddr_to_vaddr() on arm64 needs phys_base. */
>> +               if (!get_phys_base())
>> +                       return FALSE;
>>
>> -       if (!vmcoreinfo) {
>>                  if (!get_sys_kernel_vmcoreinfo(&vmcoreinfo_addr, &vmcoreinfo_len))
>>                          return FALSE;
>>
>> --
>> 2.31.1
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-07-26  0:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-25  1:57 [PATCH] makedumpfile: call initial before use cache Lichen Liu
2024-07-17  7:44 ` HAGIO KAZUHITO(萩尾 一仁)
2024-07-20  7:38   ` Lichen Liu
2024-07-22  6:46     ` HAGIO KAZUHITO(萩尾 一仁)
2024-07-23  7:31       ` Lichen Liu
2024-07-26  0:05         ` HAGIO KAZUHITO(萩尾 一仁)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox