Generic multi-page exclusion
Subsystems (2)
v2 cover Generic multi-page exclusion
Kumagai-san,
this patch was inspired by this post of yours:
http://lists.infradead.org/pipermail/kexec/2013-November/010445.html
Unfortunately, this rewrite never happened, so I took the liberty of overtaking the job. I hope you don't mind.
This is in fact a preparatory series to add hugepage support without having to care about the appropriate size of the cyclic buffer.
Changelog:
* v2:
- Keep excluded regions per mem_map_data
- Process excluded pages in chunks
Petr Tesarik (3):
Pass mmd pointer to __exclude_unnecessary_pages
Generic handling of multi-page exclusions
Get rid of overrun adjustments
makedumpfile.c | 118 +++++++++++++++++++-------------------------------------- makedumpfile.h | 7 ++++ 2 files changed, 46 insertions(+), 79 deletions(-)
Signature · 7 lines
1.8.4.5 _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
PATCH v2 1/3 Pass mmd pointer to __exclude_unnecessary_pages
Pass the mmd pointer itself instead of the individual fields, so additional information can be kept in mem_map_data.
--- makedumpfile.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
makedumpfile.c +6 -6
@@ -4658,9 +4658,11 @@ initialize_2nd_bitmap_cyclic(struct cycle *cycle) }int -__exclude_unnecessary_pages (unsigned long mem_map, -unsigned long long pfn_start,unsigned long long pfn_end,struct cycle *cycle) +__exclude_unnecessary_pages (struct mem_map_data *mmd,struct cycle *cycle) { +unsigned long mem_map = mmd->mem_map; +unsigned long long pfn_start = mmd->pfn_start; +unsigned long long pfn_end = mmd->pfn_end;unsigned long long pfn, pfn_mm, maddr;unsigned long long pfn_read_start, pfn_read_end, index_pg;unsigned char page_cache[SIZE (page) *PGMM_CACHED ];
@@ -4809,8 +4811,7 @@ exclude_unnecessary_pages(void)if (mmd->mem_map ==NOT_MEMMAP_ADDR )continue ; -if (!__exclude_unnecessary_pages (mmd->mem_map, - mmd->pfn_start, mmd->pfn_end,NULL )) +if (!__exclude_unnecessary_pages (mmd,NULL ))return FALSE; }
@@ -4860,8 +4861,7 @@ exclude_unnecessary_pages_cyclic(struct cycle *cycle)if (mmd->pfn_end >= cycle->start_pfn && mmd->pfn_start <= cycle->end_pfn) { -if (!__exclude_unnecessary_pages (mmd->mem_map, - mmd->pfn_start, mmd->pfn_end, cycle)) +if (!__exclude_unnecessary_pages (mmd, cycle))return FALSE; } }
Signature · 7 lines
1.8.4.5 _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
PATCH v2 3/3 Get rid of overrun adjustments
In reply to Petr TesarikThanks to the previous commit, __exclude_unnecessary_pages does not require any specific size of the cycle.
--- makedumpfile.c | 59 ---------------------------------------------------------- 1 file changed, 59 deletions(-)
makedumpfile.c +0 -59
@@ -88,7 +88,6 @@ do { \ *ptr_long_table = value; \ }while (0 ) -static void check_cyclic_buffer_overrun(void); static void setup_page_is_buddy(void);void
@@ -3254,9 +3253,6 @@ out: !sadump_generate_elf_note_from_dumpfile())return FALSE; -if (info->flag_cyclic && info->dump_level &DL_EXCLUDE_FREE ) -check_cyclic_buffer_overrun (); - }else {if (!get_mem_map_without_mm ())return FALSE;
@@ -4274,61 +4270,6 @@ exclude_free_page(struct cycle *cycle) }/* -* Let C be a cyclic buffer size and B a bitmap size used for -* representing maximum block size managed by buddy allocator. -* -* For some combinations of C and B, clearing operation can overrun -* the cyclic buffer. Let's consider three cases. -* -* - If C == B, this is trivially safe. -* -* - If B > C, overrun can easily happen. -* -* - In case of C > B, if C mod B != 0, then there exist n > m > 0, -* B > b > 0 such that n x C = m x B + b. This means that clearing -* operation overruns cyclic buffer (B - b)-bytes in the -* combination of n-th cycle and m-th block. -* -* Note that C mod B != 0 iff (m x C) mod B != 0 for some m. -* -* If C == B, C mod B == 0 always holds. Again, if B > C, C mod B != 0 -* always holds. Hence, it's always sufficient to check the condition -* C mod B != 0 in order to determine whether overrun can happen or -* not. -* -* The bitmap size used for maximum block size B is calculated from -* MAX_ORDER as: -* -* B := DIVIDE_UP((1 << (MAX_ORDER - 1)), BITS_PER_BYTE) -* -* Normally, MAX_ORDER is 11 at default. This is configurable through -* CONFIG_FORCE_MAX_ZONEORDER. -*/ -static void -check_cyclic_buffer_overrun (void ) -{ -int max_order =ARRAY_LENGTH (zone.free_area); -int max_order_nr_pages =1 << (max_order -1 ); -unsigned long max_block_size =divideup (max_order_nr_pages,BITPERBYTE ); - -if (info->bufsize_cyclic % max_block_size) { -unsigned long bufsize; - -if (max_block_size > info->bufsize_cyclic) { -MSG ("WARNING: some free pages are not filtered.\n" ); -return ; - } - - bufsize = info->bufsize_cyclic; - info->bufsize_cyclic =round (bufsize, max_block_size); - info->pfn_cyclic = info->bufsize_cyclic *BITPERBYTE ; - -MSG ("cyclic buffer size has been changed: %lu => %lu\n" , - bufsize, info->bufsize_cyclic); - } -} - -/* * For the kernel versions from v2.6.17 to v2.6.37. */ static int
Signature · 7 lines
1.8.4.5 _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
PATCH v2 2/3 Generic handling of multi-page exclusions
In reply to Petr TesarikWhen multiple pages are excluded from the dump, store the extents in the mem_map_data structure, and check if anything is still pending on the next invocation of __exclude_unnecessary_pages for the same mem_map.
The start PFN of the excluded extent is set to the end of the current cycle (which is equal to the start of the next cycle, see update_cycle), so only the part of the excluded region which falls beyond current cycle buffer is valid. If the excluded region is completely processed in the current cycle, the start PFN is even bigger than the end PFN. That causes nothing to be done at the beginning of the next cycle.
There is no check whether the adjusted pfn_start is still within the current cycle. Nothing bad happens if it isn't, because exclude_range() is used again to exclude the remaining part, so even if the excluded region happens to span more than two cycles, the code will still work correctly.
Note that clear_bit_on_2nd_bitmap_for_kernel() accepts PFNs outside the current cyclic range. It willreturn FALSE, so such PFNs are not counted.
--- makedumpfile.c | 47 +++++++++++++++++++++++++++++++++-------------- makedumpfile.h | 7 +++++++ 2 files changed, 40 insertions(+), 14 deletions(-)
makedumpfile.c +33 -14
@@ -2385,6 +2385,9 @@ dump_mem_map(unsigned long long pfn_start, mmd->pfn_end = pfn_end; mmd->mem_map = mem_map; + mmd->exclude_pfn_start =0ULL ; + mmd->exclude_pfn_end =0ULL ; +DEBUG_MSG ("mem_map (%d)\n" , num_mm);DEBUG_MSG (" mem_map : %lx\n" , mem_map);DEBUG_MSG (" pfn_start : %llx\n" , pfn_start);
@@ -4657,6 +4660,21 @@ initialize_2nd_bitmap_cyclic(struct cycle *cycle)return TRUE; } +static void +exclude_range (unsigned long long *counter,struct mem_map_data *mmd, +unsigned long long pfn,unsigned long long endpfn,struct cycle *cycle) +{ +while (pfn < endpfn) { +if (clear_bit_on_2nd_bitmap_for_kernel (pfn, cycle)) + (*counter)++; + ++pfn; + } + + mmd->exclude_pfn_start = cycle ? cycle->end_pfn :ULONGLONG_MAX ; + mmd->exclude_pfn_end = endpfn; + mmd->exclude_pfn_counter = counter; +} +int __exclude_unnecessary_pages (struct mem_map_data *mmd,struct cycle *cycle) {
@@ -4671,6 +4689,18 @@ __exclude_unnecessary_pages(struct mem_map_data *mmd, struct cycle *cycle)unsigned long flags, mapping, private =0 ;/* +* If a multi-page exclusion is pending, do it first +*/ +if (mmd->exclude_pfn_start < mmd->exclude_pfn_end) { +exclude_range (mmd->exclude_pfn_counter, mmd, + mmd->exclude_pfn_start, mmd->exclude_pfn_end, + cycle); + + mem_map += (mmd->exclude_pfn_end - pfn_start) *SIZE (page); + pfn_start = mmd->exclude_pfn_end; + } + +/* * Refresh the buffer of struct page, when changing mem_map. */ pfn_read_start =ULONGLONG_MAX ;
@@ -4734,21 +4764,10 @@ __exclude_unnecessary_pages(struct mem_map_data *mmd, struct cycle *cycle)if ((info->dump_level &DL_EXCLUDE_FREE ) && info->page_is_buddy && info->page_is_buddy (flags, _mapcount, private, _count)) { -int i, nr_pages =1 << private; +int nr_pages =1 << private; + +exclude_range (&pfn_free, mmd, pfn, pfn + nr_pages, cycle); -for (i =0 ; i < nr_pages; ++i) { -/* -* According to combination of -* MAX_ORDER and size of cyclic -* buffer, this clearing bit operation -* can overrun the cyclic buffer. -* -* See check_cyclic_buffer_overrun() -* for the detail. -*/ -if (clear_bit_on_2nd_bitmap_for_kernel ((pfn + i), cycle)) - pfn_free++; - } pfn += nr_pages -1 ; mem_map += (nr_pages -1 ) *SIZE (page); }
makedumpfile.h +7 -0
@@ -816,6 +816,13 @@ struct mem_map_data {unsigned long long pfn_start;unsigned long long pfn_end;unsigned long mem_map; + +/* +* for excluding multi-page regions +*/ +unsigned long exclude_pfn_start; +unsigned long exclude_pfn_end; +unsigned long long *exclude_pfn_counter; };struct dump_bitmap {
Signature · 7 lines
1.8.4.5 _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
From: Petr Tesarik <[email protected]>
Subject: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Fri, 4 Apr 2014 19:25:08 +0200
Quoted 43 lines
When multiple pages are excluded from the dump, store the extents in the
mem_map_datastructure, and check if anything is still pending on the next invocation of__exclude_unnecessary_pagesfor the samemem_map.The start PFN of the excluded extent is set to the end of the current cycle (which is equal to the start of the next cycle, see
update_cycle), so only the part of the excluded region which falls beyond current cycle buffer is valid. If the excluded region is completely processed in the current cycle, the start PFN is even bigger than the end PFN. That causes nothing to be done at the beginning of the next cycle.There is no check whether the adjusted
pfn_startis still within the current cycle. Nothing bad happens if it isn't, becauseexclude_range()is used again to exclude the remaining part, so even if the excluded region happens to span more than two cycles, the code will still work correctly.Note that
clear_bit_on_2nd_bitmap_for_kernel()accepts PFNs outside the current cyclic range. It willreturn FALSE, so such PFNs are not counted.Signed-off-by Petr Tesarik <[email protected]>--- makedumpfile.c | 47 +++++++++++++++++++++++++++++++++-------------- makedumpfile.h | 7 +++++++ 2 files changed, 40 insertions(+), 14 deletions(-)makedumpfile.c +3
-0@@ -2385,6 +2385,9 @@ dump_mem_map(unsigned long long pfn_start, mmd->pfn_end = pfn_end; mmd->mem_map = mem_map; + mmd->exclude_pfn_start =0ULL ; + mmd->exclude_pfn_end =0ULL ; +DEBUG_MSG ("mem_map (%d)\n" , num_mm);DEBUG_MSG (" mem_map : %lx\n" , mem_map);DEBUG_MSG (" pfn_start : %llx\n" , pfn_start);@@ -4657,6 +4660,21 @@ initialize_2nd_bitmap_cyclic(struct cycle *cycle)return TRUE; }
makedumpfile.c initialize_2nd_bitmap_cyclic(struct cycle *cycle) · line 4671
Quoted 10 lines
makedumpfile.c +9
-0+static void +exclude_range (unsigned long long *counter,struct mem_map_data *mmd, +unsigned long long pfn,unsigned long long endpfn,struct cycle *cycle) +{ +while (pfn < endpfn) { +if (clear_bit_on_2nd_bitmap_for_kernel (pfn, cycle)) + (*counter)++; + ++pfn; + }
Here endpfn is pfn + nr_pages in __exclude_unnecessary_pages(), and the pfn could be cycle->end_pfn <= pfn < endpfn.
while (pfn < MIN(endpfn, cycle->end_pfn) {
if (clear_bit_on_2nd_bitmap_for_kernel(pfn, cycle))
(*counter)++;
++pfn;
}makedumpfile.c initialize_2nd_bitmap_cyclic(struct cycle *cycle) · line 4673
Quoted 2 lines
makedumpfile.c +2
-0+ + mmd->exclude_pfn_start = cycle ? cycle->end_pfn :ULONGLONG_MAX ;
When does cycle become NULL?
Along with the above point,
mmd->exclude_pfn_start = MIN(endpfn, cycle->end_pfn);
and then we can continue the processing in the next cycle.
Quoted 56 lines
makedumpfile.c +19
-14+ mmd->exclude_pfn_end = endpfn; + mmd->exclude_pfn_counter = counter; +} +int __exclude_unnecessary_pages (struct mem_map_data *mmd,struct cycle *cycle) {@@ -4671,6 +4689,18 @@ __exclude_unnecessary_pages(struct mem_map_data *mmd, struct cycle *cycle)unsigned long flags, mapping, private =0 ;/* +* If a multi-page exclusion is pending, do it first +*/ +if (mmd->exclude_pfn_start < mmd->exclude_pfn_end) { +exclude_range (mmd->exclude_pfn_counter, mmd, + mmd->exclude_pfn_start, mmd->exclude_pfn_end, + cycle); + + mem_map += (mmd->exclude_pfn_end - pfn_start) *SIZE (page); + pfn_start = mmd->exclude_pfn_end; + } + +/* * Refresh the buffer of struct page, when changing mem_map. */ pfn_read_start =ULONGLONG_MAX ;@@ -4734,21 +4764,10 @@ __exclude_unnecessary_pages(struct mem_map_data *mmd, struct cycle *cycle)if ((info->dump_level &DL_EXCLUDE_FREE ) && info->page_is_buddy && info->page_is_buddy (flags, _mapcount, private, _count)) { -int i, nr_pages =1 << private; +int nr_pages =1 << private; + +exclude_range (&pfn_free, mmd, pfn, pfn + nr_pages, cycle); -for (i =0 ; i < nr_pages; ++i) { -/* -* According to combination of -* MAX_ORDER and size of cyclic -* buffer, this clearing bit operation -* can overrun the cyclic buffer. -* -* See check_cyclic_buffer_overrun() -* for the detail. -*/ -if (clear_bit_on_2nd_bitmap_for_kernel ((pfn + i), cycle)) - pfn_free++; - } pfn += nr_pages -1 ; mem_map += (nr_pages -1 ) *SIZE (page); }makedumpfile.h +0
-0
makedumpfile.h struct mem_map_data { · line 824
Quoted 9 lines
makedumpfile.h +6
-0@@ -816,6 +816,13 @@ struct mem_map_data {unsigned long long pfn_start;unsigned long long pfn_end;unsigned long mem_map; + +/* +* for excluding multi-page regions +*/ +unsigned long exclude_pfn_start; +unsigned long exclude_pfn_end;
unsigned long long exclude_pfn_start;
unsigned long long exclude_pfn_end;
The integers representing page frame numbers need to be defined as unsigned long long for architectures where physical address can have 64-bit length but unsigned long has 32-bit only, such as x86 PAE.
Kumagai-san, I saw this sometimes in the past. How about introducing specific abstract type for page frame number like below?
typedef unsigned long long pfn_t;
maybe with some prefix. I think this also helps code readability because unsigned long long is too long.
makedumpfile.h struct mem_map_data { · line 826
Quoted 2 lines
makedumpfile.h +1
-0+unsigned long long *exclude_pfn_counter; };
Also, it seems to me better to introduce a new type for this processing rather than extending existing code. struct mem_map_data is not specific for the excluding processing.
Thanks.
HATAYAMA, Daisuke
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
Quoted 61 lines · On Tue, 08 Apr 2014 10:49:07 +0900 (JST) HATAYAMA Daisuke…
From: Petr Tesarik <[email protected]>
Subject: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Fri, 4 Apr 2014 19:25:08 +0200Quoted 43 lines
When multiple pages are excluded from the dump, store the extents in the
mem_map_datastructure, and check if anything is still pending on the next invocation of__exclude_unnecessary_pagesfor the samemem_map.The start PFN of the excluded extent is set to the end of the current cycle (which is equal to the start of the next cycle, see
update_cycle), so only the part of the excluded region which falls beyond current cycle buffer is valid. If the excluded region is completely processed in the current cycle, the start PFN is even bigger than the end PFN. That causes nothing to be done at the beginning of the next cycle.There is no check whether the adjusted
pfn_startis still within the current cycle. Nothing bad happens if it isn't, becauseexclude_range()is used again to exclude the remaining part, so even if the excluded region happens to span more than two cycles, the code will still work correctly.Note that
clear_bit_on_2nd_bitmap_for_kernel()accepts PFNs outside the current cyclic range. It willreturn FALSE, so such PFNs are not counted.Signed-off-by Petr Tesarik <[email protected]>--- makedumpfile.c | 47 +++++++++++++++++++++++++++++++++-------------- makedumpfile.h | 7 +++++++ 2 files changed, 40 insertions(+), 14 deletions(-)makedumpfile.c +3
-0@@ -2385,6 +2385,9 @@ dump_mem_map(unsigned long long pfn_start, mmd->pfn_end = pfn_end; mmd->mem_map = mem_map; + mmd->exclude_pfn_start =0ULL ; + mmd->exclude_pfn_end =0ULL ; +DEBUG_MSG ("mem_map (%d)\n" , num_mm);DEBUG_MSG (" mem_map : %lx\n" , mem_map);DEBUG_MSG (" pfn_start : %llx\n" , pfn_start);@@ -4657,6 +4660,21 @@ initialize_2nd_bitmap_cyclic(struct cycle *cycle)return TRUE; }makedumpfile.c initialize_2nd_bitmap_cyclic(struct cycle *cycle) · line 4671
Quoted 10 lines
makedumpfile.c +9
-0+static void +exclude_range (unsigned long long *counter,struct mem_map_data *mmd, +unsigned long long pfn,unsigned long long endpfn,struct cycle *cycle) +{ +while (pfn < endpfn) { +if (clear_bit_on_2nd_bitmap_for_kernel (pfn, cycle)) + (*counter)++; + ++pfn; + }Here endpfn is pfn +
nr_pagesin__exclude_unnecessary_pages(), and the pfn could becycle->end_pfn<= pfn < endpfn.
Quoted 5 lines
while (pfn < MIN(endpfn, cycle->end_pfn) {
if (clear_bit_on_2nd_bitmap_for_kernel(pfn, cycle))
(*counter)++;
++pfn;
}This is a non-issue: clear_bitmap_cyclic() checks the extents, and I even mentioned it in the commit message. All right, we can save some loop iterations by moving the check out of the loop body...
Quoted 4 lines
makedumpfile.c initialize_2nd_bitmap_cyclic(struct cycle *cycle) · line 4673
Quoted 2 lines
makedumpfile.c +2
-0+ + mmd->exclude_pfn_start = cycle ? cycle->end_pfn :ULONGLONG_MAX ;When does cycle become NULL?
When __exclude_unnecessary_pages() is called fromexclude_unnecessary_pages, i.e. non-cyclic.
Quoted 5 lines
Along with the above point,
mmd->exclude_pfn_start=MIN(endpfn, cycle->end_pfn);and then we can continue the processing in the next cycle.
Again, this is a non-issue. These stored extents are validated before use in __exclude_unnecessary_pages. Why should I check them twice? And by the way, this is also mentioned in the commit message.
Quoted 69 lines
Quoted 56 lines
makedumpfile.c +19
-14+ mmd->exclude_pfn_end = endpfn; + mmd->exclude_pfn_counter = counter; +} +int __exclude_unnecessary_pages (struct mem_map_data *mmd,struct cycle *cycle) {@@ -4671,6 +4689,18 @@ __exclude_unnecessary_pages(struct mem_map_data *mmd, struct cycle *cycle)unsigned long flags, mapping, private =0 ;/* +* If a multi-page exclusion is pending, do it first +*/ +if (mmd->exclude_pfn_start < mmd->exclude_pfn_end) { +exclude_range (mmd->exclude_pfn_counter, mmd, + mmd->exclude_pfn_start, mmd->exclude_pfn_end, + cycle); + + mem_map += (mmd->exclude_pfn_end - pfn_start) *SIZE (page); + pfn_start = mmd->exclude_pfn_end; + } + +/* * Refresh the buffer of struct page, when changing mem_map. */ pfn_read_start =ULONGLONG_MAX ;@@ -4734,21 +4764,10 @@ __exclude_unnecessary_pages(struct mem_map_data *mmd, struct cycle *cycle)if ((info->dump_level &DL_EXCLUDE_FREE ) && info->page_is_buddy && info->page_is_buddy (flags, _mapcount, private, _count)) { -int i, nr_pages =1 << private; +int nr_pages =1 << private; + +exclude_range (&pfn_free, mmd, pfn, pfn + nr_pages, cycle); -for (i =0 ; i < nr_pages; ++i) { -/* -* According to combination of -* MAX_ORDER and size of cyclic -* buffer, this clearing bit operation -* can overrun the cyclic buffer. -* -* See check_cyclic_buffer_overrun() -* for the detail. -*/ -if (clear_bit_on_2nd_bitmap_for_kernel ((pfn + i), cycle)) - pfn_free++; - } pfn += nr_pages -1 ; mem_map += (nr_pages -1 ) *SIZE (page); }makedumpfile.h +0
-0makedumpfile.h struct mem_map_data { · line 824
Quoted 9 lines
makedumpfile.h +6
-0@@ -816,6 +816,13 @@ struct mem_map_data {unsigned long long pfn_start;unsigned long long pfn_end;unsigned long mem_map; + +/* +* for excluding multi-page regions +*/ +unsigned long exclude_pfn_start; +unsigned long exclude_pfn_end;unsigned long long
exclude_pfn_start;
unsigned long longexclude_pfn_end;
Quoted 3 lines
The integers representing page frame numbers need to be defined as unsigned long long for architectures where physical address can have 64-bit length but unsigned long has 32-bit only, such as x86 PAE.
Ouch. My mistake. I thought I covered all places, but somehow I missed this one. I'm going to post a fixed series.
Quoted 8 lines
Kumagai-san, I saw this sometimes in the past. How about introducing specific abstract type for page frame number like below?
typedef unsigned long long
pfn_t;maybe with some prefix. I think this also helps code readability because unsigned long long is too long.
Quoted 6 lines
makedumpfile.h struct mem_map_data { · line 826
Quoted 2 lines
makedumpfile.h +1
-0+unsigned long long *exclude_pfn_counter; };Also, it seems to me better to introduce a new type for this processing rather than extending existing code. struct
mem_map_datais not specific for the excluding processing.
Kind of agreed. OTOH it will most likely be embedded in struct mem_map_data anyway, because exactly one such object per mm is needed.
Petr T
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Hello Petr,
Quoted 11 lines
Kumagai-san,
this patch was inspired by this post of yours:
http://lists.infradead.org/pipermail/kexec/2013-November/010445.html
Unfortunately, this rewrite never happened, so I took the liberty of overtaking the job. I hope you don't mind.
This is in fact a preparatory series to add hugepage support without having to care about the appropriate size of the cyclic buffer.
First, I'm sorry for my late work. This patch set will be very helpful to me, so I'll wait for the fixed version.
I'm preparing to release v1.5.6 now and I'll focus on the work for hugepage support after releasing v1.5.6.
Thanks
Atsushi Kumagai
Quoted 14 lines
Changelog:
* v2:
- Keep excluded regions permem_map_data
- Process excluded pages in chunksPetr Tesarik (3):
Pass mmd pointer to__exclude_unnecessary_pages
Generic handling of multi-page exclusions
Get rid of overrun adjustments
makedumpfile.c| 118 +++++++++++++++++++--------------------------------------makedumpfile.h| 7 ++++2 files changed, 46 insertions(+), 79 deletions(-)
Quoted 8 lines
--
1.8.4.5_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
RE: [PATCH v2 2/3] Generic handling of multi-page exclusions
In reply to HATAYAMA Daisuke[...]
Quoted 18 lines
makedumpfile.h +0
-0makedumpfile.h struct mem_map_data { · line 824
Quoted 13 lines
index 951ed1b..dfad569 100644
makedumpfile.h +6
-0@@ -816,6 +816,13 @@ struct mem_map_data {unsigned long long pfn_start;unsigned long long pfn_end;unsigned long mem_map; + +/* +* for excluding multi-page regions +*/ +unsigned long exclude_pfn_start; +unsigned long exclude_pfn_end;unsigned long long
exclude_pfn_start;
unsigned long longexclude_pfn_end;
Quoted 8 lines
The integers representing page frame numbers need to be defined as unsigned long long for architectures where physical address can have 64-bit length but unsigned long has 32-bit only, such as x86 PAE.
Kumagai-san, I saw this sometimes in the past. How about introducing specific abstract type for page frame number like below?
typedef unsigned long long
pfn_t;
Good idea! We should do it.
Quoted 9 lines
maybe with some prefix. I think this also helps code readability because unsigned long long is too long.
makedumpfile.h struct mem_map_data { · line 826
Quoted 2 lines
makedumpfile.h +1
-0+unsigned long long *exclude_pfn_counter; };Also, it seems to me better to introduce a new type for this processing rather than extending existing code. struct
mem_map_datais not specific for the excluding processing.
I agree, too.
Thanks
Atsushi Kumagai
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
In reply to Petr TesarikFrom: Petr Tesarik <[email protected]>
Subject: Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Tue, 8 Apr 2014 08:54:36 +0200
Quoted 70 lines
Quoted 61 lines · On Tue, 08 Apr 2014 10:49:07 +0900 (JST) HATAYAMA Daisuke…
From: Petr Tesarik <[email protected]>
Subject: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Fri, 4 Apr 2014 19:25:08 +0200Quoted 43 lines
When multiple pages are excluded from the dump, store the extents in the
mem_map_datastructure, and check if anything is still pending on the next invocation of__exclude_unnecessary_pagesfor the samemem_map.The start PFN of the excluded extent is set to the end of the current cycle (which is equal to the start of the next cycle, see
update_cycle), so only the part of the excluded region which falls beyond current cycle buffer is valid. If the excluded region is completely processed in the current cycle, the start PFN is even bigger than the end PFN. That causes nothing to be done at the beginning of the next cycle.There is no check whether the adjusted
pfn_startis still within the current cycle. Nothing bad happens if it isn't, becauseexclude_range()is used again to exclude the remaining part, so even if the excluded region happens to span more than two cycles, the code will still work correctly.Note that
clear_bit_on_2nd_bitmap_for_kernel()accepts PFNs outside the current cyclic range. It willreturn FALSE, so such PFNs are not counted.Signed-off-by Petr Tesarik <[email protected]>--- makedumpfile.c | 47 +++++++++++++++++++++++++++++++++-------------- makedumpfile.h | 7 +++++++ 2 files changed, 40 insertions(+), 14 deletions(-)makedumpfile.c +3
-0@@ -2385,6 +2385,9 @@ dump_mem_map(unsigned long long pfn_start, mmd->pfn_end = pfn_end; mmd->mem_map = mem_map; + mmd->exclude_pfn_start =0ULL ; + mmd->exclude_pfn_end =0ULL ; +DEBUG_MSG ("mem_map (%d)\n" , num_mm);DEBUG_MSG (" mem_map : %lx\n" , mem_map);DEBUG_MSG (" pfn_start : %llx\n" , pfn_start);@@ -4657,6 +4660,21 @@ initialize_2nd_bitmap_cyclic(struct cycle *cycle)return TRUE; }makedumpfile.c initialize_2nd_bitmap_cyclic(struct cycle *cycle) · line 4671
Quoted 10 lines
makedumpfile.c +9
-0+static void +exclude_range (unsigned long long *counter,struct mem_map_data *mmd, +unsigned long long pfn,unsigned long long endpfn,struct cycle *cycle) +{ +while (pfn < endpfn) { +if (clear_bit_on_2nd_bitmap_for_kernel (pfn, cycle)) + (*counter)++; + ++pfn; + }Here endpfn is pfn +
nr_pagesin__exclude_unnecessary_pages(), and the pfn could becycle->end_pfn<= pfn < endpfn.Quoted 5 lines
while (pfn < MIN(endpfn, cycle->end_pfn) { if (clear_bit_on_2nd_bitmap_for_kernel(pfn, cycle)) (*counter)++; ++pfn; }
Quoted 4 lines
This is a non-issue:
clear_bitmap_cyclic()checks the extents, and I even mentioned it in the commit message. All right, we can save some loop iterations by moving the check out of the loop body...
Ah, OK, I interpreted somehow your code like:
+ mmd->exclude_pfn_start = cycle ? endpfn: ULONGLONG_MAX;
So there's no logically wrong point.
BTW, hmm, the behaviour of clear_bit_on_2nd_bitmap_for_kernel() is not what I expect, that is, I don't expect sanity check in set_bitmap_cyclic(), which should have been removed by clean up we did some times ago.
intset_bitmap_cyclic(char *bitmap, unsigned long long pfn, int val, struct cycle *cycle)
{
int byte, bit;
if (pfn < cycle->start_pfn || cycle->end_pfn <= pfn) <-- this
return FALSE;The code some time ago changes region of cycles in many places and it was difficult to figure out. So, in the clean up, we introduced struct cycle and for_each_cycle() to obtain invariance that cycle is not changed under for_each_cycle().
So, could you fix also this if possible?
Quoted 109 lines
Quoted 4 lines
makedumpfile.c initialize_2nd_bitmap_cyclic(struct cycle *cycle) · line 4673
Quoted 2 lines
makedumpfile.c +2
-0+ + mmd->exclude_pfn_start = cycle ? cycle->end_pfn :ULONGLONG_MAX ;When does cycle become NULL?
When
__exclude_unnecessary_pages()is called fromexclude_unnecessary_pages, i.e. non-cyclic.Quoted 5 lines
Along with the above point,
mmd->exclude_pfn_start=MIN(endpfn, cycle->end_pfn);and then we can continue the processing in the next cycle.
Again, this is a non-issue. These stored extents are validated before use in
__exclude_unnecessary_pages. Why should I check them twice? And by the way, this is also mentioned in the commit message.Quoted 69 lines
Quoted 56 lines
makedumpfile.c +19
-14+ mmd->exclude_pfn_end = endpfn; + mmd->exclude_pfn_counter = counter; +} +int __exclude_unnecessary_pages (struct mem_map_data *mmd,struct cycle *cycle) {@@ -4671,6 +4689,18 @@ __exclude_unnecessary_pages(struct mem_map_data *mmd, struct cycle *cycle)unsigned long flags, mapping, private =0 ;/* +* If a multi-page exclusion is pending, do it first +*/ +if (mmd->exclude_pfn_start < mmd->exclude_pfn_end) { +exclude_range (mmd->exclude_pfn_counter, mmd, + mmd->exclude_pfn_start, mmd->exclude_pfn_end, + cycle); + + mem_map += (mmd->exclude_pfn_end - pfn_start) *SIZE (page); + pfn_start = mmd->exclude_pfn_end; + } + +/* * Refresh the buffer of struct page, when changing mem_map. */ pfn_read_start =ULONGLONG_MAX ;@@ -4734,21 +4764,10 @@ __exclude_unnecessary_pages(struct mem_map_data *mmd, struct cycle *cycle)if ((info->dump_level &DL_EXCLUDE_FREE ) && info->page_is_buddy && info->page_is_buddy (flags, _mapcount, private, _count)) { -int i, nr_pages =1 << private; +int nr_pages =1 << private; + +exclude_range (&pfn_free, mmd, pfn, pfn + nr_pages, cycle); -for (i =0 ; i < nr_pages; ++i) { -/* -* According to combination of -* MAX_ORDER and size of cyclic -* buffer, this clearing bit operation -* can overrun the cyclic buffer. -* -* See check_cyclic_buffer_overrun() -* for the detail. -*/ -if (clear_bit_on_2nd_bitmap_for_kernel ((pfn + i), cycle)) - pfn_free++; - } pfn += nr_pages -1 ; mem_map += (nr_pages -1 ) *SIZE (page); }makedumpfile.h +0
-0makedumpfile.h struct mem_map_data { · line 824
Quoted 9 lines
makedumpfile.h +6
-0@@ -816,6 +816,13 @@ struct mem_map_data {unsigned long long pfn_start;unsigned long long pfn_end;unsigned long mem_map; + +/* +* for excluding multi-page regions +*/ +unsigned long exclude_pfn_start; +unsigned long exclude_pfn_end;unsigned long long
exclude_pfn_start;
unsigned long longexclude_pfn_end;Quoted 3 lines
The integers representing page frame numbers need to be defined as unsigned long long for architectures where physical address can have 64-bit length but unsigned long has 32-bit only, such as x86 PAE.
Ouch. My mistake. I thought I covered all places, but somehow I missed this one. I'm going to post a fixed series.
Quoted 8 lines
Kumagai-san, I saw this sometimes in the past. How about introducing specific abstract type for page frame number like below?
typedef unsigned long long
pfn_t;maybe with some prefix. I think this also helps code readability because unsigned long long is too long.
Quoted 6 lines
makedumpfile.h struct mem_map_data { · line 826
Quoted 2 lines
makedumpfile.h +1
-0+unsigned long long *exclude_pfn_counter; };Also, it seems to me better to introduce a new type for this processing rather than extending existing code. struct
mem_map_datais not specific for the excluding processing.
Quoted 4 lines
Kind of agreed. OTOH it will most likely be embedded in struct
mem_map_dataanyway, because exactly one such object per mm is needed.Petr T
I don't understand well. It seems to me a single object is enough. Is it possible to nr_pages cover multiple mm's?
Thanks.
HATAYAMA, Daisuke
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
RE: [PATCH v2 2/3] Generic handling of multi-page exclusions
Quoted 83 lines
From: Petr Tesarik <[email protected]>
Subject: Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Tue, 8 Apr 2014 08:54:36 +0200Quoted 70 lines
Quoted 61 lines · On Tue, 08 Apr 2014 10:49:07 +0900 (JST) HATAYAMA Daisuke…
From: Petr Tesarik <[email protected]>
Subject: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Fri, 4 Apr 2014 19:25:08 +0200Quoted 43 lines
When multiple pages are excluded from the dump, store the extents in the
mem_map_datastructure, and check if anything is still pending on the next invocation of__exclude_unnecessary_pagesfor the samemem_map.The start PFN of the excluded extent is set to the end of the current cycle (which is equal to the start of the next cycle, see
update_cycle), so only the part of the excluded region which falls beyond current cycle buffer is valid. If the excluded region is completely processed in the current cycle, the start PFN is even bigger than the end PFN. That causes nothing to be done at the beginning of the next cycle.There is no check whether the adjusted
pfn_startis still within the current cycle. Nothing bad happens if it isn't, becauseexclude_range()is used again to exclude the remaining part, so even if the excluded region happens to span more than two cycles, the code will still work correctly.Note that
clear_bit_on_2nd_bitmap_for_kernel()accepts PFNs outside the current cyclic range. It willreturn FALSE, so such PFNs are not counted.Signed-off-by Petr Tesarik <[email protected]>--- makedumpfile.c | 47 +++++++++++++++++++++++++++++++++-------------- makedumpfile.h | 7 +++++++ 2 files changed, 40 insertions(+), 14 deletions(-)makedumpfile.c +3
-0@@ -2385,6 +2385,9 @@ dump_mem_map(unsigned long long pfn_start, mmd->pfn_end = pfn_end; mmd->mem_map = mem_map; + mmd->exclude_pfn_start =0ULL ; + mmd->exclude_pfn_end =0ULL ; +DEBUG_MSG ("mem_map (%d)\n" , num_mm);DEBUG_MSG (" mem_map : %lx\n" , mem_map);DEBUG_MSG (" pfn_start : %llx\n" , pfn_start);@@ -4657,6 +4660,21 @@ initialize_2nd_bitmap_cyclic(struct cycle *cycle)return TRUE; }makedumpfile.c initialize_2nd_bitmap_cyclic(struct cycle *cycle) · line 4671
Quoted 10 lines
makedumpfile.c +9
-0+static void +exclude_range (unsigned long long *counter,struct mem_map_data *mmd, +unsigned long long pfn,unsigned long long endpfn,struct cycle *cycle) +{ +while (pfn < endpfn) { +if (clear_bit_on_2nd_bitmap_for_kernel (pfn, cycle)) + (*counter)++; + ++pfn; + }Here endpfn is pfn +
nr_pagesin__exclude_unnecessary_pages(), and the pfn could becycle->end_pfn<= pfn < endpfn.Quoted 5 lines
while (pfn < MIN(endpfn, cycle->end_pfn) { if (clear_bit_on_2nd_bitmap_for_kernel(pfn, cycle)) (*counter)++; ++pfn; }Quoted 4 lines
This is a non-issue:
clear_bitmap_cyclic()checks the extents, and I even mentioned it in the commit message. All right, we can save some loop iterations by moving the check out of the loop body...Ah, OK, I interpreted somehow your code like:
(quoted diff) +1
-0+ mmd->exclude_pfn_start = cycle ? endpfn: ULONGLONG_MAX;
Quoted 6 lines
So there's no logically wrong point.
BTW, hmm, the behaviour of
clear_bit_on_2nd_bitmap_for_kernel()is not what I expect, that is, I don't expect sanity check inset_bitmap_cyclic(), which should have been removed by clean up we did some times ago.
At that time, I chose the current code since it was simpler and safer.
http://www.mail-archive.com/kexec%40lists.infradead.org/msg10207.html
Don't you like this ?
Thanks
Atsushi Kumagai
Quoted 127 lines
int
set_bitmap_cyclic(char *bitmap, unsigned long long pfn, int val, struct cycle *cycle){ int byte, bit; if (pfn < cycle->start_pfn || cycle->end_pfn <= pfn) <-- this return FALSE;The code some time ago changes region of cycles in many places and it was difficult to figure out. So, in the clean up, we introduced struct cycle and
for_each_cycle()to obtain invariance that cycle is not changed underfor_each_cycle().So, could you fix also this if possible?
Quoted 112 lines
Quoted 4 lines
makedumpfile.c initialize_2nd_bitmap_cyclic(struct cycle *cycle) · line 4673
Quoted 2 lines
makedumpfile.c +2
-0+ + mmd->exclude_pfn_start = cycle ? cycle->end_pfn :ULONGLONG_MAX ;When does cycle become NULL?
When
__exclude_unnecessary_pages()is called fromexclude_unnecessary_pages, i.e. non-cyclic.Quoted 5 lines
Along with the above point,
mmd->exclude_pfn_start=MIN(endpfn, cycle->end_pfn);and then we can continue the processing in the next cycle.
Again, this is a non-issue. These stored extents are validated before use in
__exclude_unnecessary_pages. Why should I check them twice? And by the way, this is also mentioned in the commit message.Quoted 69 lines
Quoted 56 lines
makedumpfile.c +19
-14+ mmd->exclude_pfn_end = endpfn; + mmd->exclude_pfn_counter = counter; +} +int __exclude_unnecessary_pages (struct mem_map_data *mmd,struct cycle *cycle) {@@ -4671,6 +4689,18 @@ __exclude_unnecessary_pages(struct mem_map_data *mmd, struct cycle *cycle)unsigned long flags, mapping, private =0 ;/* +* If a multi-page exclusion is pending, do it first +*/ +if (mmd->exclude_pfn_start < mmd->exclude_pfn_end) { +exclude_range (mmd->exclude_pfn_counter, mmd, + mmd->exclude_pfn_start, mmd->exclude_pfn_end, + cycle); + + mem_map += (mmd->exclude_pfn_end - pfn_start) *SIZE (page); + pfn_start = mmd->exclude_pfn_end; + } + +/* * Refresh the buffer of struct page, when changing mem_map. */ pfn_read_start =ULONGLONG_MAX ;@@ -4734,21 +4764,10 @@ __exclude_unnecessary_pages(struct mem_map_data *mmd, struct cycle *cycle)if ((info->dump_level &DL_EXCLUDE_FREE ) && info->page_is_buddy && info->page_is_buddy (flags, _mapcount, private, _count)) { -int i, nr_pages =1 << private; +int nr_pages =1 << private; + +exclude_range (&pfn_free, mmd, pfn, pfn + nr_pages, cycle); -for (i =0 ; i < nr_pages; ++i) { -/* -* According to combination of -* MAX_ORDER and size of cyclic -* buffer, this clearing bit operation -* can overrun the cyclic buffer. -* -* See check_cyclic_buffer_overrun() -* for the detail. -*/ -if (clear_bit_on_2nd_bitmap_for_kernel ((pfn + i), cycle)) - pfn_free++; - } pfn += nr_pages -1 ; mem_map += (nr_pages -1 ) *SIZE (page); }makedumpfile.h +0
-0makedumpfile.h struct mem_map_data { · line 824
Quoted 9 lines
makedumpfile.h +6
-0@@ -816,6 +816,13 @@ struct mem_map_data {unsigned long long pfn_start;unsigned long long pfn_end;unsigned long mem_map; + +/* +* for excluding multi-page regions +*/ +unsigned long exclude_pfn_start; +unsigned long exclude_pfn_end;unsigned long long
exclude_pfn_start;
unsigned long longexclude_pfn_end;Quoted 3 lines
The integers representing page frame numbers need to be defined as unsigned long long for architectures where physical address can have 64-bit length but unsigned long has 32-bit only, such as x86 PAE.
Ouch. My mistake. I thought I covered all places, but somehow I missed this one. I'm going to post a fixed series.
Quoted 8 lines
Kumagai-san, I saw this sometimes in the past. How about introducing specific abstract type for page frame number like below?
typedef unsigned long long
pfn_t;maybe with some prefix. I think this also helps code readability because unsigned long long is too long.
Quoted 6 lines
makedumpfile.h struct mem_map_data { · line 826
Quoted 2 lines
makedumpfile.h +1
-0+unsigned long long *exclude_pfn_counter; };Also, it seems to me better to introduce a new type for this processing rather than extending existing code. struct
mem_map_datais not specific for the excluding processing.Kind of agreed. OTOH it will most likely be embedded in struct
mem_map_dataanyway, because exactly one such object per mm is needed.
Quoted 7 lines
Quoted 1 line
Petr T
I don't understand well. It seems to me a single object is enough. Is it possible to
nr_pagescover multiple mm's?Thanks.
HATAYAMA, Daisuke
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
In reply to HATAYAMA DaisukeQuoted 11 lines · On Tue, 08 Apr 2014 17:19:03 +0900 (JST) HATAYAMA Daisuke…
From: Petr Tesarik <[email protected]>
Subject: Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Tue, 8 Apr 2014 08:54:36 +0200Quoted 7 lines
Quoted 4 lines · On Tue, 08 Apr 2014 10:49:07 +0900 (JST) HATAYAMA Daisuke…
Also, it seems to me better to introduce a new type for this processing rather than extending existing code. struct
mem_map_datais not specific for the excluding processing.
Quoted 7 lines
Quoted 4 lines
Kind of agreed. OTOH it will most likely be embedded in struct
mem_map_dataanyway, because exactly one such object per mm is needed.Petr T
I don't understand well. It seems to me a single object is enough. Is it possible to
nr_pagescover multiple mm's?
In fact, I believe it may be sufficient, but the loop nesting is like this (pseudo-code):
for each cycle:
for each mm:
call __exclude_unnecessary_pages
I'm not 100% sure that the region cannot change back and forth between two consecutive calls to __exclude_unnecessary_pages.
Thinking about it some more, this can be harmful only if there are overlapping memory maps... Can it happen, or can I safely ignore this possibility?
Petr T
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
From: Petr Tesarik <[email protected]>
Subject: Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Tue, 8 Apr 2014 12:07:28 +0200
Quoted 29 lines
Quoted 11 lines · On Tue, 08 Apr 2014 17:19:03 +0900 (JST) HATAYAMA Daisuke…
From: Petr Tesarik <[email protected]>
Subject: Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Tue, 8 Apr 2014 08:54:36 +0200Quoted 7 lines
Quoted 4 lines · On Tue, 08 Apr 2014 10:49:07 +0900 (JST) HATAYAMA Daisuke…
Also, it seems to me better to introduce a new type for this processing rather than extending existing code. struct
mem_map_datais not specific for the excluding processing.Quoted 7 lines
Quoted 4 lines
Kind of agreed. OTOH it will most likely be embedded in struct
mem_map_dataanyway, because exactly one such object per mm is needed.Petr T
I don't understand well. It seems to me a single object is enough. Is it possible to
nr_pagescover multiple mm's?In fact, I believe it may be sufficient, but the loop nesting is like this (pseudo-code):
for each cycle:
for each mm:
call__exclude_unnecessary_pages
Quoted 8 lines
I'm not 100% sure that the region cannot change back and forth between two consecutive calls to
__exclude_unnecessary_pages.Thinking about it some more, this can be harmful only if there are overlapping memory maps... Can it happen, or can I safely ignore this possibility?
Petr T
The list of mm is sorted and not overlapping. See get_mm_sparsemem(), get_mm_sparsemem(), get_mm_discontigmem() and get_mm_flatmem() from get_mem_map(). In particular, in get_mm_discontigmem(), sorting and removing overlapping portions are done explicitly.
Thanks.
HATAYAMA, Daisuke
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
In reply to Atsushi KumagaiFrom: Atsushi Kumagai <[email protected]>
Subject: RE: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Tue, 8 Apr 2014 09:00:19 +0000
Quoted 6 lines
At that time, I chose the current code since it was simpler and safer.
http://www.mail-archive.com/kexec%40lists.infradead.org/msg10207.htmlDon't you like this ?
Sorry. I had forgotten this. We should keep the sanity check there. But in our policy, we should not pass to set_bitmap_cyclic(), pfn and cycle where pfn is not in the cycle. We should chceck that in the caller side and pass pfn in the cycle only.
Also, on the current implementation, even if pfn outside a current cycle is passed to set_bitmap_cyclic(), we don't have any means to know that.
So, how about warning that only once at runtime?
Thanks.
HATAYAMA, Daisuke
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
In reply to Atsushi KumagaiQuoted 23 lines · On Tue, 8 Apr 2014 07:06:34 +0000 Atsushi Kumagai…
[...]
Quoted 22 lines
makedumpfile.h +0
-0makedumpfile.h struct mem_map_data { · line 824
Quoted 13 lines
index 951ed1b..dfad569 100644
makedumpfile.h +6
-0@@ -816,6 +816,13 @@ struct mem_map_data {unsigned long long pfn_start;unsigned long long pfn_end;unsigned long mem_map; + +/* +* for excluding multi-page regions +*/ +unsigned long exclude_pfn_start; +unsigned long exclude_pfn_end;unsigned long long
exclude_pfn_start;
unsigned long longexclude_pfn_end;The integers representing page frame numbers need to be defined as unsigned long long for architectures where physical address can have 64-bit length but unsigned long has 32-bit only, such as x86 PAE.
Quoted 6 lines
Quoted 4 lines
Kumagai-san, I saw this sometimes in the past. How about introducing specific abstract type for page frame number like below?
typedef unsigned long long
pfn_t;Good idea! We should do it.
Like the following patch?
From 9f3f6876bf1e8c93690097c510dff9982651bfa5 Mon Sep 17 00:00:00 2001
From: Petr Tesarik <[email protected]>
Date: Thu, 10 Apr 2014 12:40:31 +0200
Subject: [PATCH] Introduce the pfn_t type
Replace unsigned long long with pfn_t where:
a. the variable denotes a PFN
b. the variable is a number of pages
The number of pages is converted to a pfn_t, because it is a result of subtracting two PFNs or incremented in a loop over a range of PFNs, so it can get as large as a PFN.
--- makedumpfile.c | 151 +++++++++++++++++++++++++++++---------------------------- makedumpfile.h | 46 +++++++++--------- sadump_info.c | 15 +++--- sadump_info.h | 4 +- 4 files changed, 113 insertions(+), 103 deletions(-)
makedumpfile.c +78 -73
@@ -37,7 +37,7 @@ struct DumpInfo *info = NULL;char filename_stdout[] =FILENAME_STDOUT ; -static void first_cycle (unsigned long long start,unsigned long long max,struct cycle *cycle) +static void first_cycle (pfn_t start,pfn_t max,struct cycle *cycle) { cycle->start_pfn =round (start, info->pfn_cyclic); cycle->end_pfn = cycle->start_pfn + info->pfn_cyclic;
@@ -46,7 +46,7 @@ static void first_cycle(unsigned long long start, unsigned long long max, struct cycle->end_pfn = max; } -static void update_cycle (unsigned long long max,struct cycle *cycle) +static void update_cycle (pfn_t max,struct cycle *cycle) { cycle->start_pfn= cycle->end_pfn; cycle->end_pfn= cycle->start_pfn + info->pfn_cyclic;
@@ -55,7 +55,7 @@ static void update_cycle(unsigned long long max, struct cycle *cycle) cycle->end_pfn = max; } -static int end_cycle (unsigned long long max,struct cycle *cycle) +static int end_cycle (pfn_t max,struct cycle *cycle) {return (cycle->start_pfn >= max)?TRUE:FALSE; }
@@ -67,15 +67,15 @@ static int end_cycle(unsigned long long max, struct cycle *cycle)/* * The numbers of the excluded pages */ -unsigned long long pfn_zero; -unsigned long long pfn_memhole; -unsigned long long pfn_cache; -unsigned long long pfn_cache_private; -unsigned long long pfn_user; -unsigned long long pfn_free; -unsigned long long pfn_hwpoison; +pfn_t pfn_zero; +pfn_t pfn_memhole; +pfn_t pfn_cache; +pfn_t pfn_cache_private; +pfn_t pfn_user; +pfn_t pfn_free; +pfn_t pfn_hwpoison; -unsigned long long num_dumped; +pfn_t num_dumped;int retcd =FAILED ;/* return code */
@@ -122,7 +122,9 @@ unsigned long long ptom_xen(unsigned long long paddr) {unsigned long mfn; -unsigned long long maddr, pfn, mfn_idx, frame_idx; +unsigned long long maddr; +pfn_t pfn; +unsigned long long mfn_idx, frame_idx; pfn =paddr_to_pfn (paddr); mfn_idx = pfn /MFNS_PER_FRAME ;
@@ -226,12 +228,13 @@ is_in_same_page(unsigned long vaddr1, unsigned long vaddr2) }#define BITMAP_SECT_LEN 4096 -static inline int is_dumpable (struct dump_bitmap *,unsigned long long ); -static inline int is_dumpable_cyclic (char *bitmap,unsigned long long ,struct cycle *cycle); +static inline int is_dumpable (struct dump_bitmap *,pfn_t ); +static inline int is_dumpable_cyclic (char *bitmap,pfn_t ,struct cycle *cycle);unsigned long -pfn_to_pos (unsigned long long pfn) +pfn_to_pos (pfn_t pfn) { -unsigned long desc_pos, i; +unsigned long desc_pos; +pfn_t i; desc_pos = info->valid_pages[pfn /BITMAP_SECT_LEN ];for (i =round (pfn,BITMAP_SECT_LEN ); i < pfn; i++)
@@ -246,7 +249,7 @@ read_page_desc(unsigned long long paddr, page_desc_t *pd) {struct disk_dump_header *dh;unsigned long desc_pos; -unsigned long long pfn; +pfn_t pfn;off_t offset;/*
@@ -2375,8 +2378,7 @@ pgdat4: }void -dump_mem_map (unsigned long long pfn_start, -unsigned long long pfn_end,unsigned long mem_map,int num_mm) +dump_mem_map (pfn_t pfn_start,pfn_t pfn_end,unsigned long mem_map,int num_mm) {struct mem_map_data *mmd;
@@ -2802,7 +2804,7 @@ intget_mm_sparsemem (void) {unsigned int section_nr, mem_section_size, num_section; -unsigned long long pfn_start, pfn_end; +pfn_t pfn_start, pfn_end;unsigned long section, mem_map;unsigned long *mem_sec =NULL ;
@@ -2886,7 +2888,7 @@ get_mem_map_without_mm(void)int get_mem_map (void ) { -unsigned long long max_pfn =0 ; +pfn_t max_pfn =0 ;unsigned int i;int ret;
@@ -2944,7 +2946,7 @@ initialize_bitmap_memory(void)struct dump_bitmap *bmp;off_t bitmap_offset;off_t bitmap_len, max_sect_len; -unsigned long pfn; +pfn_t pfn;int i, j;long block_size;
@@ -3309,8 +3311,7 @@ initialize_2nd_bitmap(struct dump_bitmap *bitmap) }int -set_bitmap (struct dump_bitmap *bitmap,unsigned long long pfn, -int val) +set_bitmap (struct dump_bitmap *bitmap,pfn_t pfn,int val) {int byte, bit;off_t old_offset, new_offset;
@@ -3358,7 +3359,7 @@ set_bitmap(struct dump_bitmap *bitmap, unsigned long long pfn, }int -set_bitmap_cyclic (char *bitmap,unsigned long long pfn,int val,struct cycle *cycle) +set_bitmap_cyclic (char *bitmap,pfn_t pfn,int val,struct cycle *cycle) {int byte, bit;
@@ -3418,7 +3419,7 @@ sync_2nd_bitmap(void) }int -set_bit_on_1st_bitmap (unsigned long long pfn,struct cycle *cycle) +set_bit_on_1st_bitmap (pfn_t pfn,struct cycle *cycle) {if (info->flag_cyclic) {return set_bitmap_cyclic (info->partial_bitmap1, pfn,1 , cycle);
@@ -3428,7 +3429,7 @@ set_bit_on_1st_bitmap(unsigned long long pfn, struct cycle *cycle) }int -clear_bit_on_1st_bitmap (unsigned long long pfn,struct cycle *cycle) +clear_bit_on_1st_bitmap (pfn_t pfn,struct cycle *cycle) {if (info->flag_cyclic) {return set_bitmap_cyclic (info->partial_bitmap1, pfn,0 , cycle);
@@ -3438,7 +3439,7 @@ clear_bit_on_1st_bitmap(unsigned long long pfn, struct cycle *cycle) }int -clear_bit_on_2nd_bitmap (unsigned long long pfn,struct cycle *cycle) +clear_bit_on_2nd_bitmap (pfn_t pfn,struct cycle *cycle) {if (info->flag_cyclic) {return set_bitmap_cyclic (info->partial_bitmap2, pfn,0 , cycle);
@@ -3448,7 +3449,7 @@ clear_bit_on_2nd_bitmap(unsigned long long pfn, struct cycle *cycle) }int -clear_bit_on_2nd_bitmap_for_kernel (unsigned long long pfn,struct cycle *cycle) +clear_bit_on_2nd_bitmap_for_kernel (pfn_t pfn,struct cycle *cycle) {unsigned long long maddr;
@@ -3465,7 +3466,7 @@ clear_bit_on_2nd_bitmap_for_kernel(unsigned long long pfn, struct cycle *cycle) }int -set_bit_on_2nd_bitmap (unsigned long long pfn,struct cycle *cycle) +set_bit_on_2nd_bitmap (pfn_t pfn,struct cycle *cycle) {if (info->flag_cyclic) {return set_bitmap_cyclic (info->partial_bitmap2, pfn,1 , cycle);
@@ -3475,7 +3476,7 @@ set_bit_on_2nd_bitmap(unsigned long long pfn, struct cycle *cycle) }int -set_bit_on_2nd_bitmap_for_kernel (unsigned long long pfn,struct cycle *cycle) +set_bit_on_2nd_bitmap_for_kernel (pfn_t pfn,struct cycle *cycle) {unsigned long long maddr;
@@ -3786,11 +3787,11 @@ rearrange_dumpdata(void)return TRUE; } -unsigned long long +pfn_t page_to_pfn (unsigned long page) {unsigned int num; -unsigned long long pfn =ULONGLONG_MAX ; +pfn_t pfn =ULONGLONG_MAX ;unsigned long long index =0 ;struct mem_map_data *mmd;
@@ -3820,7 +3821,7 @@ reset_bitmap_of_free_pages(unsigned long node_zones, struct cycle *cycle)int order, i, migrate_type, migrate_types;unsigned long curr, previous, head, curr_page, curr_prev;unsigned long addr_free_pages, free_pages =0 , found_free_pages =0 ; -unsigned long long pfn, start_pfn; +pfn_t pfn, start_pfn;/* * On linux-2.6.24 or later, free_list is divided into the array.
@@ -4423,7 +4424,7 @@ create_1st_bitmap(void)int i;unsigned int num_pt_loads =get_num_pt_loads ();char buf[info->page_size]; -unsigned long long pfn, pfn_start, pfn_end, pfn_bitmap1; +pfn_t pfn, pfn_start, pfn_end, pfn_bitmap1;unsigned long long phys_start, phys_end;struct timeval tv_start;off_t offset_page;
@@ -4495,10 +4496,10 @@ int create_1st_bitmap_cyclic(struct cycle *cycle) {int i; -unsigned long long pfn, pfn_bitmap1; +pfn_t pfn, pfn_bitmap1;unsigned long long phys_start, phys_end; -unsigned long long pfn_start, pfn_end; -unsigned long long pfn_start_roundup, pfn_end_round; +pfn_t pfn_start, pfn_end; +pfn_t pfn_start_roundup, pfn_end_round;unsigned long pfn_start_byte, pfn_end_byte;/*
@@ -4556,7 +4557,8 @@ create_1st_bitmap_cyclic(struct cycle *cycle)int exclude_zero_pages (void ) { -unsigned long long pfn, paddr; +pfn_t pfn; +unsigned long long paddr;struct dump_bitmap bitmap2;struct timeval tv_start;unsigned char buf[info->page_size];
@@ -4608,10 +4610,10 @@ static int initialize_2nd_bitmap_cyclic(struct cycle *cycle) {int i; -unsigned long long pfn; +pfn_t pfn;unsigned long long phys_start, phys_end; -unsigned long long pfn_start, pfn_end; -unsigned long long pfn_start_roundup, pfn_end_round; +pfn_t pfn_start, pfn_end; +pfn_t pfn_start_roundup, pfn_end_round;unsigned long pfn_start_byte, pfn_end_byte;/*
@@ -4659,10 +4661,12 @@ initialize_2nd_bitmap_cyclic(struct cycle *cycle)int __exclude_unnecessary_pages (unsigned long mem_map, -unsigned long long pfn_start,unsigned long long pfn_end,struct cycle *cycle) +pfn_t pfn_start,pfn_t pfn_end,struct cycle *cycle) { -unsigned long long pfn, pfn_mm, maddr; -unsigned long long pfn_read_start, pfn_read_end, index_pg; +pfn_t pfn; +unsigned long index_pg, pfn_mm; +unsigned long long maddr; +pfn_t pfn_read_start, pfn_read_end;unsigned char page_cache[SIZE (page) *PGMM_CACHED ];unsigned char *pcache;unsigned int _count, _mapcount =0 ;
@@ -5162,7 +5166,7 @@ get_loads_dumpfile(void) {int i, phnum, num_new_load =0 ;long page_size = info->page_size; -unsigned long long pfn, pfn_start, pfn_end, num_excluded; +pfn_t pfn, pfn_start, pfn_end, num_excluded;unsigned long frac_head, frac_tail;Elf64_Phdr load;struct dump_bitmap bitmap2;
@@ -5612,10 +5616,10 @@ out:return ret; } -unsigned long long +pfn_t get_num_dumpable (void ) { -unsigned long long pfn, num_dumpable; +pfn_t pfn, num_dumpable;struct dump_bitmap bitmap2;initialize_2nd_bitmap (&bitmap2);
@@ -5627,10 +5631,10 @@ get_num_dumpable(void)return num_dumpable; } -unsigned long long +pfn_t get_num_dumpable_cyclic (void ) { -unsigned long long pfn, num_dumpable=0 ; +pfn_t pfn, num_dumpable=0 ;struct cycle cycle = {0 };for_each_cycle (0 , info->max_mapnr, &cycle)
@@ -5692,8 +5696,9 @@ write_elf_pages(struct cache_data *cd_header, struct cache_data *cd_page) {int i, phnum;long page_size = info->page_size; -unsigned long long pfn, pfn_start, pfn_end, paddr, num_excluded; -unsigned long long num_dumpable, per; +pfn_t pfn, pfn_start, pfn_end, num_excluded; +unsigned long long paddr; +pfn_t num_dumpable, per;unsigned long long memsz, filesz;unsigned long frac_head, frac_tail;off_t off_seg_load, off_memory;
@@ -5878,7 +5883,7 @@ write_elf_pages(struct cache_data *cd_header, struct cache_data *cd_page) }int -read_pfn (unsigned long long pfn,unsigned char *buf) +read_pfn (pfn_t pfn,unsigned char *buf) {unsigned long long paddr;
@@ -5897,7 +5902,7 @@ get_loads_dumpfile_cyclic(void)int i, phnum, num_new_load =0 ;long page_size = info->page_size;unsigned char buf[info->page_size]; -unsigned long long pfn, pfn_start, pfn_end, num_excluded; +pfn_t pfn, pfn_start, pfn_end, num_excluded;unsigned long frac_head, frac_tail;Elf64_Phdr load;struct cycle cycle = {0 };
@@ -5969,8 +5974,8 @@ write_elf_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page)int i, phnum;long page_size = info->page_size;unsigned char buf[info->page_size]; -unsigned long long pfn, pfn_start, pfn_end, paddr, num_excluded; -unsigned long long num_dumpable, per; +pfn_t pfn, pfn_start, pfn_end, num_excluded, num_dumpable, per; +unsigned long long paddr;unsigned long long memsz, filesz;unsigned long frac_head, frac_tail;off_t off_seg_load, off_memory;
@@ -6190,8 +6195,8 @@ write_elf_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page)int write_kdump_pages (struct cache_data *cd_header,struct cache_data *cd_page) { -unsigned long long pfn, per, num_dumpable; -unsigned long long start_pfn, end_pfn; +pfn_t pfn, per, num_dumpable; +pfn_t start_pfn, end_pfn;unsigned long size_out;struct page_desc pd, pd_zero;off_t offset_data =0 ;
@@ -6397,8 +6402,8 @@ int write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page, struct page_desc *pd_zero, off_t *offset_data, struct cycle *cycle) { - unsigned long long pfn, per; -unsigned long long start_pfn, end_pfn; + pfn_t pfn, per; +pfn_t start_pfn, end_pfn;unsigned long size_out;struct page_desc pd;unsigned char buf[info->page_size], *buf_out =NULL ;
@@ -7504,7 +7509,7 @@ read_vmcoreinfo_xen(void) }int -allocated_in_map (unsigned long long pfn) +allocated_in_map (pfn_t pfn) {static unsigned long long cur_idx =-1 ;static unsigned long cur_word;
@@ -7550,8 +7555,8 @@ exclude_xen3_user_domain(void)unsigned int num_pt_loads =get_num_pt_loads ();unsigned long page_info_addr;unsigned long long phys_start, phys_end; -unsigned long long pfn, pfn_end; -unsigned long long j, size; +pfn_t pfn, pfn_end; +pfn_t j, size;/* * NOTE: the first half of bitmap is not used for Xen extraction
@@ -7614,8 +7619,8 @@ exclude_xen4_user_domain(void)unsigned int num_pt_loads =get_num_pt_loads ();unsigned long page_info_addr;unsigned long long phys_start, phys_end; -unsigned long long pfn, pfn_end; -unsigned long long j, size; +pfn_t pfn, pfn_end; +pfn_t j, size;/* * NOTE: the first half of bitmap is not used for Xen extraction
@@ -7840,7 +7845,7 @@ print_vtop(void)void print_report (void ) { -unsigned long long pfn_original, pfn_excluded, shrinking; +pfn_t pfn_original, pfn_excluded, shrinking;/* * /proc/vmcore doesn't contain the memory hole area.
@@ -7945,9 +7950,9 @@ intsetup_splitting (void) {int i; -unsigned long long j, pfn_per_dumpfile; -unsigned long long start_pfn, end_pfn; -unsigned long long num_dumpable =get_num_dumpable (); +pfn_t j, pfn_per_dumpfile; +pfn_t start_pfn, end_pfn; +pfn_t num_dumpable =get_num_dumpable ();struct dump_bitmap bitmap2;if (info->num_dumpfile <=1 )
@@ -8274,7 +8279,7 @@ voidsort_splitting_info (void) {int i, j; -unsigned long long start_pfn, end_pfn; +pfn_t start_pfn, end_pfn;char *name_dumpfile;/*
@@ -8310,7 +8315,7 @@ intcheck_splitting_info (void) {int i; -unsigned long long end_pfn; +pfn_t end_pfn;/* * Check whether there are not lack of /proc/vmcore.
@@ -8537,8 +8542,8 @@ reassemble_kdump_pages(void)int i, fd =0 , ret = FALSE;off_t offset_first_ph, offset_ph_org, offset_eraseinfo;off_t offset_data_new, offset_zero_page =0 ; -unsigned long long pfn, start_pfn, end_pfn; -unsigned long long num_dumpable; +pfn_t pfn, start_pfn, end_pfn; +pfn_t num_dumpable;unsigned long size_eraseinfo;struct dump_bitmap bitmap2;struct disk_dump_header dh;
makedumpfile.h +24 -22
136 lines not on this page · raw message
sadump_info.c +9 -6
@@ -94,7 +94,7 @@ static int read_device_diskset(struct sadump_diskset_info *sdi, void *buf, size_t bytes, ulong *offset);static int read_sadump_header (char *filename);static int read_sadump_header_diskset (int diskid,struct sadump_diskset_info *sdi); -static unsigned long long pfn_to_block (unsigned long long pfn); +static unsigned long long pfn_to_block (pfn_t pfn);static int lookup_diskset (unsigned long long whole_offset,int *diskid,unsigned long long *disk_offset);static int max_mask_cpu (void );
@@ -202,7 +202,8 @@ sadump_copy_1st_bitmap_from_memory(void)* modify bitmap accordingly. */ if (si->kdump_backed_up) { -unsigned long long paddr, pfn, backup_src_pfn; +unsigned long long paddr; +pfn_t pfn, backup_src_pfn; for (paddr = si->backup_src_start; paddr < si->backup_src_start + si->backup_src_size;
@@ -754,7 +755,8 @@ sadump_initialize_bitmap_memory(void)struct sadump_header *sh = si->sh_memory;struct dump_bitmap *bmp;unsigned long dumpable_bitmap_offset; -unsigned long long section, max_section, pfn; +unsigned long long section, max_section; +pfn_t pfn;unsigned long long *block_table; dumpable_bitmap_offset =
@@ -901,7 +903,7 @@ sadump_set_timestamp(struct timeval *ts)return TRUE; } -unsigned long long +pfn_t sadump_get_max_mapnr (void ) {return si->sh_memory->max_mapnr;
@@ -951,7 +953,8 @@ failed:int readpage_sadump (unsigned long long paddr,void *bufptr) { -unsigned long long pfn, block, whole_offset, perdisk_offset; +pfn_t pfn; +unsigned long long block, whole_offset, perdisk_offset;int fd_memory; if (si->kdump_backed_up &&
@@ -1117,7 +1120,7 @@ sadump_check_debug_info(void) }static unsigned long long -pfn_to_block (unsigned long long pfn) +pfn_to_block (pfn_t pfn) {unsigned long long block, section, p;
sadump_info.h +2 -2
@@ -42,7 +42,7 @@ int sadump_copy_1st_bitmap_from_memory(void);int sadump_initialize_bitmap_memory (void );int sadump_num_online_cpus (void );int sadump_set_timestamp (struct timeval *ts); -unsigned long long sadump_get_max_mapnr (void ); +pfn_t sadump_get_max_mapnr (void );int readpage_sadump (unsigned long long paddr,void *bufptr);int sadump_check_debug_info (void );int sadump_generate_vmcoreinfo_from_vmlinux (size_t *vmcoreinfo_size);
@@ -92,7 +92,7 @@ static inline int sadump_set_timestamp(struct timeval *ts)return FALSE; } -static inline unsigned long long sadump_get_max_mapnr (void ) +static inline pfn_t sadump_get_max_mapnr (void ) {return 0 ; }
Signature · 7 lines
1.8.4.5 _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
From: Petr Tesarik <[email protected]>
Subject: Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Thu, 10 Apr 2014 12:47:17 +0200
Quoted 42 lines
Quoted 23 lines · On Tue, 8 Apr 2014 07:06:34 +0000 Atsushi Kumagai…
[...]
Quoted 22 lines
makedumpfile.h +0
-0makedumpfile.h struct mem_map_data { · line 824
Quoted 13 lines
index 951ed1b..dfad569 100644
makedumpfile.h +6
-0@@ -816,6 +816,13 @@ struct mem_map_data {unsigned long long pfn_start;unsigned long long pfn_end;unsigned long mem_map; + +/* +* for excluding multi-page regions +*/ +unsigned long exclude_pfn_start; +unsigned long exclude_pfn_end;unsigned long long
exclude_pfn_start;
unsigned long longexclude_pfn_end;The integers representing page frame numbers need to be defined as unsigned long long for architectures where physical address can have 64-bit length but unsigned long has 32-bit only, such as x86 PAE.
Quoted 6 lines
Quoted 4 lines
Kumagai-san, I saw this sometimes in the past. How about introducing specific abstract type for page frame number like below?
typedef unsigned long long
pfn_t;Good idea! We should do it.
Like the following patch?
From
9f3f6876bf1e8c93690097c510dff9982651bfa5Mon Sep 17 00:00:00 2001
From: Petr Tesarik <[email protected]>
Date: Thu, 10 Apr 2014 12:40:31 +0200
Subject: [PATCH] Introduce thepfn_ttypeReplace unsigned long long with
pfn_twhere:
Quoted 8 lines
a. the variable denotes a PFN
b. the variable is a number of pagesThe number of pages is converted to a
pfn_t, because it is a result of subtracting two PFNs or incremented in a loop over a range of PFNs, so it can get as large as a PFN.Signed-off-by Petr Tesarik <[email protected]>
Thanks!
Only concern is that pfn_t could be too generic; could collide with symbols from any other libraries. On kernel, I found that kvm and um defines pfn_t.
So, it's better to prefix pfn_t with the letters indicating that this is relevant to makedumpfile.
But I don't come up with good prefix...
Thanks.
HATAYAMA, Daisuke
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
RE: [PATCH v2 2/3] Generic handling of multi-page exclusions
Quoted 57 lines
From: Petr Tesarik <[email protected]>
Subject: Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Thu, 10 Apr 2014 12:47:17 +0200Quoted 42 lines
Quoted 23 lines · On Tue, 8 Apr 2014 07:06:34 +0000 Atsushi Kumagai…
[...]
Quoted 22 lines
makedumpfile.h +0
-0makedumpfile.h struct mem_map_data { · line 824
Quoted 13 lines
index 951ed1b..dfad569 100644
makedumpfile.h +6
-0@@ -816,6 +816,13 @@ struct mem_map_data {unsigned long long pfn_start;unsigned long long pfn_end;unsigned long mem_map; + +/* +* for excluding multi-page regions +*/ +unsigned long exclude_pfn_start; +unsigned long exclude_pfn_end;unsigned long long
exclude_pfn_start;
unsigned long longexclude_pfn_end;The integers representing page frame numbers need to be defined as unsigned long long for architectures where physical address can have 64-bit length but unsigned long has 32-bit only, such as x86 PAE.
Quoted 6 lines
Quoted 4 lines
Kumagai-san, I saw this sometimes in the past. How about introducing specific abstract type for page frame number like below?
typedef unsigned long long
pfn_t;Good idea! We should do it.
Like the following patch?
From
9f3f6876bf1e8c93690097c510dff9982651bfa5Mon Sep 17 00:00:00 2001
From: Petr Tesarik <[email protected]>
Date: Thu, 10 Apr 2014 12:40:31 +0200
Subject: [PATCH] Introduce thepfn_ttypeReplace unsigned long long with
pfn_twhere:Quoted 8 lines
a. the variable denotes a PFN
b. the variable is a number of pagesThe number of pages is converted to a
pfn_t, because it is a result of subtracting two PFNs or incremented in a loop over a range of PFNs, so it can get as large as a PFN.Signed-off-by Petr Tesarik <[email protected]>Thanks!
Quoted 8 lines
Only concern is that
pfn_tcould be too generic; could collide with symbols from any other libraries. On kernel, I found that kvm and um definespfn_t.So, it's better to prefix
pfn_twith the letters indicating that this is relevant to makedumpfile.But I don't come up with good prefix...
We don't need to be serious, just keeping identity is the important thing. So how about kdump_pfn_t or mdf_pfn_t? ("makedumpfile" is a long name and it hasn't a good shortening, it might be better to rename it :-P)
Thanks
Atsushi Kumagai
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
RE: [PATCH v2 2/3] Generic handling of multi-page exclusions
In reply to HATAYAMA DaisukeQuoted 11 lines
Quoted 5 lines
At that time, I chose the current code since it was simpler and safer.
http://www.mail-archive.com/kexec%40lists.infradead.org/msg10207.htmlDon't you like this ?
Sorry. I had forgotten this. We should keep the sanity check there. But in our policy, we should not pass to
set_bitmap_cyclic(), pfn and cycle where pfn is not in the cycle. We should chceck that in the caller side and pass pfn in the cycle only.
Quoted 5 lines
Also, on the current implementation, even if pfn outside a current cycle is passed to
set_bitmap_cyclic(), we don't have any means to know that.So, how about warning that only once at runtime?
Sounds good, it will be helpful to detect bugs in caller side. Like this?
makedumpfile.c +8 -1
@@ -3361,9 +3361,16 @@ int set_bitmap_cyclic(char *bitmap, unsigned long long pfn, int val, struct cycle *cycle) {int byte, bit; +static int warning =0 ; -if (pfn < cycle->start_pfn || cycle->end_pfn <= pfn) +if (pfn < cycle->start_pfn || cycle->end_pfn <= pfn) { +if (!warning) { +MSG ("WARNING: PFN out of cycle range. (pfn:%llx, " , pfn); +MSG ("cycle:[%llx-%llx])\n" , cycle->start_pfn, cycle->end_pfn); + warning =1 ; + }return FALSE; + }/* * If val is 0, clear bit on the bitmap.
Thanks
Atsushi Kumagai
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
In reply to Atsushi KumagaiV Tue, 15 Apr 2014 01:20:46 +0000
Atsushi Kumagai <[email protected]> napsáno:
Quoted 61 lines
Quoted 61 lines
From: Petr Tesarik <[email protected]>
Subject: Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Thu, 10 Apr 2014 12:47:17 +0200Quoted 42 lines
Quoted 23 lines · On Tue, 8 Apr 2014 07:06:34 +0000 Atsushi Kumagai…
[...]
Quoted 22 lines
makedumpfile.h +0
-0makedumpfile.h struct mem_map_data { · line 824
Quoted 13 lines
index 951ed1b..dfad569 100644
makedumpfile.h +6
-0@@ -816,6 +816,13 @@ struct mem_map_data {unsigned long long pfn_start;unsigned long long pfn_end;unsigned long mem_map; + +/* +* for excluding multi-page regions +*/ +unsigned long exclude_pfn_start; +unsigned long exclude_pfn_end;unsigned long long
exclude_pfn_start;
unsigned long longexclude_pfn_end;The integers representing page frame numbers need to be defined as unsigned long long for architectures where physical address can have 64-bit length but unsigned long has 32-bit only, such as x86 PAE.
Quoted 6 lines
Quoted 4 lines
Kumagai-san, I saw this sometimes in the past. How about introducing specific abstract type for page frame number like below?
typedef unsigned long long
pfn_t;Good idea! We should do it.
Like the following patch?
From
9f3f6876bf1e8c93690097c510dff9982651bfa5Mon Sep 17 00:00:00 2001
From: Petr Tesarik <[email protected]>
Date: Thu, 10 Apr 2014 12:40:31 +0200
Subject: [PATCH] Introduce thepfn_ttypeReplace unsigned long long with
pfn_twhere:Quoted 8 lines
a. the variable denotes a PFN
b. the variable is a number of pagesThe number of pages is converted to a
pfn_t, because it is a result of subtracting two PFNs or incremented in a loop over a range of PFNs, so it can get as large as a PFN.Signed-off-by Petr Tesarik <[email protected]>Thanks!
Only concern is that
pfn_tcould be too generic; could collide with symbols from any other libraries. On kernel, I found that kvm and um definespfn_t.
Quoted 7 lines
Quoted 4 lines
So, it's better to prefix
pfn_twith the letters indicating that this is relevant to makedumpfile.But I don't come up with good prefix...
We don't need to be serious, just keeping identity is the important thing. So how about
kdump_pfn_tormdf_pfn_t?
I vote for mdf_pfn_t, because MDF is already used as an abbreviation for makedumpfile in the crash utility source files.
My two cents,
Petr T
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
In reply to Atsushi KumagaiFrom: Atsushi Kumagai <[email protected]>
Subject: RE: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Tue, 15 Apr 2014 01:20:46 +0000
Quoted 64 lines
Quoted 64 lines
From: Petr Tesarik <[email protected]>
Subject: Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Thu, 10 Apr 2014 12:47:17 +0200Quoted 42 lines
Quoted 23 lines · On Tue, 8 Apr 2014 07:06:34 +0000 Atsushi Kumagai…
[...]
Quoted 22 lines
makedumpfile.h +0
-0makedumpfile.h struct mem_map_data { · line 824
Quoted 13 lines
index 951ed1b..dfad569 100644
makedumpfile.h +6
-0@@ -816,6 +816,13 @@ struct mem_map_data {unsigned long long pfn_start;unsigned long long pfn_end;unsigned long mem_map; + +/* +* for excluding multi-page regions +*/ +unsigned long exclude_pfn_start; +unsigned long exclude_pfn_end;unsigned long long
exclude_pfn_start;
unsigned long longexclude_pfn_end;The integers representing page frame numbers need to be defined as unsigned long long for architectures where physical address can have 64-bit length but unsigned long has 32-bit only, such as x86 PAE.
Quoted 6 lines
Quoted 4 lines
Kumagai-san, I saw this sometimes in the past. How about introducing specific abstract type for page frame number like below?
typedef unsigned long long
pfn_t;Good idea! We should do it.
Like the following patch?
From
9f3f6876bf1e8c93690097c510dff9982651bfa5Mon Sep 17 00:00:00 2001
From: Petr Tesarik <[email protected]>
Date: Thu, 10 Apr 2014 12:40:31 +0200
Subject: [PATCH] Introduce thepfn_ttypeReplace unsigned long long with
pfn_twhere:Quoted 8 lines
a. the variable denotes a PFN
b. the variable is a number of pagesThe number of pages is converted to a
pfn_t, because it is a result of subtracting two PFNs or incremented in a loop over a range of PFNs, so it can get as large as a PFN.Signed-off-by Petr Tesarik <[email protected]>Thanks!
Only concern is that
pfn_tcould be too generic; could collide with symbols from any other libraries. On kernel, I found that kvm and um definespfn_t.So, it's better to prefix
pfn_twith the letters indicating that this is relevant to makedumpfile.
Quoted 7 lines
Quoted 1 line
But I don't come up with good prefix...
We don't need to be serious, just keeping identity is the important thing. So how about
kdump_pfn_tormdf_pfn_t? ("makedumpfile" is a long name and it hasn't a good shortening, it might be better to rename it :-P)
It seems to me the latter mdf_ is better.
Thanks.
HATAYAMA, Daisuke
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 2/3] Generic handling of multi-page exclusions
In reply to Atsushi KumagaiFrom: Atsushi Kumagai <[email protected]>
Subject: RE: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Tue, 15 Apr 2014 04:18:31 +0000
Quoted 33 lines
Quoted 11 lines
Quoted 5 lines
At that time, I chose the current code since it was simpler and safer.
http://www.mail-archive.com/kexec%40lists.infradead.org/msg10207.htmlDon't you like this ?
Sorry. I had forgotten this. We should keep the sanity check there. But in our policy, we should not pass to
set_bitmap_cyclic(), pfn and cycle where pfn is not in the cycle. We should chceck that in the caller side and pass pfn in the cycle only.Quoted 5 lines
Also, on the current implementation, even if pfn outside a current cycle is passed to
set_bitmap_cyclic(), we don't have any means to know that.So, how about warning that only once at runtime?
Sounds good, it will be helpful to detect bugs in caller side. Like this?
makedumpfile.c +2
-1@@ -3361,9 +3361,16 @@ int set_bitmap_cyclic(char *bitmap, unsigned long long pfn, int val, struct cycle *cycle) {int byte, bit; +static int warning =0 ; -if (pfn < cycle->start_pfn || cycle->end_pfn <= pfn) +if (pfn < cycle->start_pfn || cycle->end_pfn <= pfn) {
makedumpfile.c int · line 3376
Quoted 10 lines
makedumpfile.c +6
-0+if (!warning) { +MSG ("WARNING: PFN out of cycle range. (pfn:%llx, " , pfn); +MSG ("cycle:[%llx-%llx])\n" , cycle->start_pfn, cycle->end_pfn); + warning =1 ; + }return FALSE; + }/* * If val is 0, clear bit on the bitmap.
Yes, I thought this logic.
--
Thanks.
HATAYAMA, Daisuke
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
RE: [PATCH v2 2/3] Generic handling of multi-page exclusions
Quoted 44 lines
From: Atsushi Kumagai <[email protected]>
Subject: RE: [PATCH v2 2/3] Generic handling of multi-page exclusions
Date: Tue, 15 Apr 2014 04:18:31 +0000Quoted 40 lines
Quoted 11 lines
Quoted 5 lines
At that time, I chose the current code since it was simpler and safer.
http://www.mail-archive.com/kexec%40lists.infradead.org/msg10207.htmlDon't you like this ?
Sorry. I had forgotten this. We should keep the sanity check there. But in our policy, we should not pass to
set_bitmap_cyclic(), pfn and cycle where pfn is not in the cycle. We should chceck that in the caller side and pass pfn in the cycle only.Quoted 5 lines
Also, on the current implementation, even if pfn outside a current cycle is passed to
set_bitmap_cyclic(), we don't have any means to know that.So, how about warning that only once at runtime?
Sounds good, it will be helpful to detect bugs in caller side. Like this?
makedumpfile.c +8
-1@@ -3361,9 +3361,16 @@ int set_bitmap_cyclic(char *bitmap, unsigned long long pfn, int val, struct cycle *cycle) {int byte, bit; +static int warning =0 ; -if (pfn < cycle->start_pfn || cycle->end_pfn <= pfn) +if (pfn < cycle->start_pfn || cycle->end_pfn <= pfn) { +if (!warning) { +MSG ("WARNING: PFN out of cycle range. (pfn:%llx, " , pfn); +MSG ("cycle:[%llx-%llx])\n" , cycle->start_pfn, cycle->end_pfn); + warning =1 ; + }return FALSE; + }
Quoted 5 lines
makedumpfile.c int · line 3376
Quoted 3 lines
makedumpfile.c +0
-0/* * If val is 0, clear bit on the bitmap. Yes, I thought this logic.
Thanks, I've pushed this into devel branch.
Atsushi Kumagai
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec