Hi HATAYAMA and Atsushi,

I think v2 is better than v1, since update_cyclic_region can be used
with a more flexible calling.

What's your opinion about this?

On 11/23/13 at 05:29pm, Baoquan He wrote:
> Function update_cyclic_region has a name which doesn't match its
> functionality. it does 3 tings: update the cyclic region; if elf
> dump, call create_1st_bitmap_cyclic in this region; call
> exclude_unnecessary_pages_cyclic to exclude pages in this region.
> 
> However not all 3 things are needed to be done everywhere. Somewhere
> unnecessary thing is done with much efforts, e.g when get dumpable
> page numbers for elf cyclic dump 1st bigmap creating is not needed.
> Somewhere thing is done, but it also cause errors, e.g when write
> 1st bitmap cyclic for compressed kdump it will add free page numbers
> twice, namely pfn_free is doubled. Then dump information will be wrong
> like below:
> 
> Original pages  : 0x000000000007539c
>   Excluded pages   : 0x00000000000d986a
>     Pages filled with zero  : 0x0000000000001196
>     Cache pages             : 0x0000000000025008
>     Cache pages + private   : 0x0000000000004baa
>     User process data pages : 0x00000000000127d0
>     Free pages              : 0x000000000009c352
>     Hwpoison pages          : 0x0000000000000000
>   Remaining pages  : 0xfffffffffff9bb32
>   (The number of pages is reduced to 38418230895101%.)
> Memory Hole     : 0x000000000000ac62
> --------------------------------------------------
> Total pages     : 0x000000000007fffe
> 
> So add 2 more parameters for update_cyclic_region. By these adding
> update_cyclic_region can be called in a refined grain. Then effort
> can be saved and above error can be fixed.
> 
> And in kdump compressed dump case, prepare_bitmap2_buffer_cyclic is
> called, that means partial_bitmap2 is malloced twice. Here remove
> the 2nd call.
> ---
>  makedumpfile.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 3746cf6..8cfc2a5 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -4725,7 +4725,7 @@ exclude_unnecessary_pages_cyclic(void)
>  }
>  
>  int
> -update_cyclic_region(unsigned long long pfn)
> +update_cyclic_region(unsigned long long pfn, int bmp1_set, int exclude)
>  {
>       if (is_cyclic_region(pfn))
>               return TRUE;
> @@ -4736,10 +4736,10 @@ update_cyclic_region(unsigned long long pfn)
>       if (info->cyclic_end_pfn > info->max_mapnr)
>               info->cyclic_end_pfn = info->max_mapnr;
>  
> -     if (info->flag_elf_dumpfile && !create_1st_bitmap_cyclic())
> +     if (bmp1_set && info->flag_elf_dumpfile && !create_1st_bitmap_cyclic())
>               return FALSE;
>  
> -     if (!exclude_unnecessary_pages_cyclic())
> +     if (exclude && !exclude_unnecessary_pages_cyclic())
>               return FALSE;
>  
>       return TRUE;
> @@ -5502,7 +5502,7 @@ get_num_dumpable_cyclic(void)
>       unsigned long long pfn, num_dumpable=0;
>  
>       for (pfn = 0; pfn < info->max_mapnr; pfn++) {
> -             if (!update_cyclic_region(pfn))
> +             if (!update_cyclic_region(pfn, 0, 1))
>                       return FALSE;
>  
>               if (is_dumpable_cyclic(info->partial_bitmap2, pfn))
> @@ -5803,7 +5803,7 @@ get_loads_dumpfile_cyclic(void)
>                        * Update target region and bitmap
>                        */
>                       if (!is_cyclic_region(pfn)) {
> -                             if (!update_cyclic_region(pfn))
> +                             if (!update_cyclic_region(pfn, 0, 1))
>                                       return FALSE;
>                       }
>  
> @@ -5872,7 +5872,7 @@ write_elf_pages_cyclic(struct cache_data *cd_header, 
> struct cache_data *cd_page)
>  
>       info->cyclic_start_pfn = 0;
>       info->cyclic_end_pfn = 0;
> -     if (!update_cyclic_region(0))
> +     if (!update_cyclic_region(0, 1, 1))
>               return FALSE;
>  
>       if (!(phnum = get_phnum_memory()))
> @@ -5910,7 +5910,7 @@ write_elf_pages_cyclic(struct cache_data *cd_header, 
> struct cache_data *cd_page)
>                       /*
>                        * Update target region and partial bitmap if necessary.
>                        */
> -                     if (!update_cyclic_region(pfn))
> +                     if (!update_cyclic_region(pfn, 1, 1))
>                               return FALSE;
>  
>                       if (!is_dumpable_cyclic(info->partial_bitmap2, pfn)) {
> @@ -6805,7 +6805,7 @@ write_kdump_pages_and_bitmap_cyclic(struct cache_data 
> *cd_header, struct cache_d
>       for (pfn = 0; pfn < info->max_mapnr; pfn++) {
>               if (is_cyclic_region(pfn))
>                       continue;
> -             if (!update_cyclic_region(pfn))
> +             if (!update_cyclic_region(pfn, 0, 0))
>                       return FALSE;
>               if (!create_1st_bitmap_cyclic())
>                       return FALSE;
> @@ -6815,9 +6815,6 @@ write_kdump_pages_and_bitmap_cyclic(struct cache_data 
> *cd_header, struct cache_d
>  
>       free_bitmap1_buffer();
>  
> -     if (!prepare_bitmap2_buffer_cyclic())
> -             return FALSE;
> -
>       /*
>        * Write pages and bitmap cyclically.
>        */
> @@ -6827,7 +6824,7 @@ write_kdump_pages_and_bitmap_cyclic(struct cache_data 
> *cd_header, struct cache_d
>               if (is_cyclic_region(pfn))
>                       continue;
>  
> -             if (!update_cyclic_region(pfn))
> +             if (!update_cyclic_region(pfn, 0, 1))
>                          return FALSE;
>  
>               if (!write_kdump_pages_cyclic(cd_header, cd_page, &pd_zero, 
> &offset_data))
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> kexec mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Reply via email to