x86, numa: exit early on numa_reset_distance()

PATCH In review Yinghai Lu 3 participants 20 messages Feb 16, 2011 – Feb 24, 2011 5 messages not in the archive

Subsystems (4)

x86 ACPI x86 MM Xen hypervisor x86

lore

Share & tools

lore

Yinghai Lu

author

2011-02-16 20:57 UTC #0 lore raw

PATCH 1/1 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.

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

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

arch/x86/mm/numa_64.c +6 -0

Index: 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);

Yinghai Lu

author

2011-02-16 20:59 UTC #1 lore raw

PATCH 1/1 x86, numa: refactoring numa_register_memblks

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 <[email protected]>

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

arch/x86/mm/numa_64.c +26 ~~-21~~

Index: 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);

Yinghai Lu

author

2011-02-16 21:00 UTC #5 lore raw

PATCH 1/1 x86, numa: seperate alloc_numa_distance from numa_reset_distance.

In reply to Yinghai Lu

alloc code is much bigger the setting self.

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

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

arch/x86/mm/numa_64.c +42 ~~-35~~

Index: 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",

Cyrill Gorcunov

2011-02-16 21:44 UTC #6 lore raw

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

Quoted 3 lines · Yinghai Lu

alloc code is much bigger the setting self.

...

arch/x86/mm/numa_64.c static void __init numa_reset_distance(v · line 423

Quoted 12 lines

arch/x86/mm/numa_64.c +12 -0

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

Signature · 1 line

    Cyrill

Yinghai Lu

author

2011-02-16 22:30 UTC #7 lore raw

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

Quoted 23 lines · Cyrill Gorcunov

Quoted 3 lines · Yinghai Lu
alloc code is much bigger the setting self.
...
Review
Quoted 13 lines
arch/x86/mm/numa_64.c +10 -0
+
+    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);
(quoted diff) +2 -0
+}
+
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

Quoted 6 lines

    /* 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];

...

Yinghai Lu

author

2011-02-16 22:32 UTC #10 lore raw

PATCH 1/1 x86, numa: cleanup x86_acpi_numa_init()

In reply to 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 <[email protected]>

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

arch/x86/mm/srat_64.c +12 -1

Index: 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)

drivers/acpi/numa.c +3 -5

Index: 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)

Yinghai Lu

author

2011-02-16 22:32 UTC #13 lore raw

PATCH 1/1 x86, numa: put dummy_numa_init in init section

In reply to Yinghai Lu

like other __numa_init

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

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

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

Index: 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");

Tejun Heo

2011-02-16 22:39 UTC #22 lore raw

In reply to Yinghai Lu

Quoted 6 lines · 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 <[email protected]>

Thanks for spotting this.

Quoted 5 lines

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

arch/x86/mm/numa_64.c static void __init numa_reset_distance(v · line 379

Quoted 9 lines

arch/x86/mm/numa_64.c +6 -0

@@ -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

Tejun Heo

2011-02-16 22:43 UTC #2 lore raw

Re: [PATCH] x86, numa: refactoring numa_register_memblks

In reply to Yinghai Lu

Hello,

Quoted 4 lines · Yinghai Lu

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?

Quoted 2 lines

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.

arch/x86/mm/numa_64.c void __init initmem_init(void) · line 975

Quoted 8 lines

arch/x86/mm/numa_64.c +4 -0

@@ -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

Tejun Heo

Acked-by on v1 1/1

2011-02-16 22:44 UTC #14 lore raw

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

In reply to Yinghai Lu

Quoted 4 lines · Yinghai Lu

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

Acked-by Tejun Heo <[email protected]>

--
tejun

Cyrill Gorcunov

2011-02-16 22:47 UTC #8 lore raw

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

In reply to Yinghai Lu

Quoted 41 lines · Yinghai Lu

Quoted 23 lines · Cyrill Gorcunov
Quoted 3 lines · Yinghai Lu
alloc code is much bigger the setting self.
...
Review
Quoted 13 lines
arch/x86/mm/numa_64.c +10 -0
+
+    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);
(quoted diff) +2 -0
+}
+
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
Quoted 6 lines
     /* 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);

Quoted 7 lines

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.

Signature · 1 line

    Cyrill

Tejun Heo

2011-02-16 22:49 UTC #11 lore raw

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

In reply to Yinghai Lu

Quoted 6 lines · 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 <[email protected]>

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

Tejun Heo

2011-02-16 22:51 UTC #9 lore raw

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

In reply to Yinghai Lu

Quoted 4 lines · Yinghai Lu

alloc code is much bigger the setting self.
Signed-off-by Yinghai Lu <[email protected]>

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

--
tejun

Yinghai Lu

author

2011-02-16 22:55 UTC #3 lore raw

Re: [PATCH] x86, numa: refactoring numa_register_memblks

In reply to Tejun Heo

Quoted 9 lines · Tejun Heo

Hello,
Quoted 4 lines · Yinghai Lu
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.

Quoted 6 lines

Quoted 3 lines
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.

Quoted 5 lines

arch/x86/mm/numa_64.c +0 -0

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

Quoted 8 lines

arch/x86/mm/numa_64.c void __init initmem_init(void) · line 975
Quoted 4 lines
arch/x86/mm/numa_64.c +4 -0
+		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

Yinghai Lu

author

2011-02-16 22:57 UTC #12 lore raw

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

In reply to Tejun Heo

Quoted 10 lines · Tejun Heo

Quoted 6 lines · 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 <[email protected]>
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

Tejun Heo

2011-02-16 23:03 UTC #4 lore raw

Re: [PATCH] x86, numa: refactoring numa_register_memblks

In reply to Yinghai Lu

Quoted 6 lines · Yinghai Lu

Quoted 3 lines
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

Yinghai Lu

author

2011-02-16 23:10 UTC #23 lore raw

In reply to Tejun Heo

Quoted 18 lines · Tejun Heo

Quoted 6 lines · 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 <[email protected]>
Thanks for spotting this.
Quoted 8 lines
Index: linux-2.6/arch/x86/mm/numa_64.c
===================================================================
arch/x86/mm/numa_64.c +0 -0
@@ -371,6 +371,12 @@ static void __init numa_reset_distance(v
 {
 	size_t size;
 

Quoted 9 lines

arch/x86/mm/numa_64.c static void __init numa_reset_distance(v · line 379
Quoted 6 lines
arch/x86/mm/numa_64.c +6 -0
+	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 <[email protected]>

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

arch/x86/mm/numa_64.c +6 -4

Index: 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;
 }
 
 /*

Tejun Heo

Acked-by

2011-02-16 23:16 UTC #24 lore raw

Quoted 4 lines · Yinghai Lu

Quoted 2 lines
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.

Quoted 5 lines

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

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

arch/x86/mm/numa_64.c static void __init numa_reset_distance(v · line 378

Quoted 10 lines

arch/x86/mm/numa_64.c +6 -3

-	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 <[email protected]>

Thanks.

--
tejun

Yinghai Lu

author

2011-02-24 07:23 UTC #20 lore raw

PATCH 1/1 x86: Rename e820_table_* to pgt_buf_*

In reply to a message not in the archive

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 <[email protected]>

---
 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(-)

arch/x86/include/asm/init.h +3 -3

Index: 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 */

arch/x86/mm/init.c +10 ~~-10~~

Index: 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);

arch/x86/mm/init_32.c +4 -4

Index: 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;

arch/x86/mm/init_64.c +2 -2

Index: 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);

arch/x86/xen/mmu.c +1 -1

Index: 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;

Tejun Heo

2011-02-24 14:00 UTC #21 lore raw

PATCH 1/1 x86: Rename e820_table_* to pgt_buf_*

Quoted 1 line

From d1b19426b04787e48f2689923e28d37b488969b0 Mon Sep 17 00:00:00 2001

From: Yinghai Lu <[email protected]>
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 <[email protected]>

Signed-off-by Tejun Heo <[email protected]>

---
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(-)

arch/x86/include/asm/init.h +3 -3

index 36fb1a6..8dbe353 100644
@@ -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 */

arch/x86/mm/init.c +10 ~~-10~~

index b8054e0..286d289 100644
@@ -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);

arch/x86/mm/init_32.c +4 -4

index 5d43fa5..73ad7eb 100644
@@ -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;

arch/x86/mm/init_64.c +2 -2

index 4f1f461..470cc47 100644
@@ -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);

arch/x86/xen/mmu.c +1 -1

index 5e92b61..13783a1 100644
@@ -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;

Signature · 1 line

1.7.1