[PATCH] makedumpfile: clean up readmem() by removing its recursive call

[Atsushi Kumagai]

Hello HATAYAMA-san,

On Tue, 19 Feb 2013 05:05:39 +0000
"Hatayama, Daisuke" <d.hatayama at jp.fujitsu.com> wrote:

> Currently, readmem() calls itself recursively and its depth grows in
> proportin to the number of requested pages.
> 
> For example, in __exclude_unnecessary_pages(), PGMM_CACHED is defined
> as 512 and page structure is of 56 bytes on x86_64, so size of page
> cache on x86_64 is now 56 * 512 = 28KiB, and this is 7 pages. This
> means that on the existing implementation readmem() is called
> recursively 7 times when reading mem_map array, but 6 of the 7 are
> in fact unnecessary.
> 
> This patch cleans up readmem() by removing the recursive
> call. Instead, readmem() proceeds to the processing for next page by
> jump.
> 
> Also, by this change, read operation is done in increasing order. This
> is more natural than the existing implementation in decreasing order.
> 
> Signed-off-by: HATAYAMA Daisuke <d.hatayama at jp.fujitsu.com>

Thanks, I'll merge this patch into v1.5.4.


Atsushi Kumagai

> ---
>  makedumpfile.c |   28 ++++++++++++++--------------
>  1 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index acb1b21..0bc8e1c 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -337,13 +337,12 @@ readpage_kdump_compressed(unsigned long long paddr, void *bufptr)
>  int
>  readmem(int type_addr, unsigned long long addr, void *bufptr, size_t size)
>  {
> -	size_t read_size, next_size;
> -	unsigned long long next_addr;
> +	size_t read_size, size_orig = size;
>  	unsigned long long paddr, maddr = NOT_PADDR;
>  	unsigned long long pgaddr;
>  	void *pgbuf;
> -	char *next_ptr;
>  
> +next_page:
>  	switch (type_addr) {
>  	case VADDR:
>  		if ((paddr = vaddr_to_paddr(addr)) == NOT_PADDR) {
> @@ -386,21 +385,14 @@ readmem(int type_addr, unsigned long long addr, void *bufptr, size_t size)
>  		goto error;
>  	}
>  
> -	read_size = size;
> -
>  	/*
>  	 * Read each page, because pages are not necessarily continuous.
>  	 * Ex) pages in vmalloc area
>  	 */
> -	if (!is_in_same_page(addr, addr + size - 1)) {
> -		read_size = info->page_size - (addr % info->page_size);
> -		next_addr = roundup(addr + 1, info->page_size);
> -		next_size = size - read_size;
> -		next_ptr  = (char *)bufptr + read_size;
> +	read_size = info->page_size - PAGEOFFSET(paddr);
>  
> -		if (!readmem(type_addr, next_addr, next_ptr, next_size))
> -			goto error;
> -	}
> +	if (read_size > size)
> +		read_size = size;
>  
>  	pgaddr = PAGEBASE(paddr);
>  	pgbuf = cache_search(pgaddr);
> @@ -423,7 +415,15 @@ readmem(int type_addr, unsigned long long addr, void *bufptr, size_t size)
>  	}
>  
>  	memcpy(bufptr, pgbuf + PAGEOFFSET(paddr), read_size);
> -	return size;
> +
> +	addr += read_size;
> +	bufptr += read_size;
> +	size -= read_size;
> +
> +	if (size > 0)
> +		goto next_page;
> +
> +	return size_orig;
>  
>  error:
>  	ERRMSG("type_addr: %d, addr:%llx, size:%zd\n", type_addr, addr, size);
> -- 
> 1.7.7.6
>