[PATCH v2 4/5] Add module of calculating start_pfn and end_pfn in each dumpfile

HATAYAMA Daisuke d.hatayama at jp.fujitsu.com
Tue Oct 28 00:43:12 PDT 2014


From: Zhou Wenjian <zhouwj-fnst at cn.fujitsu.com>
Subject: [PATCH v2 4/5] Add module of calculating start_pfn and end_pfn in each dumpfile
Date: Mon, 13 Oct 2014 17:34:25 +0800

> When --split is specified in cyclic mode, start_pfn and end_pfn of each dumpfile
> will be calculated to make each dumpfile have the same size.
> 
> Signed-off-by: HATAYAMA Daisuke <d.hatayama at jp.fujitsu.com>

Please remove this Signed-off-by.

> Signed-off-by: Qiao Nuohan <qiaonuohan at cn.fujitsu.com>
> Signed-off-by: Zhou Wenjian <zhouwj-fnst at cn.fujitsu.com>
> ---
>  makedumpfile.c |  109 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 files changed, 104 insertions(+), 5 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index a6f0be4..32c0919 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -8190,6 +8190,103 @@ out:
>  		return ret;
>  }
>  
> +/*
> + * calculate end pfn in incomplete splitblock or memory not managed by splitblock
> + */
> +mdf_pfn_t
> +calculate_end_pfn_in_cycle(mdf_pfn_t start, mdf_pfn_t max,
> +			    mdf_pfn_t end_pfn, long long pfn_needed_by_per_dumpfile)
> +{
> +	struct cycle cycle;

Please add a line.

> +	for_each_cycle(start,max,&cycle) {
> +		if (!exclude_unnecessary_pages_cyclic(&cycle))
> +			return FALSE;
> +		while (end_pfn < cycle.end_pfn) {
> +			end_pfn++;
> +			if (is_dumpable_cyclic(info->partial_bitmap2, end_pfn, &cycle)){

This line exceeds 80 columns. It would be enough to add linebreaks in
each argument posision.

			if (is_dumpable_cyclic(info->partial_bitmap2,
                                               end_pfn,
                                               &cycle)) {

> +				if (--pfn_needed_by_per_dumpfile <= 0)
> +					return ++end_pfn;

				pfn_needed_by_per_dumpfile--;
				if (pfn_needed_by_per_dumpfile <= 0) {
					end_pfn++;
					return end_pfn;
				}


> +			}
> +		}
> +	}
> +	return ++end_pfn;
> +}
> +
> +/*
> + * calculate end_pfn of one dumpfile.
> + * try to make every output file have the same size.
> + * splitblock_table is used to reduce calculate time.
> + */
> +
> +#define CURRENT_SPLITBLOCK_PFN_NUM (*current_splitblock * splitblock->page_per_splitblock)
> +mdf_pfn_t
> +calculate_end_pfn_by_splitblock(mdf_pfn_t start_pfn,
> +			   int *current_splitblock,
> +			   long long *current_splitblock_pfns){

			   long long *current_splitblock_pfns)
{

> +	mdf_pfn_t end_pfn;
> +	long long pfn_needed_by_per_dumpfile,offset;

Please add a space:

	long long pfn_needed_by_per_dumpfile, offset;

Also, some variable names are too long: current_splitblock,
current_splitblock_pfns and pfn_needed_by_per_dumpfile.

>From current_splitblock, I imagine a SplitBlock object, but this is in
fact an inter representing an index of splitblocks.

For current_splitblock_pfns, is this really neccesary? Please see a
comment that will occur in later part.

> +	pfn_needed_by_per_dumpfile = info->num_dumpable / info->num_dumpfile;
> +	offset = *current_splitblock * splitblock->entry_size;
> +	end_pfn = start_pfn;

This is not a declaration. Please move downwards.

> +	char *splitblock_inner = splitblock->table + offset;

Please add a line.

> +	//calculate the part containing complete splitblock

Please use /* */ style.

> +	while (*current_splitblock < splitblock->num && pfn_needed_by_per_dumpfile > 0) {
> +		if (*current_splitblock_pfns > 0) {
> +			pfn_needed_by_per_dumpfile -= *current_splitblock_pfns ;
> +			*current_splitblock_pfns = 0 ;
> +		}
> +		else
> +		pfn_needed_by_per_dumpfile -= read_value_from_splitblock_table(splitblock_inner);

Ah, this line exceeds 80 columns: Please:

		} else {
pfn_needed_by_per_dumpfile -= read_value_from_splitblock_table(splitblock_inner);
		}

Again, pfn_needed_by_per_dumpfile is too long.

> +		splitblock_inner += splitblock->entry_size;
> +		++*current_splitblock;
> +	}

Please add a line.

> +	//deal with complete splitblock

Please use /* */ style.

What does "complete" mean?

> +	if (pfn_needed_by_per_dumpfile == 0)
> +		end_pfn = CURRENT_SPLITBLOCK_PFN_NUM;

Please add a line.

> +	//deal with incomplete splitblock

Please use /* */ style.

What does "incomplete" mean?

> +	if (pfn_needed_by_per_dumpfile < 0) {

Just as I've already commented in another mail, I think filtering here
is unnecessary. Is it enough to add this splitblock into a current
dumpfile, not next dumpfile? Then, this code is unecessary, and also
current_splitblock_pfns is unnecessary.

> +		--*current_splitblock;
> +		splitblock_inner -= splitblock->entry_size;
> +		end_pfn = CURRENT_SPLITBLOCK_PFN_NUM;
> +		*current_splitblock_pfns = (-1) * pfn_needed_by_per_dumpfile;
> +		pfn_needed_by_per_dumpfile += read_value_from_splitblock_table(splitblock_inner);
> +		end_pfn = calculate_end_pfn_in_cycle(CURRENT_SPLITBLOCK_PFN_NUM,
> +						     CURRENT_SPLITBLOCK_PFN_NUM+splitblock->page_per_splitblock,
> +						     end_pfn,pfn_needed_by_per_dumpfile);
> +	}

Please add a line.

> +	//deal with memory not managed by splitblock

Please use /* */ style.

> +	if (pfn_needed_by_per_dumpfile > 0 && *current_splitblock >= splitblock->num) {
> +		mdf_pfn_t cycle_start_pfn = MAX(CURRENT_SPLITBLOCK_PFN_NUM,end_pfn);
> +		end_pfn=calculate_end_pfn_in_cycle(cycle_start_pfn,
> +						   info->max_mapnr,
> +						   end_pfn,
> +						   pfn_needed_by_per_dumpfile);
> +	}
> +	return end_pfn;
> +}

Please add a line.

> +/*
> + * calculate start_pfn and end_pfn in each output file.
> + */
> +static int setup_splitting_cyclic(void)
> +{
> +	int i;
> +	mdf_pfn_t start_pfn, end_pfn;
> +	long long current_splitblock_pfns = 0;
> +	int current_splitblock = 0;

Please add a line.

> +	start_pfn = end_pfn = 0;
> +	for (i = 0; i < info->num_dumpfile - 1; i++) {
> +		start_pfn = end_pfn;
> +		end_pfn = calculate_end_pfn_by_splitblock(start_pfn,
> +							  &current_splitblock,
> +							  &current_splitblock_pfns);
> +		SPLITTING_START_PFN(i) = start_pfn;
> +		SPLITTING_END_PFN(i) = end_pfn;
> +	}
> +	SPLITTING_START_PFN(info->num_dumpfile - 1) = end_pfn;
> +	SPLITTING_END_PFN(info->num_dumpfile - 1) = info->max_mapnr;
> +	return TRUE;
> +}
> +
>  int
>  setup_splitting(void)
>  {
> @@ -8203,12 +8300,14 @@ setup_splitting(void)
>  		return FALSE;
>  
>  	if (info->flag_cyclic) {
> -		for (i = 0; i < info->num_dumpfile; i++) {
> -			SPLITTING_START_PFN(i) = divideup(info->max_mapnr, info->num_dumpfile) * i;
> -			SPLITTING_END_PFN(i)   = divideup(info->max_mapnr, info->num_dumpfile) * (i + 1);
> +		int ret = FALSE;

Please add a line.

> +		if(!prepare_bitmap2_buffer_cyclic()){
> +			free_bitmap_buffer();
> +			return ret;
>  		}
> -		if (SPLITTING_END_PFN(i-1) > info->max_mapnr)
> -			SPLITTING_END_PFN(i-1) = info->max_mapnr;
> +		ret = setup_splitting_cyclic();
> +		free_bitmap2_buffer_cyclic();
> +		return ret;
>          } else {
>  		initialize_2nd_bitmap(&bitmap2);
>  
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
--
Thanks.
HATAYAMA, Daisuke




More information about the kexec mailing list