* [Patch] Change interface of add/remove_memory() from paddr to pfn
@ 2006-04-04 5:06 Yasunori Goto
2006-04-07 14:41 ` Dave Hansen
0 siblings, 1 reply; 2+ messages in thread
From: Yasunori Goto @ 2006-04-04 5:06 UTC (permalink / raw)
To: Andrew Morton; +Cc: Dave Hansen, Linux Kernel ML
This patch is to change interfaces of add/remove_memory()
from physicall address to pfn.
Current add_memory() of each architecture changes paddr to pfn,
and __add_pages() are called by pfn after all.
So, it is not for un-alignment memory.
Using pfn is a bit better to avoid misunderstanding to use add_memory().
In addition, this patch reduces a few lines of kernel.
(Unfortunately, x86-64 and powerpc look using paddr for some
reasons.)
This patch is against 2.6.16-mm2.
Please apply.
Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
arch/i386/mm/init.c | 4 +---
arch/ia64/mm/init.c | 6 ++----
arch/powerpc/mm/mem.c | 12 +++++-------
arch/x86_64/mm/init.c | 6 +++---
drivers/acpi/acpi_memhotplug.c | 2 +-
drivers/base/memory.c | 6 +++---
include/linux/memory_hotplug.h | 4 ++--
7 files changed, 17 insertions(+), 23 deletions(-)
Index: pgdat10/arch/ia64/mm/init.c
===================================================================
--- pgdat10.orig/arch/ia64/mm/init.c 2006-03-31 14:43:24.000000000 +0900
+++ pgdat10/arch/ia64/mm/init.c 2006-03-31 15:43:07.000000000 +0900
@@ -646,12 +646,10 @@ void online_page(struct page *page)
num_physpages++;
}
-int add_memory(u64 start, u64 size)
+int add_memory(unsigned long start_pfn, unsigned long nr_pages)
{
pg_data_t *pgdat;
struct zone *zone;
- unsigned long start_pfn = start >> PAGE_SHIFT;
- unsigned long nr_pages = size >> PAGE_SHIFT;
int ret;
pgdat = NODE_DATA(0);
@@ -666,7 +664,7 @@ int add_memory(u64 start, u64 size)
return ret;
}
-int remove_memory(u64 start, u64 size)
+int remove_memory(unsigned long start_pfn, unsigned long nr_pages)
{
return -EINVAL;
}
Index: pgdat10/arch/powerpc/mm/mem.c
===================================================================
--- pgdat10.orig/arch/powerpc/mm/mem.c 2006-03-31 14:43:25.000000000 +0900
+++ pgdat10/arch/powerpc/mm/mem.c 2006-03-31 15:43:07.000000000 +0900
@@ -114,13 +114,13 @@ void online_page(struct page *page)
num_physpages++;
}
-int __devinit add_memory(u64 start, u64 size)
+int __devinit add_memory(unsigned long start_pfn, unsigned long nr_pages)
{
struct pglist_data *pgdata;
struct zone *zone;
+ u64 start = start_pfn << PAGE_SHIFT;
+ u64 size = nr_pages << PAGE_SHIFT;
int nid;
- unsigned long start_pfn = start >> PAGE_SHIFT;
- unsigned long nr_pages = size >> PAGE_SHIFT;
nid = hot_add_scn_to_nid(start);
pgdata = NODE_DATA(nid);
@@ -140,13 +140,11 @@ int __devinit add_memory(u64 start, u64
* First pass at this code will check to determine if the remove
* request is within the RMO. Do not allow removal within the RMO.
*/
-int __devinit remove_memory(u64 start, u64 size)
+int __devinit remove_memory(unsigned long start_pfn, unsigned long nr_pages)
{
struct zone *zone;
- unsigned long start_pfn, end_pfn, nr_pages;
+ unsigned long end_pfn;
- start_pfn = start >> PAGE_SHIFT;
- nr_pages = size >> PAGE_SHIFT;
end_pfn = start_pfn + nr_pages;
printk("%s(): Attempting to remove memoy in range "
Index: pgdat10/arch/x86_64/mm/init.c
===================================================================
--- pgdat10.orig/arch/x86_64/mm/init.c 2006-03-31 14:43:25.000000000 +0900
+++ pgdat10/arch/x86_64/mm/init.c 2006-03-31 15:43:07.000000000 +0900
@@ -538,12 +538,12 @@ int __add_pages(struct zone *z, unsigned
}
#endif
-int add_memory(u64 start, u64 size)
+int add_memory(unsigned long start_pfn, unsigned long nr_pages)
{
struct pglist_data *pgdat = NODE_DATA(0);
struct zone *zone = pgdat->node_zones + MAX_NR_ZONES-2;
- unsigned long start_pfn = start >> PAGE_SHIFT;
- unsigned long nr_pages = size >> PAGE_SHIFT;
+ unsigned long start = start_pfn << PAGE_SHIFT;
+ unsigned long size = nr_pages << PAGE_SHIFT;
int ret;
ret = __add_pages(zone, start_pfn, nr_pages);
Index: pgdat10/drivers/base/memory.c
===================================================================
--- pgdat10.orig/drivers/base/memory.c 2006-03-31 14:43:25.000000000 +0900
+++ pgdat10/drivers/base/memory.c 2006-03-31 15:43:07.000000000 +0900
@@ -186,8 +186,8 @@ memory_block_action(struct memory_block
mem->state = MEM_GOING_OFFLINE;
memory_notify(MEM_GOING_OFFLINE, NULL);
start_paddr = page_to_pfn(first_page) << PAGE_SHIFT;
- ret = remove_memory(start_paddr,
- PAGES_PER_SECTION << PAGE_SHIFT);
+ ret = remove_memory(start_paddr >> PAGE_SHIFT,
+ PAGES_PER_SECTION);
if (ret) {
mem->state = old_state;
break;
@@ -310,7 +310,7 @@ memory_probe_store(struct class *class,
phys_addr = simple_strtoull(buf, NULL, 0);
- ret = add_memory(phys_addr, PAGES_PER_SECTION << PAGE_SHIFT);
+ ret = add_memory(phys_addr >> PAGE_SHIFT, PAGES_PER_SECTION);
if (ret)
count = ret;
Index: pgdat10/arch/i386/mm/init.c
===================================================================
--- pgdat10.orig/arch/i386/mm/init.c 2006-03-31 14:43:24.000000000 +0900
+++ pgdat10/arch/i386/mm/init.c 2006-03-31 15:43:07.000000000 +0900
@@ -652,12 +652,10 @@ void __init mem_init(void)
* memory to the highmem for now.
*/
#ifndef CONFIG_NEED_MULTIPLE_NODES
-int add_memory(u64 start, u64 size)
+int add_memory(unsigned long start_pfn, unsigned long nr_pages)
{
struct pglist_data *pgdata = &contig_page_data;
struct zone *zone = pgdata->node_zones + MAX_NR_ZONES-1;
- unsigned long start_pfn = start >> PAGE_SHIFT;
- unsigned long nr_pages = size >> PAGE_SHIFT;
return __add_pages(zone, start_pfn, nr_pages);
}
Index: pgdat10/include/linux/memory_hotplug.h
===================================================================
--- pgdat10.orig/include/linux/memory_hotplug.h 2006-03-31 14:43:32.000000000 +0900
+++ pgdat10/include/linux/memory_hotplug.h 2006-03-31 15:44:53.000000000 +0900
@@ -106,8 +106,8 @@ static inline int __remove_pages(struct
#if defined(CONFIG_MEMORY_HOTPLUG) || defined(CONFIG_ACPI_HOTPLUG_MEMORY) \
|| defined(CONFIG_ACPI_MEMORY_HOTPLUG_MODULE)
-extern int add_memory(u64 start, u64 size);
-extern int remove_memory(u64 start, u64 size);
+extern int add_memory(unsigned long start_pfn, unsigned long nr_pages);
+extern int remove_memory(unsigned long start_pfn, unsigned long nr_pages);
#endif
#endif /* __LINUX_MEMORY_HOTPLUG_H */
Index: pgdat10/drivers/acpi/acpi_memhotplug.c
===================================================================
--- pgdat10.orig/drivers/acpi/acpi_memhotplug.c 2006-03-31 14:43:25.000000000 +0900
+++ pgdat10/drivers/acpi/acpi_memhotplug.c 2006-03-31 15:43:07.000000000 +0900
@@ -245,7 +245,7 @@ static int acpi_memory_enable_device(str
continue;
}
- result = add_memory(info->start_addr, info->length);
+ result = add_memory(start_pfn, info->length >> PAGE_SHIFT);
if (result)
continue;
info->enabled = 1;
--
Yasunori Goto
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [Patch] Change interface of add/remove_memory() from paddr to pfn
2006-04-04 5:06 [Patch] Change interface of add/remove_memory() from paddr to pfn Yasunori Goto
@ 2006-04-07 14:41 ` Dave Hansen
0 siblings, 0 replies; 2+ messages in thread
From: Dave Hansen @ 2006-04-07 14:41 UTC (permalink / raw)
To: Yasunori Goto; +Cc: Andrew Morton, Linux Kernel ML
On Tue, 2006-04-04 at 14:06 +0900, Yasunori Goto wrote:
> This patch is to change interfaces of add/remove_memory()
> from physicall address to pfn.
> Current add_memory() of each architecture changes paddr to pfn,
> and __add_pages() are called by pfn after all.
> So, it is not for un-alignment memory.
> Using pfn is a bit better to avoid misunderstanding to use add_memory().
>
> In addition, this patch reduces a few lines of kernel.
> (Unfortunately, x86-64 and powerpc look using paddr for some
> reasons.)
Sorry for the horribly late response. I've been without email since
shortly before you sent this.
I don't have a horribly serious problem with this patch, but I would
prefer that it not go in.
I really wanted to have a uniform, easily understood interface for each
of the firmware drivers which will do memory hotplug. As far as I have
seen, they almost exclusively deal in physical addresses.
I just think that keeping the interfaces at u64 is a _clearer_
interface, although it does cost a few shifts in each implementation. I
would have less of a problem with something like
__add_memory_pfn_range() that sits under add_memory() where and the
architectures only implement *it*.
-- Dave
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2006-04-07 14:42 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-04-04 5:06 [Patch] Change interface of add/remove_memory() from paddr to pfn Yasunori Goto
2006-04-07 14:41 ` Dave Hansen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).