lkml.org 
[lkml]   [2006]   [Jun]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: 2.6.17-mm1
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 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/

\
 
 \ /
  Last update: 2006-06-23 18:41    [W:0.146 / U:0.160 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site