diff mbox

[3/6] x86/mm: Factor out of top-down direct mapping setup

Message ID 52416431.1090107@cn.fujitsu.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Yanfei Zhang Sept. 24, 2013, 10:06 a.m. UTC 
From: Tang Chen <tangchen@cn.fujitsu.com>

This patch introduces a new function memory_map_top_down to
factor out of the top-down direct memory mapping pagetable
setup. This is also a preparation for the following patch,
which will introduce the bottom-up memory mapping. That said,
we will put the two ways of pagetable setup into separate
functions, and choose to use which way in init_mem_mapping,
which makes the code more clear.

Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
 arch/x86/mm/init.c |   70 ++++++++++++++++++++++++++++++++-------------------
 1 files changed, 44 insertions(+), 26 deletions(-)

Comments

Tejun Heo Sept. 24, 2013, 12:27 p.m. UTC | #1 
On Tue, Sep 24, 2013 at 06:06:41PM +0800, Zhang Yanfei wrote:
> +/**
> + * memory_map_top_down - Map [map_start, map_end) top down
> + * @map_start: start address of the target memory range
> + * @map_end: end address of the target memory range
> + *
> + * This function will setup direct mapping for memory range [map_start, map_end)
> + * in a heuristic way. In the beginning, step_size is small. The more memory we
> + * map memory in the next loop.
> + */

The comment reads a bit weird to me.  The step size is increased
gradually but that really isn't really a heuristic and it doesn't
mention mapping direction.

...
> @@ -430,19 +430,13 @@ void __init init_mem_mapping(void)
>  	min_pfn_mapped = real_end >> PAGE_SHIFT;
>  	last_start = start = real_end;
>  
> -	/*
> -	 * We start from the top (end of memory) and go to the bottom.
> -	 * The memblock_find_in_range() gets us a block of RAM from the
> -	 * end of RAM in [min_pfn_mapped, max_pfn_mapped) used as new pages
> -	 * for page table.
> -	 */

I think this comment should stay here with the variable names
updated.

> -	while (last_start > ISA_END_ADDRESS) {
> +	while (last_start > map_start) {
>  		if (last_start > step_size) {
>  			start = round_down(last_start - 1, step_size);
> -			if (start < ISA_END_ADDRESS)
> -				start = ISA_END_ADDRESS;
> +			if (start < map_start)
> +				start = map_start;
>  		} else
> -			start = ISA_END_ADDRESS;
> +			start = map_start;
>  		new_mapped_ram_size = init_range_memory_mapping(start,
>  							last_start);
>  		last_start = start;
> @@ -453,8 +447,32 @@ void __init init_mem_mapping(void)
>  		mapped_ram_size += new_mapped_ram_size;
>  	}
>  
> -	if (real_end < end)
> -		init_range_memory_mapping(real_end, end);
> +	if (real_end < map_end)
> +		init_range_memory_mapping(real_end, map_end);
> +}
> +
> +void __init init_mem_mapping(void)
> +{
> +	unsigned long end;
> +
> +	probe_page_size_mask();
> +
> +#ifdef CONFIG_X86_64
> +	end = max_pfn << PAGE_SHIFT;
> +#else
> +	end = max_low_pfn << PAGE_SHIFT;
> +#endif
> +
> +	/* the ISA range is always mapped regardless of memory holes */
> +	init_memory_mapping(0, ISA_END_ADDRESS);
> +
> +	/*
> +	 * We start from the top (end of memory) and go to the bottom.
> +	 * The memblock_find_in_range() gets us a block of RAM from the
> +	 * end of RAM in [min_pfn_mapped, max_pfn_mapped) used as new pages
> +	 * for page table.
> +	 */

And just mention the range and direction in the comment here?

> +	memory_map_top_down(ISA_END_ADDRESS, end);

Thanks.
Zhang Yanfei Sept. 24, 2013, 1:20 p.m. UTC | #2 
Hello tejun,

On 09/24/2013 08:27 PM, Tejun Heo wrote:
> On Tue, Sep 24, 2013 at 06:06:41PM +0800, Zhang Yanfei wrote:
>> +/**
>> + * memory_map_top_down - Map [map_start, map_end) top down
>> + * @map_start: start address of the target memory range
>> + * @map_end: end address of the target memory range
>> + *
>> + * This function will setup direct mapping for memory range [map_start, map_end)
>> + * in a heuristic way. In the beginning, step_size is small. The more memory we
>> + * map memory in the next loop.
>> + */
> 
> The comment reads a bit weird to me.  The step size is increased
> gradually but that really isn't really a heuristic and it doesn't
> mention mapping direction.

Ok, will change the words and add the comment of direction.

> 
> ...
>> @@ -430,19 +430,13 @@ void __init init_mem_mapping(void)
>>  	min_pfn_mapped = real_end >> PAGE_SHIFT;
>>  	last_start = start = real_end;
>>  
>> -	/*
>> -	 * We start from the top (end of memory) and go to the bottom.
>> -	 * The memblock_find_in_range() gets us a block of RAM from the
>> -	 * end of RAM in [min_pfn_mapped, max_pfn_mapped) used as new pages
>> -	 * for page table.
>> -	 */
> 
> I think this comment should stay here with the variable names
> updated.

OK, agreed

> 
>> -	while (last_start > ISA_END_ADDRESS) {
>> +	while (last_start > map_start) {
>>  		if (last_start > step_size) {
>>  			start = round_down(last_start - 1, step_size);
>> -			if (start < ISA_END_ADDRESS)
>> -				start = ISA_END_ADDRESS;
>> +			if (start < map_start)
>> +				start = map_start;
>>  		} else
>> -			start = ISA_END_ADDRESS;
>> +			start = map_start;
>>  		new_mapped_ram_size = init_range_memory_mapping(start,
>>  							last_start);
>>  		last_start = start;
>> @@ -453,8 +447,32 @@ void __init init_mem_mapping(void)
>>  		mapped_ram_size += new_mapped_ram_size;
>>  	}
>>  
>> -	if (real_end < end)
>> -		init_range_memory_mapping(real_end, end);
>> +	if (real_end < map_end)
>> +		init_range_memory_mapping(real_end, map_end);
>> +}
>> +
>> +void __init init_mem_mapping(void)
>> +{
>> +	unsigned long end;
>> +
>> +	probe_page_size_mask();
>> +
>> +#ifdef CONFIG_X86_64
>> +	end = max_pfn << PAGE_SHIFT;
>> +#else
>> +	end = max_low_pfn << PAGE_SHIFT;
>> +#endif
>> +
>> +	/* the ISA range is always mapped regardless of memory holes */
>> +	init_memory_mapping(0, ISA_END_ADDRESS);
>> +
>> +	/*
>> +	 * We start from the top (end of memory) and go to the bottom.
>> +	 * The memblock_find_in_range() gets us a block of RAM from the
>> +	 * end of RAM in [min_pfn_mapped, max_pfn_mapped) used as new pages
>> +	 * for page table.
>> +	 */
> 
> And just mention the range and direction in the comment here?

OK, agreed.

Thanks

> 
>> +	memory_map_top_down(ISA_END_ADDRESS, end);
> 
> Thanks.
>
diff mbox

Patch

diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 04664cd..73e79e6 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -401,27 +401,27 @@  static unsigned long __init init_range_memory_mapping(
 
 /* (PUD_SHIFT-PMD_SHIFT)/2 */
 #define STEP_SIZE_SHIFT 5
-void __init init_mem_mapping(void)
+
+/**
+ * memory_map_top_down - Map [map_start, map_end) top down
+ * @map_start: start address of the target memory range
+ * @map_end: end address of the target memory range
+ *
+ * This function will setup direct mapping for memory range [map_start, map_end)
+ * in a heuristic way. In the beginning, step_size is small. The more memory we
+ * map memory in the next loop.
+ */
+static void __init memory_map_top_down(unsigned long map_start,
+				       unsigned long map_end)
 {
-	unsigned long end, real_end, start, last_start;
+	unsigned long real_end, start, last_start;
 	unsigned long step_size;
 	unsigned long addr;
 	unsigned long mapped_ram_size = 0;
 	unsigned long new_mapped_ram_size;
 
-	probe_page_size_mask();
-
-#ifdef CONFIG_X86_64
-	end = max_pfn << PAGE_SHIFT;
-#else
-	end = max_low_pfn << PAGE_SHIFT;
-#endif
-
-	/* the ISA range is always mapped regardless of memory holes */
-	init_memory_mapping(0, ISA_END_ADDRESS);
-
 	/* xen has big range in reserved near end of ram, skip it at first.*/
-	addr = memblock_find_in_range(ISA_END_ADDRESS, end, PMD_SIZE, PMD_SIZE);
+	addr = memblock_find_in_range(map_start, map_end, PMD_SIZE, PMD_SIZE);
 	real_end = addr + PMD_SIZE;
 
 	/* step_size need to be small so pgt_buf from BRK could cover it */
@@ -430,19 +430,13 @@  void __init init_mem_mapping(void)
 	min_pfn_mapped = real_end >> PAGE_SHIFT;
 	last_start = start = real_end;
 
-	/*
-	 * We start from the top (end of memory) and go to the bottom.
-	 * The memblock_find_in_range() gets us a block of RAM from the
-	 * end of RAM in [min_pfn_mapped, max_pfn_mapped) used as new pages
-	 * for page table.
-	 */
-	while (last_start > ISA_END_ADDRESS) {
+	while (last_start > map_start) {
 		if (last_start > step_size) {
 			start = round_down(last_start - 1, step_size);
-			if (start < ISA_END_ADDRESS)
-				start = ISA_END_ADDRESS;
+			if (start < map_start)
+				start = map_start;
 		} else
-			start = ISA_END_ADDRESS;
+			start = map_start;
 		new_mapped_ram_size = init_range_memory_mapping(start,
 							last_start);
 		last_start = start;
@@ -453,8 +447,32 @@  void __init init_mem_mapping(void)
 		mapped_ram_size += new_mapped_ram_size;
 	}
 
-	if (real_end < end)
-		init_range_memory_mapping(real_end, end);
+	if (real_end < map_end)
+		init_range_memory_mapping(real_end, map_end);
+}
+
+void __init init_mem_mapping(void)
+{
+	unsigned long end;
+
+	probe_page_size_mask();
+
+#ifdef CONFIG_X86_64
+	end = max_pfn << PAGE_SHIFT;
+#else
+	end = max_low_pfn << PAGE_SHIFT;
+#endif
+
+	/* the ISA range is always mapped regardless of memory holes */
+	init_memory_mapping(0, ISA_END_ADDRESS);
+
+	/*
+	 * We start from the top (end of memory) and go to the bottom.
+	 * The memblock_find_in_range() gets us a block of RAM from the
+	 * end of RAM in [min_pfn_mapped, max_pfn_mapped) used as new pages
+	 * for page table.
+	 */
+	memory_map_top_down(ISA_END_ADDRESS, end);
 
 #ifdef CONFIG_X86_64
 	if (max_pfn > max_low_pfn) {