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

[HAGIO KAZUHITO(萩尾 一仁)]

-----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_WILLNEED for madvise, it will trigger read ahead for
>     reading the bitmap.

Thanks for the update.

With removing the unused variables (will be fixed when merging),
Acked-by: Kazuhito Hagio <k-hagio-ab at nec.com>

$ make clean ; make warn

diskdump.c: In function ‘read_dump_header’:
diskdump.c:543:10: warning: unused variable ‘bytes_read’ [-Wunused-variable]
  ssize_t bytes_read;
          ^~~~~~~~~~
diskdump.c:542:9: warning: unused variable ‘len’ [-Wunused-variable]
  size_t len;
         ^~~
diskdump.c:541:8: warning: unused variable ‘bufptr’ [-Wunused-variable]
  char *bufptr;
        ^~~~~~

As for a second ack, please wait for a while.
Lianbo is taking a leave and will be back soon.

Thanks,
Kazu

> 
> 2.) Test.
> 
>     The files:
>     vmlinux: 272M
>     vmcore : 23G (bitmap_len: 4575985664)
> 
>  #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    0m44.097s
> 	user    0m19.031s
> 	sys     0m1.620s
>      ............................
> 
> Signed-off-by: Huang Shijie <shijie at os.amperecomputing.com>
> ---
> v1 --> v2:
> 	Use MADV_WILLNEED for mmap, not MADV_SEQUENTIAL.
> 
> ---
>  diskdump.c | 39 ++++++++++++++++-----------------------
>  1 file changed, 16 insertions(+), 23 deletions(-)
> 
> diff --git a/diskdump.c b/diskdump.c
> index ff1e9a3..102062b 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_WILLNEED);
>  	}
> 
>  	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