[PATCH 2/3] makedumpfile: Mark unstored ELF pages as filtered

[Atsushi Kumagai]

Hello Petr,

Sorry for my late comment:

>Pages that were filtered in the original dump file should also be filtered
>in the destination file, i.e. pages between p_filesz and p_memsz must have
>their corresponding bit cleared in the 2nd bitmap. Note that in theory,
>such ELF segments might not refer to filtered pages, because the crash
>kernel copies the program headers verbatim from elfcorehdr=. However, all
>common sources of /proc/vmcore program headers use the same value for each
>p_memsz and p_filesz:
>
>a. kexec(8)
>   Program headers are created in two places, but in both cases the value
>   of p_memsz is equal to p_filesz.
>   Reference: kexec/crashdump-elf.c in kexec-tools
>
>b. kexec_file_load(2)
>   Conceptually the same code as kexec(8): two places, both set p_filesz
>   and p_memsz to the same value.
>   Reference: kexec/crashdump-elf.c in Linux kernel
>
>c. created in the crash kernel (s390)
>   On s390, program headers are created in the crash kernel, but both
>   p_filesz and p_memsz are set to "end - start", i.e. the same value.
>   Reference: arch/s390/kernel/crash_dump.c in Linux kernel
>
>The above means that p_memsz > p_filesz only in ELF files that were
>processed by makedumpfile, hence it can be safely assumed that pages
>between p_filesz and p_memsz were filtered out.
>
>Signed-off-by: Petr Tesarik <ptesarik at suse.com>
>
>---
> elf_info.c     |   26 ++++++++++++++++++++++++++
> elf_info.h     |    5 +++++
> makedumpfile.c |   32 ++++++++++++++++++++++++++++++++
> 3 files changed, 63 insertions(+)
>
>--- a/elf_info.c
>+++ b/elf_info.c
>@@ -1096,6 +1096,32 @@ get_pt_load(int idx,
> 	return TRUE;
> }
>
>+int
>+get_pt_load_extents(int idx,
>+	unsigned long long *phys_start,
>+	unsigned long long *phys_end,
>+	off_t *file_offset,
>+	off_t *file_size)
>+{
>+	struct pt_load_segment *pls;
>+
>+	if (num_pt_loads <= idx)
>+		return FALSE;
>+
>+	pls = &pt_loads[idx];
>+
>+	if (phys_start)
>+		*phys_start  = pls->phys_start;
>+	if (phys_end)
>+		*phys_end    = pls->phys_end;
>+	if (file_offset)
>+		*file_offset = pls->file_offset;
>+	if (file_size)
>+		*file_size   = pls->file_size;
>+
>+	return TRUE;
>+}
>+
> unsigned int
> get_num_pt_loads(void)
> {
>--- a/elf_info.h
>+++ b/elf_info.h
>@@ -61,6 +61,11 @@ int get_pt_load(int idx,
> 	unsigned long long *phys_end,
> 	unsigned long long *virt_start,
> 	unsigned long long *virt_end);
>+int get_pt_load_extents(int idx,
>+	unsigned long long *phys_start,
>+	unsigned long long *phys_end,
>+	off_t *file_offset,
>+	off_t *file_size);
> unsigned int get_num_pt_loads(void);
>
> void set_nr_cpus(int num);
>--- a/makedumpfile.c
>+++ b/makedumpfile.c
>@@ -4395,6 +4395,32 @@ read_start_flat_header(void)
> 	return TRUE;
> }
>
>+static void
>+exclude_nodata_pages(struct cycle *cycle)
>+{
>+	int i;
>+	unsigned long long phys_start, phys_end;
>+	off_t file_size;
>+
>+	i = 0;
>+	while (get_pt_load_extents(i, &phys_start, &phys_end,
>+				   NULL, &file_size)) {
>+		unsigned long long pfn, pfn_end;
>+
>+		pfn = paddr_to_pfn(phys_start + file_size);
>+		pfn_end = paddr_to_pfn(phys_end);

Does this code exclude the first pfn of out of PT_LOAD even if there is
no unstored pages ? pfn and pfn_end will point at the next pfn to the
last pfn of PT_LOAD.
This will be problem for the environments which have a overlapped PT_LOAD
segment like ia64.

>+		if (pfn < cycle->start_pfn)
>+			pfn = cycle->start_pfn;
>+		if (pfn_end >= cycle->end_pfn)
>+			pfn_end = cycle->end_pfn - 1;
>+		while (pfn <= pfn_end) {

Should we change this condition to "pfn < pfn_end" ?

>+			clear_bit_on_2nd_bitmap(pfn, cycle);
>+			++pfn;
>+		}
>+		++i;
>+	}
>+}

Thanks,
Atsushi Kumagai

>+
> int
> read_flat_data_header(struct makedumpfile_data_header *fdh)
> {
>@@ -6087,6 +6113,12 @@ create_2nd_bitmap(struct cycle *cycle)
> 	}
>
> 	/*
>+	 * If re-filtering ELF dump, exclude pages that were already
>+	 * excluded in the original file.
>+	 */
>+	exclude_nodata_pages(cycle);
>+
>+	/*
> 	 * Exclude cache pages, cache private pages, user data pages,
> 	 * and hwpoison pages.
> 	 */
>
>_______________________________________________
>kexec mailing list
>kexec at lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/kexec