On 10/16/2013 03:22 AM, Dave Anderson wrote:

Hello Jingbai,

I have a few additional comments below:

----- Original Message -----
The patch will add support for new compressed dumpfile header_version 6.

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 corresponding patch for makedumpfile can be found here:
http://lists.infradead.org/pipermail/kexec/2013-October/009727.html

Changelog:
v3:
- Fix a bug that failed to work with old split format kdumps.

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.
- Add a 64bit max_mapnr in struct diskdump_data. The max_mapnr_64 in
   the sub-header only exists in compressed kdump file format, so can't
   be used in diskdump file format.
- Merge a patch from Dave Anderson that fixed bitmap_len issue.

v1:
- http://lists.infradead.org/pipermail/kexec/2013-September/009663.html

Signed-off-by: Jingbai Ma<[email protected]>
Tested-by: Lisa Mitchell<[email protected]>
---
  diskdump.c |  122
  ++++++++++++++++++++++++++++++++++++++++++++++++------------
  diskdump.h |   17 +++++++-
  2 files changed, 112 insertions(+), 27 deletions(-)

diff --git a/diskdump.c b/diskdump.c
index 0819a3f..bb7a33e 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -40,11 +40,13 @@ struct diskdump_data {
        struct disk_dump_sub_header     *sub_header;
        struct kdump_sub_header         *sub_header_kdump;

+       unsigned long long      max_mapnr;      /* 64bit max_mapnr */
+
        size_t  data_offset;
        int     block_size;
        int     block_shift;
        char    *bitmap;
-       int     bitmap_len;
+       off_t   bitmap_len;
        char    *dumpable_bitmap;
        int     byte, bit;
        char    *compressed_page;       /* copy of compressed page data */
@@ -170,9 +172,9 @@ add_diskdump_data(char* name)
        dd->filename = name;

        if (CRASHDEBUG(1))
-               fprintf(fp, "%s: start_pfn=%lu, end_pfn=%lu\n", name,
-                       dd->sub_header_kdump->start_pfn,
-                       dd->sub_header_kdump->end_pfn);
+               fprintf(fp, "%s: start_pfn=%llu, end_pfn=%llu\n", name,
+                       dd->sub_header_kdump->start_pfn_64,
+                       dd->sub_header_kdump->end_pfn_64);
  }

  static void
@@ -199,13 +201,13 @@ get_bit(char *map, int byte, int bit)
  }

  static inline int
-page_is_ram(unsigned int nr)
+page_is_ram(unsigned long nr)
  {
        return get_bit(dd->bitmap, nr>>  3, nr&  7);
  }

  static inline int
-page_is_dumpable(unsigned int nr)
+page_is_dumpable(unsigned long nr)
  {
        return dd->dumpable_bitmap[nr>>3]&  (1<<  (nr&  7));
  }
@@ -214,7 +216,7 @@ static inline int
  dump_is_partial(const struct disk_dump_header *header)
  {
        return header->bitmap_blocks>=
-           divideup(divideup(header->max_mapnr, 8), dd->block_size) * 2;
+           divideup(divideup(dd->max_mapnr, 8), dd->block_size) * 2;
  }

  static int
@@ -321,6 +323,9 @@ x86_process_elf_notes(void *note_ptr, unsigned long
size_note)
   * [40]    unsigned long   size_note;          /  header_version 4 and later
   /
   * [44]    off_t           offset_eraseinfo;   /  header_version 5 and later
   /
   * [52]    unsigned long   size_eraseinfo;     /  header_version 5 and later
   /
+ * [56]    unsigned long long   start_pfn_64;  /  header_version 6 and later
/
+ * [64]    unsigned long long   end_pfn_64;    /  header_version 6 and later
/
+ * [72]    unsigned long long   max_mapnr_64;  /  header_version 6 and later
/
   * };
   *
   * But when compiled on an ARM processor, each 64-bit "off_t" would be
   pushed
@@ -337,7 +342,10 @@ x86_process_elf_notes(void *note_ptr, unsigned long
size_note)
   * [40]    off_t           offset_note;        /  header_version 4 and later
   /
   * [48]    unsigned long   size_note;          /  header_version 4 and later
   /
   * [56]    off_t           offset_eraseinfo;   /  header_version 5 and later
   /
- * [62]    unsigned long   size_eraseinfo;     /  header_version 5 and later
/
+ * [64]    unsigned long   size_eraseinfo;     /  header_version 5 and later
/
+ * [72]    unsigned long long   start_pfn_64;  /  header_version 6 and later
/
+ * [80]    unsigned long long   end_pfn_64;    /  header_version 6 and later
/
+ * [88]    unsigned long long   max_mapnr_64;  /  header_version 6 and later
/
   * };
   *
   */
@@ -357,6 +365,10 @@ struct kdump_sub_header_ARM_target {
        int             pad3;
          off_t           offset_eraseinfo;   /* header_version 5 and later */
          unsigned long   size_eraseinfo;     /* header_version 5 and later */
+       int             pad4;
+       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 */
  };

  static void
@@ -380,6 +392,15 @@ arm_kdump_header_adjust(int header_version)
                kdsh->offset_eraseinfo = kdsh_ARM_target->offset_eraseinfo;
                kdsh->size_eraseinfo = kdsh_ARM_target->size_eraseinfo;
        }
+       if (header_version>= 6) {
+               kdsh->start_pfn_64 = kdsh_ARM_target->start_pfn_64;
+               kdsh->end_pfn_64 = kdsh_ARM_target->end_pfn_64;
+               kdsh->max_mapnr_64 = kdsh_ARM_target->map_mapnr_64;
+       } else {
+               kdsh->start_pfn_64 = kdsh_ARM_target->start_pfn;
+               kdsh->end_pfn_64 = kdsh_ARM_target->end_pfn;
+               kdsh->max_mapnr_64 = dd->map_mapnr;
+       }
  }
  #endif  /* __i386__&&  ARM */

@@ -390,7 +411,10 @@ read_dump_header(char *file)
        struct disk_dump_sub_header *sub_header = NULL;
        struct kdump_sub_header *sub_header_kdump = NULL;
        size_t size;
-       int bitmap_len;
+       off_t bitmap_len;
+       char *bufptr;
+       size_t len;
+       size_t bytes_read;
        int block_size = (int)sysconf(_SC_PAGESIZE);
        off_t offset;
        const off_t failed = (off_t)-1;
@@ -540,8 +564,27 @@ restart:
  #if defined(__i386__)&&  defined(ARM)
                arm_kdump_header_adjust(header->header_version);
  #endif
+               /* use 64bit max_mapnr in compressed kdump file sub-header */
+               if (header->header_version>= 6)
+                       dd->max_mapnr = dd->sub_header_kdump->max_mapnr_64;
+               else {
+                       dd->sub_header_kdump->start_pfn_64
+                               = dd->sub_header_kdump->start_pfn;
+                       dd->sub_header_kdump->end_pfn_64
+                               = dd->sub_header_kdump->end_pfn;
+               }
+       } else {
+               /* the 64bit max_mapnr only exists in sub-header of compressed
+                * kdump file, if it's not a compressed kdump file, we have to
+                * use the old 32bit max_mapnr in dumpfile header.
+                * max_mapnr may be truncated here.
+                */
+                dd->max_mapnr = header->max_mapnr;
        }

The additional "} else {" segment above doesn't make sense because either
DISKDUMP_VALID() or KDUMP_CMPRS_VALID() are true, so the "else" part will
never be executed.

The patch works because you follow the above section with this piece:

+       if (header->header_version<  6)
+               dd->max_mapnr = header->max_mapnr;
+
        /* read memory bitmap */
        bitmap_len = block_size * header->bitmap_blocks;
        dd->bitmap_len = bitmap_len;
@@ -571,11 +614,27 @@ restart:
                                DISKDUMP_VALID() ? "diskdump" : "compressed 
kdump");
                        goto err;
                }
+#ifdef OLDWAY
                if (read(dd->dfd, dd->bitmap, bitmap_len)<  bitmap_len) {
                        error(INFO, "%s: cannot read memory bitmap\n",
                                DISKDUMP_VALID() ? "diskdump" : "compressed 
kdump");
                        goto err;
                }
+#else
+               bufptr = dd->bitmap;
+               len = bitmap_len;
+               while (len) {
+                       bytes_read = read(dd->dfd, bufptr, len);
+                       if (bytes_read<  0) {
+                               error(INFO, "%s: cannot read memory bitmap\n",
+                                       DISKDUMP_VALID() ? "diskdump"
+                                       : "compressed kdump");
+                               goto err;
+                       }
+                       len -= bytes_read;
+                       bufptr += bytes_read;
+               }
+#endif

You can get rid of the "#ifdef OLDWAY" section -- I left that there when
I gave Lisa my debug version of the function so that the difference would
be obvious.

        }

        if (dump_is_partial(header))
@@ -679,13 +738,13 @@ restart:
        }

        if (!is_split) {
-               max_sect_len = divideup(header->max_mapnr, BITMAP_SECT_LEN);
+               max_sect_len = divideup(dd->max_mapnr, BITMAP_SECT_LEN);
                pfn = 0;
                dd->filename = file;
        }
        else {
-               ulong start = sub_header_kdump->start_pfn;
-               ulong end = sub_header_kdump->end_pfn;
+               unsigned long long start = sub_header_kdump->start_pfn_64;
+               unsigned long long end = sub_header_kdump->end_pfn_64;
                max_sect_len = divideup(end - start + 1, BITMAP_SECT_LEN);
                pfn = start;
        }
@@ -727,8 +786,9 @@ pfn_to_pos(ulong pfn)
        ulong p1, p2;

        if (KDUMP_SPLIT()) {
-               p1 = pfn - dd->sub_header_kdump->start_pfn;
-               p2 = round(p1, BITMAP_SECT_LEN) + 
dd->sub_header_kdump->start_pfn;
+               p1 = pfn - dd->sub_header_kdump->start_pfn_64;
+               p2 = round(p1, BITMAP_SECT_LEN)
+                       + dd->sub_header_kdump->start_pfn_64;
        }
        else {
                p1 = pfn;
@@ -1034,12 +1094,12 @@ read_diskdump(int fd, void *bufptr, int cnt, ulong
addr, physaddr_t paddr)
        if (KDUMP_SPLIT()) {
                /* Find proper dd */
                int i;
-               unsigned long start_pfn;
-               unsigned long end_pfn;
+               unsigned long long start_pfn;
+               unsigned long long end_pfn;

                for (i=0; i<num_dumpfiles; i++) {
-                       start_pfn = dd_list[i]->sub_header_kdump->start_pfn;
-                       end_pfn = dd_list[i]->sub_header_kdump->end_pfn;
+                       start_pfn = dd_list[i]->sub_header_kdump->start_pfn_64;
+                       end_pfn = dd_list[i]->sub_header_kdump->end_pfn_64;
                        if ((pfn>= start_pfn)&&  (pfn<= end_pfn)) {
                                dd = dd_list[i];
                                break;
@@ -1058,14 +1118,14 @@ read_diskdump(int fd, void *bufptr, int cnt, ulong
addr, physaddr_t paddr)
        curpaddr = paddr&  ~((physaddr_t)(dd->block_size-1));
        page_offset = paddr&  ((physaddr_t)(dd->block_size-1));

-       if ((pfn>= dd->header->max_mapnr) || !page_is_ram(pfn)) {
+       if ((pfn>= dd->max_mapnr) || !page_is_ram(pfn)) {
                if (CRASHDEBUG(8)) {
                        fprintf(fp, "read_diskdump: SEEK_ERROR: "
                            "paddr/pfn: %llx/%lx ",
                                (ulonglong)paddr, pfn);
-                       if (pfn>= dd->header->max_mapnr)
-                               fprintf(fp, "max_mapnr: %x\n",
-                                       dd->header->max_mapnr);
+                       if (pfn>= dd->max_mapnr)
+                               fprintf(fp, "max_mapnr: %llx\n",
+                                       dd->max_mapnr);
                        else
                                fprintf(fp, "!page_is_ram\n");
                }
@@ -1517,7 +1577,7 @@ __diskdump_memory_dump(FILE *fp)
        fprintf(fp, "          block_size: %d\n", dh->block_size);
        fprintf(fp, "        sub_hdr_size: %d\n", dh->sub_hdr_size);
        fprintf(fp, "       bitmap_blocks: %u\n", dh->bitmap_blocks);
-       fprintf(fp, "           max_mapnr: %u\n", dh->max_mapnr);
+       fprintf(fp, "           max_mapnr: %llu\n", dd->max_mapnr);

Please display the original dh->max_mapnr as it exists in the dumpfile
header, regardless whether it is the obsolete version or not.

Your new "dd->max_mapnr" should be displayed further down in the function
where the other diskdump_data fields are shown.

        fprintf(fp, "    total_ram_blocks: %u\n", dh->total_ram_blocks);
        fprintf(fp, "       device_blocks: %u\n", dh->device_blocks);
        fprintf(fp, "      written_blocks: %u\n", dh->written_blocks);
@@ -1662,6 +1722,20 @@ __diskdump_memory_dump(FILE *fp)
                                dump_eraseinfo(fp);
                        }
                }
+               if (dh->header_version>= 6) {
+                       fprintf(fp, "           start_pfn_64: ");
+                       if (KDUMP_SPLIT())
+                               fprintf(fp, "%lld (0x%llx)\n",
+                                       kdsh->start_pfn_64, kdsh->start_pfn_64);
+                       else
+                               fprintf(fp, "(unused)\n");
+                       fprintf(fp, "             end_pfn_64: ");
+                       if (KDUMP_SPLIT())
+                               fprintf(fp, "%lld (0x%llx)\n",
+                                       kdsh->end_pfn_64, kdsh->end_pfn_64);
+                       else
+                               fprintf(fp, "(unused)\n");
+               }
                fprintf(fp, "\n");
        } else
                fprintf(fp, "(n/a)\n\n");
@@ -1670,7 +1744,7 @@ __diskdump_memory_dump(FILE *fp)
        fprintf(fp, "        block_size: %d\n", dd->block_size);
        fprintf(fp, "       block_shift: %d\n", dd->block_shift);
        fprintf(fp, "            bitmap: %lx\n", (ulong)dd->bitmap);
-       fprintf(fp, "        bitmap_len: %d\n", dd->bitmap_len);
+       fprintf(fp, "        bitmap_len: %ld\n", dd->bitmap_len);
        fprintf(fp, "   dumpable_bitmap: %lx\n", (ulong)dd->dumpable_bitmap);
        fprintf(fp, "              byte: %d\n", dd->byte);
        fprintf(fp, "               bit: %d\n", dd->bit);
diff --git a/diskdump.h b/diskdump.h
index 9ab10b6..0492351 100644
--- a/diskdump.h
+++ b/diskdump.h
@@ -42,7 +42,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! */

You can change this comment to read "obsolete" so that it is similar to 
Daisuke's
suggestion for makedumpfile.

        unsigned int            total_ram_blocks;/* Number of blocks should be
                                                   written */
        unsigned int            device_blocks;  /* Number of total blocks in
@@ -61,14 +63,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.

Same here...

+                                              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 */

Thanks,
   Dave



I will fix all these issues and send out a new patch.
Thanks!


--
Thanks,
Jingbai Ma

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Reply via email to