CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

[Andrew Morton]

WARNING: "remove_memory" [drivers/acpi/acpi_memhotplug.ko] undefined!
WARNING: "add_memory" [drivers/acpi/acpi_memhotplug.ko] undefined!
WARNING: "acpi_os_allocate" [drivers/acpi/acpi_memhotplug.ko] undefined!

Does it actually make sense to load acpi_memhotplug.ko as a module?  Will
it all work?

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

[Yasunori Goto]

> WARNING: "remove_memory" [drivers/acpi/acpi_memhotplug.ko] undefined!
> WARNING: "add_memory" [drivers/acpi/acpi_memhotplug.ko] undefined!
> WARNING: "acpi_os_allocate" [drivers/acpi/acpi_memhotplug.ko] undefined!
> 
> Does it actually make sense to load acpi_memhotplug.ko as a module?  Will
> it all work?

Are these warning 2.6.16-rc6-mm2?
I tried it on ia64. 
If EXPORT_SYMBOL_GPL for them are defined, it worked well as a module. 
Probably, x86-64 will work by same definition.

In 2.6.16 stock kernel, EXPORT_SYMBOL is already defined for
add_memory() and remove_memory() of x86-64. (only x86-64, not for ia64).
acpi_os_allocate() is not called in acpi_memhotplug.ko of 2.6.16.
it is called only on 2.6-16-rc6-mm2.

Bye.

-- 
Yasunori Goto 

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

[Andrew Morton]

Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>
> > WARNING: "remove_memory" [drivers/acpi/acpi_memhotplug.ko] undefined!
> > WARNING: "add_memory" [drivers/acpi/acpi_memhotplug.ko] undefined!
> > WARNING: "acpi_os_allocate" [drivers/acpi/acpi_memhotplug.ko] undefined!
> > 
> > Does it actually make sense to load acpi_memhotplug.ko as a module?  Will
> > it all work?
> 
> Are these warning 2.6.16-rc6-mm2?
> I tried it on ia64. 
> If EXPORT_SYMBOL_GPL for them are defined, it worked well as a module. 
> Probably, x86-64 will work by same definition.
> 
> In 2.6.16 stock kernel, EXPORT_SYMBOL is already defined for
> add_memory() and remove_memory() of x86-64. (only x86-64, not for ia64).
> acpi_os_allocate() is not called in acpi_memhotplug.ko of 2.6.16.
> it is called only on 2.6-16-rc6-mm2.
> 

I think the problem was introduced by one of the newly-queued patches in
Andi's tree, possibly
ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt-current/patches/hotadd-reserve

If acpi_memhotplug.ko works OK as a module then I guess adding the exports
is the right fix.

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

[KAMEZAWA Hiroyuki]

On Wed, 22 Mar 2006 03:02:08 -0800
Andrew Morton <akpm@osdl.org> wrote:

> Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
> I think the problem was introduced by one of the newly-queued patches in
> Andi's tree, possibly
> ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt-current/patches/hotadd-reserve
> 
I didn't notice this patch and am shocked now....

First, e820 doesn't holds information of memory which doesn't exist at boot
time, I think. This patch can work only with emulated environ. 

Next, Goto-san is now working on new patches , node-hot-add.
It includes add-new-zone/rebuild-zonelist...etc patches, which will be necessary
also for x86_64, even if it's not NUMA. 
I doesn't think this patch should be merged *now*.

-- Kame

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

[Andi Kleen]

On Tue, Mar 21, 2006 at 10:23:32PM -0800, Andrew Morton wrote:
> WARNING: "remove_memory" [drivers/acpi/acpi_memhotplug.ko] undefined!
> WARNING: "add_memory" [drivers/acpi/acpi_memhotplug.ko] undefined!
> WARNING: "acpi_os_allocate" [drivers/acpi/acpi_memhotplug.ko] undefined!
> 
> Does it actually make sense to load acpi_memhotplug.ko as a module?  Will

Yes.

> it all work?

Just need to export the symbols I think. Thanks for the headsup.

-Andi

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

[Andi Kleen]

> It includes add-new-zone/rebuild-zonelist...etc patches, which will be necessary
> also for x86_64, even if it's not NUMA. 

Hmm? x86_64 supports NUMA systems.

-Andi

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

[Andi Kleen]

On Tue, Mar 21, 2006 at 10:23:32PM -0800, Andrew Morton wrote:
> WARNING: "remove_memory" [drivers/acpi/acpi_memhotplug.ko] undefined!
> WARNING: "add_memory" [drivers/acpi/acpi_memhotplug.ko] undefined!
> WARNING: "acpi_os_allocate" [drivers/acpi/acpi_memhotplug.ko] undefined!
> 
> Does it actually make sense to load acpi_memhotplug.ko as a module?  Will
> it all work?
> 
Actually I found the real bug. All the places that check
for #ifdef CONFIG_ACPI_HOTPLUG_MEMORY need to check for
CONFIG_ACPI_HOTPLUG_MEMORY_MODULE (what a mouthful!) too.

-Andi

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

[KAMEZAWA Hiroyuki]

On 22 Mar 2006 20:21:34 +0100
Andi Kleen <ak@muc.de> wrote:

> > It includes add-new-zone/rebuild-zonelist...etc patches, which will be necessary
> > also for x86_64, even if it's not NUMA. 
> 
> Hmm? x86_64 supports NUMA systems.
> 
Ah, I know.
I wrote "even if.." just because we cannot reserve mem_map for not exisiting node.

-- Kame

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

[Andi Kleen]

On Thu, Mar 23, 2006 at 08:27:32AM +0900, KAMEZAWA Hiroyuki wrote:
> On 22 Mar 2006 20:21:34 +0100
> Andi Kleen <ak@muc.de> wrote:
> 
> > > It includes add-new-zone/rebuild-zonelist...etc patches, which will be necessary
> > > also for x86_64, even if it's not NUMA. 
> > 
> > Hmm? x86_64 supports NUMA systems.
> > 
> Ah, I know.
> I wrote "even if.." just because we cannot reserve mem_map for not exisiting node.

Hmm actually I haven't tested it but in theory the reserve hotplug
code should just work if you list the new nodes already in SRAT as empty
hotplug PXMs.

Ok there is no code yet to add hotplug cpus to specific nodes later
but that would be easy to add.

The only problem is that mem_map has to be in another node so it will be 
slightly slower.

-Andi

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

[KAMEZAWA Hiroyuki]

On 23 Mar 2006 01:07:59 +0100
Andi Kleen <ak@muc.de> wrote:

> On Thu, Mar 23, 2006 at 08:27:32AM +0900, KAMEZAWA Hiroyuki wrote:
> > On 22 Mar 2006 20:21:34 +0100
> > Andi Kleen <ak@muc.de> wrote:
> > 
> > > > It includes add-new-zone/rebuild-zonelist...etc patches, which will be necessary
> > > > also for x86_64, even if it's not NUMA. 
> > > 
> > > Hmm? x86_64 supports NUMA systems.
> > > 
> > Ah, I know.
> > I wrote "even if.." just because we cannot reserve mem_map for not exisiting node.
> 
> Hmm actually I haven't tested it but in theory the reserve hotplug
> code should just work if you list the new nodes already in SRAT as empty
> hotplug PXMs.
> 
SRAT is required by Microsoft, then most of servers will equip it, I think.
And SRAT just tells each cpu's/memory range's  a pxm.

But allocating memmap for SRAT entry has some problem in big system.

For example, our server (ia64/Fujitsu PrimeQuest) can equip memory from
4G to 1T(maybe 2T in future), and SRAT will *always* say  we have possible 1T memory.
(Microsoft requires "write all possible memory in SRAT") 
When we reserve memmap for possible 1T memory, Linux will not work well in minimum 4G
configuraion ;) 
So, I recommend allocate memmap dynamically.

> Ok there is no code yet to add hotplug cpus to specific nodes later
> but that would be easy to add.
> 
We found hot-add-memory-less-node problem now. So, "cpu hot add to a new node"  
will be supported later.

-- Kame

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

[Andi Kleen]

On Thu, Mar 23, 2006 at 09:36:09AM +0900, KAMEZAWA Hiroyuki wrote:
> On 23 Mar 2006 01:07:59 +0100
> Andi Kleen <ak@muc.de> wrote:
> 
> > On Thu, Mar 23, 2006 at 08:27:32AM +0900, KAMEZAWA Hiroyuki wrote:
> > > On 22 Mar 2006 20:21:34 +0100
> > > Andi Kleen <ak@muc.de> wrote:
> > > 
> > > > > It includes add-new-zone/rebuild-zonelist...etc patches, which will be necessary
> > > > > also for x86_64, even if it's not NUMA. 
> > > > 
> > > > Hmm? x86_64 supports NUMA systems.
> > > > 
> > > Ah, I know.
> > > I wrote "even if.." just because we cannot reserve mem_map for not exisiting node.
> > 
> > Hmm actually I haven't tested it but in theory the reserve hotplug
> > code should just work if you list the new nodes already in SRAT as empty
> > hotplug PXMs.
> > 
> SRAT is required by Microsoft, then most of servers will equip it, I think.
> And SRAT just tells each cpu's/memory range's  a pxm.
> 
> But allocating memmap for SRAT entry has some problem in big system.
> 
> For example, our server (ia64/Fujitsu PrimeQuest) can equip memory from
> 4G to 1T(maybe 2T in future), and SRAT will *always* say  we have possible 1T memory.
> (Microsoft requires "write all possible memory in SRAT") 
> When we reserve memmap for possible 1T memory, Linux will not work well in minimum 4G

Don't do that.
The x86-64 kernel will preallocate memmaps when everything is enabled soon.
Ok that won't concern your IA64 machine immediately.


> > Ok there is no code yet to add hotplug cpus to specific nodes later
> > but that would be easy to add.
> > 
> We found hot-add-memory-less-node problem now. So, "cpu hot add to a new node"  
> will be supported later.

Again should be pretty easy.

-Andi

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

[KAMEZAWA Hiroyuki]

On 23 Mar 2006 03:43:06 +0100
Andi Kleen <ak@muc.de> wrote:

> On Thu, Mar 23, 2006 at 09:36:09AM +0900, KAMEZAWA Hiroyuki wrote:
> > On 23 Mar 2006 01:07:59 +0100
> > Andi Kleen <ak@muc.de> wrote:
> > 
> > > On Thu, Mar 23, 2006 at 08:27:32AM +0900, KAMEZAWA Hiroyuki wrote:
> > > > On 22 Mar 2006 20:21:34 +0100
> > > > Andi Kleen <ak@muc.de> wrote:
> > > > 
> > > > > > It includes add-new-zone/rebuild-zonelist...etc patches, which will be necessary
> > > > > > also for x86_64, even if it's not NUMA. 
> > > > > 
> > > > > Hmm? x86_64 supports NUMA systems.
> > > > > 
> > > > Ah, I know.
> > > > I wrote "even if.." just because we cannot reserve mem_map for not exisiting node.
> > > 
> > > Hmm actually I haven't tested it but in theory the reserve hotplug
> > > code should just work if you list the new nodes already in SRAT as empty
> > > hotplug PXMs.
> > > 
> > SRAT is required by Microsoft, then most of servers will equip it, I think.
> > And SRAT just tells each cpu's/memory range's  a pxm.
> > 
> > But allocating memmap for SRAT entry has some problem in big system.
> > 
> > For example, our server (ia64/Fujitsu PrimeQuest) can equip memory from
> > 4G to 1T(maybe 2T in future), and SRAT will *always* say  we have possible 1T memory.
> > (Microsoft requires "write all possible memory in SRAT") 
> > When we reserve memmap for possible 1T memory, Linux will not work well in minimum 4G
> 
> Don't do that.
Don't do what ?

> The x86-64 kernel will preallocate memmaps when everything is enabled soon.
> Ok that won't concern your IA64 machine immediately.
> 

BTW, why don't you just use SPARSEMEM ? and Goto-san has to consider x86_64 special case 
or just should ignore x86_64 ?

- Kame

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

[KAMEZAWA Hiroyuki]

On 23 Mar 2006 03:43:06 +0100
Andi Kleen <ak@muc.de> wrote:

>
> Don't do that.
> The x86-64 kernel will preallocate memmaps when everything is enabled soon.
> Ok that won't concern your IA64 machine immediately.
> 
> 

I read hotadd-reserve patches again.. Okay, please preallocate if you want.

Problem I found is:
1) Goto's rebuild_zonelist patch will not work if CONFIG_MEMORY_HOTPLUG=n.
   rebuild zonelist is necessary when the system has just memory < 4G at boot,   
   and hot add memory > 4G. because x86_64 has DMA32, ZONE_NORAML is not included into
   zonelist at boot time if system doesn't have memory >4G at boot.

2) zone and node's spanned_pages and present_pages are not incremented. you should do.

Bye.
-- Kame

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

[KAMEZAWA Hiroyuki]

On 23 Mar 2006 03:43:06 +0100
Andi Kleen <ak@muc.de> wrote:

> > For example, our server (ia64/Fujitsu PrimeQuest) can equip memory from
> > 4G to 1T(maybe 2T in future), and SRAT will *always* say  we have possible 1T memory.
> > (Microsoft requires "write all possible memory in SRAT") 
> > When we reserve memmap for possible 1T memory, Linux will not work well in minimum 4G
> 
> Don't do that.
> The x86-64 kernel will preallocate memmaps when everything is enabled soon.
> Ok that won't concern your IA64 machine immediately.
> 

Okay... I took several cups of coffee and understood what you say.and what x86_64 does.

If X86_64=y and CONFIG_MEMORY_HOTPLUG=n, system works like this..

1. preallocate all memmap, for memory range in SRAT. initialize all.
   just don't free pages (page struct) at boot time.

2. pre-initialize all pgdat/zone. even if ther are empty.(but have huge amount of
   reserved pages(memmap))

3. When add_memory() is called, just call free_page(page) and make pages populated. 

4. 

5. Only problem is firmware configurated as "small memory + huge SRAT entry".

Correct ?

Note: "enabled" in SRAT just means "you can read this entry". not means "memory is present".

--Kame

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree

[Andi Kleen]

On Thu, Mar 23, 2006 at 12:22:50PM +0900, KAMEZAWA Hiroyuki wrote:
> On 23 Mar 2006 03:43:06 +0100
> Andi Kleen <ak@muc.de> wrote:
> 
> >
> > Don't do that.
> > The x86-64 kernel will preallocate memmaps when everything is enabled soon.
> > Ok that won't concern your IA64 machine immediately.
> > 
> > 
> 
> I read hotadd-reserve patches again.. Okay, please preallocate if you want.
> 
> Problem I found is:
> 1) Goto's rebuild_zonelist patch will not work if CONFIG_MEMORY_HOTPLUG=n.
>    rebuild zonelist is necessary when the system has just memory < 4G at boot,   
>    and hot add memory > 4G. because x86_64 has DMA32, ZONE_NORAML is not included into
>    zonelist at boot time if system doesn't have memory >4G at boot.
> 
> 2) zone and node's spanned_pages and present_pages are not incremented. you should do.

Thanks for the review. All good points. Will fix. 

I think I just can force the higher zones to be built at boot when there 
are any SRATs > 4GB.

I will also add some throttling to prevent the kernel not booting
in the "1TB of hotplug ram" scenario you outlined. Probably just limit
the memory map preallocated to not more than some percentage of the 
free RAM.

Just need to figure out a good limit.

Currently mem_map costs about 1.5% of RAM, so not allocating more
than e.g. 5% of RAM to hotplug memmaps would limit the hotplug memory
on a 1GB machine to 3GB. Hmm, maybe 10% but users could be annoyed
if they lose that much memory due to hotplug.

Another problem is that mem_map needs to be contiguous by node,
but that can be handled with existing functions by checking the e820 map.

-Andi

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree II

[Andi Kleen]

Hi,

I started to implement your suggestion now.

But one problem I noticed and I don't see how the current sparsemem code
handles correctly is that it makes no attempt to handle hotadd
areas that cross zone boundaries. Is there code somewhere that rejects
them?

-Andi

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree II

[KAMEZAWA Hiroyuki]

On 30 Mar 2006 15:41:33 +0200
Andi Kleen <ak@muc.de> wrote:

> 
> Hi,
> 
> I started to implement your suggestion now.
> 
> But one problem I noticed and I don't see how the current sparsemem code
> handles correctly is that it makes no attempt to handle hotadd
> areas that cross zone boundaries. Is there code somewhere that rejects
> them?
> 

add_memory() selects a zone which pages are added, before adding memory regardless
of its address. Now this is hard-coded.

ia64 adds memory to NORMAL.
i386 adds memory to HIGHMEM.
x86_64 adds memory to NORMAL.

if x86_64 people wants to add memory to DMA32 or some other zone, please add code.


- Kame

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree II

[Andi Kleen]

On Thu, Mar 30, 2006 at 10:54:59PM +0900, KAMEZAWA Hiroyuki wrote:
> On 30 Mar 2006 15:41:33 +0200
> Andi Kleen <ak@muc.de> wrote:
> 
> > 
> > Hi,
> > 
> > I started to implement your suggestion now.
> > 
> > But one problem I noticed and I don't see how the current sparsemem code
> > handles correctly is that it makes no attempt to handle hotadd
> > areas that cross zone boundaries. Is there code somewhere that rejects
> > them?
> > 
> 
> add_memory() selects a zone which pages are added, before adding memory regardless
> of its address. Now this is hard-coded.
> 
> ia64 adds memory to NORMAL.
> i386 adds memory to HIGHMEM.
> x86_64 adds memory to NORMAL.

Yes but what stops ACPI from trying to add memory that crosses 
a zone? (e.g. from 2GB to 6GB) 

> 
> if x86_64 people wants to add memory to DMA32 or some other zone, please add code.

I added a check for it in the reserve hotadd code now.

-Andi

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree II

[KAMEZAWA Hiroyuki]

On 30 Mar 2006 16:18:11 +0200
Andi Kleen <ak@muc.de> wrote:

> On Thu, Mar 30, 2006 at 10:54:59PM +0900, KAMEZAWA Hiroyuki wrote:
> > On 30 Mar 2006 15:41:33 +0200
> > Andi Kleen <ak@muc.de> wrote:
> > 
> > > 
> > > Hi,
> > > 
> > > I started to implement your suggestion now.
> > > 
> > > But one problem I noticed and I don't see how the current sparsemem code
> > > handles correctly is that it makes no attempt to handle hotadd
> > > areas that cross zone boundaries. Is there code somewhere that rejects
> > > them?
> > > 
> > 
> > add_memory() selects a zone which pages are added, before adding memory regardless
> > of its address. Now this is hard-coded.
> > 
> > ia64 adds memory to NORMAL.
> > i386 adds memory to HIGHMEM.
> > x86_64 adds memory to NORMAL.
> 
> Yes but what stops ACPI from trying to add memory that crosses 
> a zone? (e.g. from 2GB to 6GB) 
> 
2GB to 6GB pages are all added to NORMAL, not DMA32. (2.6.16)
If this is not sane, x86_64's add_memory should have to check address of start/end.



> > 
> > if x86_64 people wants to add memory to DMA32 or some other zone, please add code.
> 
> I added a check for it in the reserve hotadd code now.
> 
great. I'll look into x86_64's add_memory() (with CONFIG_MEMORY_HOTPLUG=y) 
and fix it if I can.

Thanks,
- Kame

Re: CONFIG_ACPI_HOTPLUG_MEMORY=m with x86_64 devel tree II

[Andi Kleen]

> 2GB to 6GB pages are all added to NORMAL, not DMA32. (2.6.16)
> If this is not sane, x86_64's add_memory should have to check address of start/end.

Hmm thinking about it again it might not be that bad. It could
cause some additional bounces during IO, but shouldn't cause
any malfunction.

-Andi