[PATCH] x86, numa: exit early on numa_reset_distance()

[PATCH] x86, numa: exit early on numa_reset_distance()

[Yinghai Lu]

Do not call __pa(numa_distance), if it is not allocated before.

it will get BUG_ON if VIRTUAL_DEBUG is on.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/mm/numa_64.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: linux-2.6/arch/x86/mm/numa_64.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/numa_64.c
+++ linux-2.6/arch/x86/mm/numa_64.c
@@ -371,6 +371,12 @@ static void __init numa_reset_distance(v
 {
 	size_t size;
 
+	if (!numa_distance_cnt) {
+		numa_distance = NULL;
+
+		return;
+	}
+
 	size = numa_distance_cnt * sizeof(numa_distance[0]);
 	memblock_x86_free_range(__pa(numa_distance),
 				__pa(numa_distance) + size);

[PATCH] x86, numa: seperate alloc_numa_distance from numa_reset_distance.

[Yinghai Lu]

alloc code is much bigger the setting self.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/mm/numa_64.c |   77 +++++++++++++++++++++++++++-----------------------
 1 file changed, 42 insertions(+), 35 deletions(-)

Index: linux-2.6/arch/x86/mm/numa_64.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/numa_64.c
+++ linux-2.6/arch/x86/mm/numa_64.c
@@ -384,6 +384,43 @@ static void __init numa_reset_distance(v
 	numa_distance_cnt = 0;
 }
 
+static void __init alloc_numa_distance(void)
+{
+	nodemask_t nodes_parsed;
+	size_t size;
+	int i, j, cnt = 0;
+	u64 phys;
+
+	/* size the new table and allocate it */
+	nodes_parsed = numa_nodes_parsed;
+	numa_nodemask_from_meminfo(&nodes_parsed, &numa_meminfo);
+
+	for_each_node_mask(i, nodes_parsed)
+		cnt = i;
+	size = ++cnt * sizeof(numa_distance[0]);
+
+	phys = memblock_find_in_range(0,
+				      (u64)max_pfn_mapped << PAGE_SHIFT,
+				      size, PAGE_SIZE);
+	if (phys == MEMBLOCK_ERROR) {
+		pr_warning("NUMA: Warning: can't allocate distance table!\n");
+		/* don't retry until explicitly reset */
+		numa_distance = (void *)1LU;
+		return;
+	}
+	memblock_x86_reserve_range(phys, phys + size, "NUMA DIST");
+
+	numa_distance = __va(phys);
+	numa_distance_cnt = cnt;
+
+	/* fill with the default distances */
+	for (i = 0; i < cnt; i++)
+		for (j = 0; j < cnt; j++)
+			numa_distance[i * cnt + j] = i == j ?
+				LOCAL_DISTANCE : REMOTE_DISTANCE;
+	printk(KERN_DEBUG "NUMA: Initialized distance table, cnt=%d\n", cnt);
+}
+
 /*
  * Set the distance between node @from to @to to @distance.  If distance
  * table doesn't exist, one which is large enough to accomodate all the
@@ -391,41 +428,11 @@ static void __init numa_reset_distance(v
  */
 void __init numa_set_distance(int from, int to, int distance)
 {
-	if (!numa_distance) {
-		nodemask_t nodes_parsed;
-		size_t size;
-		int i, j, cnt = 0;
-		u64 phys;
-
-		/* size the new table and allocate it */
-		nodes_parsed = numa_nodes_parsed;
-		numa_nodemask_from_meminfo(&nodes_parsed, &numa_meminfo);
-
-		for_each_node_mask(i, nodes_parsed)
-			cnt = i;
-		size = ++cnt * sizeof(numa_distance[0]);
-
-		phys = memblock_find_in_range(0,
-					      (u64)max_pfn_mapped << PAGE_SHIFT,
-					      size, PAGE_SIZE);
-		if (phys == MEMBLOCK_ERROR) {
-			pr_warning("NUMA: Warning: can't allocate distance table!\n");
-			/* don't retry until explicitly reset */
-			numa_distance = (void *)1LU;
-			return;
-		}
-		memblock_x86_reserve_range(phys, phys + size, "NUMA DIST");
-
-		numa_distance = __va(phys);
-		numa_distance_cnt = cnt;
-
-		/* fill with the default distances */
-		for (i = 0; i < cnt; i++)
-			for (j = 0; j < cnt; j++)
-				numa_distance[i * cnt + j] = i == j ?
-					LOCAL_DISTANCE : REMOTE_DISTANCE;
-		printk(KERN_DEBUG "NUMA: Initialized distance table, cnt=%d\n", cnt);
-	}
+	if (!numa_distance)
+		alloc_numa_distance();
+
+	if (!numa_distance_cnt)
+		return;
 
 	if (from >= numa_distance_cnt || to >= numa_distance_cnt) {
 		printk_once(KERN_DEBUG "NUMA: Debug: distance out of bound, from=%d to=%d distance=%d\n",

Re: [PATCH] x86, numa: seperate alloc_numa_distance from numa_reset_distance.

[Cyrill Gorcunov]

On 02/16/2011 11:57 PM, Yinghai Lu wrote:
>
> alloc code is much bigger the setting self.
>
...
> +
> +	numa_distance = __va(phys);
> +	numa_distance_cnt = cnt;
> +
> +	/* fill with the default distances */
> +	for (i = 0; i<  cnt; i++)
> +		for (j = 0; j<  cnt; j++)
> +			numa_distance[i * cnt + j] = i == j ?
> +				LOCAL_DISTANCE : REMOTE_DISTANCE;
> +	printk(KERN_DEBUG "NUMA: Initialized distance table, cnt=%d\n", cnt);
> +}
> +

   Hi Yinghai, btw would it be convenient to use node_distance() helper here
(since this snippet is touched anyway and there is a big merging of numa
  code), ie

	/* fill with the default distances */
	for (i = 0; i < cnt; i++)
		for (j = 0; j <  cnt; j++)
			numa_distance[i * cnt + j] = node_distance(i, j);
	
though it should be patch on top of yours to not mix with plain code move
-- 
     Cyrill

Re: [PATCH] x86, numa: seperate alloc_numa_distance from numa_reset_distance.

[Yinghai Lu]

On 02/16/2011 01:44 PM, Cyrill Gorcunov wrote:
> On 02/16/2011 11:57 PM, Yinghai Lu wrote:
>>
>> alloc code is much bigger the setting self.
>>
> ...
>> +
>> +    numa_distance = __va(phys);
>> +    numa_distance_cnt = cnt;
>> +
>> +    /* fill with the default distances */
>> +    for (i = 0; i<  cnt; i++)
>> +        for (j = 0; j<  cnt; j++)
>> +            numa_distance[i * cnt + j] = i == j ?
>> +                LOCAL_DISTANCE : REMOTE_DISTANCE;
>> +    printk(KERN_DEBUG "NUMA: Initialized distance table, cnt=%d\n",
>> cnt);
>> +}
>> +
> 
>   Hi Yinghai, btw would it be convenient to use node_distance() helper here
> (since this snippet is touched anyway and there is a big merging of numa
>  code), ie
> 
>     /* fill with the default distances */
>     for (i = 0; i < cnt; i++)
>         for (j = 0; j <  cnt; j++)
>             numa_distance[i * cnt + j] = node_distance(i, j);
>     
> though it should be patch on top of yours to not mix with plain code move

do you mean

int __node_distance(int from, int to)
{
        if (from >= numa_distance_cnt || to >= numa_distance_cnt)
                return from == to ? LOCAL_DISTANCE : REMOTE_DISTANCE;
        return numa_distance[from * numa_distance_cnt + to];
}
EXPORT_SYMBOL(__node_distance);

or node_distance()?

then you will have 

 numa_distance[i * cnt + j] =  numa_distance[i * cnt + j];

...

Re: [PATCH] x86, numa: seperate alloc_numa_distance from numa_reset_distance.

[Cyrill Gorcunov]

On 02/17/2011 01:29 AM, Yinghai Lu wrote:
> On 02/16/2011 01:44 PM, Cyrill Gorcunov wrote:
>> On 02/16/2011 11:57 PM, Yinghai Lu wrote:
>>>
>>> alloc code is much bigger the setting self.
>>>
>> ...
>>> +
>>> +    numa_distance = __va(phys);
>>> +    numa_distance_cnt = cnt;
>>> +
>>> +    /* fill with the default distances */
>>> +    for (i = 0; i<   cnt; i++)
>>> +        for (j = 0; j<   cnt; j++)
>>> +            numa_distance[i * cnt + j] = i == j ?
>>> +                LOCAL_DISTANCE : REMOTE_DISTANCE;
>>> +    printk(KERN_DEBUG "NUMA: Initialized distance table, cnt=%d\n",
>>> cnt);
>>> +}
>>> +
>>
>>    Hi Yinghai, btw would it be convenient to use node_distance() helper here
>> (since this snippet is touched anyway and there is a big merging of numa
>>   code), ie
>>
>>      /* fill with the default distances */
>>      for (i = 0; i<  cnt; i++)
>>          for (j = 0; j<   cnt; j++)
>>              numa_distance[i * cnt + j] = node_distance(i, j);
>>
>> though it should be patch on top of yours to not mix with plain code move
>
> do you mean
>
> int __node_distance(int from, int to)
> {
>          if (from>= numa_distance_cnt || to>= numa_distance_cnt)
>                  return from == to ? LOCAL_DISTANCE : REMOTE_DISTANCE;
>          return numa_distance[from * numa_distance_cnt + to];
> }
> EXPORT_SYMBOL(__node_distance);
>
> or node_distance()?
>
> then you will have
>
>   numa_distance[i * cnt + j] =  numa_distance[i * cnt + j];
>
> ...

Ah, yeah, I mentally swapped >= to <= which is not true of course,
drop this idea, sorry.

-- 
     Cyrill

Re: [PATCH] x86, numa: seperate alloc_numa_distance from numa_reset_distance.

[Tejun Heo]

On Wed, Feb 16, 2011 at 12:57:36PM -0800, Yinghai Lu wrote:
> 
> alloc code is much bigger the setting self.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

Again, the same thing.  It just depends.  Maybe it's slightly more
readable this way.  Still falls in the realm of bikeshedding.

--
tejun

[PATCH] x86, numa: refactoring numa_register_memblks

[Yinghai Lu]

Don't hide init_memory_mapping and setup_bootmem into that __register__ function

those are really work.

Also We don't need to scan two times for setup_node_bootmem() becase we
are using mapped memblock for node_data already.

let num_register_memblks only take care of register them into early_node_map[]

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/mm/numa_64.c |   47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

Index: linux-2.6/arch/x86/mm/numa_64.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/numa_64.c
+++ linux-2.6/arch/x86/mm/numa_64.c
@@ -491,7 +491,7 @@ static bool __init numa_meminfo_cover_me
 
 static int __init numa_register_memblks(struct numa_meminfo *mi)
 {
-	int i, j, nid;
+	int i;
 
 	/* Account for nodes with cpus and no memory */
 	node_possible_map = numa_nodes_parsed;
@@ -515,33 +515,34 @@ static int __init numa_register_memblks(
 	if (!numa_meminfo_cover_memory(mi))
 		return -EINVAL;
 
-	init_memory_mapping_high();
+	return 0;
+}
+
+static void __init setup_numa_bootmem(struct numa_meminfo *mi)
+{
+	int i, nid;
 
 	/*
-	 * Finally register nodes.  Do it twice in case setup_node_bootmem
-	 * missed one due to missing bootmem.
+	 * Do not do that twice, not needed!
+	 *   We are using mapped memblock directly for node data
 	 */
-	for (i = 0; i < 2; i++) {
-		for_each_node_mask(nid, node_possible_map) {
-			u64 start = (u64)max_pfn << PAGE_SHIFT;
-			u64 end = 0;
-
-			if (node_online(nid))
-				continue;
+	for_each_node_mask(nid, node_possible_map) {
+		u64 start = (u64)max_pfn << PAGE_SHIFT;
+		u64 end = 0;
 
-			for (j = 0; j < mi->nr_blks; j++) {
-				if (nid != mi->blk[j].nid)
-					continue;
-				start = min(mi->blk[j].start, start);
-				end = max(mi->blk[j].end, end);
-			}
+		if (node_online(nid))
+			continue;
 
-			if (start < end)
-				setup_node_bootmem(nid, start, end);
+		for (i = 0; i < mi->nr_blks; i++) {
+			if (nid != mi->blk[i].nid)
+				continue;
+			start = min(mi->blk[i].start, start);
+			end = max(mi->blk[i].end, end);
 		}
-	}
 
-	return 0;
+		if (start < end)
+			setup_node_bootmem(nid, start, end);
+	}
 }
 
 #ifdef CONFIG_NUMA_EMU
@@ -968,6 +969,10 @@ void __init initmem_init(void)
 		if (numa_register_memblks(&numa_meminfo) < 0)
 			continue;
 
+		init_memory_mapping_high();
+
+		setup_numa_bootmem(&numa_meminfo);
+
 		for (j = 0; j < nr_cpu_ids; j++) {
 			int nid = early_cpu_to_node(j);
 

Re: [PATCH] x86, numa: refactoring numa_register_memblks

[Tejun Heo]

Hello,

On Wed, Feb 16, 2011 at 12:58:41PM -0800, Yinghai Lu wrote:
> 
> Don't hide init_memory_mapping and setup_bootmem into that __register__ function
> 
> those are really work.

What does that mean?  What isn't really work?

> Also We don't need to scan two times for setup_node_bootmem() becase we
> are using mapped memblock for node_data already.

If this isn't necessary, please make this change in a separate patch.
This involves behavior change.

> @@ -968,6 +969,10 @@ void __init initmem_init(void)
>  		if (numa_register_memblks(&numa_meminfo) < 0)
>  			continue;
>  
> +		init_memory_mapping_high();
> +
> +		setup_numa_bootmem(&numa_meminfo);
> +

Sorry, nack.  This squarely falls in the realm of bikeshedding and I
plan on collapsing init_memory_mapping_high() into the register
function.

Thanks.

--
tejun

Re: [PATCH] x86, numa: refactoring numa_register_memblks

[Yinghai Lu]

On 02/16/2011 02:43 PM, Tejun Heo wrote:
> Hello,
> 
> On Wed, Feb 16, 2011 at 12:58:41PM -0800, Yinghai Lu wrote:
>>
>> Don't hide init_memory_mapping and setup_bootmem into that __register__ function
>>
>> those are really work.
> 
> What does that mean?  What isn't really work?

i mean: really initmem init work.

> 
>> Also We don't need to scan two times for setup_node_bootmem() becase we
>> are using mapped memblock for node_data already.
> 
> If this isn't necessary, please make this change in a separate patch.
> This involves behavior change.

ok.


> 
>> @@ -968,6 +969,10 @@ void __init initmem_init(void)
>>  		if (numa_register_memblks(&numa_meminfo) < 0)
>>  			continue;
>>  
>> +		init_memory_mapping_high();
>> +
>> +		setup_numa_bootmem(&numa_meminfo);
>> +
> 
> Sorry, nack.  This squarely falls in the realm of bikeshedding and I
> plan on collapsing init_memory_mapping_high() into the register
> function.

no. init_memory_mapping_high now it is with early_node_map[], aka it is e820 and srat table overlapping one.

Yinghai

Re: [PATCH] x86, numa: refactoring numa_register_memblks

[Tejun Heo]

On Wed, Feb 16, 2011 at 02:53:02PM -0800, Yinghai Lu wrote:
> > Sorry, nack.  This squarely falls in the realm of bikeshedding and I
> > plan on collapsing init_memory_mapping_high() into the register
> > function.
> 
> no. init_memory_mapping_high now it is with early_node_map[], aka it
> is e820 and srat table overlapping one.

Hmmm... okay, I'll think more about it but currently it looks silly.
It isn't necessary for !NUMA and is done in rather convoluted way in
NUMA.  The name is misleading too.  There's nothing like mapped high
memory.  Maybe it would be better to use suffix like _above_4g.  At
any rate, I don't really see the point of moving those functions
calls.

The way I see it is that these things are inherently subjective.  When
you're writing or updating the part, it's natural to follow your own
preference.  When reviewing or working on other's code, trying to
enforce all those subjective details doesn't really help anyone.  Push
the ones with technical advantages; otherwise, just let it go.

Thanks.

--
tejun

[PATCH] x86, numa: put dummy_numa_init in init section

[Yinghai Lu]

like other __numa_init

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/mm/numa_64.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/arch/x86/mm/numa_64.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/numa_64.c
+++ linux-2.6/arch/x86/mm/numa_64.c
@@ -897,7 +897,7 @@ static bool __init numa_emulation(void)
 }
 #endif /* CONFIG_NUMA_EMU */
 
-static int dummy_numa_init(void)
+static int __init dummy_numa_init(void)
 {
 	printk(KERN_INFO "%s\n",
 	       numa_off ? "NUMA turned off" : "No NUMA configuration found");

Re: [PATCH] x86, numa: put dummy_numa_init in init section

[Tejun Heo]

On Wed, Feb 16, 2011 at 02:30:54PM -0800, Yinghai Lu wrote:
> 
> like other __numa_init
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

Acked-by: Tejun Heo <tj@kernel.org>

--
tejun

[PATCH -v2] x86: Rename e820_table_* to pgt_buf_*

[Yinghai Lu]

Now it is found from memblock way.

Change the name to purpose related.

-v2: Ingo found "x86: Use early pre-allocated page table buffer top-down"
     cause 32bit crash.
     and need to drop it, So update this one accordingly.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/include/asm/init.h |    6 +++---
 arch/x86/mm/init.c          |   20 ++++++++++----------
 arch/x86/mm/init_32.c       |    8 ++++----
 arch/x86/mm/init_64.c       |    4 ++--
 arch/x86/xen/mmu.c          |    2 +-
 5 files changed, 20 insertions(+), 20 deletions(-)

Index: linux-2.6/arch/x86/include/asm/init.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/init.h
+++ linux-2.6/arch/x86/include/asm/init.h
@@ -11,8 +11,8 @@ kernel_physical_mapping_init(unsigned lo
 			     unsigned long page_size_mask);
 
 
-extern unsigned long __initdata e820_table_start;
-extern unsigned long __meminitdata e820_table_end;
-extern unsigned long __meminitdata e820_table_top;
+extern unsigned long __initdata pgt_buf_start;
+extern unsigned long __meminitdata pgt_buf_end;
+extern unsigned long __meminitdata pgt_buf_top;
 
 #endif /* _ASM_X86_INIT_32_H */
Index: linux-2.6/arch/x86/mm/init.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/init.c
+++ linux-2.6/arch/x86/mm/init.c
@@ -18,9 +18,9 @@
 
 DEFINE_PER_CPU(struct mmu_gather, mmu_gathers);
 
-unsigned long __initdata e820_table_start;
-unsigned long __meminitdata e820_table_end;
-unsigned long __meminitdata e820_table_top;
+unsigned long __initdata pgt_buf_start;
+unsigned long __meminitdata pgt_buf_end;
+unsigned long __meminitdata pgt_buf_top;
 
 int after_bootmem;
 
@@ -73,12 +73,12 @@ static void __init find_early_table_spac
 	if (base == MEMBLOCK_ERROR)
 		panic("Cannot find space for the kernel page tables");
 
-	e820_table_start = base >> PAGE_SHIFT;
-	e820_table_end = e820_table_start;
-	e820_table_top = e820_table_start + (tables >> PAGE_SHIFT);
+	pgt_buf_start = base >> PAGE_SHIFT;
+	pgt_buf_end = pgt_buf_start;
+	pgt_buf_top = pgt_buf_start + (tables >> PAGE_SHIFT);
 
 	printk(KERN_DEBUG "kernel direct mapping tables up to %lx @ %lx-%lx\n",
-		end, e820_table_start << PAGE_SHIFT, e820_table_top << PAGE_SHIFT);
+		end, pgt_buf_start << PAGE_SHIFT, pgt_buf_top << PAGE_SHIFT);
 }
 
 struct map_range {
@@ -272,9 +272,9 @@ unsigned long __init_refok init_memory_m
 
 	__flush_tlb_all();
 
-	if (!after_bootmem && e820_table_end > e820_table_start)
-		memblock_x86_reserve_range(e820_table_start << PAGE_SHIFT,
-				 e820_table_end << PAGE_SHIFT, "PGTABLE");
+	if (!after_bootmem && pgt_buf_end > pgt_buf_start)
+		memblock_x86_reserve_range(pgt_buf_start << PAGE_SHIFT,
+				 pgt_buf_end << PAGE_SHIFT, "PGTABLE");
 
 	if (!after_bootmem)
 		early_memtest(start, end);
Index: linux-2.6/arch/x86/mm/init_32.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/init_32.c
+++ linux-2.6/arch/x86/mm/init_32.c
@@ -62,10 +62,10 @@ bool __read_mostly __vmalloc_start_set =
 
 static __init void *alloc_low_page(void)
 {
-	unsigned long pfn = e820_table_end++;
+	unsigned long pfn = pgt_buf_end++;
 	void *adr;
 
-	if (pfn >= e820_table_top)
+	if (pfn >= pgt_buf_top)
 		panic("alloc_low_page: ran out of memory");
 
 	adr = __va(pfn * PAGE_SIZE);
@@ -163,8 +163,8 @@ static pte_t *__init page_table_kmap_che
 	if (pmd_idx_kmap_begin != pmd_idx_kmap_end
 	    && (vaddr >> PMD_SHIFT) >= pmd_idx_kmap_begin
 	    && (vaddr >> PMD_SHIFT) <= pmd_idx_kmap_end
-	    && ((__pa(pte) >> PAGE_SHIFT) < e820_table_start
-		|| (__pa(pte) >> PAGE_SHIFT) >= e820_table_end)) {
+	    && ((__pa(pte) >> PAGE_SHIFT) < pgt_buf_start
+		|| (__pa(pte) >> PAGE_SHIFT) >= pgt_buf_end)) {
 		pte_t *newpte;
 		int i;
 
Index: linux-2.6/arch/x86/mm/init_64.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/init_64.c
+++ linux-2.6/arch/x86/mm/init_64.c
@@ -314,7 +314,7 @@ void __init cleanup_highmap(void)
 
 static __ref void *alloc_low_page(unsigned long *phys)
 {
-	unsigned long pfn = e820_table_end++;
+	unsigned long pfn = pgt_buf_end++;
 	void *adr;
 
 	if (after_bootmem) {
@@ -324,7 +324,7 @@ static __ref void *alloc_low_page(unsign
 		return adr;
 	}
 
-	if (pfn >= e820_table_top)
+	if (pfn >= pgt_buf_top)
 		panic("alloc_low_page: ran out of memory");
 
 	adr = early_memremap(pfn * PAGE_SIZE, PAGE_SIZE);
Index: linux-2.6/arch/x86/xen/mmu.c
===================================================================
--- linux-2.6.orig/arch/x86/xen/mmu.c
+++ linux-2.6/arch/x86/xen/mmu.c
@@ -1443,7 +1443,7 @@ static __init pte_t mask_rw_pte(pte_t *p
 	 * early_ioremap fixmap slot, make sure it is RO.
 	 */
 	if (!is_early_ioremap_ptep(ptep) &&
-	    pfn >= e820_table_start && pfn < e820_table_end)
+	    pfn >= pgt_buf_start && pfn < pgt_buf_end)
 		pte = pte_wrprotect(pte);
 
 	return pte;

[PATCH] x86: Rename e820_table_* to pgt_buf_*

[Tejun Heo]

>From d1b19426b04787e48f2689923e28d37b488969b0 Mon Sep 17 00:00:00 2001
From: Yinghai Lu <yinghai@kernel.org>
Date: Thu, 24 Feb 2011 14:46:24 +0100

e820_table_{start|end|top}, which are used to buffer page table
allocation during early boot, are now derived from memblock and don't
have much to do with e820.  Change the names so that they reflect what
they're used for.

This patch doesn't introduce any behavior change.

-v2: Ingo found that earlier patch "x86: Use early pre-allocated page
     table buffer top-down" caused crash on 32bit and needed to be
     dropped.  This patch was updated to reflect the change.

-tj: Updated commit description.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
Committed to misc:x86-numa with description updated.  Thanks.

 arch/x86/include/asm/init.h |    6 +++---
 arch/x86/mm/init.c          |   20 ++++++++++----------
 arch/x86/mm/init_32.c       |    8 ++++----
 arch/x86/mm/init_64.c       |    4 ++--
 arch/x86/xen/mmu.c          |    2 +-
 5 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/x86/include/asm/init.h b/arch/x86/include/asm/init.h
index 36fb1a6..8dbe353 100644
--- a/arch/x86/include/asm/init.h
+++ b/arch/x86/include/asm/init.h
@@ -11,8 +11,8 @@ kernel_physical_mapping_init(unsigned long start,
 			     unsigned long page_size_mask);
 
 
-extern unsigned long __initdata e820_table_start;
-extern unsigned long __meminitdata e820_table_end;
-extern unsigned long __meminitdata e820_table_top;
+extern unsigned long __initdata pgt_buf_start;
+extern unsigned long __meminitdata pgt_buf_end;
+extern unsigned long __meminitdata pgt_buf_top;
 
 #endif /* _ASM_X86_INIT_32_H */
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index b8054e0..286d289 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -18,9 +18,9 @@
 
 DEFINE_PER_CPU(struct mmu_gather, mmu_gathers);
 
-unsigned long __initdata e820_table_start;
-unsigned long __meminitdata e820_table_end;
-unsigned long __meminitdata e820_table_top;
+unsigned long __initdata pgt_buf_start;
+unsigned long __meminitdata pgt_buf_end;
+unsigned long __meminitdata pgt_buf_top;
 
 int after_bootmem;
 
@@ -73,12 +73,12 @@ static void __init find_early_table_space(unsigned long end, int use_pse,
 	if (base == MEMBLOCK_ERROR)
 		panic("Cannot find space for the kernel page tables");
 
-	e820_table_start = base >> PAGE_SHIFT;
-	e820_table_end = e820_table_start;
-	e820_table_top = e820_table_start + (tables >> PAGE_SHIFT);
+	pgt_buf_start = base >> PAGE_SHIFT;
+	pgt_buf_end = pgt_buf_start;
+	pgt_buf_top = pgt_buf_start + (tables >> PAGE_SHIFT);
 
 	printk(KERN_DEBUG "kernel direct mapping tables up to %lx @ %lx-%lx\n",
-		end, e820_table_start << PAGE_SHIFT, e820_table_top << PAGE_SHIFT);
+		end, pgt_buf_start << PAGE_SHIFT, pgt_buf_top << PAGE_SHIFT);
 }
 
 struct map_range {
@@ -272,9 +272,9 @@ unsigned long __init_refok init_memory_mapping(unsigned long start,
 
 	__flush_tlb_all();
 
-	if (!after_bootmem && e820_table_end > e820_table_start)
-		memblock_x86_reserve_range(e820_table_start << PAGE_SHIFT,
-				 e820_table_end << PAGE_SHIFT, "PGTABLE");
+	if (!after_bootmem && pgt_buf_end > pgt_buf_start)
+		memblock_x86_reserve_range(pgt_buf_start << PAGE_SHIFT,
+				 pgt_buf_end << PAGE_SHIFT, "PGTABLE");
 
 	if (!after_bootmem)
 		early_memtest(start, end);
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index 5d43fa5..73ad7eb 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -62,10 +62,10 @@ bool __read_mostly __vmalloc_start_set = false;
 
 static __init void *alloc_low_page(void)
 {
-	unsigned long pfn = e820_table_end++;
+	unsigned long pfn = pgt_buf_end++;
 	void *adr;
 
-	if (pfn >= e820_table_top)
+	if (pfn >= pgt_buf_top)
 		panic("alloc_low_page: ran out of memory");
 
 	adr = __va(pfn * PAGE_SIZE);
@@ -163,8 +163,8 @@ static pte_t *__init page_table_kmap_check(pte_t *pte, pmd_t *pmd,
 	if (pmd_idx_kmap_begin != pmd_idx_kmap_end
 	    && (vaddr >> PMD_SHIFT) >= pmd_idx_kmap_begin
 	    && (vaddr >> PMD_SHIFT) <= pmd_idx_kmap_end
-	    && ((__pa(pte) >> PAGE_SHIFT) < e820_table_start
-		|| (__pa(pte) >> PAGE_SHIFT) >= e820_table_end)) {
+	    && ((__pa(pte) >> PAGE_SHIFT) < pgt_buf_start
+		|| (__pa(pte) >> PAGE_SHIFT) >= pgt_buf_end)) {
 		pte_t *newpte;
 		int i;
 
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 4f1f461..470cc47 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -314,7 +314,7 @@ void __init cleanup_highmap(void)
 
 static __ref void *alloc_low_page(unsigned long *phys)
 {
-	unsigned long pfn = e820_table_end++;
+	unsigned long pfn = pgt_buf_end++;
 	void *adr;
 
 	if (after_bootmem) {
@@ -324,7 +324,7 @@ static __ref void *alloc_low_page(unsigned long *phys)
 		return adr;
 	}
 
-	if (pfn >= e820_table_top)
+	if (pfn >= pgt_buf_top)
 		panic("alloc_low_page: ran out of memory");
 
 	adr = early_memremap(pfn * PAGE_SIZE, PAGE_SIZE);
diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 5e92b61..13783a1 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -1443,7 +1443,7 @@ static __init pte_t mask_rw_pte(pte_t *ptep, pte_t pte)
 	 * early_ioremap fixmap slot, make sure it is RO.
 	 */
 	if (!is_early_ioremap_ptep(ptep) &&
-	    pfn >= e820_table_start && pfn < e820_table_end)
+	    pfn >= pgt_buf_start && pfn < pgt_buf_end)
 		pte = pte_wrprotect(pte);
 
 	return pte;
-- 
1.7.1

[PATCH] x86, numa: cleanup x86_acpi_numa_init()

[Yinghai Lu]

make it more readable. put valid checking together.

Also restore old acpi_numa_init(). we don't need to touch it
because already have x86 own wrapper.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/mm/srat_64.c |   13 ++++++++++++-
 drivers/acpi/numa.c   |    8 +++-----
 2 files changed, 15 insertions(+), 6 deletions(-)

Index: linux-2.6/arch/x86/mm/srat_64.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/srat_64.c
+++ linux-2.6/arch/x86/mm/srat_64.c
@@ -240,9 +240,20 @@ int __init x86_acpi_numa_init(void)
 	int ret;
 
 	ret = acpi_numa_init();
+
+	/* no srat */
+	if (!ret)
+		return -ENOENT;
+
+	/* parse failed */
 	if (ret < 0)
 		return ret;
-	return srat_disabled() ? -EINVAL : 0;
+
+	/* bad_srat() */
+	if (srat_disabled())
+		return -EINVAL;
+
+	return 0;
 }
 
 #if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) || defined(CONFIG_ACPI_HOTPLUG_MEMORY)
Index: linux-2.6/drivers/acpi/numa.c
===================================================================
--- linux-2.6.orig/drivers/acpi/numa.c
+++ linux-2.6/drivers/acpi/numa.c
@@ -274,7 +274,7 @@ acpi_table_parse_srat(enum acpi_srat_typ
 
 int __init acpi_numa_init(void)
 {
-	int cnt = 0;
+	int ret = 0;
 
 	/*
 	 * Should not limit number with cpu num that is from NR_CPUS or nr_cpus=
@@ -288,7 +288,7 @@ int __init acpi_numa_init(void)
 				     acpi_parse_x2apic_affinity, 0);
 		acpi_table_parse_srat(ACPI_SRAT_TYPE_CPU_AFFINITY,
 				     acpi_parse_processor_affinity, 0);
-		cnt = acpi_table_parse_srat(ACPI_SRAT_TYPE_MEMORY_AFFINITY,
+		ret = acpi_table_parse_srat(ACPI_SRAT_TYPE_MEMORY_AFFINITY,
 					    acpi_parse_memory_affinity,
 					    NR_NODE_MEMBLKS);
 	}
@@ -298,9 +298,7 @@ int __init acpi_numa_init(void)
 
 	acpi_numa_arch_fixup();
 
-	if (cnt <= 0)
-		return cnt ?: -ENOENT;
-	return 0;
+	return ret;
 }
 
 int acpi_get_pxm(acpi_handle h)

Re: [PATCH] x86, numa: cleanup x86_acpi_numa_init()

[Tejun Heo]

On Wed, Feb 16, 2011 at 02:31:34PM -0800, Yinghai Lu wrote:
> make it more readable. put valid checking together.
> 
> Also restore old acpi_numa_init(). we don't need to touch it
> because already have x86 own wrapper.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

What's the point of this patch?  If someone else likes it, I don't
object but I have no idea why this is any better than before.

--
tejun

Re: [PATCH] x86, numa: cleanup x86_acpi_numa_init()

[Yinghai Lu]

On 02/16/2011 02:49 PM, Tejun Heo wrote:
> On Wed, Feb 16, 2011 at 02:31:34PM -0800, Yinghai Lu wrote:
>> make it more readable. put valid checking together.
>>
>> Also restore old acpi_numa_init(). we don't need to touch it
>> because already have x86 own wrapper.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> What's the point of this patch?  If someone else likes it, I don't
> object but I have no idea why this is any better than before.

before you added x86_acpi_numa_init(), you modified acpi_numa_init...

you really don't need to touch other acpi code, if we can limit that in x86 code.

Yinghai

Re: [PATCH] x86, numa: exit early on numa_reset_distance()

[Tejun Heo]

On Wed, Feb 16, 2011 at 12:56:47PM -0800, Yinghai Lu wrote:
> 
> Do not call __pa(numa_distance), if it is not allocated before.
> 
> it will get BUG_ON if VIRTUAL_DEBUG is on.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

Thanks for spotting this.

> Index: linux-2.6/arch/x86/mm/numa_64.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/mm/numa_64.c
> +++ linux-2.6/arch/x86/mm/numa_64.c
> @@ -371,6 +371,12 @@ static void __init numa_reset_distance(v
>  {
>  	size_t size;
>  
> +	if (!numa_distance_cnt) {
> +		numa_distance = NULL;
> +
> +		return;
> +	}
> +

But please move the existing numa_distance = NULL before the
conditional and do if (!numa_distance_cnt) return;

Thanks.

--
tejun

Re: [PATCH] x86, numa: exit early on numa_reset_distance()

[Yinghai Lu]

On 02/16/2011 02:39 PM, Tejun Heo wrote:
> On Wed, Feb 16, 2011 at 12:56:47PM -0800, Yinghai Lu wrote:
>>
>> Do not call __pa(numa_distance), if it is not allocated before.
>>
>> it will get BUG_ON if VIRTUAL_DEBUG is on.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> Thanks for spotting this.
> 
>> Index: linux-2.6/arch/x86/mm/numa_64.c
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/mm/numa_64.c
>> +++ linux-2.6/arch/x86/mm/numa_64.c
>> @@ -371,6 +371,12 @@ static void __init numa_reset_distance(v
>>  {
>>  	size_t size;
>>  
>> +	if (!numa_distance_cnt) {
>> +		numa_distance = NULL;
>> +
>> +		return;
>> +	}
>> +
> 
> But please move the existing numa_distance = NULL before the
> conditional and do if (!numa_distance_cnt) return;

confused. Do you mean this one ?

[PATCH -v2] x86, numa: exit early on numa_reset_distance()

Do not call __pa(numa_distance), if it is not allocated before.

it will get BUG_ON if VIRTUAL_DEBUG is on.

-v2: change to check existing path as tj requested.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/mm/numa_64.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Index: linux-2.6/arch/x86/mm/numa_64.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/numa_64.c
+++ linux-2.6/arch/x86/mm/numa_64.c
@@ -371,11 +371,13 @@ static void __init numa_reset_distance(v
 {
 	size_t size;
 
-	size = numa_distance_cnt * sizeof(numa_distance[0]);
-	memblock_x86_free_range(__pa(numa_distance),
-				__pa(numa_distance) + size);
+	if (numa_distance_cnt) {
+		size = numa_distance_cnt * sizeof(numa_distance[0]);
+		memblock_x86_free_range(__pa(numa_distance),
+					__pa(numa_distance) + size);
+		numa_distance_cnt = 0;
+	}
 	numa_distance = NULL;
-	numa_distance_cnt = 0;
 }
 
 /*

Re: [PATCH] x86, numa: exit early on numa_reset_distance()

[Tejun Heo]

On Wed, Feb 16, 2011 at 03:08:57PM -0800, Yinghai Lu wrote:
> > But please move the existing numa_distance = NULL before the
> > conditional and do if (!numa_distance_cnt) return;
> 
> confused. Do you mean this one ?

Heh, I was confused.

> @@ -371,11 +371,13 @@ static void __init numa_reset_distance(v
>  {
>  	size_t size;
>  
> -	size = numa_distance_cnt * sizeof(numa_distance[0]);
> -	memblock_x86_free_range(__pa(numa_distance),
> -				__pa(numa_distance) + size);
> +	if (numa_distance_cnt) {
> +		size = numa_distance_cnt * sizeof(numa_distance[0]);
> +		memblock_x86_free_range(__pa(numa_distance),
> +					__pa(numa_distance) + size);
> +		numa_distance_cnt = 0;
> +	}
>  	numa_distance = NULL;
> -	numa_distance_cnt = 0;

But I like this one better.

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

--
tejun