[PATCH] makedumpfile: Error on re-filtering the dump file with no free pages

[PATCH] makedumpfile: Error on re-filtering the dump file with no free pages

[Zaslonko Mikhail]

[-- Attachment #1: Type: text/plain, Size: 39 bytes --]

Please find the enclosed patch below.


[-- Attachment #2: 0001-makedumpfile-Error-on-re-filtering-the-dump-file-wit.patch --]
[-- Type: text/plain, Size: 2501 bytes --]

From 627b4c8085269251f6d66627dada447909bceafd Mon Sep 17 00:00:00 2001
From: root <root@s8345025.boeblingen.de.ibm.com>
Date: Tue, 16 May 2017 14:08:14 +0200
Subject: [PATCH] makedumpfile: Error on re-filtering the dump file with no
 free pages

When re-filtering the dump file after the free pages have already been
stripped out we get an error "Can't get necessary symbols for excluding
free pages" if newly specified dump level is below 16 (preserves free
pages).
According to the code, the check for the new dump level is done BEFORE
the new dump level is actually set (based on the dump level specified in
the command and the one from the input dump file).
Moving the new_dump_level calculation ahead would fix the error.

Signed-off-by: Mikhail Zaslonko <zaslonko@linux.vnet.ibm.com>
---
 makedumpfile.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index e69b6df..24f99fc 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -9774,10 +9774,25 @@ writeout_multiple_dumpfiles(void)
 	return ret;
 }

+void
+update_dump_level(void)
+{
+	int new_level;
+
+	new_level = info->dump_level | info->kh_memory->dump_level;
+	if (new_level != info->dump_level) {
+		info->dump_level = new_level;
+		MSG("dump_level is changed to %d, " \
+			"because %s was created by dump_level(%d).",
+			new_level, info->name_memory,
+			info->kh_memory->dump_level);
+	}
+}
+
 int
 create_dumpfile(void)
 {
-	int num_retry, status, new_level;
+	int num_retry, status;

 	if (!open_files_for_creating_dumpfile())
 		return FALSE;
@@ -9786,6 +9801,10 @@ create_dumpfile(void)
 		if (!get_elf_info(info->fd_memory, info->name_memory))
 			return FALSE;
 	}
+
+	if (info->flag_refiltering)
+		update_dump_level();
+
 	if (!initial())
 		return FALSE;

@@ -9804,17 +9823,8 @@ create_dumpfile(void)

 	num_retry = 0;
 retry:
-	if (info->flag_refiltering) {
-		/* Change dump level */
-		new_level = info->dump_level | info->kh_memory->dump_level;
-		if (new_level != info->dump_level) {
-			info->dump_level = new_level;
-			MSG("dump_level is changed to %d, " \
-				"because %s was created by dump_level(%d).",
-				new_level, info->name_memory,
-				info->kh_memory->dump_level);
-		}
-	}
+	if (info->flag_refiltering)
+		update_dump_level();

 	if ((info->name_filterconfig || info->name_eppic_config)
 			&& !gather_filter_info())
-- 
1.8.3.1


[-- Attachment #3: Type: text/plain, Size: 143 bytes --]

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

RE: [PATCH] makedumpfile: Error on re-filtering the dump file with no free pages

[Atsushi Kumagai]

Hello Zaslonko,

> When re-filtering the dump file after the free pages have already been
> stripped out we get an error "Can't get necessary symbols for excluding
> free pages" if newly specified dump level is below 16 (preserves free
> pages).
> According to the code, the check for the new dump level is done BEFORE
> the new dump level is actually set (based on the dump level specified in
> the command and the one from the input dump file).
> Moving the new_dump_level calculation ahead would fix the error.

Well, I guess the real problem is different.
The error you said is printed by exclude_free_page():

        if ((SYMBOL(node_data) == NOT_FOUND_SYMBOL)
            && (SYMBOL(pgdat_list) == NOT_FOUND_SYMBOL)
            && (SYMBOL(contig_page_data) == NOT_FOUND_SYMBOL)) {
                ERRMSG("Can't get necessary symbols for excluding free pages.\n");
                return FALSE;
        }

I think the availability of these symbols are not related to free pages.
exclude_free_page() is called if info->page_is_buddy is null, I estimate that
this situation occurs only if the kernel is older(2.6.16 or before). 

However, I don't think you use such too old kernel, so I suspect that
setup_page_is_buddy() should be updated for newer kernel.
Could you tell me your kernel version and how to reproduce this issue ?


Thanks,
Atsushi Kumagai

> Signed-off-by: Mikhail Zaslonko <zaslonko@linux.vnet.ibm.com>
> ---
>  makedumpfile.c | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index e69b6df..24f99fc 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -9774,10 +9774,25 @@ writeout_multiple_dumpfiles(void)
>  	return ret;
>  }
> 
> +void
> +update_dump_level(void)
> +{
> +	int new_level;
> +
> +	new_level = info->dump_level | info->kh_memory->dump_level;
> +	if (new_level != info->dump_level) {
> +		info->dump_level = new_level;
> +		MSG("dump_level is changed to %d, " \
> +			"because %s was created by dump_level(%d).",
> +			new_level, info->name_memory,
> +			info->kh_memory->dump_level);
> +	}
> +}
> +
>  int
>  create_dumpfile(void)
>  {
> -	int num_retry, status, new_level;
> +	int num_retry, status;
> 
>  	if (!open_files_for_creating_dumpfile())
>  		return FALSE;
> @@ -9786,6 +9801,10 @@ create_dumpfile(void)
>  		if (!get_elf_info(info->fd_memory, info->name_memory))
>  			return FALSE;
>  	}
> +
> +	if (info->flag_refiltering)
> +		update_dump_level();
> +
>  	if (!initial())
>  		return FALSE;
> 
> @@ -9804,17 +9823,8 @@ create_dumpfile(void)
> 
>  	num_retry = 0;
>  retry:
> -	if (info->flag_refiltering) {
> -		/* Change dump level */
> -		new_level = info->dump_level | info->kh_memory->dump_level;
> -		if (new_level != info->dump_level) {
> -			info->dump_level = new_level;
> -			MSG("dump_level is changed to %d, " \
> -				"because %s was created by dump_level(%d).",
> -				new_level, info->name_memory,
> -				info->kh_memory->dump_level);
> -		}
> -	}
> +	if (info->flag_refiltering)
> +		update_dump_level();
> 
>  	if ((info->name_filterconfig || info->name_eppic_config)
>  			&& !gather_filter_info())
> -- 
> 1.8.3.1
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] makedumpfile: Error on re-filtering the dump file with no free pages

[Michael Holzheu]

Am Thu, 18 May 2017 04:37:20 +0000
schrieb Atsushi Kumagai <ats-kumagai@wm.jp.nec.com>:

> Hello Zaslonko,
> 
> > When re-filtering the dump file after the free pages have already been
> > stripped out we get an error "Can't get necessary symbols for excluding
> > free pages" if newly specified dump level is below 16 (preserves free
> > pages).
> > According to the code, the check for the new dump level is done BEFORE
> > the new dump level is actually set (based on the dump level specified in
> > the command and the one from the input dump file).
> > Moving the new_dump_level calculation ahead would fix the error.
> 
> Well, I guess the real problem is different.
> The error you said is printed by exclude_free_page():
> 
>         if ((SYMBOL(node_data) == NOT_FOUND_SYMBOL)
>             && (SYMBOL(pgdat_list) == NOT_FOUND_SYMBOL)
>             && (SYMBOL(contig_page_data) == NOT_FOUND_SYMBOL)) {
>                 ERRMSG("Can't get necessary symbols for excluding free pages.\n");
>                 return FALSE;
>         }
> 
> I think the availability of these symbols are not related to free pages.
> exclude_free_page() is called if info->page_is_buddy is null, I estimate that
> this situation occurs only if the kernel is older(2.6.16 or before). 
> 
> However, I don't think you use such too old kernel, so I suspect that
> setup_page_is_buddy() should be updated for newer kernel.

Mikhail is on vacation now - so I try to explain:

The test case is as follows:

 1) We have a -d31 filtered dump "dump.d31"
 2) We want to compress the dump with "makedumpfile -c dump.31 dump31.compressed"

This fails with:

makedumpfile -c dump.31 dump.31.compressed
Excluding unnecessary pages        : [100.0 %]
exclude_free_page: Can't get necessary symbols for excluding free pages.
dump_level is changed to 31, because dump.31 was created by dump_level(31).
makedumpfile Failed.

The problem is that setup_page_is_buddy() is not called in this case because
info->dump_level is still 0 since it was not adjusted early enough:

        if (info->dump_level & DL_EXCLUDE_FREE)
                setup_page_is_buddy();

Because it is not set info->page_is_buddy is NULL and therefore the following
if condition gets true:

        if ((info->dump_level & DL_EXCLUDE_FREE) && !info->page_is_buddy)
                if (!exclude_free_page(cycle))
                        return FALSE;

Since we don't have the symbols in VMCOREINFO (and IMHO don't need it?) the
exclude_free_page() functions fails with the described error message.

So our fix is to adjust the info->level before setup_page_is_buddy() is called.

Best Regards
Michael

> Could you tell me your kernel version and how to reproduce this issue ?



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

RE: [PATCH] makedumpfile: Error on re-filtering the dump file with no free pages

[Atsushi Kumagai]

>> Hello Zaslonko,
>>
>> > When re-filtering the dump file after the free pages have already been
>> > stripped out we get an error "Can't get necessary symbols for excluding
>> > free pages" if newly specified dump level is below 16 (preserves free
>> > pages).
>> > According to the code, the check for the new dump level is done BEFORE
>> > the new dump level is actually set (based on the dump level specified in
>> > the command and the one from the input dump file).
>> > Moving the new_dump_level calculation ahead would fix the error.
>>
>> Well, I guess the real problem is different.
>> The error you said is printed by exclude_free_page():
>>
>>         if ((SYMBOL(node_data) == NOT_FOUND_SYMBOL)
>>             && (SYMBOL(pgdat_list) == NOT_FOUND_SYMBOL)
>>             && (SYMBOL(contig_page_data) == NOT_FOUND_SYMBOL)) {
>>                 ERRMSG("Can't get necessary symbols for excluding free pages.\n");
>>                 return FALSE;
>>         }
>>
>> I think the availability of these symbols are not related to free pages.
>> exclude_free_page() is called if info->page_is_buddy is null, I estimate that
>> this situation occurs only if the kernel is older(2.6.16 or before).
>>
>> However, I don't think you use such too old kernel, so I suspect that
>> setup_page_is_buddy() should be updated for newer kernel.
>
>Mikhail is on vacation now - so I try to explain:

Thanks for your explanation, I understand this issue properly.

>The test case is as follows:
>
> 1) We have a -d31 filtered dump "dump.d31"
> 2) We want to compress the dump with "makedumpfile -c dump.31 dump31.compressed"
>
>This fails with:
>
>makedumpfile -c dump.31 dump.31.compressed
>Excluding unnecessary pages        : [100.0 %]
>exclude_free_page: Can't get necessary symbols for excluding free pages.
>dump_level is changed to 31, because dump.31 was created by dump_level(31).
>makedumpfile Failed.
>
>The problem is that setup_page_is_buddy() is not called in this case because
>info->dump_level is still 0 since it was not adjusted early enough:
>
>        if (info->dump_level & DL_EXCLUDE_FREE)
>                setup_page_is_buddy();
>
>Because it is not set info->page_is_buddy is NULL and therefore the following
>if condition gets true:
>
>        if ((info->dump_level & DL_EXCLUDE_FREE) && !info->page_is_buddy)
>                if (!exclude_free_page(cycle))
>                        return FALSE;
>
>Since we don't have the symbols in VMCOREINFO (and IMHO don't need it?) the
>exclude_free_page() functions fails with the described error message.

It seems that it's better if I update the condition check of exclude_free_page()
for recent kernels, but those symbols are unnecessary in this case as you thought
anyway. exclude_free_page() shouldn't be called for recent kernels, I don't think
this is an actual problem.

>So our fix is to adjust the info->level before setup_page_is_buddy() is called.

I'm sure this fix is reasonable, I'll merge this into v1.6.2.

Thanks,
Atsushi Kumagai


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

Re: [PATCH] makedumpfile: Error on re-filtering the dump file with no free pages

[Michael Holzheu]

Am Fri, 19 May 2017 02:35:40 +0000
schrieb Atsushi Kumagai <ats-kumagai@wm.jp.nec.com>:

> >> Hello Zaslonko,
> >>

[snip]

> >Since we don't have the symbols in VMCOREINFO (and IMHO don't need it?) the
> >exclude_free_page() functions fails with the described error message.
> 
> It seems that it's better if I update the condition check of exclude_free_page()
> for recent kernels, but those symbols are unnecessary in this case as you thought
> anyway. exclude_free_page() shouldn't be called for recent kernels, I don't think
> this is an actual problem.
> 
> >So our fix is to adjust the info->level before setup_page_is_buddy() is called.
> 
> I'm sure this fix is reasonable, I'll merge this into v1.6.2.

Thanks!

Michael


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