[Crash-utility] [PATCH] diskdump: use mmap/madvise to improve the start-up

[HAGIO KAZUHITO(萩尾 一仁)]

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 <shijie at os.amperecomputing.com>
> ---
> 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