[Makedumpfile Patch v3 0/7] Fix --mem-usage /proc/kcore

[Makedumpfile Patch v3 0/7] Fix --mem-usage /proc/kcore

[Pratyush Anand]

`makedumpfile --mem-usage /proc/kcore` has been broken after kaslr specific
modifications. Kernel commit commit 464920104bf7 (/proc/kcore: update
physical address for kcore ram and text) provides now valid physical
address for direct mapped regions and invalid (-1) for other areas.  This
helps to fix the issue in makedumpfile for both the case of kaslr enabled
and disabled.

Changes since v2:
 - Fixed a memory leak issue and updated man page and usage info
Changes since v1:
 - One more patch..Now mem-usage with older kernel (< 4.11) will only work
   when that has been rightly patched and -f has been used in makedumpfile
   command line.


Baoquan He (2):
  makedumpfile: Correct the calculation of kvaddr in
    set_kcore_vmcoreinfo
  makedumpfile: Discard process_dump_load

Pratyush Anand (5):
  show_mem_usage(): calculate page offset after elf load
  initial(): call cach_init() a bit early
  x86_64: check physical address in PT_LOAD for none direct mapped
    regions
  elf_info: kcore: check for invalid physical address
  mem-usage: allow to work only with -f for kernel version < 4.11

 arch/x86_64.c  |  6 ++++--
 elf_info.c     | 25 +++++--------------------
 makedumpfile.8 |  9 ++++++++-
 makedumpfile.c | 18 ++++++++++++------
 print_info.c   |  3 ++-
 5 files changed, 31 insertions(+), 30 deletions(-)

-- 
2.9.3


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

[Makedumpfile Patch v3 1/7] show_mem_usage(): calculate page offset after elf load

[Pratyush Anand]

x86_64 calculated page offset from PT_LOAD headers. Therefore call
get_page_offset() after get_elf_loads()

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

diff --git a/makedumpfile.c b/makedumpfile.c
index e69b6df9a9ee..6942047199de 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -10944,15 +10944,15 @@ int show_mem_usage(void)
 
 	info->dump_level = MAX_DUMP_LEVEL;
 
-	if (!get_page_offset())
-		return FALSE;
-
 	if (!open_files_for_creating_dumpfile())
 		return FALSE;
 
 	if (!get_elf_loads(info->fd_memory, info->name_memory))
 		return FALSE;
 
+	if (!get_page_offset())
+		return FALSE;
+
 	if (!get_sys_kernel_vmcoreinfo(&vmcoreinfo_addr, &vmcoreinfo_len))
 		return FALSE;
 
-- 
2.9.3


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

[Makedumpfile Patch v3 2/7] initial(): call cach_init() a bit early

[Pratyush Anand]

Call cach_init() before get_kcore_dump_loads(), because latter uses
cache_search().

Call path is like this :
get_kcore_dump_loads() -> process_dump_load() -> vaddr_to_paddr() ->
vtop4_x86_64() -> readmem() -> cache_search()

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

diff --git a/makedumpfile.c b/makedumpfile.c
index 6942047199de..3b8e9810468d 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -3878,6 +3878,9 @@ initial(void)
 	if (!get_value_for_old_linux())
 		return FALSE;
 
+	if (!is_xen_memory() && !cache_init())
+		return FALSE;
+
 	if (info->flag_mem_usage && !get_kcore_dump_loads())
 		return FALSE;
 
@@ -4000,9 +4003,6 @@ out:
 		}
 	}
 
-	if (!is_xen_memory() && !cache_init())
-		return FALSE;
-
 	if (debug_info && !get_machdep_info())
 		return FALSE;
 
-- 
2.9.3


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

RE: [Makedumpfile Patch v3 2/7] initial(): call cach_init() a bit early

[Atsushi Kumagai]

Hello Pratyush,

>Call cach_init() before get_kcore_dump_loads(), because latter uses
>cache_search().
>
>Call path is like this :
>get_kcore_dump_loads() -> process_dump_load() -> vaddr_to_paddr() ->
>vtop4_x86_64() -> readmem() -> cache_search()

I found that the current cach_init() is too early, cache_init() should be
called after fallback_to_current_page_size() for older kernel
which doesn't have VMCOREINFO.
Could you confirm that the patch below is fine with you ?

Thanks,
Atsushi Kumagai

From: Atsushi Kumagai <ats-kumagai@wm.jp.nec.com>
Date: Wed, 19 Jul 2017 15:05:58 +0900
Subject: [PATCH] Fix the regression caused by moving cache_init() earlier

info->page_size should be set before calling cache_init() since it
allocates some buffers based on info->page_size. However, cache_init()
was moved before fallback_to_current_page_size() while it set
info->page_size for older kernel which doesn't have VMCOREINFO.

This patch moves cache_init() after fallback_to_current_page_size()
and reverts the commit: b98e6e0d2a(sadump: set info->page_size before cache_init())

Signed-off-by: Atsushi Kumagai <ats-kumagai@wm.jp.nec.com>
---
 makedumpfile.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index a3f0697..f85003a 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -3905,15 +3905,6 @@ initial(void)
 	if (!get_value_for_old_linux())
 		return FALSE;
 
-	if (info->flag_sadump && !set_page_size(sadump_page_size()))
-			return FALSE;
-
-	if (!is_xen_memory() && !cache_init())
-		return FALSE;
-
-	if (info->flag_mem_usage && !get_kcore_dump_loads())
-		return FALSE;
-
 	if (info->flag_refiltering) {
 		if (info->flag_elf_dumpfile) {
 			MSG("'-E' option is disable, ");
@@ -3936,6 +3927,9 @@ initial(void)
 			return FALSE;
 		}
 
+		if (!set_page_size(sadump_page_size()))
+			return FALSE;
+
 		if (!sadump_initialize_bitmap_memory())
 			return FALSE;
 
@@ -3949,9 +3943,7 @@ initial(void)
 		 * postponed until debug information becomes
 		 * available.
 		 */
-
-	} else if (!get_phys_base())
-		return FALSE;
+	}
 
 out:
 	if (!info->page_size) {
@@ -3962,6 +3954,18 @@ out:
 		if (!fallback_to_current_page_size())
 			return FALSE;
 	}
+
+	if (!is_xen_memory() && !cache_init())
+		return FALSE;
+
+	if (info->flag_mem_usage && !get_kcore_dump_loads())
+		return FALSE;
+
+	if (!info->flag_refiltering && !info->flag_sadump) {
+		if (!get_phys_base())
+			return FALSE;
+	}
+
 	if (!get_max_mapnr())
 		return FALSE;
 
-- 
2.7.2


>Signed-off-by: Pratyush Anand <panand@redhat.com>
>---
> makedumpfile.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/makedumpfile.c b/makedumpfile.c
>index 6942047199de..3b8e9810468d 100644
>--- a/makedumpfile.c
>+++ b/makedumpfile.c
>@@ -3878,6 +3878,9 @@ initial(void)
> 	if (!get_value_for_old_linux())
> 		return FALSE;
>
>+	if (!is_xen_memory() && !cache_init())
>+		return FALSE;
>+
> 	if (info->flag_mem_usage && !get_kcore_dump_loads())
> 		return FALSE;
>
>@@ -4000,9 +4003,6 @@ out:
> 		}
> 	}
>
>-	if (!is_xen_memory() && !cache_init())
>-		return FALSE;
>-
> 	if (debug_info && !get_machdep_info())
> 		return FALSE;
>
>--
>2.9.3
>



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

Re: [Makedumpfile Patch v3 2/7] initial(): call cach_init() a bit early

[Pratyush Anand]

Hi Atsushi,

On Wednesday 19 July 2017 01:16 PM, Atsushi Kumagai wrote:
> Hello Pratyush,
>
>> Call cach_init() before get_kcore_dump_loads(), because latter uses
>> cache_search().
>>
>> Call path is like this :
>> get_kcore_dump_loads() -> process_dump_load() -> vaddr_to_paddr() ->
>> vtop4_x86_64() -> readmem() -> cache_search()
>
> I found that the current cach_init() is too early, cache_init() should be
> called after fallback_to_current_page_size() for older kernel
> which doesn't have VMCOREINFO.
> Could you confirm that the patch below is fine with you ?

Its fine and works.

>
> Thanks,
> Atsushi Kumagai
>
> From: Atsushi Kumagai <ats-kumagai@wm.jp.nec.com>
> Date: Wed, 19 Jul 2017 15:05:58 +0900
> Subject: [PATCH] Fix the regression caused by moving cache_init() earlier
>
> info->page_size should be set before calling cache_init() since it
> allocates some buffers based on info->page_size. However, cache_init()
> was moved before fallback_to_current_page_size() while it set
> info->page_size for older kernel which doesn't have VMCOREINFO.
>
> This patch moves cache_init() after fallback_to_current_page_size()
> and reverts the commit: b98e6e0d2a(sadump: set info->page_size before cache_init())
>
> Signed-off-by: Atsushi Kumagai <ats-kumagai@wm.jp.nec.com>
> ---
>  makedumpfile.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/makedumpfile.c b/makedumpfile.c
> index a3f0697..f85003a 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -3905,15 +3905,6 @@ initial(void)
>  	if (!get_value_for_old_linux())
>  		return FALSE;
>
> -	if (info->flag_sadump && !set_page_size(sadump_page_size()))
> -			return FALSE;
> -
> -	if (!is_xen_memory() && !cache_init())
> -		return FALSE;
> -
> -	if (info->flag_mem_usage && !get_kcore_dump_loads())
> -		return FALSE;
> -
>  	if (info->flag_refiltering) {
>  		if (info->flag_elf_dumpfile) {
>  			MSG("'-E' option is disable, ");
> @@ -3936,6 +3927,9 @@ initial(void)
>  			return FALSE;
>  		}
>
> +		if (!set_page_size(sadump_page_size()))
> +			return FALSE;
> +
>  		if (!sadump_initialize_bitmap_memory())
>  			return FALSE;
>
> @@ -3949,9 +3943,7 @@ initial(void)
>  		 * postponed until debug information becomes
>  		 * available.
>  		 */
> -
> -	} else if (!get_phys_base())
> -		return FALSE;
> +	}
>
>  out:
>  	if (!info->page_size) {
> @@ -3962,6 +3954,18 @@ out:
>  		if (!fallback_to_current_page_size())
>  			return FALSE;
>  	}
> +
> +	if (!is_xen_memory() && !cache_init())
> +		return FALSE;
> +
> +	if (info->flag_mem_usage && !get_kcore_dump_loads())
> +		return FALSE;
> +
> +	if (!info->flag_refiltering && !info->flag_sadump) {
> +		if (!get_phys_base())
> +			return FALSE;
> +	}
> +
>  	if (!get_max_mapnr())
>  		return FALSE;
>
>

-- 
Regards
Pratyush

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

RE: [Makedumpfile Patch v3 2/7] initial(): call cach_init() a bit early

[Atsushi Kumagai]

>Hi Atsushi,
>
>On Wednesday 19 July 2017 01:16 PM, Atsushi Kumagai wrote:
>> Hello Pratyush,
>>
>>> Call cach_init() before get_kcore_dump_loads(), because latter uses
>>> cache_search().
>>>
>>> Call path is like this :
>>> get_kcore_dump_loads() -> process_dump_load() -> vaddr_to_paddr() ->
>>> vtop4_x86_64() -> readmem() -> cache_search()
>>
>> I found that the current cach_init() is too early, cache_init() should be
>> called after fallback_to_current_page_size() for older kernel
>> which doesn't have VMCOREINFO.
>> Could you confirm that the patch below is fine with you ?
>
>Its fine and works.

Thanks, I'll merge this fix into v1.6.2.

Regards,
Atsushi Kumagai

>>
>> Thanks,
>> Atsushi Kumagai
>>
>> From: Atsushi Kumagai <ats-kumagai@wm.jp.nec.com>
>> Date: Wed, 19 Jul 2017 15:05:58 +0900
>> Subject: [PATCH] Fix the regression caused by moving cache_init() earlier
>>
>> info->page_size should be set before calling cache_init() since it
>> allocates some buffers based on info->page_size. However, cache_init()
>> was moved before fallback_to_current_page_size() while it set
>> info->page_size for older kernel which doesn't have VMCOREINFO.
>>
>> This patch moves cache_init() after fallback_to_current_page_size()
>> and reverts the commit: b98e6e0d2a(sadump: set info->page_size before cache_init())
>>
>> Signed-off-by: Atsushi Kumagai <ats-kumagai@wm.jp.nec.com>
>> ---
>>  makedumpfile.c | 28 ++++++++++++++++------------
>>  1 file changed, 16 insertions(+), 12 deletions(-)
>>
>> diff --git a/makedumpfile.c b/makedumpfile.c
>> index a3f0697..f85003a 100644
>> --- a/makedumpfile.c
>> +++ b/makedumpfile.c
>> @@ -3905,15 +3905,6 @@ initial(void)
>>  	if (!get_value_for_old_linux())
>>  		return FALSE;
>>
>> -	if (info->flag_sadump && !set_page_size(sadump_page_size()))
>> -			return FALSE;
>> -
>> -	if (!is_xen_memory() && !cache_init())
>> -		return FALSE;
>> -
>> -	if (info->flag_mem_usage && !get_kcore_dump_loads())
>> -		return FALSE;
>> -
>>  	if (info->flag_refiltering) {
>>  		if (info->flag_elf_dumpfile) {
>>  			MSG("'-E' option is disable, ");
>> @@ -3936,6 +3927,9 @@ initial(void)
>>  			return FALSE;
>>  		}
>>
>> +		if (!set_page_size(sadump_page_size()))
>> +			return FALSE;
>> +
>>  		if (!sadump_initialize_bitmap_memory())
>>  			return FALSE;
>>
>> @@ -3949,9 +3943,7 @@ initial(void)
>>  		 * postponed until debug information becomes
>>  		 * available.
>>  		 */
>> -
>> -	} else if (!get_phys_base())
>> -		return FALSE;
>> +	}
>>
>>  out:
>>  	if (!info->page_size) {
>> @@ -3962,6 +3954,18 @@ out:
>>  		if (!fallback_to_current_page_size())
>>  			return FALSE;
>>  	}
>> +
>> +	if (!is_xen_memory() && !cache_init())
>> +		return FALSE;
>> +
>> +	if (info->flag_mem_usage && !get_kcore_dump_loads())
>> +		return FALSE;
>> +
>> +	if (!info->flag_refiltering && !info->flag_sadump) {
>> +		if (!get_phys_base())
>> +			return FALSE;
>> +	}
>> +
>>  	if (!get_max_mapnr())
>>  		return FALSE;
>>
>>
>
>--
>Regards
>Pratyush



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

RE: [Makedumpfile Patch v3 2/7] initial(): call cach_init() a bit early

[Hatayama, Daisuke]

> -----Original Message-----
> From: Atsushi Kumagai [mailto:ats-kumagai@wm.jp.nec.com]
> Sent: Thursday, July 20, 2017 3:54 PM
> To: Pratyush Anand <panand@redhat.com>
> Cc: dyoung@redhat.com; Hatayama, Daisuke
> <d.hatayama@jp.fujitsu.com>; kexec@lists.infradead.org; bhe@redhat.com
> Subject: RE: [Makedumpfile Patch v3 2/7] initial(): call cach_init() a bit
> early
> 
> >Hi Atsushi,
> >
> >On Wednesday 19 July 2017 01:16 PM, Atsushi Kumagai wrote:
> >> Hello Pratyush,
> >>
> >>> Call cach_init() before get_kcore_dump_loads(), because latter uses
> >>> cache_search().
> >>>
> >>> Call path is like this :
> >>> get_kcore_dump_loads() -> process_dump_load() -> vaddr_to_paddr() ->
> >>> vtop4_x86_64() -> readmem() -> cache_search()
> >>
> >> I found that the current cach_init() is too early, cache_init() should be
> >> called after fallback_to_current_page_size() for older kernel
> >> which doesn't have VMCOREINFO.
> >> Could you confirm that the patch below is fine with you ?
> >
> >Its fine and works.
> 
> Thanks, I'll merge this fix into v1.6.2.
> 

It looks good also to me.

Thanks.
HATAYAMA, Daisuke

> Regards,
> Atsushi Kumagai
> 
> >>
> >> Thanks,
> >> Atsushi Kumagai
> >>
> >> From: Atsushi Kumagai <ats-kumagai@wm.jp.nec.com>
> >> Date: Wed, 19 Jul 2017 15:05:58 +0900
> >> Subject: [PATCH] Fix the regression caused by moving cache_init() earlier
> >>
> >> info->page_size should be set before calling cache_init() since it
> >> allocates some buffers based on info->page_size. However, cache_init()
> >> was moved before fallback_to_current_page_size() while it set
> >> info->page_size for older kernel which doesn't have VMCOREINFO.
> >>
> >> This patch moves cache_init() after fallback_to_current_page_size()
> >> and reverts the commit: b98e6e0d2a(sadump: set info->page_size before
> cache_init())
> >>
> >> Signed-off-by: Atsushi Kumagai <ats-kumagai@wm.jp.nec.com>
> >> ---
> >>  makedumpfile.c | 28 ++++++++++++++++------------
> >>  1 file changed, 16 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/makedumpfile.c b/makedumpfile.c
> >> index a3f0697..f85003a 100644
> >> --- a/makedumpfile.c
> >> +++ b/makedumpfile.c
> >> @@ -3905,15 +3905,6 @@ initial(void)
> >>  	if (!get_value_for_old_linux())
> >>  		return FALSE;
> >>
> >> -	if (info->flag_sadump && !set_page_size(sadump_page_size()))
> >> -			return FALSE;
> >> -
> >> -	if (!is_xen_memory() && !cache_init())
> >> -		return FALSE;
> >> -
> >> -	if (info->flag_mem_usage && !get_kcore_dump_loads())
> >> -		return FALSE;
> >> -
> >>  	if (info->flag_refiltering) {
> >>  		if (info->flag_elf_dumpfile) {
> >>  			MSG("'-E' option is disable, ");
> >> @@ -3936,6 +3927,9 @@ initial(void)
> >>  			return FALSE;
> >>  		}
> >>
> >> +		if (!set_page_size(sadump_page_size()))
> >> +			return FALSE;
> >> +
> >>  		if (!sadump_initialize_bitmap_memory())
> >>  			return FALSE;
> >>
> >> @@ -3949,9 +3943,7 @@ initial(void)
> >>  		 * postponed until debug information becomes
> >>  		 * available.
> >>  		 */
> >> -
> >> -	} else if (!get_phys_base())
> >> -		return FALSE;
> >> +	}
> >>
> >>  out:
> >>  	if (!info->page_size) {
> >> @@ -3962,6 +3954,18 @@ out:
> >>  		if (!fallback_to_current_page_size())
> >>  			return FALSE;
> >>  	}
> >> +
> >> +	if (!is_xen_memory() && !cache_init())
> >> +		return FALSE;
> >> +
> >> +	if (info->flag_mem_usage && !get_kcore_dump_loads())
> >> +		return FALSE;
> >> +
> >> +	if (!info->flag_refiltering && !info->flag_sadump) {
> >> +		if (!get_phys_base())
> >> +			return FALSE;
> >> +	}
> >> +
> >>  	if (!get_max_mapnr())
> >>  		return FALSE;
> >>
> >>
> >
> >--
> >Regards
> >Pratyush
> 
> 
> 



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

[Makedumpfile Patch v3 3/7] x86_64: check physical address in PT_LOAD for none direct mapped regions

[Pratyush Anand]

A kcore PT_LOAD can have a section from vmalloc region. However,
physical address in that header would be invalid (-1) after kernel
commit 464920104bf7 (/proc/kcore: update physical address for kcore ram
and text). Therefore, check for valid physical address while calculating
page_offset or phys_offset.

Signed-off-by: Pratyush Anand <panand@redhat.com>
---
 arch/x86_64.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86_64.c b/arch/x86_64.c
index 893cd516fc8b..e978a36f8878 100644
--- a/arch/x86_64.c
+++ b/arch/x86_64.c
@@ -41,7 +41,8 @@ get_page_offset_x86_64(void)
 	unsigned long long virt_start;
 
 	for (i = 0; get_pt_load(i, &phys_start, NULL, &virt_start, NULL); i++) {
-		if (virt_start < __START_KERNEL_map) {
+		if (virt_start < __START_KERNEL_map
+				&& phys_start != NOT_PADDR) {
 			info->page_offset = virt_start - phys_start;
 			return TRUE;
 		}
@@ -76,7 +77,8 @@ get_phys_base_x86_64(void)
 	}
 
 	for (i = 0; get_pt_load(i, &phys_start, NULL, &virt_start, NULL); i++) {
-		if (virt_start >= __START_KERNEL_map) {
+		if (virt_start >= __START_KERNEL_map
+				&& phys_start != NOT_PADDR) {
 
 			info->phys_base = phys_start -
 			    (virt_start & ~(__START_KERNEL_map));
-- 
2.9.3


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

[Makedumpfile Patch v3 4/7] elf_info: kcore: check for invalid physical address

[Pratyush Anand]

kcore passes correct phys_start for direct mapped region and an invalid
value (-1) for all other regions after the kernel commit
464920104bf7(/proc/kcore: update physical address for kcore ram and
text). arch specific function is_phys_addr() accepts only virt_start.
Therefore, check for valid phys_start in get_kcore_dump_loads().

Signed-off-by: Pratyush Anand <panand@redhat.com>
---
 elf_info.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/elf_info.c b/elf_info.c
index 65ff333cf33a..c5743b3cab28 100644
--- a/elf_info.c
+++ b/elf_info.c
@@ -881,7 +881,8 @@ int get_kcore_dump_loads(void)
 
 	for (i = 0; i < num_pt_loads; ++i) {
 		struct pt_load_segment *p = &pt_loads[i];
-		if (!is_phys_addr(p->virt_start))
+		if (p->phys_start == NOT_PADDR
+				|| !is_phys_addr(p->virt_start))
 			continue;
 		loads++;
 	}
@@ -901,7 +902,8 @@ int get_kcore_dump_loads(void)
 
 	for (i = 0, j = 0; i < num_pt_loads; ++i) {
 		struct pt_load_segment *p = &pt_loads[i];
-		if (!is_phys_addr(p->virt_start))
+		if (p->phys_start == NOT_PADDR
+				|| !is_phys_addr(p->virt_start))
 			continue;
 		if (j >= loads)
 			return FALSE;
-- 
2.9.3


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

[Makedumpfile Patch v3 5/7] makedumpfile: Correct the calculation of kvaddr in set_kcore_vmcoreinfo

[Pratyush Anand]

From: Baoquan He <bhe@redhat.com>

In set_kcore_vmcoreinfo, we calculate the virtual address of vmcoreinfo
by OR operation as below:

	kvaddr = (ulong)vmcoreinfo_addr | PAGE_OFFSET;

When mm sections kaslr is not enabled, this is correct since the
starting address of direct mapping section is 0xffff880000000000 which
is 1T aligned.  Usually system with memory below 1T won't cause problem.

However with mm section kaslr enabled, the starting address of direct
mapping is 1G aligned. The above code makes kvaddr unsure.

So change it to adding operation:
	kvaddr = (ulong)vmcoreinfo_addr + PAGE_OFFSET;

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 elf_info.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/elf_info.c b/elf_info.c
index c5743b3cab28..100272f83c48 100644
--- a/elf_info.c
+++ b/elf_info.c
@@ -372,7 +372,7 @@ int set_kcore_vmcoreinfo(uint64_t vmcoreinfo_addr, uint64_t vmcoreinfo_len)
 	off_t offset_desc;
 
 	offset = UNINITIALIZED;
-	kvaddr = (ulong)vmcoreinfo_addr | PAGE_OFFSET;
+	kvaddr = (ulong)vmcoreinfo_addr + PAGE_OFFSET;
 
 	for (i = 0; i < num_pt_loads; ++i) {
 		struct pt_load_segment *p = &pt_loads[i];
-- 
2.9.3


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

[Makedumpfile Patch v3 6/7] makedumpfile: Discard process_dump_load

[Pratyush Anand]

From: Baoquan He <bhe@redhat.com>

Kernel commit 464920104bf7 (/proc/kcore: update physical address for
kcore ram and text) provides physical address of direct mapping kcore
program segments. So no need to calculate it specifically now. And the
old code is not correct since it calls vaddr_to_paddr() which has not
been ready at that time.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 elf_info.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/elf_info.c b/elf_info.c
index 100272f83c48..8e2437622141 100644
--- a/elf_info.c
+++ b/elf_info.c
@@ -857,22 +857,6 @@ static int exclude_segment(struct pt_load_segment **pt_loads,
 	return 0;
 }
 
-static int
-process_dump_load(struct pt_load_segment	*pls)
-{
-	unsigned long long paddr;
-
-	paddr = vaddr_to_paddr(pls->virt_start);
-	pls->phys_start  = paddr;
-	pls->phys_end    = paddr + (pls->virt_end - pls->virt_start);
-	DEBUG_MSG("process_dump_load\n");
-	DEBUG_MSG("  phys_start : %llx\n", pls->phys_start);
-	DEBUG_MSG("  phys_end   : %llx\n", pls->phys_end);
-	DEBUG_MSG("  virt_start : %llx\n", pls->virt_start);
-	DEBUG_MSG("  virt_end   : %llx\n", pls->virt_end);
-
-	return TRUE;
-}
 
 int get_kcore_dump_loads(void)
 {
@@ -917,7 +901,6 @@ int get_kcore_dump_loads(void)
 		}
 
 		pls[j] = *p;
-		process_dump_load(&pls[j]);
 		j++;
 	}
 
-- 
2.9.3


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

[Makedumpfile Patch v3 7/7] mem-usage: allow to work only with -f for kernel version < 4.11

[Pratyush Anand]

PT_LOAD of kcore does not have valid p_paddr values for kernel version
less that v4.11. Therefore, older kernel will no long work for mem-usage
with current makedumpfile code. They can only work when they are patched
with fix to "update physical address for kcore ram and text".

This patch fixes the makedumpfile so that it does not allow to work
older kernel for --mem-usage until someone is sure that kernel is
rightly patched and so uses -f in command line. It also updates man page
and usage info accordingly.

Signed-off-by: Pratyush Anand <panand@redhat.com>
---
 makedumpfile.8 | 9 ++++++++-
 makedumpfile.c | 6 ++++++
 print_info.c   | 3 ++-
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/makedumpfile.8 b/makedumpfile.8
index 9069fb18cdb6..993236486e77 100644
--- a/makedumpfile.8
+++ b/makedumpfile.8
@@ -235,13 +235,20 @@ the ELF format does not support compressed data.
 
 .TP
 \fB\-f\fR
-Force existing DUMPFILE to be overwritten.
+Force existing DUMPFILE to be overwritten and mem-usage to work with older
+kernel as well.
 .br
 .B Example:
 .br
 # makedumpfile \-f \-d 31 \-x vmlinux /proc/vmcore dumpfile
 .br
 This command overwrites \fIDUMPFILE\fR even if it already exists.
+.br
+# makedumpfile \-f \-\-mem\-usage /proc/kcore
+.br
+Kernel version lesser than v4.11 will not work with \-\-mem\-usage
+functionality until it has been patched with upstream commit 464920104bf7.
+Therefore if you have patched your older kernel then use \-f.
 
 .TP
 \fB\-x\fR \fIVMLINUX\fR
diff --git a/makedumpfile.c b/makedumpfile.c
index 3b8e9810468d..e3be1ab0a9ec 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -11269,6 +11269,12 @@ main(int argc, char *argv[])
 			MSG("Try `makedumpfile --help' for more information.\n");
 			goto out;
 		}
+		if (info->kernel_version < KERNEL_VERSION(4, 11, 0) &&
+				!info->flag_force) {
+			MSG("mem-usage not supported for this kernel.\n");
+			MSG("You can try with -f if your kernel's kcore has valid p_paddr\n");
+			goto out;
+		}
 
 		if (!show_mem_usage())
 			goto out;
diff --git a/print_info.c b/print_info.c
index 392d863a4227..72ed8fa0c059 100644
--- a/print_info.c
+++ b/print_info.c
@@ -309,7 +309,8 @@ print_usage(void)
 	MSG("      Print debugging message.\n");
 	MSG("\n");
 	MSG("  [-f]:\n");
-	MSG("      Overwrite DUMPFILE even if it already exists.\n");
+	MSG("      Overwrite DUMPFILE even if it already exists\n");
+	MSG("      Force mem-usage to work with older kernel as well.\n");
 	MSG("\n");
 	MSG("  [-h, --help]:\n");
 	MSG("      Show help message and LZO/snappy support status (enabled/disabled).\n");
-- 
2.9.3


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

RE: [Makedumpfile Patch v3 7/7] mem-usage: allow to work only with -f for kernel version < 4.11

[Atsushi Kumagai]

>PT_LOAD of kcore does not have valid p_paddr values for kernel version
>less that v4.11. Therefore, older kernel will no long work for mem-usage
>with current makedumpfile code. They can only work when they are patched
>with fix to "update physical address for kcore ram and text".
>
>This patch fixes the makedumpfile so that it does not allow to work
>older kernel for --mem-usage until someone is sure that kernel is
>rightly patched and so uses -f in command line. It also updates man page
>and usage info accordingly.
>
>Signed-off-by: Pratyush Anand <panand@redhat.com>
>---
> makedumpfile.8 | 9 ++++++++-
> makedumpfile.c | 6 ++++++
> print_info.c   | 3 ++-
> 3 files changed, 16 insertions(+), 2 deletions(-)
>
>diff --git a/makedumpfile.8 b/makedumpfile.8
>index 9069fb18cdb6..993236486e77 100644
>--- a/makedumpfile.8
>+++ b/makedumpfile.8
>@@ -235,13 +235,20 @@ the ELF format does not support compressed data.
>
> .TP
> \fB\-f\fR
>-Force existing DUMPFILE to be overwritten.
>+Force existing DUMPFILE to be overwritten and mem-usage to work with older
>+kernel as well.
> .br
> .B Example:
> .br
> # makedumpfile \-f \-d 31 \-x vmlinux /proc/vmcore dumpfile
> .br
> This command overwrites \fIDUMPFILE\fR even if it already exists.
>+.br
>+# makedumpfile \-f \-\-mem\-usage /proc/kcore
>+.br
>+Kernel version lesser than v4.11 will not work with \-\-mem\-usage
>+functionality until it has been patched with upstream commit 464920104bf7.
>+Therefore if you have patched your older kernel then use \-f.
>
> .TP
> \fB\-x\fR \fIVMLINUX\fR
>diff --git a/makedumpfile.c b/makedumpfile.c
>index 3b8e9810468d..e3be1ab0a9ec 100644
>--- a/makedumpfile.c
>+++ b/makedumpfile.c
>@@ -11269,6 +11269,12 @@ main(int argc, char *argv[])
> 			MSG("Try `makedumpfile --help' for more information.\n");
> 			goto out;
> 		}
>+		if (info->kernel_version < KERNEL_VERSION(4, 11, 0) &&
>+				!info->flag_force) {
>+			MSG("mem-usage not supported for this kernel.\n");
>+			MSG("You can try with -f if your kernel's kcore has valid p_paddr\n");
>+			goto out;
>+		}

You forgot to set COMPLETED to retcd before goto.
The behavior will be different from the v2 patch.


Thanks,
Atsushi Kumagai

>
> 		if (!show_mem_usage())
> 			goto out;
>diff --git a/print_info.c b/print_info.c
>index 392d863a4227..72ed8fa0c059 100644
>--- a/print_info.c
>+++ b/print_info.c
>@@ -309,7 +309,8 @@ print_usage(void)
> 	MSG("      Print debugging message.\n");
> 	MSG("\n");
> 	MSG("  [-f]:\n");
>-	MSG("      Overwrite DUMPFILE even if it already exists.\n");
>+	MSG("      Overwrite DUMPFILE even if it already exists\n");
>+	MSG("      Force mem-usage to work with older kernel as well.\n");
> 	MSG("\n");
> 	MSG("  [-h, --help]:\n");
> 	MSG("      Show help message and LZO/snappy support status (enabled/disabled).\n");
>--
>2.9.3
>


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

Re: [Makedumpfile Patch v3 7/7] mem-usage: allow to work only with -f for kernel version < 4.11

[Pratyush Anand]

On Friday 03 March 2017 07:40 AM, Atsushi Kumagai wrote:
>> +		if (info->kernel_version < KERNEL_VERSION(4, 11, 0) &&
>> +				!info->flag_force) {
>> +			MSG("mem-usage not supported for this kernel.\n");
>> +			MSG("You can try with -f if your kernel's kcore has valid p_paddr\n");
>> +			goto out;
>> +		}
> You forgot to set COMPLETED to retcd before goto.
> The behavior will be different from the v2 patch.


I had thought about it. Should not an unsupported feature be a FAILED case?

~Pratyush

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

Re: [Makedumpfile Patch v3 7/7] mem-usage: allow to work only with -f for kernel version < 4.11

[bhe]

On 03/03/17 at 08:18am, Pratyush Anand wrote:
> 
> 
> On Friday 03 March 2017 07:40 AM, Atsushi Kumagai wrote:
> > > +		if (info->kernel_version < KERNEL_VERSION(4, 11, 0) &&
> > > +				!info->flag_force) {
> > > +			MSG("mem-usage not supported for this kernel.\n");
> > > +			MSG("You can try with -f if your kernel's kcore has valid p_paddr\n");
> > > +			goto out;
> > > +		}
> > You forgot to set COMPLETED to retcd before goto.
> > The behavior will be different from the v2 patch.
> 
> 
> I had thought about it. Should not an unsupported feature be a FAILED case?


Judging from the result, it should be a failed case. People expect to
get a dumped vmcore, it failed to collect at last because of unmatched
kernel and tool.


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

RE: [Makedumpfile Patch v3 7/7] mem-usage: allow to work only with -f for kernel version < 4.11

[Atsushi Kumagai]

>> On Friday 03 March 2017 07:40 AM, Atsushi Kumagai wrote:
>> > > +		if (info->kernel_version < KERNEL_VERSION(4, 11, 0) &&
>> > > +				!info->flag_force) {
>> > > +			MSG("mem-usage not supported for this kernel.\n");
>> > > +			MSG("You can try with -f if your kernel's kcore has valid p_paddr\n");
>> > > +			goto out;
>> > > +		}
>> > You forgot to set COMPLETED to retcd before goto.
>> > The behavior will be different from the v2 patch.
>>
>>
>> I had thought about it. Should not an unsupported feature be a FAILED case?
>
>
>Judging from the result, it should be a failed case. People expect to
>get a dumped vmcore, it failed to collect at last because of unmatched
>kernel and tool.

When you change the design, please note it into changelog, 
it's a review point of view.

  Changes since v2:
    - Fixed a memory leak issue and updated man page and usage info

As for the return value, I guess both is OK if there is a clear intention.
If this is what you intended, I have no objection.
(it actually sounds more reasonable)

So I'll merge the v3 patch into v1.6.2, thanks for your work.


Regards,
Atsushi Kumagai


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

Re: [Makedumpfile Patch v3 7/7] mem-usage: allow to work only with -f for kernel version < 4.11

[Pratyush Anand]

On Friday 03 March 2017 10:05 AM, Atsushi Kumagai wrote:
> When you change the design, please note it into changelog,
> it's a review point of view.
>
>   Changes since v2:
>     - Fixed a memory leak issue and updated man page and usage info

Sorry about that!!

>
> As for the return value, I guess both is OK if there is a clear intention.
> If this is what you intended, I have no objection.
> (it actually sounds more reasonable)
>
> So I'll merge the v3 patch into v1.6.2, thanks for your work.


and thanks :-)


~Pratyush

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