Re: 2.6.17-mm1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 23 Jun 2006, Franck Bui-Huu wrote:

Mel Gorman wrote:
On (23/06/06 17:06), Franck Bui-Huu didst pronounce:
what do you think about this use of ARCH_PFN_OFFSET ?


I believe that arches calling free_area_init_node() directly, but using
CONFIG_FLAT_NODE_MEM_MAP will have problems with this. There is no
guarantee that pgdat->node_start_pfn will have any relation to
ARCH_PFN_OFFSET.

I agree we shouldn't make this assumption.

what about this ?

-- >8 --
[PATCH] flatmem: tiny optimisation when memory do not start at 0

The FLATMEM memory model assumes that memory is in one
contigious area based at pfn 0. Some archictectures, like
ARM, use ARCH_PFN_OFFSET to relax this requirement but it
involves one more operation during page/pfn conversion.

Instead this patch modifies the address of mem_map to make
mem_map[ARCH_PFN_OFFSET] pointing at NODE_DATA(0)->node_mem_map.
The result is a slight gain on kernel code size.

It also relaxes this requirement in two more places:
	free_area_init(...)
	init_bootmem(...)

---
include/asm-generic/memory_model.h |    6 +++---
mm/bootmem.c                       |    4 ++--
mm/page_alloc.c                    |   18 ++++++++++++------
3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/include/asm-generic/memory_model.h b/include/asm-generic/memory_model.h
index 0cfb086..2faa12f 100644
--- a/include/asm-generic/memory_model.h
+++ b/include/asm-generic/memory_model.h
@@ -34,9 +34,9 @@ #else
 */
#if defined(CONFIG_FLATMEM)

-#define pfn_to_page(pfn)	(mem_map + ((pfn) - ARCH_PFN_OFFSET))
-#define page_to_pfn(page)	((unsigned long)((page) - mem_map) + \
-				 ARCH_PFN_OFFSET)
+#define pfn_to_page(pfn)	(mem_map + (pfn))
+#define page_to_pfn(page)	((unsigned long)((page) - mem_map))
+
#elif defined(CONFIG_DISCONTIGMEM)

#define pfn_to_page(pfn)			\
diff --git a/mm/bootmem.c b/mm/bootmem.c
index d213fed..fd28eed 100644
--- a/mm/bootmem.c
+++ b/mm/bootmem.c
@@ -377,11 +377,11 @@ unsigned long __init free_all_bootmem_no
	return(free_all_bootmem_core(pgdat));
}

-unsigned long __init init_bootmem (unsigned long start, unsigned long pages)
+unsigned long __init init_bootmem(unsigned long start, unsigned long pages)
{
	max_low_pfn = pages;
	min_low_pfn = start;
-	return(init_bootmem_core(NODE_DATA(0), start, 0, pages));
+	return init_bootmem_core(NODE_DATA(0), start, ARCH_PFN_OFFSET, pages);
}


Not all arches will use init_bootmem(). Arm for example uses init_bootmem_node(). ARCH_PFN_OFFSET is only meant to affect mem_map, not any node attribute. I suspect (but don't know for a fact) that if we try and change node attributes with ARCH_PFN_OFFSET, we'll shoot ourselves in the foot.

#ifndef CONFIG_HAVE_ARCH_BOOTMEM_NODE
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 253a450..3e97bc3 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2118,15 +2118,16 @@ static void __init free_area_init_core(s

static void __init alloc_node_mem_map(struct pglist_data *pgdat)
{
+#ifdef CONFIG_FLAT_NODE_MEM_MAP
+	struct page *map = pgdat->node_mem_map;
+
	/* Skip empty nodes */
	if (!pgdat->node_spanned_pages)
		return;

-#ifdef CONFIG_FLAT_NODE_MEM_MAP
	/* ia64 gets its own node_mem_map, before this, without bootmem */
-	if (!pgdat->node_mem_map) {
+	if (!map) {
		unsigned long size, start, end;
-		struct page *map;

		/*
		 * The zone's endpoints aren't required to be MAX_ORDER
@@ -2147,7 +2148,13 @@ #ifdef CONFIG_FLATMEM
	 * With no DISCONTIG, the global mem_map is just set as node 0's
	 */
	if (pgdat == NODE_DATA(0))
-		mem_map = NODE_DATA(0)->node_mem_map;
+		/*
+		 * mem_map is assumed to be based at pfn 0 such that
+		 * 'pfn = page* - mem_map' is true. Adjust map
+		 * relative to node_mem_map to maintain this
+		 * relationship.
+		 */
+		mem_map = map - ARCH_PFN_OFFSET;

This part is fine.

#endif
#endif /* CONFIG_FLAT_NODE_MEM_MAP */
}
@@ -2174,8 +2181,7 @@ #endif

void __init free_area_init(unsigned long *zones_size)
{
-	free_area_init_node(0, NODE_DATA(0), zones_size,
-			__pa(PAGE_OFFSET) >> PAGE_SHIFT, NULL);
+	free_area_init_node(0, NODE_DATA(0), zones_size, ARCH_PFN_OFFSET, NULL);
}


Same comment applies as for init_bootmem(). I can't be sure it's a good idea.

--
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux