x86, mm: Add comments for step_size shift

As request by hpa, add comments for why we choose 5 for step size shift.

Signed-off-by Yinghai Lu <[email protected]>

Reviewed-by Tang Chen <[email protected]>

Tested-by Tang Chen <[email protected]>

---
 arch/x86/mm/init.c |   21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

Index: linux-2.6/arch/x86/mm/init.c
===================================================================

arch/x86/mm/init.c +0 -1

@@ -395,8 +395,23 @@ static unsigned long __init init_range_m
 	return mapped_ram_size;
 }
 
-/* (PUD_SHIFT-PMD_SHIFT)/2 */

arch/x86/mm/init.c +8 -0

+	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;

Quoted 20 lines · Yinghai Lu

As request by hpa, add comments for why we choose 5 for step size shift.

Signed-off-by Yinghai Lu <[email protected]>

Reviewed-by Tang Chen <[email protected]>

Tested-by Tang Chen <[email protected]>

---
 arch/x86/mm/init.c |   21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

Index: linux-2.6/arch/x86/mm/init.c
===================================================================

arch/x86/mm/init.c +0 -1

@@ -395,8 +395,23 @@ static unsigned long __init init_range_m
      return mapped_ram_size;
 }
 
-/* (PUD_SHIFT-PMD_SHIFT)/2 */

arch/x86/mm/init.c static unsigned long __init init_range_m · line 406

Quoted 10 lines

arch/x86/mm/init.c +9 -1

-#define STEP_SIZE_SHIFT 5
+static unsigned long __init get_new_step_size(unsigned long step_size)
+{
+     /*
+      * initial mapped size is PMD_SIZE, aka 2M.
+      * We can not set step_size to be PUD_SIZE aka 1G yet.
+      * In worse case, when 1G is cross the 1G boundary, and
+      * PG_LEVEL_2M is not set, we will need 1+1+512 pages (aka 2M + 8k)
+      * to map 1G range with PTE. Use 5 as shift for now.
+      */

This is much more readable:

Quoted 14 lines

arch/x86/mm/init.c +8 -0

+     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;

arch/x86/mm/init.c void __init init_mem_mapping(void) · line 465

Quoted 7 lines

arch/x86/mm/init.c +1 -1

@@ -445,7 +460,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;
      }

As-is the changelog claims it only adds comments - but it obviously does more than that ...

Yinghai, for the 1001st time, please use the customary changelog style we use in the kernel:

" Current code does (A), this has a problem when (B).
We can improve this doing (C), because (D)."

I'm also going to suggest something radical: how about you keep this sugestion of mine in mind for _all_ future patches so I don't have to repeat it for every 3rd patch like I had to for the past 4 years, non-stop? Okay?