pfn_to_nid under CONFIG_SPARSEMEM and CONFIG_NUMA

The following code/comment is in <linux/mmzone.h> if SPARSEMEM and NUMA are configured.

/*
* These are _only_ used during initialisation, therefore they
* can use __initdata ... They could have names to indicate
* this restriction.
*/
#ifdef CONFIG_NUMA
#define pfn_to_nid early_pfn_to_nid
#endif

However, pfn_to_nid is certainly used in check_pte_range() mm/mempolicy.c. I wouldn't be surprised to find more non init time uses if you follow all the call chains.

On ppc64, early_pfn_to_nid now only uses __initdata. So, I would expect policy code that calls check_pte_range to cause serious problems on ppc64.

Any suggestions on how this should really be structured? I'm thinking of removing the above definition of pfn_to_nid to force each architecture to provide a (non init only) version.

Mike

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Yes! I worried about same things.
How is this?

static inline int pfn_to_nid(unsigned long pfn)

{
	return page_to_nid(pfn_to_page(pfn));
}

page_to_nid() and pfn_to_page() is well defined. Probably, this will work on all architecture.
So, just we should check this should be used after that memmap is initialized.

Bye.

Yasunori Goto 


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

But that does not work if the pfn points to something which does not have a struct page behind it (uncached memory on ia64 for instance). At the very least you would need to ensure pfn_to_page returns a struct page * before continuing blindly.

Robin

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Ok. This was a ploy to avoid lots of code churn which has bitten us. The separation here is to indicate that pfn_to_nid isn't necessarily safe until after the memory model is init'd. When the code was initially implmented we only used pfn_to_nid in init code so it wasn't an issue. What we need to do here is break this link and make sure each user is using the right version.

I'll go and put together something now.

-apw

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

I have reviewed the uses of pfn_to_nid() in 2.6.14-mm2. The only user of the non-init pfn_to_nid is the one in check_pte_range(). So we simply need to profide a non-early pfn_to_nid() implementation for SPARSEMEM. Whilst reviewing these interfaces I found two alternative dependant interfaces which are not used.

Following this message are three patches:

kvaddr_to_nid-not-used-in-common-code: removes the unused interface kvaddr_to_nid().

pfn_to_pgdat-not-used-in-common-code: removes the unused interface pfn_to_pgdat().

sparse-provide-pfn_to_nid: provides pfn_to_nid() for SPARSEMEM. Note that this implmentation assumes the pfn has been validated prior to use. The only intree user of this call does this. We perhaps need to make this part of the signature for this function.

Mike, how does this look to you?

-apw

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

kvaddr_to_nid not used in common code

kvaddr_to_nid() isn't used in common code nor in i386 code. Remove these definitions.

---
 asm-i386/mmzone.h |    5 -----
 linux/mmzone.h    |    5 -----
 2 files changed, 10 deletions(-)

diff -upN reference/include/asm-i386/mmzone.h current/include/asm-i386/mmzone.h
@@ -76,11 +76,6 @@ static inline int pfn_to_nid(unsigned lo
  * Following are macros that each numa implmentation must define.
  */
 
-/*
- * Given a kernel address, find the home node of the underlying memory.
- */
-#define kvaddr_to_nid(kaddr)	pfn_to_nid(__pa(kaddr) >> PAGE_SHIFT)
-
 #define node_start_pfn(nid)	(NODE_DATA(nid)->node_start_pfn)
 #define node_end_pfn(nid)						\
 ({									\

diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
@@ -575,11 +575,6 @@ static inline int valid_section_nr(unsig
 	return valid_section(__nr_to_section(nr));
 }
 
-/*
- * Given a kernel address, find the home node of the underlying memory.
- */
-#define kvaddr_to_nid(kaddr)	pfn_to_nid(__pa(kaddr) >> PAGE_SHIFT)
-
 static inline struct mem_section *__pfn_to_section(unsigned long pfn)
 {
 	return __nr_to_section(pfn_to_section_nr(pfn));

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

pfn_to_pgdat not used in common code

pfn_to_pgdat() isn't used in common code. Remove definition.

---
 mmzone.h |    5 -----
 1 file changed, 5 deletions(-)

diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
@@ -607,11 +607,6 @@ static inline int pfn_valid(unsigned lon
 #define pfn_to_nid		early_pfn_to_nid
 #endif
 
-#define pfn_to_pgdat(pfn)						\
-({									\
-	NODE_DATA(pfn_to_nid(pfn));					\
-})
-
 #define early_pfn_valid(pfn)	pfn_valid(pfn)
 void sparse_init(void);
 #else

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Before SPARSEMEM is initialised we cannot provide an efficient pfn_to_nid() implmentation; before initialisation is complete we use early_pfn_to_nid() to provide location information. Until recently there was no non-init user of this functionality. Provide a post init pfn_to_nid() implementation.

Note that this implmentation assumes that the pfn passed has been validated with pfn_valid(). The current single user of this function already has this check.

---
 mmzone.h |   13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
@@ -598,14 +598,11 @@ static inline int pfn_valid(unsigned lon
 	return valid_section(__nr_to_section(pfn_to_section_nr(pfn)));
 }
 
-/*
- * These are _only_ used during initialisation, therefore they
- * can use __initdata ...  They could have names to indicate
- * this restriction.
- */
-#ifdef CONFIG_NUMA
-#define pfn_to_nid		early_pfn_to_nid
-#endif
+#define pfn_to_nid(pfn)							\
+({									\
+ 	unsigned long __pfn = (pfn);                                    \
+	page_to_nid(pfn_to_page(pfn));					\
+})
 
 #define early_pfn_valid(pfn)	pfn_valid(pfn)
 void sparse_init(void);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Gack, agreed this is all a mess. I'll take care of it.

-apw

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

There are three places we define pfn_to_nid(). Two in linux/mmzone.h and one in asm/mmzone.h. These in essence represent the three memory models. The definition in linux/mmzone.h under !NEED_MULTIPLE_NODES is both the FLATMEM definition and the optimisation for single NUMA nodes; the one under SPARSEMEM is the NUMA sparsemem one; the one in asm/mmzone.h under DISCONTIGMEM is the discontigmem one. This is not in the least bit obvious, particularly the connection between the non-NUMA optimisations and the memory models.

Following in the email are two patches:

flatmem-split-out-memory-model: simplifies the selection of
pfn_to_nid() implementations. The selection is based primarily off the memory model selected. Optimisations for non-NUMA are applied where needed.

sparse-provide-pfn_to_nid: implement pfn_to_nid() for SPARSEMEM

Boot tested on for both SPARSEMEM and DISCONTIGMEM on all my test boxes. Also compile tested for FLATMEM and SPARSEMEM without NUMA. Against 2.6.15-rc2.

Next I'll review the configuration options to see if we can simplify them any.

-apw

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

pfn_to_nid is memory model specific

The pfn_to_nid() call is memory model specific. It represents the locality identifier for the memory passed. Classically this would be a NUMA node, but not a chunk of memory under DISCONTIGMEM.

The SPARSEMEM and FLATMEM memory model non-NUMA versions of pfn_to_nid() are folded together under NEED_MULTIPLE_NODES, while DISCONTIGMEM has its own optimisation. This is all very confusing.

This patch splits out each implementation of pfn_to_nid() so that we can see them and the optimisations to each.

---
 mmzone.h |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletion(-)

diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
@@ -445,7 +445,6 @@ extern struct pglist_data contig_page_da
 #define NODE_DATA(nid)		(&contig_page_data)
 #define NODE_MEM_MAP(nid)	mem_map
 #define MAX_NODES_SHIFT		1
-#define pfn_to_nid(pfn)		(0)
 
 #else /* CONFIG_NEED_MULTIPLE_NODES */
 

@@ -480,6 +479,10 @@ extern struct pglist_data contig_page_da
 #define early_pfn_to_nid(nid)  (0UL)
 #endif
 
+#ifdef CONFIG_FLATMEM
+#define pfn_to_nid(pfn)		(0)
+#endif
+
 #define pfn_to_section_nr(pfn) ((pfn) >> PFN_SECTION_SHIFT)
 #define section_nr_to_pfn(sec) ((sec) << PFN_SECTION_SHIFT)
 

@@ -604,6 +607,8 @@ static inline int pfn_valid(unsigned lon
  */
 #ifdef CONFIG_NUMA
 #define pfn_to_nid		early_pfn_to_nid
+#else
+#define pfn_to_nid(pfn)		(0)
 #endif
 
 #define early_pfn_valid(pfn)	pfn_valid(pfn)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Before SPARSEMEM is initialised we cannot provide an efficient pfn_to_nid() implmentation; before initialisation is complete we use early_pfn_to_nid() to provide location information. Until recently there was no non-init user of this functionality. Provide a post init pfn_to_nid() implementation.

Note that this implmentation assumes that the pfn passed has been validated with pfn_valid(). The current single user of this function already has this check.

---
 mmzone.h |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletion(-)

diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
@@ -606,7 +606,11 @@ static inline int pfn_valid(unsigned lon
  * this restriction.
  */
 #ifdef CONFIG_NUMA
-#define pfn_to_nid		early_pfn_to_nid
+#define pfn_to_nid(pfn)							\
+({									\
+	unsigned long __pfn_to_nid_pfn = (pfn);				\
+	page_to_nid(pfn_to_page(__pfn_to_nid_pfn));			\
+})
 #else
 #define pfn_to_nid(pfn)		(0)
 #endif

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

I like the idea of getting rid of unused interfaces as well as getting the node information from the page structs. It works for me on powerpc.

Mike

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>