From: Zhou Wenjian <zhouwj-fnst@xxxxxxxxxxxxxx>
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,
> + ¤t_splitblock,
> + ¤t_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
