Hi Huang Shijie,

-----Original Message-----
> 1.) The bitmap of vmcore file can be very big in the server kernel panic,
>     such as over 256M.
> 
>     This patch uses mmap/madvise to improve the read speed
>     of the bitmap in the non-FLAT_FORMAT code path.
> 
>     Use MADV_SEQUENTIAL for madvise, it will trigger aggressively
>     readahead for reading the bitmap.

I'm not familiar with the detailed behavior of madvise() and a little
concerned about the sentence "may be freed soon after they are accessed"
below:

       MADV_SEQUENTIAL
              Expect page references in sequential order.   (Hence,  pages  in
              the given range can be aggressively read ahead, and may be freed
              soon after they are accessed.)

       MADV_WILLNEED
              Expect access in the near future.  (Hence, it might  be  a  good
              idea to read some pages ahead.)

dd->bitmap is often used after the initialization process, is there
no side-effect?

So if MADV_WILLNEED has same or some good effect, I think it will be
easier to accept.  Any benchmark and information?

Thanks,
Kazu

> 
> 2.) Test.
> 
>     The files:
>     vmlinux: 272M
>     vmcore : 23G (bitmap is 281018368 bytes)
> 
>  #cat ./commands.txt
>       quit
> 
>  Before this patch:
>     #echo  3 > /proc/sys/vm/drop_caches;
>     #time ./crash -i ./commands.txt /root/t/vmlinux /root/t/vmcore > 
> /dev/null 2>&1
>      ............................
>       real    0m55.217s
>       user    0m15.114s
>       sys     0m3.560s
>      ............................
> 
>  After this patch:
>     #echo  3 > /proc/sys/vm/drop_caches;
>     #time ./crash -i ./commands.txt /root/t/vmlinux /root/t/vmcore > 
> /dev/null 2>&1
>      ............................
>       real    0m46.781s
>       user    0m19.693s
>       sys     0m1.642s
>      ............................
> 
> Signed-off-by: Huang Shijie <[email protected]>
> ---
> This patch is based on patch:
>    diskdump: Optimize the boot time
> 
> ---
>  diskdump.c | 39 ++++++++++++++++-----------------------
>  1 file changed, 16 insertions(+), 23 deletions(-)
> 
> diff --git a/diskdump.c b/diskdump.c
> index d30db9d..c60b283 100644
> --- a/diskdump.c
> +++ b/diskdump.c
> @@ -724,10 +724,6 @@ restart:
> 
>       offset = (off_t)block_size * (1 + header->sub_hdr_size);
> 
> -     if ((dd->bitmap = malloc(bitmap_len)) == NULL)
> -             error(FATAL, "%s: cannot malloc bitmap buffer\n",
> -                     DISKDUMP_VALID() ? "diskdump" : "compressed kdump");
> -
>       dd->dumpable_bitmap = calloc(bitmap_len, 1);
> 
>       if (CRASHDEBUG(8))
> @@ -736,30 +732,23 @@ restart:
>                       (ulonglong)offset);
> 
>       if (FLAT_FORMAT()) {
> +             if ((dd->bitmap = malloc(bitmap_len)) == NULL)
> +                     error(FATAL, "%s: cannot malloc bitmap buffer\n",
> +                             DISKDUMP_VALID() ? "diskdump" : "compressed 
> kdump");
> +
>               if (!read_flattened_format(dd->dfd, offset, dd->bitmap, 
> bitmap_len)) {
>                       error(INFO, "%s: cannot read memory bitmap\n",
>                               DISKDUMP_VALID() ? "diskdump" : "compressed 
> kdump");
>                       goto err;
>               }
>       } else {
> -             if (lseek(dd->dfd, offset, SEEK_SET) == failed) {
> -                     error(INFO, "%s: cannot lseek memory bitmap\n",
> +             dd->bitmap = mmap(NULL, bitmap_len, PROT_READ,
> +                                     MAP_SHARED, dd->dfd, offset);
> +             if (dd->bitmap == MAP_FAILED)
> +                     error(FATAL, "%s: cannot mmap bitmap buffer\n",
>                               DISKDUMP_VALID() ? "diskdump" : "compressed 
> kdump");
> -                     goto err;
> -             }
> -             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;
> -             }
> +
> +             madvise(dd->bitmap, bitmap_len, MADV_SEQUENTIAL);
>       }
> 
>       if (dump_is_partial(header))
> @@ -920,8 +909,12 @@ err:
>               free(sub_header);
>       if (sub_header_kdump)
>               free(sub_header_kdump);
> -     if (dd->bitmap)
> -             free(dd->bitmap);
> +     if (dd->bitmap) {
> +             if (FLAT_FORMAT())
> +                     free(dd->bitmap);
> +             else
> +                     munmap(dd->bitmap, dd->bitmap_len);
> +     }
>       if (dd->dumpable_bitmap)
>               free(dd->dumpable_bitmap);
>       if (dd->notes_buf)
> --
> 2.30.2

--
Crash-utility mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki

Reply via email to