Re: [PATCH 1/1] Only use ARCH_PFN_OFFSET once during boot

From: Mel Gorman
Date: Thu Jul 06 2006 - 15:54:25 EST


On Thu, 6 Jul 2006, Franck Bui-Huu wrote:

> 2006/7/6, Mel Gorman <mel@xxxxxxxxx>:
>>
>> I think my patch does the job of moving ARCH_PFN_OFFSET out of the hot
>> path in a less risky fashion. However, if you are sure that callers to
>> free_area_init() and ARCH_PFN_OFFSET are ok after your patch, I'd be happy
>> to go with it. If you're not sure, I reckon my patch would be the way to
>> go.
>>
>
> Ok I try to explain better what I have in mind. Your patch changes the
> behaviour of free_area_init_node() in the sense that it doesn't work
> as expected if its fourth parameter is different from ARCH_PFN_OFFSET,
> it even becomes boggus IMHO. And I think it's valid to use it when
> FLATMEM model is selected.

I'm missing something silly here.

Before the patch, we have the following
o Call free_area_initSOMETHING()
o Set mem_map to NODE_DATA(0)->node_mem_map
o At each call to page_to_pfn() or pfn_to_page(), offset mem_map by
ARCH_PFN_OFFSET

After the patch, we have

o Call free_area_initSOMETHING()
o Set mem_map to NODE_DATA(0)->node_mem_map - ARCH_PFN_OFFSET
o At each call to page_to_pfn() or pfn_to_page(), use mem_map without
any additional offset

I don't see how free_area_init_node() changed except for callers
using mem_map directly.

....

using mem_map directly. uh uh

Both of our patches are broken.

page_to_pfn() and pfn_to_page() both need ARCH_PFN_OFFSET to get PFNs,
that's fine. However, I forgot that another assumption of the FLATMEM memory
model is that mem_map[0] is the first valid struct page in the system. A
number of architectures walk mem_map[] directly (cris and frv are examples)
without offsetting based on this assumption.

This means that any patch that moves mem_map without altering every direct
user of the mem_map[] array will break further down the line. If nothing else,
this shows that ARCH_PFN_OFFSET could have done with a comment :) .

> <rest of mail snipped>


diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.17-mm6-clean/include/asm-generic/memory_model.h linux-2.6.17-mm6-documentarchpfn/include/asm-generic/memory_model.h
--- linux-2.6.17-mm6-clean/include/asm-generic/memory_model.h 2006-07-05 14:31:17.000000000 +0100
+++ linux-2.6.17-mm6-documentarchpfn/include/asm-generic/memory_model.h 2006-07-06 20:48:46.000000000 +0100
@@ -6,6 +6,16 @@

#if defined(CONFIG_FLATMEM)

+/*
+ * The FLATMEM memory model assumes that memory is one contiguous block of
+ * memory starting at PFN 0 with the first valid struct page at mem_map[0].
+ * mem_map is initialised to point to NODE_DATA(0)->node_mem_map.
+ *
+ * Architectures that do not start memory at PFN 0 are required to
+ * define ARCH_PFN_OFFSET so that __page_to_pfn(&mem_map[0]) == 0 and
+ * __pfn_to_page(0) == &mem_map[0]
+ *
+ */
#ifndef ARCH_PFN_OFFSET
#define ARCH_PFN_OFFSET (0UL)
#endif

--
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/