pfn_to_nid under CONFIG_SPARSEMEM and CONFIG_NUMA

pfn_to_nid under CONFIG_SPARSEMEM and CONFIG_NUMA

[Mike Kravetz]

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

Re: pfn_to_nid under CONFIG_SPARSEMEM and CONFIG_NUMA

[Yasunori Goto]

On Tue, 15 Nov 2005 14:10:03 -0800
Mike Kravetz <kravetz@us.ibm.com> wrote:

> 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.

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 

Re: pfn_to_nid under CONFIG_SPARSEMEM and CONFIG_NUMA

[Robin Holt]

On Wed, Nov 16, 2005 at 12:14:18PM +0900, Yasunori Goto wrote:
> static inline int pfn_to_nid(unsigned long pfn)
> {
> 	return page_to_nid(pfn_to_page(pfn));

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.

> 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.

Robin

Re: pfn_to_nid under CONFIG_SPARSEMEM and CONFIG_NUMA

[Andy Whitcroft]

Mike Kravetz wrote:
> 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

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

[PATCH 0/3] SPARSEMEM: pfn_to_nid implementation

[Andy Whitcroft]

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

[PATCH 1/3] kvaddr_to_nid not used in common code

[Andy Whitcroft]

kvaddr_to_nid not used in common code

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

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 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
--- 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
--- 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));

[PATCH 2/3] pfn_to_pgdat not used in common code

[Andy Whitcroft]

pfn_to_pgdat not used in common code

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

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 mmzone.h |    5 -----
 1 file changed, 5 deletions(-)
diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
--- 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

[PATCH 3/3] sparse provide pfn_to_nid

[Andy Whitcroft]

sparsemem: provide pfn_to_nid

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.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 mmzone.h |   13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)
diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
--- 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);

Re: [PATCH 0/3] SPARSEMEM: pfn_to_nid implementation

[Mike Kravetz]

On Wed, Nov 16, 2005 at 10:59:53PM +0000, Andy Whitcroft wrote:
> 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?

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

Re: [PATCH 3/3] sparse provide pfn_to_nid

[Andrew Morton]

Andy Whitcroft <apw@shadowen.org> wrote:
>
> sparsemem: provide pfn_to_nid
> 
> 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.
> 
> Signed-off-by: Andy Whitcroft <apw@shadowen.org>
> ---
>  mmzone.h |   13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
> --- 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);

This causes a problem because we already have a definition of pfn_to_nid()
in include/linux/mmzone.h.  Effectively:

#ifndef CONFIG_NEED_MULTIPLE_NODES

#define pfn_to_nid(pfn)		(0)

#else /* CONFIG_NEED_MULTIPLE_NODES */

#include <asm/mmzone.h>

#endif /* !CONFIG_NEED_MULTIPLE_NODES */


If someone does !CONFIG_NEED_MULTIPLE_NODES, pfn_to_nid() gets a duplicate
definition (from inspection).

If someone does CONFIG_NEED_MULTIPLE_NODES && CONFIG_DISCONTIGMEM we get
duplicate definitions of pfn_to_nid(): one in include/linux/mmzone.h and
one in include/asm/mmzone.h.

It's a big mess - can someone please fix it up?  The maze of config options
is just over the top.

Meanwhile, I'll drop this patch.

Re: [PATCH 3/3] sparse provide pfn_to_nid

[Andy Whitcroft]

Andrew Morton wrote:

> It's a big mess - can someone please fix it up?  The maze of config options
> is just over the top.
> 
> Meanwhile, I'll drop this patch.

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

-apw

[PATCH 0/2] SPARSEMEM: pfn_to_nid implementation v2

[Andy Whitcroft]

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

[PATCH 1/2] flatmem split out memory model

[Andy Whitcroft]

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.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 mmzone.h |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletion(-)
diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
--- 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)

[PATCH 2/2] sparse provide pfn_to_nid

[Andy Whitcroft]

sparsemem: provide pfn_to_nid

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.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 mmzone.h |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletion(-)
diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
--- 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