2007-09-11 07:25:28

by Paul Mundt

[permalink] [raw]
Subject: [PATCH -mm] mm: Fix memory hotplug + sparsemem build.

Building with CONFIG_MEMORY_HOTPLUG_SPARSE enabled results in:

CC mm/memory_hotplug.o
mm/memory_hotplug.c: In function 'online_pages':
mm/memory_hotplug.c:215: error: 'struct zone' has no member named 'node'
make[1]: *** [mm/memory_hotplug.o] Error 1
make: *** [mm] Error 2

Fix it up.

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

--

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

--- linux-2.6.23-rc4-mm1.orig/mm/memory_hotplug.c 2007-09-11 15:15:56.000000000 +0900
+++ linux-2.6.23-rc4-mm1/mm/memory_hotplug.c 2007-09-11 16:20:03.000000000 +0900
@@ -212,7 +212,7 @@
zone->present_pages += onlined_pages;
zone->zone_pgdat->node_present_pages += onlined_pages;
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();


2007-09-11 08:19:16

by Yasunori Goto

[permalink] [raw]
Subject: Re: [PATCH -mm] mm: Fix memory hotplug + sparsemem build.

> 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
===================================================================
--- current.orig/mm/memory_hotplug.c 2007-09-07 18:08:07.000000000 +0900
+++ current/mm/memory_hotplug.c 2007-09-11 17:29:19.000000000 +0900
@@ -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();
@@ -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 */


--
Yasunori Goto


2007-09-11 08:26:20

by Paul Mundt

[permalink] [raw]
Subject: Re: [PATCH -mm] mm: Fix memory hotplug + sparsemem build.

Hi Goto-san,

On Tue, Sep 11, 2007 at 05:18:01PM +0900, Yasunori Goto wrote:
> > 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.
>
Looks fine to me!

2007-09-11 09:15:46

by Andy Whitcroft

[permalink] [raw]
Subject: Re: [PATCH -mm] mm: Fix memory hotplug + sparsemem build.

On Tue, Sep 11, 2007 at 05:18:01PM +0900, Yasunori Goto wrote:
> > 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
> ===================================================================
> --- current.orig/mm/memory_hotplug.c 2007-09-07 18:08:07.000000000 +0900
> +++ current/mm/memory_hotplug.c 2007-09-11 17:29:19.000000000 +0900
> @@ -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){

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

> + kswapd_run(zone_to_nid(zone));
> + node_set_state(zone_to_nid(zone), N_HIGH_MEMORY);
> + }
>
> if (need_zonelists_rebuild)
> build_all_zonelists();
> @@ -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 */

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

-apw

2007-09-11 09:38:42

by Yasunori Goto

[permalink] [raw]
Subject: Re: [PATCH -mm] mm: Fix memory hotplug + sparsemem build.


> > + if (onlined_pages){
>
> Nit, needs a space there before the '{'.

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

> 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
===================================================================
--- current.orig/mm/memory_hotplug.c 2007-09-07 18:08:07.000000000 +0900
+++ current/mm/memory_hotplug.c 2007-09-11 17:29:19.000000000 +0900
@@ -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();
@@ -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 */

--
Yasunori Goto


2007-09-14 01:45:58

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH -mm] mm: Fix memory hotplug + sparsemem build.

On Tue, 11 Sep 2007 18:37:12 +0900 Yasunori Goto <[email protected]> wrote:

>
> > > + if (onlined_pages){
> >
> > Nit, needs a space there before the '{'.
>
> Ah, Ok. I attached fixed patch in this mail.
>
> > 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
> ===================================================================
> --- current.orig/mm/memory_hotplug.c 2007-09-07 18:08:07.000000000 +0900
> +++ current/mm/memory_hotplug.c 2007-09-11 17:29:19.000000000 +0900
> @@ -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();
> @@ -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.

2007-09-14 02:04:14

by Yasunori Goto

[permalink] [raw]
Subject: Re: [PATCH -mm] mm: Fix memory hotplug + sparsemem build.

> On Tue, 11 Sep 2007 18:37:12 +0900 Yasunori Goto <[email protected]> wrote:
>
> >
> > > > + if (onlined_pages){
> > >
> > > Nit, needs a space there before the '{'.
> >
> > Ah, Ok. I attached fixed patch in this mail.
> >
> > > 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
> > ===================================================================
> > --- current.orig/mm/memory_hotplug.c 2007-09-07 18:08:07.000000000 +0900
> > +++ current/mm/memory_hotplug.c 2007-09-11 17:29:19.000000000 +0900
> > @@ -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();
> > @@ -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.

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


Thanks.
--
Yasunori Goto


2007-09-14 02:42:27

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH -mm] mm: Fix memory hotplug + sparsemem build.

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

> > > /* 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.
>
> 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.

2007-09-14 05:15:36

by Yasunori Goto

[permalink] [raw]
Subject: Re: [PATCH -mm] mm: Fix memory hotplug + sparsemem build.


> On Fri, 14 Sep 2007 11:02:43 +0900 Yasunori Goto <[email protected]> wrote:
>
> > > > /* 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.
> >
> > 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?

I'm not sure exact meaning of "just-added".
But, update-n_high_memory-node-state-for-memory-hotadd-fix.patch is
necessary for 2.6.23-rc4-mm1.

> b) Which patch in 2.6.22-rc4-mm1 does

2.6.23-rc4-mm1?

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

In my understanding,
update-n_high_memory-node-state-for-memory-hotadd.patch should be folded
with all of memoryless-nodes-xxxxxxxxxxxx.patch.
It sets N_HIGH_MEMORY for a new node-with-memory.

But if you need specifing of more detail patch, becase N_HIGH_MEMORY is
set in memoryless-nodes-introduce-ask-of-nodes-with-memory.patch,
I suppose update-n_high_memory-node-state-for-memory-hotadd.patch
should be fold with it.


update-n_high_memory-node-state-for-memory-hotadd-fix.patch
^^^
is fixes of update-n_high_memory-node-state-for-memory-hotadd.patch
and memoryless-nodes-no-need-for-kswapd.patch


Is it enough for your question? Or more confuse?


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

Sorry for your confusing...


--
Yasunori Goto


2007-09-14 14:09:41

by Lee Schermerhorn

[permalink] [raw]
Subject: Re: [PATCH -mm] mm: Fix memory hotplug + sparsemem build.

On Thu, 2007-09-13 at 19:41 -0700, Andrew Morton wrote:
> On Fri, 14 Sep 2007 11:02:43 +0900 Yasunori Goto <[email protected]> wrote:
>
> > > > /* 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.
> >
> > 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?

Andrew:

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?

Lee


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

2007-09-14 20:52:51

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH -mm] mm: Fix memory hotplug + sparsemem build.

On Fri, 14 Sep 2007 10:09:27 -0400
Lee Schermerhorn <[email protected]> wrote:

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

not really ;)

See, when I get some rinky-dink little fix for a patch in -mm I will
position that patch immediately after the patch which it is fixing, with a
filename which is derived from the fixed patch's name. So when
send-to-Linus time comes, I can fold the fixes into the base patch. This
practice also keeps the patches in a sensible presentation order, with
minimum interdependencies and good git-bisect friendliness.

However it sometimes (rarely) takes considerable effort to work out which
patch in -mm a particular fix is fixing. That was the case with
update-n_high_memory-node-state-for-memory-hotadd.patch.

It helps me quite a bit if people tell me which patch they're fixing.
Usually they don't and I get to work it out. Usually it's fairly obvious.