remove_memory() does not remove memory but just offlines memory. The patch
changes name of it to offline_memory().
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/acpi/acpi_memhotplug.c | 2 +-
drivers/base/memory.c | 4 ++--
include/linux/memory_hotplug.h | 2 +-
mm/memory_hotplug.c | 6 +++---
4 files changed, 7 insertions(+), 7 deletions(-)
Index: linux-3.5-rc4/drivers/acpi/acpi_memhotplug.c
===================================================================
When offline_pages() is called to offlined memory, the function fails since
all memory has been offlined. In this case, the function should succeed.
The patch adds the check function into offline_pages().
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/base/memory.c | 20 ++++++++++++++++++++
include/linux/memory.h | 1 +
mm/memory_hotplug.c | 5 +++++
3 files changed, 26 insertions(+)
Index: linux-3.5-rc4/drivers/base/memory.c
===================================================================
When (hot)adding memory into system, /sys/firmware/memmap/X/{end, start, type}
sysfs files are created. But there is no code to remove these files. The patch
implements the function to remove them.
Note : The code does not free firmware_map_entry since there is no way to free
memory which is allocated by bootmem.
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/firmware/memmap.c | 71 +++++++++++++++++++++++++++++++++++++++++++
include/linux/firmware-map.h | 6 +++
mm/memory_hotplug.c | 6 +++
3 files changed, 82 insertions(+), 1 deletion(-)
Index: linux-3.5-rc4/mm/memory_hotplug.c
===================================================================
Since applying a patch(de7f0cba96786c), release_mem_region() has been changed
as called in PAGES_PER_SECTION chunks because register_memory_resource() is
called in PAGES_PER_SECTION chunks by add_memory(). But it seems firmware
dependency. If CRS are written in the PAGES_PER_SECTION chunks in ACPI DSDT
Table, register_memory_resource() is called in PAGES_PER_SECTION chunks.
But if CRS are written in the DIMM unit in ACPI DSDT Table,
register_memory_resource() is called in DIMM unit. So release_mem_region()
should not be called in PAGES_PER_SECTION chunks. The patch fixes it.
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
arch/powerpc/platforms/pseries/hotplug-memory.c | 13 +++++++++----
mm/memory_hotplug.c | 4 ++--
2 files changed, 11 insertions(+), 6 deletions(-)
Index: linux-3.5-rc4/mm/memory_hotplug.c
===================================================================
@@ -358,11 +358,11 @@ int __remove_pages(struct zone *zone, unBUG_ON(phys_start_pfn&~PAGE_SECTION_MASK);BUG_ON(nr_pages%PAGES_PER_SECTION);+release_mem_region(phys_start_pfn<<PAGE_SHIFT,nr_pages*PAGE_SIZE);+sections_to_remove=nr_pages/PAGES_PER_SECTION;for(i=0;i<sections_to_remove;i++){unsignedlongpfn=phys_start_pfn+i*PAGES_PER_SECTION;-release_mem_region(pfn<<PAGE_SHIFT,-PAGES_PER_SECTION<<PAGE_SHIFT);ret=__remove_section(zone,__pfn_to_section(pfn));if(ret)break;
The patch adds __remove_pages() to remove_memory(). Then the range of
phys_start_pfn argument and nr_pages argument in __remove_pagse() may
have different zone. So zone argument is removed from __remove_pages()
and __remove_pages() caluculates zone in each section.
When CONFIG_SPARSEMEM_VMEMMAP is defined, there is no way to remove a memmap.
So __remove_section only calls unregister_memory_section().
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
arch/powerpc/platforms/pseries/hotplug-memory.c | 5 +----
include/linux/memory_hotplug.h | 3 +--
mm/memory_hotplug.c | 20 +++++++++++++-------
3 files changed, 15 insertions(+), 13 deletions(-)
Index: linux-3.5-rc4/mm/memory_hotplug.c
===================================================================
@@ -363,6 +366,7 @@ int __remove_pages(struct zone *zone, unsections_to_remove=nr_pages/PAGES_PER_SECTION;for(i=0;i<sections_to_remove;i++){unsignedlongpfn=phys_start_pfn+i*PAGES_PER_SECTION;+zone=page_zone(pfn_to_page(pfn));ret=__remove_section(zone,__pfn_to_section(pfn));if(ret)break;
@@ -89,8 +89,7 @@ extern bool is_pageblock_removable_noloc/* reasonably generic interface to expand the physical pages in a zone */externint__add_pages(intnid,structzone*zone,unsignedlongstart_pfn,unsignedlongnr_pages);-externint__remove_pages(structzone*zone,unsignedlongstart_pfn,-unsignedlongnr_pages);+externint__remove_pages(unsignedlongstart_pfn,unsignedlongnr_pages);#ifdef CONFIG_NUMAexternintmemory_add_physaddr_to_nid(u64start);
There is a possibility that get_page_bootmem() is called to the same page many
times. So when get_page_bootmem is called to the same page, the function only
increments page->_count.
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
Index: linux-3.5-rc4/mm/memory_hotplug.c
===================================================================
For removing memmap region of sparse-vmemmap which is allocated bootmem,
memmap region of sparse-vmemmap needs to be registered by get_page_bootmem().
So the patch searches pages of virtual mapping and registers the pages by
get_page_bootmem().
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
arch/x86/mm/init_64.c | 53 +++++++++++++++++++++++++++++++++++++++++
include/linux/memory_hotplug.h | 2 +
include/linux/mm.h | 3 +-
mm/memory_hotplug.c | 23 +++++++++++++++--
4 files changed, 77 insertions(+), 4 deletions(-)
Index: linux-3.5-rc4/mm/memory_hotplug.c
===================================================================
I don't think that all pages of virtual mapping in removed memory can be
freed, since page which type is MIX_SECTION_INFO is difficult to free.
So, the patch only frees page which type is SECTION_INFO at first.
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
arch/x86/mm/init_64.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/mm.h | 2 +
mm/memory_hotplug.c | 5 ++
mm/sparse.c | 5 +-
4 files changed, 99 insertions(+), 2 deletions(-)
Index: linux-3.5-rc4/include/linux/mm.h
===================================================================
@@ -614,12 +614,13 @@ static inline struct page *kmalloc_secti/* This will make the necessary allocations eventually. */returnsparse_mem_map_populate(pnum,nid);}-staticvoid__kfree_section_memmap(structpage*memmap,unsignedlongnr_pages)+staticvoid__kfree_section_memmap(structpage*page,unsignedlongnr_pages){-return;/* XXX: Not implemented yet */+vmemmap_kfree(page,nr_pages);}staticvoidfree_map_bootmem(structpage*page,unsignedlongnr_pages){+vmemmap_free_bootmem(page,nr_pages);}#elsestaticstructpage*__kmalloc_section_memmap(unsignedlongnr_pages)
When calling unregister_node(), the function shows following message at
device_release().
Device 'node2' does not have a release() function, it is broken and must be fixed.
So the patch implements node_device_release()
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
Index: linux-3.5-rc1/drivers/base/node.c
===================================================================
From: David Rientjes <rientjes@google.com> Date: 2012-06-27 06:10:38
On Wed, 27 Jun 2012, Yasuaki Ishimatsu wrote:
remove_memory() does not remove memory but just offlines memory. The patch
changes name of it to offline_memory().
The kernel is never going to physically remove the memory itself, so I
don't see the big problem with calling it remove_memory(). If you're
going to change it to offline_memory(), which is just as good but not
better, then I'd suggest changing add_memory() to online_memory() for
completeness.
@@ -887,6 +887,11 @@ static int __ref offline_pages(unsignedlock_memory_hotplug();+if(memory_is_offline(start_pfn,end_pfn)){+ret=0;+gotoout;+}+zone=page_zone(pfn_to_page(start_pfn));node=zone_to_nid(zone);nr_pages=end_pfn-start_pfn;
Are there additional prerequisites for this patch? Otherwise it changes
the return value of offline_memory() which will now call
acpi_memory_powerdown_device() in the acpi memhotplug case when disabling.
Is that a problem?
remove_memory() does not remove memory but just offlines memory. The patch
changes name of it to offline_memory().
There are 3 functions in the kernel:
1. add_memory()
2. online_pages()
3. remove_memory()
So I think offline_pages() is better than offline_memory().
Thanks
Wen Congyang
quoted hunk
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/acpi/acpi_memhotplug.c | 2 +-
drivers/base/memory.c | 4 ++--
include/linux/memory_hotplug.h | 2 +-
mm/memory_hotplug.c | 6 +++---
4 files changed, 7 insertions(+), 7 deletions(-)
Index: linux-3.5-rc4/drivers/acpi/acpi_memhotplug.c
===================================================================
remove_memory() does not remove memory but just offlines memory. The patch
changes name of it to offline_memory().
There are 3 functions in the kernel:
1. add_memory()
2. online_pages()
3. remove_memory()
So I think offline_pages() is better than offline_memory().
There is already a function named offline_pages(). So we
should call offline_pages() instead of remove_memory() in
memory_block_action(), and there is no need to rename
remove_memory().
Thanks
Wen Congyang
Thanks
Wen Congyang
quoted
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/acpi/acpi_memhotplug.c | 2 +-
drivers/base/memory.c | 4 ++--
include/linux/memory_hotplug.h | 2 +-
mm/memory_hotplug.c | 6 +++---
4 files changed, 7 insertions(+), 7 deletions(-)
Index: linux-3.5-rc4/drivers/acpi/acpi_memhotplug.c
===================================================================
When offline_pages() is called to offlined memory, the function fails since
all memory has been offlined. In this case, the function should succeed.
The patch adds the check function into offline_pages().
You miss such case: some pages are online, while some pages are offline.
offline_pages() will fail too in such case.
Thanks
Wen Congyang
quoted hunk
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/base/memory.c | 20 ++++++++++++++++++++
include/linux/memory.h | 1 +
mm/memory_hotplug.c | 5 +++++
3 files changed, 26 insertions(+)
Index: linux-3.5-rc4/drivers/base/memory.c
===================================================================
Hi David and Wen,
Thank you for reviewing my patch.
2012/06/27 17:47, Wen Congyang wrote:
At 06/27/2012 03:14 PM, Wen Congyang Wrote:
quoted
At 06/27/2012 01:42 PM, Yasuaki Ishimatsu Wrote:
quoted
remove_memory() does not remove memory but just offlines memory. The patch
changes name of it to offline_memory().
There are 3 functions in the kernel:
1. add_memory()
2. online_pages()
3. remove_memory()
So I think offline_pages() is better than offline_memory().
There is already a function named offline_pages(). So we
should call offline_pages() instead of remove_memory() in
memory_block_action(), and there is no need to rename
remove_memory().
As Wen says, Linux has 4 functions for memory hotplug already.
In my recognition, these functions are prepared for following purpose.
1. add_memory : add physical memory
2. online_pages : online logical memory
3. remove_memory : offline logical memory
4. offline_pages : offline logical memory
add_memory() is used for adding physical memory. I think remove_memory()
would rather be used for removing physical memory than be used for removing
logical memory. So I renamed remove_memory() to offline_memory().
How do you think?
Regards,
Yasuaki Ishimatsu
Thanks
Wen Congyang
quoted
Thanks
Wen Congyang
quoted
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/acpi/acpi_memhotplug.c | 2 +-
drivers/base/memory.c | 4 ++--
include/linux/memory_hotplug.h | 2 +-
mm/memory_hotplug.c | 6 +++---
4 files changed, 7 insertions(+), 7 deletions(-)
Index: linux-3.5-rc4/drivers/acpi/acpi_memhotplug.c
===================================================================
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Hi David and Wen,
Thank you for reviewing my patch.
2012/06/27 17:47, Wen Congyang wrote:
quoted
At 06/27/2012 03:14 PM, Wen Congyang Wrote:
quoted
At 06/27/2012 01:42 PM, Yasuaki Ishimatsu Wrote:
quoted
remove_memory() does not remove memory but just offlines memory. The patch
changes name of it to offline_memory().
There are 3 functions in the kernel:
1. add_memory()
2. online_pages()
3. remove_memory()
So I think offline_pages() is better than offline_memory().
There is already a function named offline_pages(). So we
should call offline_pages() instead of remove_memory() in
memory_block_action(), and there is no need to rename
remove_memory().
As Wen says, Linux has 4 functions for memory hotplug already.
In my recognition, these functions are prepared for following purpose.
1. add_memory : add physical memory
2. online_pages : online logical memory
3. remove_memory : offline logical memory
4. offline_pages : offline logical memory
add_memory() is used for adding physical memory. I think remove_memory()
would rather be used for removing physical memory than be used for removing
logical memory. So I renamed remove_memory() to offline_memory().
How do you think?
Hmm, remove_memory() will revert all things we do in add_memory(), so I think
there is no need to rename it. If we rename it to offline_memory(), we should
also rename add_memory() to online_memory().
Thanks
Wen Congyang
Hi David and Wen,
Thank you for reviewing my patch.
2012/06/27 17:47, Wen Congyang wrote:
quoted
At 06/27/2012 03:14 PM, Wen Congyang Wrote:
quoted
At 06/27/2012 01:42 PM, Yasuaki Ishimatsu Wrote:
quoted
remove_memory() does not remove memory but just offlines memory. The patch
changes name of it to offline_memory().
There are 3 functions in the kernel:
1. add_memory()
2. online_pages()
3. remove_memory()
So I think offline_pages() is better than offline_memory().
There is already a function named offline_pages(). So we
should call offline_pages() instead of remove_memory() in
memory_block_action(), and there is no need to rename
remove_memory().
As Wen says, Linux has 4 functions for memory hotplug already.
In my recognition, these functions are prepared for following purpose.
1. add_memory : add physical memory
2. online_pages : online logical memory
3. remove_memory : offline logical memory
4. offline_pages : offline logical memory
add_memory() is used for adding physical memory. I think remove_memory()
would rather be used for removing physical memory than be used for removing
logical memory. So I renamed remove_memory() to offline_memory().
How do you think?
Hmm, remove_memory() will revert all things we do in add_memory(), so I think
I think so too.
add_memory() prepares to use physical memory. It prepares some structures
(pgdat, page table, node, etc) for using the physical memory at the system.
But it does not online the meomory. For onlining the memory, we use
online_pages().
So I think that remove_memory() should remove these structures which are
prepared by add_memory() not offline memory. But current remove_memory() code
only calls offline_pages() and offlines memory.
The patch series recreates remove_memory() for removing these structures
after [RFC PATCH 3/12]. The reason to change the name of remove_memory() is a
preparation to recreate it.
Thanks,
Yasuaki Ishimatsu
there is no need to rename it. If we rename it to offline_memory(), we should
also rename add_memory() to online_memory().
Thanks
Wen Congyang
When offline_pages() is called to offlined memory, the function fails since
all memory has been offlined. In this case, the function should succeed.
The patch adds the check function into offline_pages().
You miss such case: some pages are online, while some pages are offline.
offline_pages() will fail too in such case.
You are right. But current code fails, when the function is called to offline
memory. In this case, the function should succeed. So the patch confirms
whether the memory was offlined or not. And if memory has already been
offlined, offline_pages return 0.
Thanks,
Yasuaki Ishimatsu
Thanks
Wen Congyang
quoted
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/base/memory.c | 20 ++++++++++++++++++++
include/linux/memory.h | 1 +
mm/memory_hotplug.c | 5 +++++
3 files changed, 26 insertions(+)
Index: linux-3.5-rc4/drivers/base/memory.c
===================================================================
On Wed, Jun 27, 2012 at 1:44 AM, Yasuaki Ishimatsu
[off-list ref] wrote:
When offline_pages() is called to offlined memory, the function fails sin=
ce
all memory has been offlined. In this case, the function should succeed.
The patch adds the check function into offline_pages().
I don't understand your point. I think following misoperation should
fail. Otherwise
administrator have no way to know their fault.
$ echo offline > memoryN/state
$ echo offline > memoryN/state
In general, we don't like to ignore an error except the standard require it=
.
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
=A0drivers/base/memory.c =A0| =A0 20 ++++++++++++++++++++
=A0include/linux/memory.h | =A0 =A01 +
=A0mm/memory_hotplug.c =A0 =A0| =A0 =A05 +++++
=A03 files changed, 26 insertions(+)
Index: linux-3.5-rc4/drivers/base/memory.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
This seems to have strong sparse dependency.
Hm, I wonder why memory-hotplug.c can enable when X86_64_ACPI_NUMA.
# eventually, we can have this option just 'select SPARSEMEM'
config MEMORY_HOTPLUG
bool "Allow for memory hot-add"
depends on SPARSEMEM || X86_64_ACPI_NUMA
On Thu, Jun 28, 2012 at 1:06 AM, Yasuaki Ishimatsu
[off-list ref] wrote:
Hi Wen,
2012/06/27 17:49, Wen Congyang wrote:
quoted
At 06/27/2012 01:44 PM, Yasuaki Ishimatsu Wrote:
quoted
When offline_pages() is called to offlined memory, the function fails since
all memory has been offlined. In this case, the function should succeed.
The patch adds the check function into offline_pages().
You miss such case: some pages are online, while some pages are offline.
offline_pages() will fail too in such case.
You are right. But current code fails, when the function is called to offline
memory. In this case, the function should succeed. So the patch confirms
whether the memory was offlined or not. And if memory has already been
offlined, offline_pages return 0.
Can you please explain why the caller can't check it? I hope to avoid
an ignorance
as far as we can.
Hi Kosaki-san,
2012/06/28 14:27, KOSAKI Motohiro wrote:
On Thu, Jun 28, 2012 at 1:06 AM, Yasuaki Ishimatsu
[off-list ref] wrote:
quoted
Hi Wen,
2012/06/27 17:49, Wen Congyang wrote:
quoted
At 06/27/2012 01:44 PM, Yasuaki Ishimatsu Wrote:
quoted
When offline_pages() is called to offlined memory, the function fails since
all memory has been offlined. In this case, the function should succeed.
The patch adds the check function into offline_pages().
You miss such case: some pages are online, while some pages are offline.
offline_pages() will fail too in such case.
You are right. But current code fails, when the function is called to offline
memory. In this case, the function should succeed. So the patch confirms
whether the memory was offlined or not. And if memory has already been
offlined, offline_pages return 0.
Can you please explain why the caller can't check it? I hope to avoid
an ignorance
as far as we can.
Of course, caller side can check it. But there is a possibility that
offline_pages() is called by many functions. So I do not think that it
is good that all functions which call offline_pages() check it.
Thanks,
Yasuaki Ishimatsu
When (hot)adding memory into system, /sys/firmware/memmap/X/{end, start, type}
sysfs files are created. But there is no code to remove these files. The patch
implements the function to remove them.
Note : The code does not free firmware_map_entry since there is no way to free
memory which is allocated by bootmem.
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/firmware/memmap.c | 71 +++++++++++++++++++++++++++++++++++++++++++
include/linux/firmware-map.h | 6 +++
mm/memory_hotplug.c | 6 +++
3 files changed, 82 insertions(+), 1 deletion(-)
Index: linux-3.5-rc4/mm/memory_hotplug.c
===================================================================
@@ -123,6 +123,16 @@ static int firmware_map_add_entry(u64 streturn0;}+/**+*firmware_map_remove_entry()-Doestherealworktoremoveafirmware+*memmapentry.+*@entry:removedentry.+**/+staticinlinevoidfirmware_map_remove_entry(structfirmware_map_entry*entry)+{+list_del(&entry->list);+}+/**Addmemmapentryonsysfs*/
@@ -144,6 +154,31 @@ static int add_sysfs_fw_map_entry(structreturn0;}+/*+*Removememmapentryonsysfs+*/+staticinlinevoidremove_sysfs_fw_map_entry(structfirmware_map_entry*entry)+{+kobject_del(&entry->kobj);+}++/*+*Searchmemmapentry+*/++structfirmware_map_entry*__meminit+find_firmware_map_entry(u64start,u64end,constchar*type)+{+structfirmware_map_entry*entry;++list_for_each_entry(entry,&map_entries,list)+if((entry->start==start)&&(entry->end==end)&&+(!strcmp(entry->type,type)))+returnentry;++returnNULL;+}+/***firmware_map_add_hotplug()-Addsafirmwaremappingentrywhenwedo*memoryhotplug.
@@ -196,6 +231,42 @@ int __init firmware_map_add_early(u64 streturnfirmware_map_add_entry(start,end,type,entry);}+voidrelease_firmware_map_entry(structfirmware_map_entry*entry)+{+/*+*FIXME:Thereisnoidea.+*Howtofreetheentrywhichallocatedbootmem?+*/+}++/**+*firmware_map_remove()-removeafirmwaremappingentry+*@start:Startofthememoryrange.+*@end:Endofthememoryrange(inclusive).+*@type:Typeofthememoryrange.+*+*removesafirmwaremappingentry.+*+*Returns0onsuccess,or-EINVALifnoentry.+**/+int__meminitfirmware_map_remove(u64start,u64end,constchar*type)+{+structfirmware_map_entry*entry;++entry=find_firmware_map_entry(start,end,type);
Hmm, we cannot find the entry easily, because the end can be:
1. start + size
2. start + size - 1
So, We should fix this bug first.
quoted hunk
+ if (!entry)+ return -EINVAL;++ /* remove the memmap entry */+ remove_sysfs_fw_map_entry(entry);++ firmware_map_remove_entry(entry);++ release_firmware_map_entry(entry);
I guess you want to free the memory in the above function. But I think it is
not a good idea to free it here. We should free it when the entry->kobj's kref
is decreased to 0.
Thanks
Wen Congyang
+
+ return 0;
+}
+
/*
* Sysfs functions -------------------------------------------------------------
*/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Hi Kosaki-san,
2012/06/28 14:26, KOSAKI Motohiro wrote:
On Wed, Jun 27, 2012 at 1:44 AM, Yasuaki Ishimatsu
[off-list ref] wrote:
quoted
When offline_pages() is called to offlined memory, the function fails since
all memory has been offlined. In this case, the function should succeed.
The patch adds the check function into offline_pages().
I don't understand your point. I think following misoperation should
fail. Otherwise
administrator have no way to know their fault.
$ echo offline > memoryN/state
$ echo offline > memoryN/state
In general, we don't like to ignore an error except the standard require it.
I understood the intention of previous mail (why the caller can't check it? ).
I'll move memory_is_offline() to caller side.
quoted
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/base/memory.c | 20 ++++++++++++++++++++
include/linux/memory.h | 1 +
mm/memory_hotplug.c | 5 +++++
3 files changed, 26 insertions(+)
Index: linux-3.5-rc4/drivers/base/memory.c
===================================================================
Thanks.
I will consider other CONFIG_.
Thanks.
Yasuaki Ishimatsu
Hm, I wonder why memory-hotplug.c can enable when X86_64_ACPI_NUMA.
# eventually, we can have this option just 'select SPARSEMEM'
config MEMORY_HOTPLUG
bool "Allow for memory hot-add"
depends on SPARSEMEM || X86_64_ACPI_NUMA
quoted
+ if (!mem)
+ continue;
+ if (mem->state == MEM_OFFLINE)
+ continue;
+ return false;
+ }
+
+ return true;
+}
+
/*
* register_memory - Setup a sysfs device for a memory block
*/
Index: linux-3.5-rc4/include/linux/memory.h
===================================================================
@@ -887,6 +887,11 @@ static int __ref offline_pages(unsignedlock_memory_hotplug();+if(memory_is_offline(start_pfn,end_pfn)){+ret=0;+gotoout;+}+zone=page_zone(pfn_to_page(start_pfn));node=zone_to_nid(zone);nr_pages=end_pfn-start_pfn;--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
@@ -887,6 +887,11 @@ static int __ref offline_pages(unsignedlock_memory_hotplug();+if(memory_is_offline(start_pfn,end_pfn)){+ret=0;+gotoout;+}+zone=page_zone(pfn_to_page(start_pfn));node=zone_to_nid(zone);nr_pages=end_pfn-start_pfn;
Are there additional prerequisites for this patch? Otherwise it changes
the return value of offline_memory() which will now call
acpi_memory_powerdown_device() in the acpi memhotplug case when disabling.
Is that a problem?
I have understood there is a person who expects "offline_pages()" to fail
in this case by kosaki's comment. So I'll move memory_is_offline to caller
side.
Thanks,
Yasuaki Ishimatsu
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
When (hot)adding memory into system, /sys/firmware/memmap/X/{end, start, type}
sysfs files are created. But there is no code to remove these files. The patch
implements the function to remove them.
Note : The code does not free firmware_map_entry since there is no way to free
memory which is allocated by bootmem.
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/firmware/memmap.c | 71 +++++++++++++++++++++++++++++++++++++++++++
include/linux/firmware-map.h | 6 +++
mm/memory_hotplug.c | 6 +++
3 files changed, 82 insertions(+), 1 deletion(-)
Index: linux-3.5-rc4/mm/memory_hotplug.c
===================================================================
@@ -123,6 +123,16 @@ static int firmware_map_add_entry(u64 streturn0;}+/**+*firmware_map_remove_entry()-Doestherealworktoremoveafirmware+*memmapentry.+*@entry:removedentry.+**/+staticinlinevoidfirmware_map_remove_entry(structfirmware_map_entry*entry)+{+list_del(&entry->list);+}+/**Addmemmapentryonsysfs*/
@@ -144,6 +154,31 @@ static int add_sysfs_fw_map_entry(structreturn0;}+/*+*Removememmapentryonsysfs+*/+staticinlinevoidremove_sysfs_fw_map_entry(structfirmware_map_entry*entry)+{+kobject_del(&entry->kobj);+}++/*+*Searchmemmapentry+*/++structfirmware_map_entry*__meminit+find_firmware_map_entry(u64start,u64end,constchar*type)+{+structfirmware_map_entry*entry;++list_for_each_entry(entry,&map_entries,list)+if((entry->start==start)&&(entry->end==end)&&+(!strcmp(entry->type,type)))+returnentry;++returnNULL;+}+/***firmware_map_add_hotplug()-Addsafirmwaremappingentrywhenwedo*memoryhotplug.
@@ -196,6 +231,42 @@ int __init firmware_map_add_early(u64 streturnfirmware_map_add_entry(start,end,type,entry);}+voidrelease_firmware_map_entry(structfirmware_map_entry*entry)+{+/*+*FIXME:Thereisnoidea.+*Howtofreetheentrywhichallocatedbootmem?+*/+}++/**+*firmware_map_remove()-removeafirmwaremappingentry+*@start:Startofthememoryrange.+*@end:Endofthememoryrange(inclusive).+*@type:Typeofthememoryrange.+*+*removesafirmwaremappingentry.+*+*Returns0onsuccess,or-EINVALifnoentry.+**/+int__meminitfirmware_map_remove(u64start,u64end,constchar*type)+{+structfirmware_map_entry*entry;++entry=find_firmware_map_entry(start,end,type);
Hmm, we cannot find the entry easily, because the end can be:
1. start + size
2. start + size - 1
So, We should fix this bug first.
This is not a bug.
start and size arguments of firmware_map_remove() include
acpi_memory_info->{start_addr, length}. And when creating a firmware_map_entry,
the entry is created by acpi_memory_info->{start_addr, length}. So I don't
think that we need care your comment.
quoted
+ if (!entry)+ return -EINVAL;++ /* remove the memmap entry */+ remove_sysfs_fw_map_entry(entry);++ firmware_map_remove_entry(entry);++ release_firmware_map_entry(entry);
I guess you want to free the memory in the above function. But I think it is
not a good idea to free it here. We should free it when the entry->kobj's kref
is decreased to 0.
Thanks.
I'll update your comment at next version.
Thanks,
Yasuaki Ishimatsu
Thanks
Wen Congyang
quoted
+
+ return 0;
+}
+
/*
* Sysfs functions -------------------------------------------------------------
*/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
When (hot)adding memory into system, /sys/firmware/memmap/X/{end,
start, type}
sysfs files are created. But there is no code to remove these files.
The patch
implements the function to remove them.
Note : The code does not free firmware_map_entry since there is no
way to free
memory which is allocated by bootmem.
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/firmware/memmap.c | 71
+++++++++++++++++++++++++++++++++++++++++++
include/linux/firmware-map.h | 6 +++
mm/memory_hotplug.c | 6 +++
3 files changed, 82 insertions(+), 1 deletion(-)
Index: linux-3.5-rc4/mm/memory_hotplug.c
===================================================================
@@ -196,6 +231,42 @@ int __init firmware_map_add_early(u64 st return firmware_map_add_entry(start, end, type, entry); }+void release_firmware_map_entry(struct firmware_map_entry *entry)+{+ /*+ * FIXME : There is no idea.+ * How to free the entry which allocated bootmem?+ */+}++/**+ * firmware_map_remove() - remove a firmware mapping entry+ * @start: Start of the memory range.+ * @end: End of the memory range (inclusive).+ * @type: Type of the memory range.+ *+ * removes a firmware mapping entry.+ *+ * Returns 0 on success, or -EINVAL if no entry.+ **/+int __meminit firmware_map_remove(u64 start, u64 end, const char *type)+{+ struct firmware_map_entry *entry;++ entry = find_firmware_map_entry(start, end, type);
Hmm, we cannot find the entry easily, because the end can be:
1. start + size
2. start + size - 1
So, We should fix this bug first.
This is not a bug.
start and size arguments of firmware_map_remove() include
acpi_memory_info->{start_addr, length}. And when creating a
firmware_map_entry,
the entry is created by acpi_memory_info->{start_addr, length}. So I don't
think that we need care your comment.
If the memory device is hotpluged before the os starts, and the memory
map is included in e820 map, the entry will be created by firmware_map_add_early().
The function firmware_map_add_early() is called by e820_reserve_resources():
=====================
for (i = 0; i < e820_saved.nr_map; i++) {
struct e820entry *entry = &e820_saved.map[i];
firmware_map_add_early(entry->addr,
entry->addr + entry->size - 1,
e820_type_to_string(entry->type));
}
=====================
Note: the end is addr + size - 1, not addr + size.
In such case, you cannot find the entry.
Thanks
Wen Congyang
quoted
quoted
+ if (!entry)+ return -EINVAL;++ /* remove the memmap entry */+ remove_sysfs_fw_map_entry(entry);++ firmware_map_remove_entry(entry);++ release_firmware_map_entry(entry);
I guess you want to free the memory in the above function. But I think
it is
not a good idea to free it here. We should free it when the
entry->kobj's kref
is decreased to 0.
Thanks.
I'll update your comment at next version.
Thanks,
Yasuaki Ishimatsu
quoted
Thanks
Wen Congyang
quoted
+
+ return 0;
+}
+
/*
* Sysfs functions
-------------------------------------------------------------
*/
--
To unsubscribe from this list: send the line "unsubscribe
linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
When (hot)adding memory into system, /sys/firmware/memmap/X/{end,
start, type}
sysfs files are created. But there is no code to remove these files.
The patch
implements the function to remove them.
Note : The code does not free firmware_map_entry since there is no
way to free
memory which is allocated by bootmem.
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/firmware/memmap.c | 71
+++++++++++++++++++++++++++++++++++++++++++
include/linux/firmware-map.h | 6 +++
mm/memory_hotplug.c | 6 +++
3 files changed, 82 insertions(+), 1 deletion(-)
Index: linux-3.5-rc4/mm/memory_hotplug.c
===================================================================
@@ -196,6 +231,42 @@ int __init firmware_map_add_early(u64 st return firmware_map_add_entry(start, end, type, entry); }+void release_firmware_map_entry(struct firmware_map_entry *entry)+{+ /*+ * FIXME : There is no idea.+ * How to free the entry which allocated bootmem?+ */+}++/**+ * firmware_map_remove() - remove a firmware mapping entry+ * @start: Start of the memory range.+ * @end: End of the memory range (inclusive).+ * @type: Type of the memory range.+ *+ * removes a firmware mapping entry.+ *+ * Returns 0 on success, or -EINVAL if no entry.+ **/+int __meminit firmware_map_remove(u64 start, u64 end, const char *type)+{+ struct firmware_map_entry *entry;++ entry = find_firmware_map_entry(start, end, type);
Hmm, we cannot find the entry easily, because the end can be:
1. start + size
2. start + size - 1
So, We should fix this bug first.
This is not a bug.
start and size arguments of firmware_map_remove() include
acpi_memory_info->{start_addr, length}. And when creating a
firmware_map_entry,
the entry is created by acpi_memory_info->{start_addr, length}. So I don't
think that we need care your comment.
If the memory device is hotpluged before the os starts, and the memory
map is included in e820 map, the entry will be created by firmware_map_add_early().
The function firmware_map_add_early() is called by e820_reserve_resources():
=====================
for (i = 0; i < e820_saved.nr_map; i++) {
struct e820entry *entry = &e820_saved.map[i];
firmware_map_add_early(entry->addr,
entry->addr + entry->size - 1,
e820_type_to_string(entry->type));
}
=====================
Note: the end is addr + size - 1, not addr + size.
In such case, you cannot find the entry.
Thank you for your explanation. I understood it.
end argument of firmware_map_add_hotplug() has always "addr + size",
not "addr + size - 1". I think that changing argument of
firmware_map_add_early() is high risk since the function has been used
since early times. So, I will unify to "addr + size - 1".
Thanks,
Yasuaki Ishimatsu
Thanks
Wen Congyang
quoted
quoted
quoted
+ if (!entry)+ return -EINVAL;++ /* remove the memmap entry */+ remove_sysfs_fw_map_entry(entry);++ firmware_map_remove_entry(entry);++ release_firmware_map_entry(entry);
I guess you want to free the memory in the above function. But I think
it is
not a good idea to free it here. We should free it when the
entry->kobj's kref
is decreased to 0.
Thanks.
I'll update your comment at next version.
Thanks,
Yasuaki Ishimatsu
quoted
Thanks
Wen Congyang
quoted
+
+ return 0;
+}
+
/*
* Sysfs functions
-------------------------------------------------------------
*/
--
To unsubscribe from this list: send the line "unsubscribe
linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Jiang Liu <hidden> Date: 2012-06-30 15:47:12
On 06/27/2012 01:44 PM, Yasuaki Ishimatsu wrote:
quoted hunk
When offline_pages() is called to offlined memory, the function fails since
all memory has been offlined. In this case, the function should succeed.
The patch adds the check function into offline_pages().
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/base/memory.c | 20 ++++++++++++++++++++
include/linux/memory.h | 1 +
mm/memory_hotplug.c | 5 +++++
3 files changed, 26 insertions(+)
Index: linux-3.5-rc4/drivers/base/memory.c
===================================================================
@@ -887,6 +887,11 @@ static int __ref offline_pages(unsignedlock_memory_hotplug();+if(memory_is_offline(start_pfn,end_pfn)){+ret=0;+gotoout;+}+zone=page_zone(pfn_to_page(start_pfn));node=zone_to_nid(zone);nr_pages=end_pfn-start_pfn;--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Jiang Liu <hidden> Date: 2012-06-30 15:51:33
On 06/27/2012 01:44 PM, Yasuaki Ishimatsu wrote:
quoted hunk
When offline_pages() is called to offlined memory, the function fails since
all memory has been offlined. In this case, the function should succeed.
The patch adds the check function into offline_pages().
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/base/memory.c | 20 ++++++++++++++++++++
include/linux/memory.h | 1 +
mm/memory_hotplug.c | 5 +++++
3 files changed, 26 insertions(+)
Index: linux-3.5-rc4/drivers/base/memory.c
===================================================================
@@ -887,6 +887,11 @@ static int __ref offline_pages(unsignedlock_memory_hotplug();+if(memory_is_offline(start_pfn,end_pfn)){+ret=0;+gotoout;+}+zone=page_zone(pfn_to_page(start_pfn));node=zone_to_nid(zone);nr_pages=end_pfn-start_pfn;--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Jiang Liu <hidden> Date: 2012-06-30 15:58:27
On 06/27/2012 01:56 PM, Yasuaki Ishimatsu wrote:
quoted hunk
I don't think that all pages of virtual mapping in removed memory can be
freed, since page which type is MIX_SECTION_INFO is difficult to free.
So, the patch only frees page which type is SECTION_INFO at first.
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
arch/x86/mm/init_64.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/mm.h | 2 +
mm/memory_hotplug.c | 5 ++
mm/sparse.c | 5 +-
4 files changed, 99 insertions(+), 2 deletions(-)
Index: linux-3.5-rc4/include/linux/mm.h
===================================================================
@@ -614,12 +614,13 @@ static inline struct page *kmalloc_secti/* This will make the necessary allocations eventually. */returnsparse_mem_map_populate(pnum,nid);}-staticvoid__kfree_section_memmap(structpage*memmap,unsignedlongnr_pages)+staticvoid__kfree_section_memmap(structpage*page,unsignedlongnr_pages){-return;/* XXX: Not implemented yet */+vmemmap_kfree(page,nr_pages);}staticvoidfree_map_bootmem(structpage*page,unsignedlongnr_pages){+vmemmap_free_bootmem(page,nr_pages);}#elsestaticstructpage*__kmalloc_section_memmap(unsignedlongnr_pages)
I think the third parameter should be "struct page **pp" instead of "struct page *page".
And "page = pte_page(*pte)" should be "*pp = pte_page(*pte)".
Otherwise the found page pointer can't be returned to the caller and vmemmap_kfree()
just sees random value in variable "page".
quoted hunk
+{
+ pgd_t *pgd;
+ pud_t *pud;
+ pmd_t *pmd;
+ pte_t *pte;
+ unsigned long next;
+
+ page = NULL;
+
+ pgd = pgd_offset_k(addr);
+ if (pgd_none(*pgd))
+ return PAGE_SIZE;
+
+ pud = pud_offset(pgd, addr);
+ if (pud_none(*pud))
+ return PAGE_SIZE;
+
+ if (!cpu_has_pse) {
+ next = (addr + PAGE_SIZE) & PAGE_MASK;
+ pmd = pmd_offset(pud, addr);
+ if (pmd_none(*pmd))
+ return next;
+
+ pte = pte_offset_kernel(pmd, addr);
+ if (pte_none(*pte))
+ return next;
+
+ page = pte_page(*pte);
+ pte_clear(&init_mm, addr, pte);
+ } else {
+ next = pmd_addr_end(addr, end);
+
+ pmd = pmd_offset(pud, addr);
+ if (pmd_none(*pmd))
+ return next;
+
+ page = pmd_page(*pmd);
+ pmd_clear(pmd);
+ }
+
+ return next;
+}
+
+void __meminit
+vmemmap_kfree(struct page *memmap, unsigned long nr_pages)
+{
+ unsigned long addr = (unsigned long)memmap;
+ unsigned long end = (unsigned long)(memmap + nr_pages);
+ unsigned long next;
+ unsigned int order;
+ struct page *page;
+
+ for (; addr < end; addr = next) {
+ next = find_and_clear_pte_page(addr, end, page);
+ if (!page)
+ continue;
+
+ if (is_vmalloc_addr(page))
+ vfree(page);
+ else {
+ order = next - addr;
+ free_pages((unsigned long)page,
+ get_order(sizeof(struct page) * order));
+ }
+ }
+}
+
+void __meminit
+vmemmap_free_bootmem(struct page *memmap, unsigned long nr_pages)
+{
+ unsigned long addr = (unsigned long)memmap;
+ unsigned long end = (unsigned long)(memmap + nr_pages);
+ unsigned long next;
+ struct page *page;
+ unsigned long magic;
+
+ for (; addr < end; addr = next) {
+ next = find_and_clear_pte_page(addr, end, page);
+ if (!page)
+ continue;
+
+ magic = (unsigned long) page->lru.next;
+ if (magic == SECTION_INFO)
+ put_page_bootmem(page);
+ }
+}
+
void __meminit
register_page_bootmem_memmap(unsigned long section_nr, struct page *start_page,
unsigned long size)
Index: linux-3.5-rc4/mm/memory_hotplug.c
===================================================================
@@ -303,6 +303,8 @@ static int __meminit __add_section(int n#ifdef CONFIG_SPARSEMEM_VMEMMAPstaticint__remove_section(structzone*zone,structmem_section*ms){+unsignedlongflags;+structpglist_data*pgdat=zone->zone_pgdat;intret;if(!valid_section(ms))
@@ -310,6 +312,9 @@ static int __remove_section(struct zoneret=unregister_memory_section(ms);+pgdat_resize_lock(pgdat,&flags);+sparse_remove_one_section(zone,ms);+pgdat_resize_unlock(pgdat,&flags);returnret;}#else--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Jiang,
Thank you for your feedback.
2012/07/01 0:46, Jiang Liu wrote:
On 06/27/2012 01:44 PM, Yasuaki Ishimatsu wrote:
quoted
When offline_pages() is called to offlined memory, the function fails since
all memory has been offlined. In this case, the function should succeed.
The patch adds the check function into offline_pages().
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/base/memory.c | 20 ++++++++++++++++++++
include/linux/memory.h | 1 +
mm/memory_hotplug.c | 5 +++++
3 files changed, 26 insertions(+)
Index: linux-3.5-rc4/drivers/base/memory.c
===================================================================
Is it possible for __nr_to_section return NULL here?
Yes. I'll add NULL check.
quoted
+ mem = find_memory_block(section);+ if (!mem)+ continue;+ if (mem->state == MEM_OFFLINE)+ continue;+ return false;+ }++ return true;+}
Need a put_dev(&mem->dev) for the last memory block device handled before return.
Thanks.
I think kobject_put(&mem->dev.kobj) should be handled for all memory block
devices found by find_memory_block(). I'll update it.
Thanks,
Yasuaki Ishimatsu
quoted
+
/*
* register_memory - Setup a sysfs device for a memory block
*/
Index: linux-3.5-rc4/include/linux/memory.h
===================================================================
@@ -887,6 +887,11 @@ static int __ref offline_pages(unsignedlock_memory_hotplug();+if(memory_is_offline(start_pfn,end_pfn)){+ret=0;+gotoout;+}+zone=page_zone(pfn_to_page(start_pfn));node=zone_to_nid(zone);nr_pages=end_pfn-start_pfn;--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
When offline_pages() is called to offlined memory, the function fails since
all memory has been offlined. In this case, the function should succeed.
The patch adds the check function into offline_pages().
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
drivers/base/memory.c | 20 ++++++++++++++++++++
include/linux/memory.h | 1 +
mm/memory_hotplug.c | 5 +++++
3 files changed, 26 insertions(+)
Index: linux-3.5-rc4/drivers/base/memory.c
===================================================================
@@ -887,6 +887,11 @@ static int __ref offline_pages(unsignedlock_memory_hotplug();+if(memory_is_offline(start_pfn,end_pfn)){+ret=0;+gotoout;+}+zone=page_zone(pfn_to_page(start_pfn));node=zone_to_nid(zone);nr_pages=end_pfn-start_pfn;--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
I don't think that all pages of virtual mapping in removed memory can be
freed, since page which type is MIX_SECTION_INFO is difficult to free.
So, the patch only frees page which type is SECTION_INFO at first.
CC: Len Brown <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Christoph Lameter <redacted>
Cc: Minchan Kim <redacted>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: KOSAKI Motohiro <redacted>
CC: Wen Congyang <redacted>
Signed-off-by: Yasuaki Ishimatsu <redacted>
---
arch/x86/mm/init_64.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/mm.h | 2 +
mm/memory_hotplug.c | 5 ++
mm/sparse.c | 5 +-
4 files changed, 99 insertions(+), 2 deletions(-)
Index: linux-3.5-rc4/include/linux/mm.h
===================================================================
@@ -614,12 +614,13 @@ static inline struct page *kmalloc_secti/* This will make the necessary allocations eventually. */returnsparse_mem_map_populate(pnum,nid);}-staticvoid__kfree_section_memmap(structpage*memmap,unsignedlongnr_pages)+staticvoid__kfree_section_memmap(structpage*page,unsignedlongnr_pages){-return;/* XXX: Not implemented yet */+vmemmap_kfree(page,nr_pages);}staticvoidfree_map_bootmem(structpage*page,unsignedlongnr_pages){+vmemmap_free_bootmem(page,nr_pages);}#elsestaticstructpage*__kmalloc_section_memmap(unsignedlongnr_pages)
I think the third parameter should be "struct page **pp" instead of "struct page *page".
And "page = pte_page(*pte)" should be "*pp = pte_page(*pte)".
Otherwise the found page pointer can't be returned to the caller and vmemmap_kfree()
just sees random value in variable "page".
Oh, you are right. I'll update it.
Thanks,
Yasuaki Ishimatsu
quoted
+{
+ pgd_t *pgd;
+ pud_t *pud;
+ pmd_t *pmd;
+ pte_t *pte;
+ unsigned long next;
+
+ page = NULL;
+
+ pgd = pgd_offset_k(addr);
+ if (pgd_none(*pgd))
+ return PAGE_SIZE;
+
+ pud = pud_offset(pgd, addr);
+ if (pud_none(*pud))
+ return PAGE_SIZE;
+
+ if (!cpu_has_pse) {
+ next = (addr + PAGE_SIZE) & PAGE_MASK;
+ pmd = pmd_offset(pud, addr);
+ if (pmd_none(*pmd))
+ return next;
+
+ pte = pte_offset_kernel(pmd, addr);
+ if (pte_none(*pte))
+ return next;
+
+ page = pte_page(*pte);
+ pte_clear(&init_mm, addr, pte);
+ } else {
+ next = pmd_addr_end(addr, end);
+
+ pmd = pmd_offset(pud, addr);
+ if (pmd_none(*pmd))
+ return next;
+
+ page = pmd_page(*pmd);
+ pmd_clear(pmd);
+ }
+
+ return next;
+}
+
+void __meminit
+vmemmap_kfree(struct page *memmap, unsigned long nr_pages)
+{
+ unsigned long addr = (unsigned long)memmap;
+ unsigned long end = (unsigned long)(memmap + nr_pages);
+ unsigned long next;
+ unsigned int order;
+ struct page *page;
+
+ for (; addr < end; addr = next) {
+ next = find_and_clear_pte_page(addr, end, page);
+ if (!page)
+ continue;
+
+ if (is_vmalloc_addr(page))
+ vfree(page);
+ else {
+ order = next - addr;
+ free_pages((unsigned long)page,
+ get_order(sizeof(struct page) * order));
+ }
+ }
+}
+
+void __meminit
+vmemmap_free_bootmem(struct page *memmap, unsigned long nr_pages)
+{
+ unsigned long addr = (unsigned long)memmap;
+ unsigned long end = (unsigned long)(memmap + nr_pages);
+ unsigned long next;
+ struct page *page;
+ unsigned long magic;
+
+ for (; addr < end; addr = next) {
+ next = find_and_clear_pte_page(addr, end, page);
+ if (!page)
+ continue;
+
+ magic = (unsigned long) page->lru.next;
+ if (magic == SECTION_INFO)
+ put_page_bootmem(page);
+ }
+}
+
void __meminit
register_page_bootmem_memmap(unsigned long section_nr, struct page *start_page,
unsigned long size)
Index: linux-3.5-rc4/mm/memory_hotplug.c
===================================================================
@@ -303,6 +303,8 @@ static int __meminit __add_section(int n#ifdef CONFIG_SPARSEMEM_VMEMMAPstaticint__remove_section(structzone*zone,structmem_section*ms){+unsignedlongflags;+structpglist_data*pgdat=zone->zone_pgdat;intret;if(!valid_section(ms))
@@ -310,6 +312,9 @@ static int __remove_section(struct zoneret=unregister_memory_section(ms);+pgdat_resize_lock(pgdat,&flags);+sparse_remove_one_section(zone,ms);+pgdat_resize_unlock(pgdat,&flags);returnret;}#else--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>