(2013/10/11 18:46), Jingbai Ma wrote: > This patch will fix a bug of makedumpfile doesn't work correctly on system > has over 44-bit addressing in compression dump mode. > This bug was posted here: > http://lists.infradead.org/pipermail/kexec/2013-September/009587.html > > This patch will add 3 new fields in struct kdump_sub_header. > unsigned long long start_pfn_64; /* header_version 6 and later */ > unsigned long long end_pfn_64; /* header_version 6 and later */ > unsigned long long max_mapnr_64; /* header_version 6 and later */ > > And the old "unsigned int max_mapnr" in struct disk_dump_header will > not be used anymore, but still be there for compatibility purpose. > > The max_mapnr_64 only exists in strcut kdump_sub_header, and that only > for compressed kdump format, so for ELF format kdump files (non-compressed), > only the max_mapnr is available, so it still may be truncated for addresses > exceed 44bit (above 16TB). > > This patch will change the header_version to 6. > > The corresponding patch for crash utility will be sent out separately. > > This patch doesn't change sadump_header. > > Changelog: > v2: > - Rename max_mapnr in struct kdump_sub_header to max_mapnr_64. > - Change type of max_mapnr_64 from unsigned long to unsigned long long. > In x86 PAE mode on x86_32 kernel, the address may exceeds 44bit limit. > - Add start_pfn_64, end_pfn_64 for struct kdump_sub_header. > - Only print 64bit start_pfn_64, end_pfn_64 and max_mapnr_64 > debug messages for disk dump header version >= 6. > - Change type of bitmap_len in struct DumpInfo, from unsigned long to > unsigned long long. > - Enhance bitmap writting function in reassemble_kdump_header(). > Prevent bitmap writting failure if the size of bitmap is too large to > fit a sigle write. > > v1: > - http://lists.infradead.org/pipermail/kexec/2013-September/009662.html > > > Signed-off-by: Jingbai Ma <jingbai.ma at hp.com> > Tested-by: Lisa Mitchell <lisa.mitchell at hp.com> > --- > IMPLEMENTATION | 17 +++++++- > diskdump_mod.h | 17 +++++++- > makedumpfile.c | 116 +++++++++++++++++++++++++++++++++++++++++--------------- > makedumpfile.h | 2 - > 4 files changed, 113 insertions(+), 39 deletions(-) > > diff --git a/IMPLEMENTATION b/IMPLEMENTATION > index f0f3135..4feda02 100644 > --- a/IMPLEMENTATION > +++ b/IMPLEMENTATION > @@ -48,7 +48,9 @@ > header in blocks */ > unsigned int bitmap_blocks; /* Size of Memory bitmap in > block */ > - unsigned int max_mapnr; /* = max_mapnr */ > + unsigned int max_mapnr; /* = max_mapnr, 32bit only, > + full 64bit in sub header. > + Do not use this anymore! */ I think ``obsolete'' is more formal than ``Do not use this anymore!''. > unsigned int total_ram_blocks;/* Number of blocks should be > written */ > unsigned int device_blocks; /* Number of total blocks in > @@ -69,14 +71,23 @@ > unsigned long phys_base; > int dump_level; /* header_version 1 and later */ > int split; /* header_version 2 and later */ > - unsigned long start_pfn; /* header_version 2 and later */ > - unsigned long end_pfn; /* header_version 2 and later */ > + unsigned long start_pfn; /* header_version 2 and later, > + 32bit only, full 64bit in > + start_pfn_64. > + Do not use this anymore! */ > + unsigned long end_pfn; /* header_version 2 and later, > + 32bit only, full 64bit in > + end_pfn_64. > + Do not use this anymore! */ > off_t offset_vmcoreinfo;/* header_version 3 and later */ > unsigned long size_vmcoreinfo; /* header_version 3 and later */ > off_t offset_note; /* header_version 4 and later */ > unsigned long size_note; /* header_version 4 and later */ > off_t offset_eraseinfo; /* header_version 5 and later */ > unsigned long size_eraseinfo; /* header_version 5 and later */ > + unsigned long long start_pfn_64; /* header_version 6 and later */ > + unsigned long long end_pfn_64; /* header_version 6 and later */ > + unsigned long long max_mapnr_64; /* header_version 6 and later */ > }; > > - 1st-bitmap > diff --git a/diskdump_mod.h b/diskdump_mod.h > index af060b6..d907775 100644 > --- a/diskdump_mod.h > +++ b/diskdump_mod.h > @@ -48,7 +48,9 @@ struct disk_dump_header { > header in blocks */ > unsigned int bitmap_blocks; /* Size of Memory bitmap in > block */ > - unsigned int max_mapnr; /* = max_mapnr */ > + unsigned int max_mapnr; /* = max_mapnr, 32bit only, > + full 64bit in sub header. > + Do not use this anymore! */ > unsigned int total_ram_blocks;/* Number of blocks should be > written */ > unsigned int device_blocks; /* Number of total blocks in > @@ -67,14 +69,23 @@ struct kdump_sub_header { > unsigned long phys_base; > int dump_level; /* header_version 1 and later */ > int split; /* header_version 2 and later */ > - unsigned long start_pfn; /* header_version 2 and later */ > - unsigned long end_pfn; /* header_version 2 and later */ > + unsigned long start_pfn; /* header_version 2 and later, > + 32bit only, full 64bit in > + start_pfn_64. > + Do not use this anymore! */ > + unsigned long end_pfn; /* header_version 2 and later, > + 32bit only, full 64bit in > + end_pfn_64. > + Do not use this anymore! */ > off_t offset_vmcoreinfo;/* header_version 3 and later */ > unsigned long size_vmcoreinfo; /* header_version 3 and later */ > off_t offset_note; /* header_version 4 and later */ > unsigned long size_note; /* header_version 4 and later */ > off_t offset_eraseinfo; /* header_version 5 and later */ > unsigned long size_eraseinfo; /* header_version 5 and later */ > + unsigned long long start_pfn_64; /* header_version 6 and later */ > + unsigned long long end_pfn_64; /* header_version 6 and later */ > + unsigned long long max_mapnr_64; /* header_version 6 and later */ > }; > > /* page flags */ > diff --git a/makedumpfile.c b/makedumpfile.c > index b42565c..3ac3ad0 100644 > --- a/makedumpfile.c > +++ b/makedumpfile.c > @@ -23,6 +23,7 @@ > #include <stddef.h> > #include <ctype.h> > #include <sys/time.h> > +#include <limits.h> > > struct symbol_table symbol_table; > struct size_table size_table; > @@ -125,7 +126,7 @@ get_max_mapnr(void) > unsigned long long max_paddr; > > if (info->flag_refiltering) { > - info->max_mapnr = info->dh_memory->max_mapnr; > + info->max_mapnr = info->kh_memory->max_mapnr_64; > return TRUE; > } > > @@ -783,6 +784,10 @@ get_kdump_compressed_header_info(char *filename) > ERRMSG("header does not have dump_level member\n"); > return FALSE; > } > + > + if (dh.header_version < 6) > + kh.max_mapnr_64 = dh.max_mapnr; > + Don't do this. This debug message should output contents of header file with no modification. > DEBUG_MSG("diskdump main header\n"); > DEBUG_MSG(" signature : %s\n", dh.signature); > DEBUG_MSG(" header_version : %d\n", dh.header_version); > @@ -790,7 +795,7 @@ get_kdump_compressed_header_info(char *filename) > DEBUG_MSG(" block_size : %d\n", dh.block_size); > DEBUG_MSG(" sub_hdr_size : %d\n", dh.sub_hdr_size); > DEBUG_MSG(" bitmap_blocks : %d\n", dh.bitmap_blocks); > - DEBUG_MSG(" max_mapnr : 0x%x\n", dh.max_mapnr); > + DEBUG_MSG(" max_mapnr(32bit) : 0x%x\n", dh.max_mapnr); I don't think annotation "(32bit)" is necessary. > DEBUG_MSG(" total_ram_blocks : %d\n", dh.total_ram_blocks); > DEBUG_MSG(" device_blocks : %d\n", dh.device_blocks); > DEBUG_MSG(" written_blocks : %d\n", dh.written_blocks); > @@ -800,8 +805,14 @@ get_kdump_compressed_header_info(char *filename) > DEBUG_MSG(" phys_base : 0x%lx\n", kh.phys_base); > DEBUG_MSG(" dump_level : %d\n", kh.dump_level); > DEBUG_MSG(" split : %d\n", kh.split); > - DEBUG_MSG(" start_pfn : 0x%lx\n", kh.start_pfn); > - DEBUG_MSG(" end_pfn : 0x%lx\n", kh.end_pfn); > + DEBUG_MSG(" start_pfn(32bit) : 0x%lx\n", kh.start_pfn); > + DEBUG_MSG(" end_pfn(32bit) : 0x%lx\n", kh.end_pfn); These, too. > + if (dh.header_version >= 6) { > + /* A dumpfile contains full 64bit values. */ > + DEBUG_MSG(" start_pfn_64 : 0x%llx\n", kh.start_pfn_64); > + DEBUG_MSG(" end_pfn_64 : 0x%llx\n", kh.end_pfn_64); > + DEBUG_MSG(" max_mapnr_64 : 0x%llx\n", kh.max_mapnr_64); > + } > > info->dh_memory = malloc(sizeof(dh)); > if (info->dh_memory == NULL) { > @@ -2766,14 +2777,16 @@ int > initialize_bitmap_memory(void) > { > struct disk_dump_header *dh; > + struct kdump_sub_header *kh; > struct dump_bitmap *bmp; > off_t bitmap_offset; > - int bitmap_len, max_sect_len; > + off_t bitmap_len, max_sect_len; > unsigned long pfn; > int i, j; > long block_size; > > dh = info->dh_memory; > + kh = info->kh_memory; > block_size = dh->block_size; > > bitmap_offset > @@ -2793,7 +2806,7 @@ initialize_bitmap_memory(void) > bmp->offset = bitmap_offset + bitmap_len / 2; > info->bitmap_memory = bmp; > > - max_sect_len = divideup(dh->max_mapnr, BITMAP_SECT_LEN); > + max_sect_len = divideup(kh->max_mapnr_64, BITMAP_SECT_LEN); > info->valid_pages = calloc(sizeof(ulong), max_sect_len); > if (info->valid_pages == NULL) { > ERRMSG("Can't allocate memory for the valid_pages. %s\n", > @@ -4705,7 +4718,7 @@ create_2nd_bitmap(void) > int > prepare_bitmap_buffer(void) > { > - unsigned long tmp; > + unsigned long long tmp; > > /* > * Create 2 bitmaps (1st-bitmap & 2nd-bitmap) on block_size boundary. > @@ -4737,7 +4750,7 @@ prepare_bitmap_buffer(void) > int > prepare_bitmap_buffer_cyclic(void) > { > - unsigned long tmp; > + unsigned long long tmp; > > /* > * Create 2 bitmaps (1st-bitmap & 2nd-bitmap) on block_size boundary. > @@ -5153,11 +5166,12 @@ write_kdump_header(void) > * Write common header > */ > strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE)); > - dh->header_version = 5; > + dh->header_version = 6; > dh->block_size = info->page_size; > dh->sub_hdr_size = sizeof(kh) + size_note; > dh->sub_hdr_size = divideup(dh->sub_hdr_size, dh->block_size); > - dh->max_mapnr = info->max_mapnr; > + /* dh->max_mapnr may be truncated, full 64bit in kh.max_mapnr_64 */ > + dh->max_mapnr = MIN(info->max_mapnr, UINT_MAX); > dh->nr_cpus = get_nr_cpus(); > dh->bitmap_blocks = divideup(info->len_bitmap, dh->block_size); > memcpy(&dh->timestamp, &info->timestamp, sizeof(dh->timestamp)); > @@ -5172,12 +5186,21 @@ write_kdump_header(void) > */ > size = sizeof(struct kdump_sub_header); > memset(&kh, 0, size); > + /* 64bit max_mapnr_64 */ > + kh.max_mapnr_64 = info->max_mapnr; > kh.phys_base = info->phys_base; > kh.dump_level = info->dump_level; > if (info->flag_split) { > kh.split = 1; > + /* start_pfn and end_pfn may be truncated, > + * only for compatibility purpose > + */ > kh.start_pfn = info->split_start_pfn; > kh.end_pfn = info->split_end_pfn; Please: kh.start_pfn = MIN(info->split_start_pfn, UINT_MAX); kh.end_pfn = MIN(info->split_end_pfn, UINT_MAX); > + > + /* 64bit start_pfn_64 and end_pfn_64 */ > + kh.start_pfn_64 = info->split_start_pfn; > + kh.end_pfn_64 = info->split_end_pfn; > } > if (has_pt_note()) { > /* > @@ -6421,7 +6444,7 @@ int > write_kdump_bitmap(void) > { > struct cache_data bm; > - long buf_size; > + long long buf_size; > off_t offset; > > int ret = FALSE; > @@ -7796,10 +7819,8 @@ store_splitting_info(void) > > if (i == 0) { > memcpy(&dh, &tmp_dh, sizeof(tmp_dh)); > - info->max_mapnr = dh.max_mapnr; > if (!set_page_size(dh.block_size)) > return FALSE; > - DEBUG_MSG("max_mapnr : %llx\n", info->max_mapnr); > DEBUG_MSG("page_size : %ld\n", info->page_size); > } > > @@ -7816,11 +7837,26 @@ store_splitting_info(void) > return FALSE; > > if (i == 0) { > + if (dh.header_version >= 6) > + info->max_mapnr = kh.max_mapnr_64; > + else > + info->max_mapnr = dh.max_mapnr; > + > + DEBUG_MSG("max_mapnr : %llx\n", info->max_mapnr); > + } > + > + if (i == 0) { > info->dump_level = kh.dump_level; > DEBUG_MSG("dump_level : %d\n", info->dump_level); > } > - SPLITTING_START_PFN(i) = kh.start_pfn; > - SPLITTING_END_PFN(i) = kh.end_pfn; > + > + if (dh.header_version >= 6) { > + SPLITTING_START_PFN(i) = kh.start_pfn_64; > + SPLITTING_END_PFN(i) = kh.end_pfn_64; > + } else { > + SPLITTING_START_PFN(i) = kh.start_pfn; > + SPLITTING_END_PFN(i) = kh.end_pfn; > + } > SPLITTING_OFFSET_EI(i) = kh.offset_eraseinfo; > SPLITTING_SIZE_EI(i) = kh.size_eraseinfo; > } > @@ -7954,6 +7990,7 @@ reassemble_kdump_header(void) > struct disk_dump_header dh; > struct kdump_sub_header kh; > char *buf_bitmap = NULL; > + ssize_t status, read_size, written_size; > > /* > * Write common header. > @@ -7981,6 +8018,8 @@ reassemble_kdump_header(void) > kh.split = 0; > kh.start_pfn = 0; > kh.end_pfn = 0; > + kh.start_pfn_64 = 0; > + kh.end_pfn_64 = 0; > > if (lseek(info->fd_dumpfile, info->page_size, SEEK_SET) < 0) { > ERRMSG("Can't seek a file(%s). %s\n", > @@ -8030,36 +8069,49 @@ reassemble_kdump_header(void) > SPLITTING_DUMPFILE(0), strerror(errno)); > goto out; > } > - if (read(fd, buf_bitmap, info->len_bitmap) != info->len_bitmap) { > - ERRMSG("Can't read a file(%s). %s\n", > - SPLITTING_DUMPFILE(0), strerror(errno)); > - goto out; > + read_size = 0; > + while (read_size < info->len_bitmap) { > + status = read(fd, buf_bitmap + read_size, info->len_bitmap > + - read_size); > + if (status < 0) { > + ERRMSG("Can't read a file(%s). %s\n", > + SPLITTING_DUMPFILE(0), strerror(errno)); > + goto out; > + } > + read_size += status; This change is not relevant to topic of this patch. > } > - > if (lseek(info->fd_dumpfile, offset, SEEK_SET) < 0) { > ERRMSG("Can't seek a file(%s). %s\n", > info->name_dumpfile, strerror(errno)); > goto out; > } > - if (write(info->fd_dumpfile, buf_bitmap, info->len_bitmap) > - != info->len_bitmap) { > - ERRMSG("Can't write a file(%s). %s\n", > - info->name_dumpfile, strerror(errno)); > - goto out; > + written_size = 0; > + while (written_size < info->len_bitmap) { > + status = write(info->fd_dumpfile, buf_bitmap + written_size, > + info->len_bitmap - written_size); > + if (status < 0) { > + ERRMSG("Can't write a file(%s). %s\n", > + info->name_dumpfile, strerror(errno)); > + goto out; > + } > + written_size += status; This, too. > } > - > if (lseek(info->fd_bitmap, 0x0, SEEK_SET) < 0) { > ERRMSG("Can't seek a file(%s). %s\n", > info->name_bitmap, strerror(errno)); > goto out; > } > - if (write(info->fd_bitmap, buf_bitmap, info->len_bitmap) > - != info->len_bitmap) { > - ERRMSG("Can't write a file(%s). %s\n", > - info->name_bitmap, strerror(errno)); > - goto out; > + written_size = 0; > + while (written_size < info->len_bitmap) { > + status = write(info->fd_bitmap, buf_bitmap + written_size, > + info->len_bitmap - written_size); > + if (status < 0) { > + ERRMSG("Can't write a file(%s). %s\n", > + info->name_bitmap, strerror(errno)); > + goto out; > + } > + written_size += status; This, too. > } > - Unintended deletion? > ret = TRUE; > out: > if (fd > 0) > diff --git a/makedumpfile.h b/makedumpfile.h > index a5826e0..4056fa0 100644 > --- a/makedumpfile.h > +++ b/makedumpfile.h > @@ -927,7 +927,7 @@ struct DumpInfo { > */ > int block_order; > off_t offset_bitmap1; > - unsigned long len_bitmap; /* size of bitmap(1st and 2nd) */ > + unsigned long long len_bitmap; /* size of bitmap(1st and 2nd) */ unsigned long can still represent upto 128 TiB physical memory. This change doesn't seem definitely needed. > struct dump_bitmap *bitmap1; > struct dump_bitmap *bitmap2; > struct disk_dump_header *dump_header; > -- Thanks. HATAYAMA, Daisuke
