x86, mm: Avoid step_size overflow and add comments
PATCH 1/1 x86, mm: Avoid step_size overflow and add comments
Current code
1. shift step_size directly, and this could have overflow problem after we shift step_size several times.
2. use MACRO to have shift to set to 5, but there is not explanation about selection.
We can fix them with:
1. Add get_new_step_size(), and check overflow by comparing new_size with old step_size.
2. add comment in it about why we are using 5.
That will make code more readable while not increasing size of init_mem_mapping().
-v2: According to Ingo, update changelog and comments.
--- arch/x86/mm/init.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)
arch/x86/mm/init.c +18 -3
@@ -399,8 +399,23 @@ static unsigned long __init init_range_mreturn mapped_ram_size; } -/* (PUD_SHIFT-PMD_SHIFT)/2 */ -#define STEP_SIZE_SHIFT 5 +static unsigned long __init get_new_step_size (unsigned long step_size) +{ +/* +* initial mapped size is PMD_SIZE (2M). +* We can not set step_size to be PUD_SIZE (1G) yet. +* In worse case, when we cross the 1G boundary, and +* PG_LEVEL_2M is not set, we will need 1+1+512 pages (2M + 8k) +* to map 1G range with PTE. Use 5 as shift for now. +*/ +unsigned long new_step_size = step_size <<5 ; + +if (new_step_size > step_size) + step_size = new_step_size; + +return step_size; +} +void __init init_mem_mapping (void ) {unsigned long end, real_end, start, last_start;
@@ -449,7 +464,7 @@ void __init init_mem_mapping(void) min_pfn_mapped = last_start >>PAGE_SHIFT ;/* only increase step_size after big range get mapped */ if (new_mapped_ram_size > mapped_ram_size) - step_size <<=STEP_SIZE_SHIFT ; + step_size =get_new_step_size (step_size); mapped_ram_size += new_mapped_ram_size; }
Quoted 1 line · Yinghai Lu
Current code
"Current code has two problems:"
Quoted 2 lines
1. shift
step_sizedirectly, and this could have overflow problem after we shiftstep_sizeseveral times.
The most important piece of information is not explained:
under what circumstances can this happen currently, what are the symptoms if this happens, roughly what kind of systems are affected, etc?
I mean the kind of information you'd put into a BIOS errata description or into any software changelog. You wouldn't primarily write about the mechanics of the change, but about the _actual impact_ of the change, to end users, right?
Quoted 10 lines
2. use MACRO to have shift to set to 5, but there is not explanation about selection.
We can fix them with:
1. Addget_new_step_size(), and check overflow by comparingnew_sizewith oldstep_size.
2. add comment in it about why we are using 5.That will make code more readable while not increasing size of
init_mem_mapping().
Well, it gets not just more readable at a small size increase, but also the above unexplained overflow bug will be fixed as well, right?
Thanks,
Ingo