mm: Fix memory hotplug + sparsemem build.

mm/memory_hotplug.c int online_pages(unsigned long pfn, unsi

Quoted 5 lines

mm/memory_hotplug.c +1 -1

 	if (onlined_pages)
-		node_set_state(zone->node, N_HIGH_MEMORY);
+		node_set_state(zone_to_nid(zone), N_HIGH_MEMORY);
 
 	setup_per_zone_pages_min();

Thanks Paul-san.

I also have another issue around here.
(Kswapd doesn't run on memory less node now. It should run when
the node has memory.)

I would like to merge them like following if you don't mind.

mm/memory_hotplug.c int online_pages(unsigned long pfn, unsi

Quoted 5 lines

mm/memory_hotplug.c +1 -1

 	if (onlined_pages)
-		node_set_state(zone->node, N_HIGH_MEMORY);
+		node_set_state(zone_to_nid(zone), N_HIGH_MEMORY);
 
 	setup_per_zone_pages_min();

Thanks Paul-san.

I also have another issue around here.
(Kswapd doesn't run on memory less node now. It should run when
the node has memory.)

I would like to merge them like following if you don't mind.

Bye.

---

Fix kswapd doesn't run when memory is added on memory-less-node. Fix compile error of zone->node when CONFIG_NUMA is off.

Signed-off-by Yasunori Goto <[email protected]>

Signed-off-by Paul Mundt <[email protected]>

---
 mm/memory_hotplug.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Index: current/mm/memory_hotplug.c
===================================================================

mm/memory_hotplug.c +3 -0

+		kswapd_run(zone_to_nid(zone));
+		node_set_state(zone_to_nid(zone), N_HIGH_MEMORY);
+	}
 
 	if (need_zonelists_rebuild)
 		build_all_zonelists();

Review

Quoted 1 line

(quoted diff) +1 -0

+	if (onlined_pages){

Nit, needs a space there before the '{'.

The problem as I see it is that when we boot the system we start a kswapd on all nodes with memory. If the hot-add adds memory to a pre-existing node with no memory we will not start one and we end up with a node with memory and no kswapd. Bad.

As kswapd_run is a no-op when a kswapd already exists this seems a safe way to fix that. Paul's ->zone conversion is obviously correct also.

Acked-by Andy Whitcroft <[email protected]>

Quoted 4 lines

Review

Quoted 2 lines

(quoted diff) +1 -0

+	if (onlined_pages){

Nit, needs a space there before the '{'.

Ah, Ok. I attached fixed patch in this mail.

Quoted 9 lines

The problem as I see it is that when we boot the system we start a kswapd on all nodes with memory. If the hot-add adds memory to a pre-existing node with no memory we will not start one and we end up with a node with memory and no kswapd. Bad.

As kswapd_run is a no-op when a kswapd already exists this seems a safe way to fix that. Paul's ->zone conversion is obviously correct also.

Acked-by Andy Whitcroft <[email protected]>

Thanks for your explanation.
You mentioned all of my intention correctly. :-)

----

Fix kswapd doesn't run when memory is added on memory-less-node. Fix compile error of zone->node when CONFIG_NUMA is off.

Signed-off-by Yasunori Goto <[email protected]>

Signed-off-by Paul Mundt <[email protected]>

Acked-by Andy Whitcroft <[email protected]>

---
 mm/memory_hotplug.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Index: current/mm/memory_hotplug.c
===================================================================

mm/memory_hotplug.c +4 -2

@@ -211,10 +211,12 @@ int online_pages(unsigned long pfn, unsi
 		online_pages_range);
 	zone->present_pages += onlined_pages;
 	zone->zone_pgdat->node_present_pages += onlined_pages;
-	if (onlined_pages)
-		node_set_state(zone->node, N_HIGH_MEMORY);
 
 	setup_per_zone_pages_min();
+	if (onlined_pages) {
+		kswapd_run(zone_to_nid(zone));
+		node_set_state(zone_to_nid(zone), N_HIGH_MEMORY);
+	}
 
 	if (need_zonelists_rebuild)
 		build_all_zonelists();

Quoted 57 lines

Quoted 5 lines

Review

Quoted 3 lines

(quoted diff) +1 -0

+	if (onlined_pages){

Nit, needs a space there before the '{'.

Ah, Ok. I attached fixed patch in this mail.

Quoted 9 lines

The problem as I see it is that when we boot the system we start a kswapd on all nodes with memory. If the hot-add adds memory to a pre-existing node with no memory we will not start one and we end up with a node with memory and no kswapd. Bad.

As kswapd_run is a no-op when a kswapd already exists this seems a safe way to fix that. Paul's ->zone conversion is obviously correct also.

Acked-by Andy Whitcroft <[email protected]>

Thanks for your explanation.
You mentioned all of my intention correctly. :-)

----

Fix kswapd doesn't run when memory is added on memory-less-node. Fix compile error of zone->node when CONFIG_NUMA is off.

Signed-off-by Yasunori Goto <[email protected]>

Signed-off-by Paul Mundt <[email protected]>

Acked-by Andy Whitcroft <[email protected]>

---
 mm/memory_hotplug.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Index: current/mm/memory_hotplug.c
===================================================================

mm/memory_hotplug.c +4 -2

@@ -211,10 +211,12 @@ int online_pages(unsigned long pfn, unsi
 		online_pages_range);
 	zone->present_pages += onlined_pages;
 	zone->zone_pgdat->node_present_pages += onlined_pages;
-	if (onlined_pages)
-		node_set_state(zone->node, N_HIGH_MEMORY);
 
 	setup_per_zone_pages_min();
+	if (onlined_pages) {
+		kswapd_run(zone_to_nid(zone));
+		node_set_state(zone_to_nid(zone), N_HIGH_MEMORY);
+	}
 
 	if (need_zonelists_rebuild)
 		build_all_zonelists();

mm/memory_hotplug.c int add_memory(int nid, u64 start, u64 s · line 276

Quoted 9 lines

mm/memory_hotplug.c +0 -3

@@ -269,9 +271,6 @@ int add_memory(int nid, u64 start, u64 s
 		if (!pgdat)
 			return -ENOMEM;
 		new_pgdat = 1;
-		ret = kswapd_run(nid);
-		if (ret)
-			goto error;
 	}
 
 	/* call arch's memory hotadd */

OK, we're getting into a mess here. This patch fixes update-n_high_memory-node-state-for-memory-hotadd.patch, but which patch does update-n_high_memory-node-state-for-memory-hotadd.patch fix?

At present I just whacked update-n_high_memory-node-state-for-memory-hotadd.patch at the end of everything, but that was lazy of me and it ends up making a mess.

Quoted 7 lines

Quoted 2 lines

/* call arch's memory hotadd */

OK, we're getting into a mess here. This patch fixes update-n_high_memory-node-state-for-memory-hotadd.patch, but which patch does update-n_high_memory-node-state-for-memory-hotadd.patch fix?

Quoted 3 lines

At present I just whacked update-n_high_memory-node-state-for-memory-hotadd.patch at the end of everything, but that was lazy of me and it ends up making a mess.

It is enough. No more patch is necessary for these issues. I already fixed about Andy-san's comment. :-)

Quoted 12 lines

Quoted 11 lines

Quoted 3 lines

/* call arch's memory hotadd */

OK, we're getting into a mess here. This patch fixes update-n_high_memory-node-state-for-memory-hotadd.patch, but which patch does update-n_high_memory-node-state-for-memory-hotadd.patch fix?

At present I just whacked update-n_high_memory-node-state-for-memory-hotadd.patch at the end of everything, but that was lazy of me and it ends up making a mess.

Quoted 2 lines

It is enough. No more patch is necessary for these issues. I already fixed about Andy-san's comment. :-)

Now I'm more confused. I have two separeate questions:

a) Is the justr-added update-n_high_memory-node-state-for-memory-hotadd-fix.patch still needed?

b) Which patch in 2.6.22-rc4-mm1 does

update-n_high_memory-node-state-for-memory-hotadd.patch fix? In other words, into which patch should I fold update-n_high_memory-node-state-for-memory-hotadd.patch prior to sending to Linus?

(I (usually) get to work this out for myself. Sometimes it is painful).

Generally, if people tell me which patch-in-mm their patch is fixing, it really helps. Adrian does this all the time.

Quoted 7 lines · On Fri, 14 Sep 2007 11:02:43 +0900 Yasunori Goto <[email protected]> wrote:

Quoted 7 lines

Quoted 2 lines

/* call arch's memory hotadd */

OK, we're getting into a mess here. This patch fixes update-n_high_memory-node-state-for-memory-hotadd.patch, but which patch does update-n_high_memory-node-state-for-memory-hotadd.patch fix?

Quoted 6 lines

Quoted 3 lines

At present I just whacked update-n_high_memory-node-state-for-memory-hotadd.patch at the end of everything, but that was lazy of me and it ends up making a mess.

It is enough. No more patch is necessary for these issues. I already fixed about Andy-san's comment. :-)

Now I'm more confused. I have two separeate questions:

a) Is the justr-added update-n_high_memory-node-state-for-memory-hotadd-fix.patch still needed?

b) Which patch in 2.6.22-rc4-mm1 does update-n_high_memory-node-state-for-memory-hotadd.patch fix? In other words, into which patch should I fold update-n_high_memory-node-state-for-memory-hotadd.patch prior to sending to Linus?

(I (usually) get to work this out for myself. Sometimes it is painful).

Generally, if people tell me which patch-in-mm their patch is fixing, it really helps. Adrian does this all the time.

Thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

I originally sent in the "update-n_high_memory..." patch against 23-rc3-mm1 on 27aug to fix a problem that I introduced when I moved the populating of N_HIGH_MEMORY state to free_area_init_nodes(). This would miss setting the "has memory" node state for hot added memory. I never saw any response, but then it ended up in 23-rc4-mm1.

This Tuesday, Paul Mundt sent in a patch to fix a build problem with MEMORY_HOTPLUG_SPARSE introduced by my patch. He replaced zone->node with zone_to_nid(zone) in the node_set_state() arguments.

The latest patch, from Yasunori-san, I believe, starts kswapd for nodes to which memory has been hot-added. As I understand it, his is needed because the memoryless nodes patch results in no kswapd for memoryless nodes.

Does that help?