[patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

I'm working on breaking out the struct page definition into its
own file.  There seem to be a ton of header dependencies that
crop up around struct page, and I'd like to start getting rid
of thise.

In order to reduce those dependencies that a structpage.h has on 
other include files, make the #define WANT_PAGE_VIRTUAL a Kconfig
option.  This keeps the number of things that structpage.h will
include to a bare minimum.

I don't have a MIPS or m68k system to test this on, but it's
pretty simple.

Signed-off-by: Dave Hansen <haveblue@us.ibm.com>
---

 apw2-dave/arch/m68k/Kconfig       |    5 +++++
 apw2-dave/arch/mips/Kconfig       |    5 +++++
 apw2-dave/include/asm-frv/page.h  |    4 ----
 apw2-dave/include/asm-m68k/page.h |    1 -
 apw2-dave/include/asm-mips/page.h |    4 ----
 apw2-dave/include/linux/mm.h      |   12 ++++++------
 apw2-dave/mm/highmem.c            |    2 +-
 apw2-dave/mm/page_alloc.c         |    2 +-
 arch/frv/Kconfig                  |    0 
 9 files changed, 18 insertions(+), 17 deletions(-)

diff -puN include/asm-mips/page.h~000-CONFIG_WANT_PAGE_VIRTUAL include/asm-mips/page.h
--- apw2/include/asm-mips/page.h~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/include/asm-mips/page.h	2004-12-16 13:50:53.000000000 -0800
@@ -144,10 +144,6 @@ static __inline__ int get_order(unsigned
 
 #endif /* defined (__KERNEL__) */
 
-#ifdef CONFIG_LIMITED_DMA
-#define WANT_PAGE_VIRTUAL
-#endif
-
 #define devmem_is_allowed(x) 1
 
 #endif /* _ASM_PAGE_H */
diff -puN include/asm-m68k/page.h~000-CONFIG_WANT_PAGE_VIRTUAL include/asm-m68k/page.h
--- apw2/include/asm-m68k/page.h~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/include/asm-m68k/page.h	2004-12-16 13:50:53.000000000 -0800
@@ -131,7 +131,6 @@ static inline int get_order(unsigned lon
 
 #ifndef CONFIG_SUN3
 
-#define WANT_PAGE_VIRTUAL
 #ifdef CONFIG_SINGLE_MEMORY_CHUNK
 extern unsigned long m68k_memoffset;
 
diff -puN include/linux/mm.h~000-CONFIG_WANT_PAGE_VIRTUAL include/linux/mm.h
--- apw2/include/linux/mm.h~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/include/linux/mm.h	2004-12-16 13:50:53.000000000 -0800
@@ -262,12 +262,12 @@ struct page {
 	 * Note that this field could be 16 bits on x86 ... ;)
 	 *
 	 * Architectures with slow multiplication can define
-	 * WANT_PAGE_VIRTUAL in asm/page.h
+	 * WANT_PAGE_VIRTUAL in their architecture's Kconfig
 	 */
-#if defined(WANT_PAGE_VIRTUAL)
+#if defined(CONFIG_WANT_PAGE_VIRTUAL)
 	void *virtual;			/* Kernel virtual address (NULL if
 					   not kmapped, ie. highmem) */
-#endif /* WANT_PAGE_VIRTUAL */
+#endif /* CONFIG_WANT_PAGE_VIRTUAL */
 };
 
 /*
@@ -445,11 +445,11 @@ static inline void *lowmem_page_address(
 	return __va(page_to_pfn(page) << PAGE_SHIFT);
 }
 
-#if defined(CONFIG_HIGHMEM) && !defined(WANT_PAGE_VIRTUAL)
+#if defined(CONFIG_HIGHMEM) && !defined(CONFIG_WANT_PAGE_VIRTUAL)
 #define HASHED_PAGE_VIRTUAL
 #endif
 
-#if defined(WANT_PAGE_VIRTUAL)
+#if defined(CONFIG_WANT_PAGE_VIRTUAL)
 #define page_address(page) ((page)->virtual)
 #define set_page_address(page, address)			\
 	do {						\
@@ -464,7 +464,7 @@ void set_page_address(struct page *page,
 void page_address_init(void);
 #endif
 
-#if !defined(HASHED_PAGE_VIRTUAL) && !defined(WANT_PAGE_VIRTUAL)
+#if !defined(HASHED_PAGE_VIRTUAL) && !defined(CONFIG_WANT_PAGE_VIRTUAL)
 #define page_address(page) lowmem_page_address(page)
 #define set_page_address(page, address)  do { } while(0)
 #define page_address_init()  do { } while(0)
diff -puN include/asm-frv/page.h~000-CONFIG_WANT_PAGE_VIRTUAL include/asm-frv/page.h
--- apw2/include/asm-frv/page.h~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/include/asm-frv/page.h	2004-12-16 13:50:53.000000000 -0800
@@ -97,8 +97,4 @@ extern unsigned long max_pfn;
 
 #endif /* __KERNEL__ */
 
-#ifdef CONFIG_CONTIGUOUS_PAGE_ALLOC
-#define WANT_PAGE_VIRTUAL	1
-#endif
-
 #endif /* _ASM_PAGE_H */
diff -puN mm/page_alloc.c~000-CONFIG_WANT_PAGE_VIRTUAL mm/page_alloc.c
--- apw2/mm/page_alloc.c~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/mm/page_alloc.c	2004-12-16 13:50:53.000000000 -0800
@@ -1597,7 +1597,7 @@ void __init memmap_init_zone(unsigned lo
 		reset_page_mapcount(page);
 		SetPageReserved(page);
 		INIT_LIST_HEAD(&page->lru);
-#ifdef WANT_PAGE_VIRTUAL
+#ifdef CONFIG_WANT_PAGE_VIRTUAL
 		/* The shift won't overflow because ZONE_NORMAL is below 4G. */
 		if (!is_highmem_idx(zone))
 			set_page_address(page, __va(start_pfn << PAGE_SHIFT));
diff -puN mm/highmem.c~000-CONFIG_WANT_PAGE_VIRTUAL mm/highmem.c
--- apw2/mm/highmem.c~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/mm/highmem.c	2004-12-16 13:50:53.000000000 -0800
@@ -602,4 +602,4 @@ void __init page_address_init(void)
 	spin_lock_init(&pool_lock);
 }
 
-#endif	/* defined(CONFIG_HIGHMEM) && !defined(WANT_PAGE_VIRTUAL) */
+#endif	/* defined(CONFIG_HIGHMEM) && !defined(CONFIG_WANT_PAGE_VIRTUAL) */
diff -puN arch/mips/Kconfig~000-CONFIG_WANT_PAGE_VIRTUAL arch/mips/Kconfig
--- apw2/arch/mips/Kconfig~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/arch/mips/Kconfig	2004-12-16 13:50:53.000000000 -0800
@@ -896,6 +896,11 @@ config LIMITED_DMA
 	bool
 	select HIGHMEM
 
+config WANT_PAGE_VIRTUAL
+	bool
+	depends on LIMITED_DMA
+	default y
+
 config MIPS_BONITO64
 	bool
 	depends on MIPS_ATLAS || MIPS_MALTA
diff -puN arch/m68k/Kconfig~000-CONFIG_WANT_PAGE_VIRTUAL arch/m68k/Kconfig
--- apw2/arch/m68k/Kconfig~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/arch/m68k/Kconfig	2004-12-16 13:50:53.000000000 -0800
@@ -230,6 +230,11 @@ config Q40
 	  Q60. Select your CPU below.  For 68LC060 don't forget to enable FPU
 	  emulation.
 
+config WANT_PAGE_VIRTUAL
+	bool
+	depends on !SUN3
+	default y
+
 comment "Processor type"
 
 config M68020
diff -puN arch/frv/Kconfig~000-CONFIG_WANT_PAGE_VIRTUAL arch/frv/Kconfig
_

[patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

I'm working on breaking out the struct page definition into its
own file.  There seem to be a ton of header dependencies that
crop up around struct page, and I'd like to start getting rid
of thise.

In order to reduce those dependencies that a structpage.h has on 
other include files, make the #define WANT_PAGE_VIRTUAL a Kconfig
option.  This keeps the number of things that structpage.h will
include to a bare minimum.

I don't have a MIPS or m68k system to test this on, but it's
pretty simple.

Signed-off-by: Dave Hansen <haveblue@us.ibm.com>
---

 apw2-dave/arch/m68k/Kconfig       |    5 +++++
 apw2-dave/arch/mips/Kconfig       |    5 +++++
 apw2-dave/include/asm-frv/page.h  |    4 ----
 apw2-dave/include/asm-m68k/page.h |    1 -
 apw2-dave/include/asm-mips/page.h |    4 ----
 apw2-dave/include/linux/mm.h      |   12 ++++++------
 apw2-dave/mm/highmem.c            |    2 +-
 apw2-dave/mm/page_alloc.c         |    2 +-
 arch/frv/Kconfig                  |    0 
 9 files changed, 18 insertions(+), 17 deletions(-)

diff -puN include/asm-mips/page.h~000-CONFIG_WANT_PAGE_VIRTUAL include/asm-mips/page.h
--- apw2/include/asm-mips/page.h~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/include/asm-mips/page.h	2004-12-16 13:50:53.000000000 -0800
@@ -144,10 +144,6 @@ static __inline__ int get_order(unsigned
 
 #endif /* defined (__KERNEL__) */
 
-#ifdef CONFIG_LIMITED_DMA
-#define WANT_PAGE_VIRTUAL
-#endif
-
 #define devmem_is_allowed(x) 1
 
 #endif /* _ASM_PAGE_H */
diff -puN include/asm-m68k/page.h~000-CONFIG_WANT_PAGE_VIRTUAL include/asm-m68k/page.h
--- apw2/include/asm-m68k/page.h~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/include/asm-m68k/page.h	2004-12-16 13:50:53.000000000 -0800
@@ -131,7 +131,6 @@ static inline int get_order(unsigned lon
 
 #ifndef CONFIG_SUN3
 
-#define WANT_PAGE_VIRTUAL
 #ifdef CONFIG_SINGLE_MEMORY_CHUNK
 extern unsigned long m68k_memoffset;
 
diff -puN include/linux/mm.h~000-CONFIG_WANT_PAGE_VIRTUAL include/linux/mm.h
--- apw2/include/linux/mm.h~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/include/linux/mm.h	2004-12-16 13:50:53.000000000 -0800
@@ -262,12 +262,12 @@ struct page {
 	 * Note that this field could be 16 bits on x86 ... ;)
 	 *
 	 * Architectures with slow multiplication can define
-	 * WANT_PAGE_VIRTUAL in asm/page.h
+	 * WANT_PAGE_VIRTUAL in their architecture's Kconfig
 	 */
-#if defined(WANT_PAGE_VIRTUAL)
+#if defined(CONFIG_WANT_PAGE_VIRTUAL)
 	void *virtual;			/* Kernel virtual address (NULL if
 					   not kmapped, ie. highmem) */
-#endif /* WANT_PAGE_VIRTUAL */
+#endif /* CONFIG_WANT_PAGE_VIRTUAL */
 };
 
 /*
@@ -445,11 +445,11 @@ static inline void *lowmem_page_address(
 	return __va(page_to_pfn(page) << PAGE_SHIFT);
 }
 
-#if defined(CONFIG_HIGHMEM) && !defined(WANT_PAGE_VIRTUAL)
+#if defined(CONFIG_HIGHMEM) && !defined(CONFIG_WANT_PAGE_VIRTUAL)
 #define HASHED_PAGE_VIRTUAL
 #endif
 
-#if defined(WANT_PAGE_VIRTUAL)
+#if defined(CONFIG_WANT_PAGE_VIRTUAL)
 #define page_address(page) ((page)->virtual)
 #define set_page_address(page, address)			\
 	do {						\
@@ -464,7 +464,7 @@ void set_page_address(struct page *page,
 void page_address_init(void);
 #endif
 
-#if !defined(HASHED_PAGE_VIRTUAL) && !defined(WANT_PAGE_VIRTUAL)
+#if !defined(HASHED_PAGE_VIRTUAL) && !defined(CONFIG_WANT_PAGE_VIRTUAL)
 #define page_address(page) lowmem_page_address(page)
 #define set_page_address(page, address)  do { } while(0)
 #define page_address_init()  do { } while(0)
diff -puN include/asm-frv/page.h~000-CONFIG_WANT_PAGE_VIRTUAL include/asm-frv/page.h
--- apw2/include/asm-frv/page.h~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/include/asm-frv/page.h	2004-12-16 13:50:53.000000000 -0800
@@ -97,8 +97,4 @@ extern unsigned long max_pfn;
 
 #endif /* __KERNEL__ */
 
-#ifdef CONFIG_CONTIGUOUS_PAGE_ALLOC
-#define WANT_PAGE_VIRTUAL	1
-#endif
-
 #endif /* _ASM_PAGE_H */
diff -puN mm/page_alloc.c~000-CONFIG_WANT_PAGE_VIRTUAL mm/page_alloc.c
--- apw2/mm/page_alloc.c~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/mm/page_alloc.c	2004-12-16 13:50:53.000000000 -0800
@@ -1597,7 +1597,7 @@ void __init memmap_init_zone(unsigned lo
 		reset_page_mapcount(page);
 		SetPageReserved(page);
 		INIT_LIST_HEAD(&page->lru);
-#ifdef WANT_PAGE_VIRTUAL
+#ifdef CONFIG_WANT_PAGE_VIRTUAL
 		/* The shift won't overflow because ZONE_NORMAL is below 4G. */
 		if (!is_highmem_idx(zone))
 			set_page_address(page, __va(start_pfn << PAGE_SHIFT));
diff -puN mm/highmem.c~000-CONFIG_WANT_PAGE_VIRTUAL mm/highmem.c
--- apw2/mm/highmem.c~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/mm/highmem.c	2004-12-16 13:50:53.000000000 -0800
@@ -602,4 +602,4 @@ void __init page_address_init(void)
 	spin_lock_init(&pool_lock);
 }
 
-#endif	/* defined(CONFIG_HIGHMEM) && !defined(WANT_PAGE_VIRTUAL) */
+#endif	/* defined(CONFIG_HIGHMEM) && !defined(CONFIG_WANT_PAGE_VIRTUAL) */
diff -puN arch/mips/Kconfig~000-CONFIG_WANT_PAGE_VIRTUAL arch/mips/Kconfig
--- apw2/arch/mips/Kconfig~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/arch/mips/Kconfig	2004-12-16 13:50:53.000000000 -0800
@@ -896,6 +896,11 @@ config LIMITED_DMA
 	bool
 	select HIGHMEM
 
+config WANT_PAGE_VIRTUAL
+	bool
+	depends on LIMITED_DMA
+	default y
+
 config MIPS_BONITO64
 	bool
 	depends on MIPS_ATLAS || MIPS_MALTA
diff -puN arch/m68k/Kconfig~000-CONFIG_WANT_PAGE_VIRTUAL arch/m68k/Kconfig
--- apw2/arch/m68k/Kconfig~000-CONFIG_WANT_PAGE_VIRTUAL	2004-12-16 13:50:53.000000000 -0800
+++ apw2-dave/arch/m68k/Kconfig	2004-12-16 13:50:53.000000000 -0800
@@ -230,6 +230,11 @@ config Q40
 	  Q60. Select your CPU below.  For 68LC060 don't forget to enable FPU
 	  emulation.
 
+config WANT_PAGE_VIRTUAL
+	bool
+	depends on !SUN3
+	default y
+
 comment "Processor type"
 
 config M68020
diff -puN arch/frv/Kconfig~000-CONFIG_WANT_PAGE_VIRTUAL arch/frv/Kconfig
_
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Roman Zippel]

Hi,

On Thu, 16 Dec 2004, Dave Hansen wrote:

> I'm working on breaking out the struct page definition into its
> own file.  There seem to be a ton of header dependencies that
> crop up around struct page, and I'd like to start getting rid
> of thise.

Why do you want to move struct page into a separate file?

bye, Roman

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Roman Zippel]

Hi,

On Thu, 16 Dec 2004, Dave Hansen wrote:

> I'm working on breaking out the struct page definition into its
> own file.  There seem to be a ton of header dependencies that
> crop up around struct page, and I'd like to start getting rid
> of thise.

Why do you want to move struct page into a separate file?

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

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

On Thu, 2004-12-16 at 16:36, Roman Zippel wrote:
> On Thu, 16 Dec 2004, Dave Hansen wrote:
> > I'm working on breaking out the struct page definition into its
> > own file.  There seem to be a ton of header dependencies that
> > crop up around struct page, and I'd like to start getting rid
> > of thise.
> 
> Why do you want to move struct page into a separate file?

Circular header dependencies suck :)

I posted another patch, shortly after the one that I cc'd you on, with
the following description.  Cristoph suggested just making it
linux/page.h and maybe combining it with page-flags.h, but otherwise the
idea remains the same.  

> There are currently 24 places in the tree where struct page is
> predeclared.  However, a good number of these places also have to
> do some kind of arithmetic on it, and end up using macros because
> static inlines wouldn't have the type fully defined at
> compile-time.
> 
> But, in reality, struct page has very few dependencies on outside
> macros or functions, and doesn't really need to be a part of the
> header include mess which surrounds many of the VM headers.
> 
> So, put 'struct page' into structpage.h, along with a nasty comment
> telling everyone to keep their grubby mitts out of the file.
> 
> Now, we can use static inlines for almost any 'struct page'
> operations with no problems, and get rid of many of the
> predeclarations.


-- Dave

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

On Thu, 2004-12-16 at 16:36, Roman Zippel wrote:
> On Thu, 16 Dec 2004, Dave Hansen wrote:
> > I'm working on breaking out the struct page definition into its
> > own file.  There seem to be a ton of header dependencies that
> > crop up around struct page, and I'd like to start getting rid
> > of thise.
> 
> Why do you want to move struct page into a separate file?

Circular header dependencies suck :)

I posted another patch, shortly after the one that I cc'd you on, with
the following description.  Cristoph suggested just making it
linux/page.h and maybe combining it with page-flags.h, but otherwise the
idea remains the same.  

> There are currently 24 places in the tree where struct page is
> predeclared.  However, a good number of these places also have to
> do some kind of arithmetic on it, and end up using macros because
> static inlines wouldn't have the type fully defined at
> compile-time.
> 
> But, in reality, struct page has very few dependencies on outside
> macros or functions, and doesn't really need to be a part of the
> header include mess which surrounds many of the VM headers.
> 
> So, put 'struct page' into structpage.h, along with a nasty comment
> telling everyone to keep their grubby mitts out of the file.
> 
> Now, we can use static inlines for almost any 'struct page'
> operations with no problems, and get rid of many of the
> predeclarations.


-- Dave

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

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Roman Zippel]

Hi,

On Thu, 16 Dec 2004, Dave Hansen wrote:

> > Why do you want to move struct page into a separate file?
> 
> Circular header dependencies suck :)

Could you explain a bit more, what exactly the problem is?

bye, Roman

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Roman Zippel]

Hi,

On Thu, 16 Dec 2004, Dave Hansen wrote:

> > Why do you want to move struct page into a separate file?
> 
> Circular header dependencies suck :)

Could you explain a bit more, what exactly the problem is?

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

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

On Thu, 2004-12-16 at 16:51, Roman Zippel wrote:
> Could you explain a bit more, what exactly the problem is?

The symptom is that you'll add some new function to a header, say
mmzone.h.  You get some kind of compile error that a structure that you
need is not fully defined (usually because it is predeclared "struct
foo;").  This happens when you do either a structure dereference on a
pointer, or do some other kind of pointer arithmetic on it outside of a
macro.

Your first instinct is usually to go find where that structure is
declared and make sure that the header in which it's declared is
included in the header in which you're working.  Doing this gets rid of
your immediate problem, but usually causes another.  This is because
something in the other header needs something that's defined in the
header that *you're* working on, and needs the reverse order of
includes.

So, what always happens now is that someone just makes a #define.  Since
these aren't evaluated until they're actually used in the code, they get
put after all of the headers naturally and everything compiles.  

Here's a prime example of what you get from include/asm-i386/mmzone.h:

#define pfn_to_page(pfn)                                                \
({                                                                      \
        unsigned long __pfn = pfn;                                      \
        int __node  = pfn_to_nid(__pfn);                                \
        &node_mem_map(__node)[node_localnr(__pfn,__node)];              \
})

Not that this is horrible, but it sure does get annoying, and tends to
be more type-unsafe than your standard static inline.

What I want to do is make sure that, when you include a header, you get
what you need, and only what you need.  Doing this pervasively presents
the possibility of actually understanding the header dependencies, and
being able to avoid almost all of the hackery that goes along with
avoiding them when you *don't* fully understand.

Remember, *structures* can never have truly circular dependencies. I'm
just trying to start expressing that in the header layout.  

-- Dave

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

On Thu, 2004-12-16 at 16:51, Roman Zippel wrote:
> Could you explain a bit more, what exactly the problem is?

The symptom is that you'll add some new function to a header, say
mmzone.h.  You get some kind of compile error that a structure that you
need is not fully defined (usually because it is predeclared "struct
foo;").  This happens when you do either a structure dereference on a
pointer, or do some other kind of pointer arithmetic on it outside of a
macro.

Your first instinct is usually to go find where that structure is
declared and make sure that the header in which it's declared is
included in the header in which you're working.  Doing this gets rid of
your immediate problem, but usually causes another.  This is because
something in the other header needs something that's defined in the
header that *you're* working on, and needs the reverse order of
includes.

So, what always happens now is that someone just makes a #define.  Since
these aren't evaluated until they're actually used in the code, they get
put after all of the headers naturally and everything compiles.  

Here's a prime example of what you get from include/asm-i386/mmzone.h:

#define pfn_to_page(pfn)                                                \
({                                                                      \
        unsigned long __pfn = pfn;                                      \
        int __node  = pfn_to_nid(__pfn);                                \
        &node_mem_map(__node)[node_localnr(__pfn,__node)];              \
})

Not that this is horrible, but it sure does get annoying, and tends to
be more type-unsafe than your standard static inline.

What I want to do is make sure that, when you include a header, you get
what you need, and only what you need.  Doing this pervasively presents
the possibility of actually understanding the header dependencies, and
being able to avoid almost all of the hackery that goes along with
avoiding them when you *don't* fully understand.

Remember, *structures* can never have truly circular dependencies. I'm
just trying to start expressing that in the header layout.  

-- Dave

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

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Roman Zippel]

Hi,

On Thu, 16 Dec 2004, Dave Hansen wrote:

> > Could you explain a bit more, what exactly the problem is?
> 
> The symptom is that you'll add some new function to a header, say
> mmzone.h.  You get some kind of compile error that a structure that you
> need is not fully defined (usually because it is predeclared "struct
> foo;").  This happens when you do either a structure dereference on a
> pointer, or do some other kind of pointer arithmetic on it outside of a
> macro.

I know this problem and I hoped you would provide a complete header 
dependency example. Anyway, I'm not against fixing this, I think that 
you're starting somewhere in the middle.
We have the same problem with core data types, like atomic_t, spinlocks, 
semaphores... Preempt made this problem worse and was "fixed" by 
separating some stuff into thread_info.
I'd prefer to fix this problem at the core first, some time ago I posted a 
few patches to separate out core data structures from the functions. This 
allows further cleanups, I just did a quick check with linux/mm.h and 
easily reduced the dependencies by half. I need to update the patches 
soon, so there are ready once 2.6.10 is out.
If you change the header dependencies, there is a big risk you break some 
architecture, the current system is rather fragile. Moving a random 
structure into a new header file doesn't always fix the problem, this 
structure might still need some other definitions and so can pull in 
different headers on every arch. Splitting a header is easy, getting the 
whole thing working again in the end is the hard part.

bye, Roman

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Roman Zippel]

Hi,

On Thu, 16 Dec 2004, Dave Hansen wrote:

> > Could you explain a bit more, what exactly the problem is?
> 
> The symptom is that you'll add some new function to a header, say
> mmzone.h.  You get some kind of compile error that a structure that you
> need is not fully defined (usually because it is predeclared "struct
> foo;").  This happens when you do either a structure dereference on a
> pointer, or do some other kind of pointer arithmetic on it outside of a
> macro.

I know this problem and I hoped you would provide a complete header 
dependency example. Anyway, I'm not against fixing this, I think that 
you're starting somewhere in the middle.
We have the same problem with core data types, like atomic_t, spinlocks, 
semaphores... Preempt made this problem worse and was "fixed" by 
separating some stuff into thread_info.
I'd prefer to fix this problem at the core first, some time ago I posted a 
few patches to separate out core data structures from the functions. This 
allows further cleanups, I just did a quick check with linux/mm.h and 
easily reduced the dependencies by half. I need to update the patches 
soon, so there are ready once 2.6.10 is out.
If you change the header dependencies, there is a big risk you break some 
architecture, the current system is rather fragile. Moving a random 
structure into a new header file doesn't always fix the problem, this 
structure might still need some other definitions and so can pull in 
different headers on every arch. Splitting a header is easy, getting the 
whole thing working again in the end is the hard part.

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

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

On Thu, 2004-12-16 at 18:50, Roman Zippel wrote:
> On Thu, 16 Dec 2004, Dave Hansen wrote:
> > > Could you explain a bit more, what exactly the problem is?
> > The symptom is that you'll add some new function to a header, say
> > mmzone.h.  You get some kind of compile error that a structure that you
> > need is not fully defined (usually because it is predeclared "struct
> > foo;").  This happens when you do either a structure dereference on a
> > pointer, or do some other kind of pointer arithmetic on it outside of a
> > macro.
> 
> I know this problem and I hoped you would provide a complete header 
> dependency example.

Sorry I didn't provide this.  My recent effort started to clean up some
ugliness in some current patches that worked around this actually
happening a few months ago.  The original example didn't survive :)

> Anyway, I'm not against fixing this, I think that 
> you're starting somewhere in the middle.

I don't disagree.  But, I do think that getting the core
(include/linux/*) files to stop depending on architecture-specific ones
is a step.  Most other cleanups aren't as obviously correct, or have
such obviously limited impact.  So, I'm starting by patches that have
the least impact rather than actual bottom or top of the problem.

> I'd prefer to fix this problem at the core first, some time ago I posted a 
> few patches to separate out core data structures from the functions.

Especially with the vm headers, I have also seen quite a few
dependencies on simple macro variables.  I'd also like to see some
constant macro only files. 

> This 
> allows further cleanups, I just did a quick check with linux/mm.h and 
> easily reduced the dependencies by half. I need to update the patches 
> soon, so there are ready once 2.6.10 is out.

I'll eagerly await your post :)

> If you change the header dependencies, there is a big risk you break some 
> architecture, the current system is rather fragile. Moving a random 
> structure into a new header file doesn't always fix the problem, this 
> structure might still need some other definitions and so can pull in 
> different headers on every arch. Splitting a header is easy, getting the 
> whole thing working again in the end is the hard part.

The nice part about detecting and fixing these kinds of changes is that
they're not very subtle.  I'd much rather fix an include problem than an
SMP race any day :)

I certainly agree that their correct place for anything pervasive is in
the beginning of a development series.

-- Dave

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

On Thu, 2004-12-16 at 18:50, Roman Zippel wrote:
> On Thu, 16 Dec 2004, Dave Hansen wrote:
> > > Could you explain a bit more, what exactly the problem is?
> > The symptom is that you'll add some new function to a header, say
> > mmzone.h.  You get some kind of compile error that a structure that you
> > need is not fully defined (usually because it is predeclared "struct
> > foo;").  This happens when you do either a structure dereference on a
> > pointer, or do some other kind of pointer arithmetic on it outside of a
> > macro.
> 
> I know this problem and I hoped you would provide a complete header 
> dependency example.

Sorry I didn't provide this.  My recent effort started to clean up some
ugliness in some current patches that worked around this actually
happening a few months ago.  The original example didn't survive :)

> Anyway, I'm not against fixing this, I think that 
> you're starting somewhere in the middle.

I don't disagree.  But, I do think that getting the core
(include/linux/*) files to stop depending on architecture-specific ones
is a step.  Most other cleanups aren't as obviously correct, or have
such obviously limited impact.  So, I'm starting by patches that have
the least impact rather than actual bottom or top of the problem.

> I'd prefer to fix this problem at the core first, some time ago I posted a 
> few patches to separate out core data structures from the functions.

Especially with the vm headers, I have also seen quite a few
dependencies on simple macro variables.  I'd also like to see some
constant macro only files. 

> This 
> allows further cleanups, I just did a quick check with linux/mm.h and 
> easily reduced the dependencies by half. I need to update the patches 
> soon, so there are ready once 2.6.10 is out.

I'll eagerly await your post :)

> If you change the header dependencies, there is a big risk you break some 
> architecture, the current system is rather fragile. Moving a random 
> structure into a new header file doesn't always fix the problem, this 
> structure might still need some other definitions and so can pull in 
> different headers on every arch. Splitting a header is easy, getting the 
> whole thing working again in the end is the hard part.

The nice part about detecting and fixing these kinds of changes is that
they're not very subtle.  I'd much rather fix an include problem than an
SMP race any day :)

I certainly agree that their correct place for anything pervasive is in
the beginning of a development series.

-- Dave

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

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Roman Zippel]

Hi,

On Thu, 16 Dec 2004, Dave Hansen wrote:

> Sorry I didn't provide this.  My recent effort started to clean up some
> ugliness in some current patches that worked around this actually
> happening a few months ago.  The original example didn't survive :)

There are two points, which did originally catch my attention.
1) Where/why do you want to remove the dependency on asm/page.h?
2) Does every structure really needs its own header?

If you want to do such a cleanup, it would be helpful to have some more 
information about where you want to go, otherwise you may create a more 
twisted maze of header files. The big question here is what further 
cleanups are possible in this area?
What basically needs to be done is to separate the definitions from its 
users, that doesn't mean every definition needs its own header file. Why 
not create a single header file which collects a number of mm related 
definitions? E.g. struct vm_area_struct is also used by a number of header 
files, although its main users have already been separated into 
asm/tlbflush.h and so created even more headers.
So to allow further header cleanup, we should look what other definitions 
can be pulled out of mm.h and related headers. mm.h should probably stay a 
high level header, but I'd also like to see a cleanup of asm/page.h. The 
page table definitions in there should be available to every mm related 
header.
I had to look closer at this, but I did this for struct task_struct and 
making it available for lowlevel header files. I rediffed the patches and 
put them at http://www.xs4all.nl/~zippel/task_patches/

bye, Roman

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Roman Zippel]

Hi,

On Thu, 16 Dec 2004, Dave Hansen wrote:

> Sorry I didn't provide this.  My recent effort started to clean up some
> ugliness in some current patches that worked around this actually
> happening a few months ago.  The original example didn't survive :)

There are two points, which did originally catch my attention.
1) Where/why do you want to remove the dependency on asm/page.h?
2) Does every structure really needs its own header?

If you want to do such a cleanup, it would be helpful to have some more 
information about where you want to go, otherwise you may create a more 
twisted maze of header files. The big question here is what further 
cleanups are possible in this area?
What basically needs to be done is to separate the definitions from its 
users, that doesn't mean every definition needs its own header file. Why 
not create a single header file which collects a number of mm related 
definitions? E.g. struct vm_area_struct is also used by a number of header 
files, although its main users have already been separated into 
asm/tlbflush.h and so created even more headers.
So to allow further header cleanup, we should look what other definitions 
can be pulled out of mm.h and related headers. mm.h should probably stay a 
high level header, but I'd also like to see a cleanup of asm/page.h. The 
page table definitions in there should be available to every mm related 
header.
I had to look closer at this, but I did this for struct task_struct and 
making it available for lowlevel header files. I rediffed the patches and 
put them at http://www.xs4all.nl/~zippel/task_patches/

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

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

On Fri, 2004-12-17 at 05:26, Roman Zippel wrote:
> On Thu, 16 Dec 2004, Dave Hansen wrote:
> > Sorry I didn't provide this.  My recent effort started to clean up some
> > ugliness in some current patches that worked around this actually
> > happening a few months ago.  The original example didn't survive :)
> 
> There are two points, which did originally catch my attention.
> 1) Where/why do you want to remove the dependency on asm/page.h?

The basic reason is that I think it's important to know exactly what
you're getting into when you include a header, especially when you're
authoring new headers.  

That said, it doesn't appear that asm/page.h is a header that causes any
real problems.  In any case, when there are variables that need to be
created based on Kconfig parameters, I believe the "correct" thing to do
is make a Kconfig variable.  But, that is a separate conversation. :)

> 2) Does every structure really needs its own header?

No.  But, I do think that most of the very basic VM structures do, as it
stands.  That's limited to struct page, zone, and pgdat as I see it
now.  

> If you want to do such a cleanup, it would be helpful to have some more 
> information about where you want to go, otherwise you may create a more 
> twisted maze of header files.

One thing I'd like to point out is that I don't necessarily think that
the normal foo.c user should see any change.  They can still include
mmzone.h for the same stuff, but *inside* the headers, there's some more
organization.  Here's an example from a new header that I'm working on
after shifting the headers around.

#include <asm/page-consts.h>      /* for PAGE_SHIFT */
#include <asm/addr-convert.h>     /* for __pa() */
#include <linux/structpage.h>     /* for struct page */
#include <linux/sparse-consts.h>  /* for NR_MEM_SECTIONS... */
#include <linux/sparse-structs.h> /* for mem_section[] */

The dependencies aren't very twisted at all.  In fact, I don't think any
of those are deeper than two.  More importantly, I never have to cope
with 'struct page;' keeping me from doing arithmetic. 

> The big question here is what further cleanups are possible in this area?
> What basically needs to be done is to separate the definitions from its 
> users, that doesn't mean every definition needs its own header file. Why 
> not create a single header file which collects a number of mm related 
> definitions? E.g. struct vm_area_struct is also used by a number of header 
> files, although its main users have already been separated into 
> asm/tlbflush.h and so created even more headers.
> So to allow further header cleanup, we should look what other definitions 
> can be pulled out of mm.h and related headers. mm.h should probably stay a 
> high level header, but I'd also like to see a cleanup of asm/page.h. The 
> page table definitions in there should be available to every mm related 
> header.

I agree that I'm probably getting a bit carried away.  But, I don't
really see the harm in having both high-level headers and more finely
grained ones.  You could even think of the finely grained ones as
internal only.

In my example above the header is called linux/sparse.h.  It end up
including linux/sparse-consts.h and linux/sparse-structs.h.  So, any .c
file users can just include linux/sparse.h and forget about it.  But, if
any crazy header authors need either of the components in a different
order, they can have it.  

-- Dave

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

On Fri, 2004-12-17 at 05:26, Roman Zippel wrote:
> On Thu, 16 Dec 2004, Dave Hansen wrote:
> > Sorry I didn't provide this.  My recent effort started to clean up some
> > ugliness in some current patches that worked around this actually
> > happening a few months ago.  The original example didn't survive :)
> 
> There are two points, which did originally catch my attention.
> 1) Where/why do you want to remove the dependency on asm/page.h?

The basic reason is that I think it's important to know exactly what
you're getting into when you include a header, especially when you're
authoring new headers.  

That said, it doesn't appear that asm/page.h is a header that causes any
real problems.  In any case, when there are variables that need to be
created based on Kconfig parameters, I believe the "correct" thing to do
is make a Kconfig variable.  But, that is a separate conversation. :)

> 2) Does every structure really needs its own header?

No.  But, I do think that most of the very basic VM structures do, as it
stands.  That's limited to struct page, zone, and pgdat as I see it
now.  

> If you want to do such a cleanup, it would be helpful to have some more 
> information about where you want to go, otherwise you may create a more 
> twisted maze of header files.

One thing I'd like to point out is that I don't necessarily think that
the normal foo.c user should see any change.  They can still include
mmzone.h for the same stuff, but *inside* the headers, there's some more
organization.  Here's an example from a new header that I'm working on
after shifting the headers around.

#include <asm/page-consts.h>      /* for PAGE_SHIFT */
#include <asm/addr-convert.h>     /* for __pa() */
#include <linux/structpage.h>     /* for struct page */
#include <linux/sparse-consts.h>  /* for NR_MEM_SECTIONS... */
#include <linux/sparse-structs.h> /* for mem_section[] */

The dependencies aren't very twisted at all.  In fact, I don't think any
of those are deeper than two.  More importantly, I never have to cope
with 'struct page;' keeping me from doing arithmetic. 

> The big question here is what further cleanups are possible in this area?
> What basically needs to be done is to separate the definitions from its 
> users, that doesn't mean every definition needs its own header file. Why 
> not create a single header file which collects a number of mm related 
> definitions? E.g. struct vm_area_struct is also used by a number of header 
> files, although its main users have already been separated into 
> asm/tlbflush.h and so created even more headers.
> So to allow further header cleanup, we should look what other definitions 
> can be pulled out of mm.h and related headers. mm.h should probably stay a 
> high level header, but I'd also like to see a cleanup of asm/page.h. The 
> page table definitions in there should be available to every mm related 
> header.

I agree that I'm probably getting a bit carried away.  But, I don't
really see the harm in having both high-level headers and more finely
grained ones.  You could even think of the finely grained ones as
internal only.

In my example above the header is called linux/sparse.h.  It end up
including linux/sparse-consts.h and linux/sparse-structs.h.  So, any .c
file users can just include linux/sparse.h and forget about it.  But, if
any crazy header authors need either of the components in a different
order, they can have it.  

-- Dave

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

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Roman Zippel]

Hi,

On Fri, 17 Dec 2004, Dave Hansen wrote:

> That said, it doesn't appear that asm/page.h is a header that causes any
> real problems.  In any case, when there are variables that need to be
> created based on Kconfig parameters, I believe the "correct" thing to do
> is make a Kconfig variable.  But, that is a separate conversation. :)

Well, that's ok (but maybe give it a better name, e.g. 
CONFIG_PAGE_VIRTUAL_PTR?)

> > 2) Does every structure really needs its own header?
> 
> No.  But, I do think that most of the very basic VM structures do, as it
> stands.  That's limited to struct page, zone, and pgdat as I see it
> now.  

Why do you want to put these into separate headers? These would be AFAICT 
good candidates to pull into a single header. Via linux/gfp.h they are 
all pulled into a lot of files, so it doesn't make sense to split them 
because they may have different users.

> One thing I'd like to point out is that I don't necessarily think that
> the normal foo.c user should see any change.  They can still include
> mmzone.h for the same stuff, but *inside* the headers, there's some more
> organization.  Here's an example from a new header that I'm working on
> after shifting the headers around.
> 
> #include <asm/page-consts.h>      /* for PAGE_SHIFT */
> #include <asm/addr-convert.h>     /* for __pa() */
> #include <linux/structpage.h>     /* for struct page */
> #include <linux/sparse-consts.h>  /* for NR_MEM_SECTIONS... */
> #include <linux/sparse-structs.h> /* for mem_section[] */

Why do you want to separate constants and structures?
There only relatively few users of mmzone.h, so fixing them is easy and 
the main problem with mmzone.h are the macros in asm/mmzone.h, which 
should be pulled into a different header and the few remaining definitions 
could be also merged with other definitions (is there really a good 
reason for a separate asm/numnodes.h or linux/numa.h?). 

> The dependencies aren't very twisted at all.  In fact, I don't think any
> of those are deeper than two.  More importantly, I never have to cope
> with 'struct page;' keeping me from doing arithmetic. 

You may be surprised. :)
Play around with "mkdir test; echo 'obj-y = test.o' > test/Makefile; echo 
'#include <linux/foo.h>' > test/test.c; make test/test.i 
CFLAGS_test.o=--trace-includes".

> I agree that I'm probably getting a bit carried away.  But, I don't
> really see the harm in having both high-level headers and more finely
> grained ones.  You could even think of the finely grained ones as
> internal only.

Well, I don't really like lots of mini header files. I think it's more 
important to collect related information and only strictly separate 
between definitions and users, when a header gets to large we can still 
split it.

bye, Roman

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Roman Zippel]

Hi,

On Fri, 17 Dec 2004, Dave Hansen wrote:

> That said, it doesn't appear that asm/page.h is a header that causes any
> real problems.  In any case, when there are variables that need to be
> created based on Kconfig parameters, I believe the "correct" thing to do
> is make a Kconfig variable.  But, that is a separate conversation. :)

Well, that's ok (but maybe give it a better name, e.g. 
CONFIG_PAGE_VIRTUAL_PTR?)

> > 2) Does every structure really needs its own header?
> 
> No.  But, I do think that most of the very basic VM structures do, as it
> stands.  That's limited to struct page, zone, and pgdat as I see it
> now.  

Why do you want to put these into separate headers? These would be AFAICT 
good candidates to pull into a single header. Via linux/gfp.h they are 
all pulled into a lot of files, so it doesn't make sense to split them 
because they may have different users.

> One thing I'd like to point out is that I don't necessarily think that
> the normal foo.c user should see any change.  They can still include
> mmzone.h for the same stuff, but *inside* the headers, there's some more
> organization.  Here's an example from a new header that I'm working on
> after shifting the headers around.
> 
> #include <asm/page-consts.h>      /* for PAGE_SHIFT */
> #include <asm/addr-convert.h>     /* for __pa() */
> #include <linux/structpage.h>     /* for struct page */
> #include <linux/sparse-consts.h>  /* for NR_MEM_SECTIONS... */
> #include <linux/sparse-structs.h> /* for mem_section[] */

Why do you want to separate constants and structures?
There only relatively few users of mmzone.h, so fixing them is easy and 
the main problem with mmzone.h are the macros in asm/mmzone.h, which 
should be pulled into a different header and the few remaining definitions 
could be also merged with other definitions (is there really a good 
reason for a separate asm/numnodes.h or linux/numa.h?). 

> The dependencies aren't very twisted at all.  In fact, I don't think any
> of those are deeper than two.  More importantly, I never have to cope
> with 'struct page;' keeping me from doing arithmetic. 

You may be surprised. :)
Play around with "mkdir test; echo 'obj-y = test.o' > test/Makefile; echo 
'#include <linux/foo.h>' > test/test.c; make test/test.i 
CFLAGS_test.o=--trace-includes".

> I agree that I'm probably getting a bit carried away.  But, I don't
> really see the harm in having both high-level headers and more finely
> grained ones.  You could even think of the finely grained ones as
> internal only.

Well, I don't really like lots of mini header files. I think it's more 
important to collect related information and only strictly separate 
between definitions and users, when a header gets to large we can still 
split it.

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

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

OK, here's my example:

Take include/asm-i386/mmzone.h and make pfn_to_page() a static inline

  CC      arch/i386/kernel/asm-offsets.s
In file included from include/linux/mmzone.h:389,
                 from include/linux/gfp.h:4,
                 from include/linux/slab.h:15,
                 from include/linux/percpu.h:4,
                 from include/linux/sched.h:34,
                 from arch/i386/kernel/asm-offsets.c:7:
include/asm/mmzone.h: In function `pfn_to_page':
include/asm/mmzone.h:109: arithmetic on pointer to an incomplete type
include/asm/mmzone.h:109: dereferencing pointer to incomplete type
include/asm/mmzone.h:110: warning: control reaches end of non-void function
You say, oh, it doesn't have a proper 'struct page' definition.  I had
better include the place where it is defined.  You go add
"#include <linux/mm.h>"
to the top of the file.  Compile again...

  CC      arch/i386/kernel/asm-offsets.s
In file included from include/asm/mmzone.h:10,
                 from include/linux/mmzone.h:389,
                 from include/linux/gfp.h:4,
                 from include/linux/slab.h:15,
                 from include/linux/percpu.h:4,
                 from include/linux/sched.h:34,
                 from arch/i386/kernel/asm-offsets.c:7:
include/linux/mm.h: In function `page_zonenum':
include/linux/mm.h:417: `MAX_NODES_SHIFT' undeclared (first use in this function)
include/linux/mm.h:417: (Each undeclared identifier is reported only once
include/linux/mm.h:417: for each function it appears in.)
On Fri, 2004-12-17 at 12:27, Roman Zippel wrote: 
> On Fri, 17 Dec 2004, Dave Hansen wrote:
> > That said, it doesn't appear that asm/page.h is a header that causes any
> > real problems.  In any case, when there are variables that need to be
> > created based on Kconfig parameters, I believe the "correct" thing to do
> > is make a Kconfig variable.  But, that is a separate conversation. :)
> 
> Well, that's ok (but maybe give it a better name, e.g. 
> CONFIG_PAGE_VIRTUAL_PTR?)
That's fine.  I was just copying the current name for now, but I don't
have a real problem changing it.

> > > 2) Does every structure really needs its own header?
> > 
> > No.  But, I do think that most of the very basic VM structures do, as it
> > stands.  That's limited to struct page, zone, and pgdat as I see it
> > now.  
> 
> Why do you want to put these into separate headers?

It enables you do do static inlines accessing struct page members
anywhere you want, such as in asm/mmzone.h, like in my example. 

> These would be AFAICT 
> good candidates to pull into a single header. Via linux/gfp.h they are 
> all pulled into a lot of files, so it doesn't make sense to split them 
> because they may have different users.

They can still all be pulled in by the same original header.   I think
we're disconnecting a bit on this point.  

Making users of structures (driver writers, etc...) include 15 different
headers is silly.  But, I'm not really talking about those users, I'm
trying to fix the problems that people who write new *headers* are
running into.

> > One thing I'd like to point out is that I don't necessarily think that
> > the normal foo.c user should see any change.  They can still include
> > mmzone.h for the same stuff, but *inside* the headers, there's some more
> > organization.  Here's an example from a new header that I'm working on
> > after shifting the headers around.
> > 
> > #include <asm/page-consts.h>      /* for PAGE_SHIFT */
> > #include <asm/addr-convert.h>     /* for __pa() */
> > #include <linux/structpage.h>     /* for struct page */
> > #include <linux/sparse-consts.h>  /* for NR_MEM_SECTIONS... */
> > #include <linux/sparse-structs.h> /* for mem_section[] */
> 
> Why do you want to separate constants and structures?

In that code, there is a lot of intermingling between the different
constants.  The sparsemem code uses extra space in page->flags to store
some extra information.  So, it has dependencies on page-flags.h, as
well as the gfp masks.  These are used to define the sizes of structures
that are used for the pfn<->page, page<->zone, page<->nid, and
__{p,v}a() conversions.

These get really complicated, until you realize that all of the
constants have no dependencies other than linux/config.h and some other
really simple headers like types.h.

So, in effect, you do all of the constants' evaluation and *then* get to
move on to all of the structures.  Separating it out in different
headers makes it very explicit what is going on, and which constants are
needed for defining which operations.

> There only relatively few users of mmzone.h, so fixing them is easy and 
> the main problem with mmzone.h are the macros in asm/mmzone.h, which 
> should be pulled into a different header and the few remaining definitions 
> could be also merged with other definitions

I think you're right here.  Making mmzone.h a much simpler header with
fewer macros declared would help.

> (is there really a good 
> reason for a separate asm/numnodes.h or linux/numa.h?). 

I think linux/numa.h is the empty header for an architecture (or config
setup) that doesn't have NUMA.  So, it needs to be separate from the
asm/ files.  

> > The dependencies aren't very twisted at all.  In fact, I don't think any
> > of those are deeper than two.  More importantly, I never have to cope
> > with 'struct page;' keeping me from doing arithmetic. 
> 
> You may be surprised. :)
> Play around with "mkdir test; echo 'obj-y = test.o' > test/Makefile; echo 
> '#include <linux/foo.h>' > test/test.c; make test/test.i 
> CFLAGS_test.o=--trace-includes".

I'm not sure what you're getting at.

	make: *** No rule to make target `test/test.i'.  Stop.

> > I agree that I'm probably getting a bit carried away.  But, I don't
> > really see the harm in having both high-level headers and more finely
> > grained ones.  You could even think of the finely grained ones as
> > internal only.
> 
> Well, I don't really like lots of mini header files. I think it's more 
> important to collect related information and only strictly separate 
> between definitions and users, when a header gets to large we can still 
> split it.

Is the problem that I described above something that you think is at the
point where splitting is required?

-- Dave

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

OK, here's my example:

Take include/asm-i386/mmzone.h and make pfn_to_page() a static inline

  CC      arch/i386/kernel/asm-offsets.s
In file included from include/linux/mmzone.h:389,
                 from include/linux/gfp.h:4,
                 from include/linux/slab.h:15,
                 from include/linux/percpu.h:4,
                 from include/linux/sched.h:34,
                 from arch/i386/kernel/asm-offsets.c:7:
include/asm/mmzone.h: In function `pfn_to_page':
include/asm/mmzone.h:109: arithmetic on pointer to an incomplete type
include/asm/mmzone.h:109: dereferencing pointer to incomplete type
include/asm/mmzone.h:110: warning: control reaches end of non-void function
You say, oh, it doesn't have a proper 'struct page' definition.  I had
better include the place where it is defined.  You go add
"#include <linux/mm.h>"
to the top of the file.  Compile again...

  CC      arch/i386/kernel/asm-offsets.s
In file included from include/asm/mmzone.h:10,
                 from include/linux/mmzone.h:389,
                 from include/linux/gfp.h:4,
                 from include/linux/slab.h:15,
                 from include/linux/percpu.h:4,
                 from include/linux/sched.h:34,
                 from arch/i386/kernel/asm-offsets.c:7:
include/linux/mm.h: In function `page_zonenum':
include/linux/mm.h:417: `MAX_NODES_SHIFT' undeclared (first use in this function)
include/linux/mm.h:417: (Each undeclared identifier is reported only once
include/linux/mm.h:417: for each function it appears in.)
On Fri, 2004-12-17 at 12:27, Roman Zippel wrote: 
> On Fri, 17 Dec 2004, Dave Hansen wrote:
> > That said, it doesn't appear that asm/page.h is a header that causes any
> > real problems.  In any case, when there are variables that need to be
> > created based on Kconfig parameters, I believe the "correct" thing to do
> > is make a Kconfig variable.  But, that is a separate conversation. :)
> 
> Well, that's ok (but maybe give it a better name, e.g. 
> CONFIG_PAGE_VIRTUAL_PTR?)
That's fine.  I was just copying the current name for now, but I don't
have a real problem changing it.

> > > 2) Does every structure really needs its own header?
> > 
> > No.  But, I do think that most of the very basic VM structures do, as it
> > stands.  That's limited to struct page, zone, and pgdat as I see it
> > now.  
> 
> Why do you want to put these into separate headers?

It enables you do do static inlines accessing struct page members
anywhere you want, such as in asm/mmzone.h, like in my example. 

> These would be AFAICT 
> good candidates to pull into a single header. Via linux/gfp.h they are 
> all pulled into a lot of files, so it doesn't make sense to split them 
> because they may have different users.

They can still all be pulled in by the same original header.   I think
we're disconnecting a bit on this point.  

Making users of structures (driver writers, etc...) include 15 different
headers is silly.  But, I'm not really talking about those users, I'm
trying to fix the problems that people who write new *headers* are
running into.

> > One thing I'd like to point out is that I don't necessarily think that
> > the normal foo.c user should see any change.  They can still include
> > mmzone.h for the same stuff, but *inside* the headers, there's some more
> > organization.  Here's an example from a new header that I'm working on
> > after shifting the headers around.
> > 
> > #include <asm/page-consts.h>      /* for PAGE_SHIFT */
> > #include <asm/addr-convert.h>     /* for __pa() */
> > #include <linux/structpage.h>     /* for struct page */
> > #include <linux/sparse-consts.h>  /* for NR_MEM_SECTIONS... */
> > #include <linux/sparse-structs.h> /* for mem_section[] */
> 
> Why do you want to separate constants and structures?

In that code, there is a lot of intermingling between the different
constants.  The sparsemem code uses extra space in page->flags to store
some extra information.  So, it has dependencies on page-flags.h, as
well as the gfp masks.  These are used to define the sizes of structures
that are used for the pfn<->page, page<->zone, page<->nid, and
__{p,v}a() conversions.

These get really complicated, until you realize that all of the
constants have no dependencies other than linux/config.h and some other
really simple headers like types.h.

So, in effect, you do all of the constants' evaluation and *then* get to
move on to all of the structures.  Separating it out in different
headers makes it very explicit what is going on, and which constants are
needed for defining which operations.

> There only relatively few users of mmzone.h, so fixing them is easy and 
> the main problem with mmzone.h are the macros in asm/mmzone.h, which 
> should be pulled into a different header and the few remaining definitions 
> could be also merged with other definitions

I think you're right here.  Making mmzone.h a much simpler header with
fewer macros declared would help.

> (is there really a good 
> reason for a separate asm/numnodes.h or linux/numa.h?). 

I think linux/numa.h is the empty header for an architecture (or config
setup) that doesn't have NUMA.  So, it needs to be separate from the
asm/ files.  

> > The dependencies aren't very twisted at all.  In fact, I don't think any
> > of those are deeper than two.  More importantly, I never have to cope
> > with 'struct page;' keeping me from doing arithmetic. 
> 
> You may be surprised. :)
> Play around with "mkdir test; echo 'obj-y = test.o' > test/Makefile; echo 
> '#include <linux/foo.h>' > test/test.c; make test/test.i 
> CFLAGS_test.o=--trace-includes".

I'm not sure what you're getting at.

	make: *** No rule to make target `test/test.i'.  Stop.

> > I agree that I'm probably getting a bit carried away.  But, I don't
> > really see the harm in having both high-level headers and more finely
> > grained ones.  You could even think of the finely grained ones as
> > internal only.
> 
> Well, I don't really like lots of mini header files. I think it's more 
> important to collect related information and only strictly separate 
> between definitions and users, when a header gets to large we can still 
> split it.

Is the problem that I described above something that you think is at the
point where splitting is required?

-- Dave

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

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Roman Zippel]

Hi,

On Fri, 17 Dec 2004, Dave Hansen wrote:

> > > No.  But, I do think that most of the very basic VM structures do, as it
> > > stands.  That's limited to struct page, zone, and pgdat as I see it
> > > now.  
> > 
> > Why do you want to put these into separate headers?
> 
> It enables you do do static inlines accessing struct page members
> anywhere you want, such as in asm/mmzone.h, like in my example. 

And by that you add more header dependencies.
We have basically this situation:

	foo.h (struct foo; inline foo();) <-> bar.h (struct bar; inline bar();)

Almost every time we had such recursive dependencies, we simply rip one 
element out and put it into a separate header:

	foo.h (inline foo();)
		-> bar.h (struct bar; inline bar();)
			-> foo_struct.h (struct foo;)

Repeat this often enough and we end up with millions of small header 
files. Instead we can reorder everything a little and can do this:

	foo.h (inline foo(); inline bar();)
		-> foo_types.h (struct foo; struct bar;)

In your case don't put the inline functions into asm/mmzone.h and we 
should merge the various definition into fewer header files.

> > > The dependencies aren't very twisted at all.  In fact, I don't think any
> > > of those are deeper than two.  More importantly, I never have to cope
> > > with 'struct page;' keeping me from doing arithmetic. 
> > 
> > You may be surprised. :)
> > Play around with "mkdir test; echo 'obj-y = test.o' > test/Makefile; echo 
> > '#include <linux/foo.h>' > test/test.c; make test/test.i 
> > CFLAGS_test.o=--trace-includes".
> 
> I'm not sure what you're getting at.
> 
> 	make: *** No rule to make target `test/test.i'.  Stop.

Sorry, I forgot to mention that you have to do this inside a kernel tree.

bye, Roman

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Roman Zippel]

Hi,

On Fri, 17 Dec 2004, Dave Hansen wrote:

> > > No.  But, I do think that most of the very basic VM structures do, as it
> > > stands.  That's limited to struct page, zone, and pgdat as I see it
> > > now.  
> > 
> > Why do you want to put these into separate headers?
> 
> It enables you do do static inlines accessing struct page members
> anywhere you want, such as in asm/mmzone.h, like in my example. 

And by that you add more header dependencies.
We have basically this situation:

	foo.h (struct foo; inline foo();) <-> bar.h (struct bar; inline bar();)

Almost every time we had such recursive dependencies, we simply rip one 
element out and put it into a separate header:

	foo.h (inline foo();)
		-> bar.h (struct bar; inline bar();)
			-> foo_struct.h (struct foo;)

Repeat this often enough and we end up with millions of small header 
files. Instead we can reorder everything a little and can do this:

	foo.h (inline foo(); inline bar();)
		-> foo_types.h (struct foo; struct bar;)

In your case don't put the inline functions into asm/mmzone.h and we 
should merge the various definition into fewer header files.

> > > The dependencies aren't very twisted at all.  In fact, I don't think any
> > > of those are deeper than two.  More importantly, I never have to cope
> > > with 'struct page;' keeping me from doing arithmetic. 
> > 
> > You may be surprised. :)
> > Play around with "mkdir test; echo 'obj-y = test.o' > test/Makefile; echo 
> > '#include <linux/foo.h>' > test/test.c; make test/test.i 
> > CFLAGS_test.o=--trace-includes".
> 
> I'm not sure what you're getting at.
> 
> 	make: *** No rule to make target `test/test.i'.  Stop.

Sorry, I forgot to mention that you have to do this inside a kernel tree.

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

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

On Fri, 2004-12-17 at 16:52, Roman Zippel wrote:
> In your case don't put the inline functions into asm/mmzone.h and we 
> should merge the various definition into fewer header files.

OK, I'm sold.  

But, what do you think we should do about the current #defines in
asm/mmzone.h, like pfn_to_page()?  Would it be feasible to put them in
another header that can use proper functions?

-- Dave

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

On Fri, 2004-12-17 at 16:52, Roman Zippel wrote:
> In your case don't put the inline functions into asm/mmzone.h and we 
> should merge the various definition into fewer header files.

OK, I'm sold.  

But, what do you think we should do about the current #defines in
asm/mmzone.h, like pfn_to_page()?  Would it be feasible to put them in
another header that can use proper functions?

-- Dave

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

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Roman Zippel]

Hi,

On Mon, 20 Dec 2004, Dave Hansen wrote:

> But, what do you think we should do about the current #defines in
> asm/mmzone.h, like pfn_to_page()?  Would it be feasible to put them in
> another header that can use proper functions?

It should be possible to move it to asm/pgtable.h, although asm/memory.h 
(as arm already has) would be a good alternative.
struct page can be moved to linux/mmzone.h and and asm/mmzone.h should die 
rather soon. It was mistake to have definitions of inline functions/macros 
in two different files (asm/page.h and asm/mmzone.h).
Below is a patch which moves thing around so pfn_to_page/page_to_pfn can 
become inline functions.

bye, Roman

 asm/mmzone.h   |   27 -----
 asm/page.h     |    8 -
 asm/pgtable.h  |   44 +++++++++
 linux/mm.h     |  192 ----------------------------------------
 linux/mmzone.h |  272 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 5 files changed, 274 insertions(+), 269 deletions(-)

Index: linux-mm/include/linux/mmzone.h
===================================================================
--- linux-mm.orig/include/linux/mmzone.h	2004-12-17 12:01:55.000000000 +0100
+++ linux-mm/include/linux/mmzone.h	2004-12-20 21:39:26.513594537 +0100
@@ -11,7 +11,168 @@
 #include <linux/cache.h>
 #include <linux/threads.h>
 #include <linux/numa.h>
-#include <asm/atomic.h>
+#include <linux/rbtree.h>
+#include <linux/prio_tree.h>
+
+#ifdef ARCH_HAS_ATOMIC_UNSIGNED
+typedef unsigned page_flags_t;
+#else
+typedef unsigned long page_flags_t;
+#endif
+
+/*
+ * Each physical page in the system has a struct page associated with
+ * it to keep track of whatever it is we are using the page for at the
+ * moment. Note that we have no way to track which tasks are using
+ * a page.
+ */
+struct page {
+	page_flags_t flags;		/* Atomic flags, some possibly
+					 * updated asynchronously */
+	atomic_t _count;		/* Usage count, see below. */
+	atomic_t _mapcount;		/* Count of ptes mapped in mms,
+					 * to show when page is mapped
+					 * & limit reverse map searches.
+					 */
+	unsigned long private;		/* Mapping-private opaque data:
+					 * usually used for buffer_heads
+					 * if PagePrivate set; used for
+					 * swp_entry_t if PageSwapCache
+					 */
+	struct address_space *mapping;	/* If low bit clear, points to
+					 * inode address_space, or NULL.
+					 * If page mapped as anonymous
+					 * memory, low bit is set, and
+					 * it points to anon_vma object:
+					 * see PAGE_MAPPING_ANON below.
+					 */
+	pgoff_t index;			/* Our offset within mapping. */
+	struct list_head lru;		/* Pageout list, eg. active_list
+					 * protected by zone->lru_lock !
+					 */
+	/*
+	 * On machines where all RAM is mapped into kernel address space,
+	 * we can simply calculate the virtual address. On machines with
+	 * highmem some memory is mapped into kernel virtual memory
+	 * dynamically, so we need a place to store that address.
+	 * Note that this field could be 16 bits on x86 ... ;)
+	 *
+	 * Architectures with slow multiplication can define
+	 * WANT_PAGE_VIRTUAL in asm/page.h
+	 */
+#if defined(WANT_PAGE_VIRTUAL)
+	void *virtual;			/* Kernel virtual address (NULL if
+					   not kmapped, ie. highmem) */
+#endif /* WANT_PAGE_VIRTUAL */
+};
+
+/*
+ * This struct defines a memory VMM memory area. There is one of these
+ * per VM-area/task.  A VM area is any part of the process virtual memory
+ * space that has a special rule for the page-fault handlers (ie a shared
+ * library, the executable area etc).
+ */
+struct vm_area_struct {
+	struct mm_struct * vm_mm;	/* The address space we belong to. */
+	unsigned long vm_start;		/* Our start address within vm_mm. */
+	unsigned long vm_end;		/* The first byte after our end address
+					   within vm_mm. */
+
+	/* linked list of VM areas per task, sorted by address */
+	struct vm_area_struct *vm_next;
+
+	pgprot_t vm_page_prot;		/* Access permissions of this VMA. */
+	unsigned long vm_flags;		/* Flags, listed below. */
+
+	struct rb_node vm_rb;
+
+	/*
+	 * For areas with an address space and backing store,
+	 * linkage into the address_space->i_mmap prio tree, or
+	 * linkage to the list of like vmas hanging off its node, or
+	 * linkage of vma in the address_space->i_mmap_nonlinear list.
+	 */
+	union {
+		struct {
+			struct list_head list;
+			void *parent;	/* aligns with prio_tree_node parent */
+			struct vm_area_struct *head;
+		} vm_set;
+
+		struct prio_tree_node prio_tree_node;
+	} shared;
+
+	/*
+	 * A file's MAP_PRIVATE vma can be in both i_mmap tree and anon_vma
+	 * list, after a COW of one of the file pages.  A MAP_SHARED vma
+	 * can only be in the i_mmap tree.  An anonymous MAP_PRIVATE, stack
+	 * or brk vma (with NULL file) can only be in an anon_vma list.
+	 */
+	struct list_head anon_vma_node;	/* Serialized by anon_vma->lock */
+	struct anon_vma *anon_vma;	/* Serialized by page_table_lock */
+
+	/* Function pointers to deal with this struct. */
+	struct vm_operations_struct * vm_ops;
+
+	/* Information about our backing store: */
+	unsigned long vm_pgoff;		/* Offset (within vm_file) in PAGE_SIZE
+					   units, *not* PAGE_CACHE_SIZE */
+	struct file * vm_file;		/* File we map to (can be NULL). */
+	void * vm_private_data;		/* was vm_pte (shared mem) */
+
+#ifdef CONFIG_NUMA
+	struct mempolicy *vm_policy;	/* NUMA policy for the VMA */
+#endif
+};
+
+/*
+ * vm_flags..
+ */
+#define VM_READ		0x00000001	/* currently active flags */
+#define VM_WRITE	0x00000002
+#define VM_EXEC		0x00000004
+#define VM_SHARED	0x00000008
+
+#define VM_MAYREAD	0x00000010	/* limits for mprotect() etc */
+#define VM_MAYWRITE	0x00000020
+#define VM_MAYEXEC	0x00000040
+#define VM_MAYSHARE	0x00000080
+
+#define VM_GROWSDOWN	0x00000100	/* general info on the segment */
+#define VM_GROWSUP	0x00000200
+#define VM_SHM		0x00000400	/* shared memory area, don't swap out */
+#define VM_DENYWRITE	0x00000800	/* ETXTBSY on write attempts.. */
+
+#define VM_EXECUTABLE	0x00001000
+#define VM_LOCKED	0x00002000
+#define VM_IO           0x00004000	/* Memory mapped I/O or similar */
+
+					/* Used by sys_madvise() */
+#define VM_SEQ_READ	0x00008000	/* App will access data sequentially */
+#define VM_RAND_READ	0x00010000	/* App will not benefit from clustered reads */
+
+#define VM_DONTCOPY	0x00020000      /* Do not copy this vma on fork */
+#define VM_DONTEXPAND	0x00040000	/* Cannot expand with mremap() */
+#define VM_RESERVED	0x00080000	/* Don't unmap it from swap_out */
+#define VM_ACCOUNT	0x00100000	/* Is a VM accounted object */
+#define VM_HUGETLB	0x00400000	/* Huge TLB Page VM */
+#define VM_NONLINEAR	0x00800000	/* Is non-linear (remap_file_pages) */
+
+#ifndef VM_STACK_DEFAULT_FLAGS		/* arch can override this */
+#define VM_STACK_DEFAULT_FLAGS VM_DATA_DEFAULT_FLAGS
+#endif
+
+#ifdef CONFIG_STACK_GROWSUP
+#define VM_STACK_FLAGS	(VM_GROWSUP | VM_STACK_DEFAULT_FLAGS | VM_ACCOUNT)
+#else
+#define VM_STACK_FLAGS	(VM_GROWSDOWN | VM_STACK_DEFAULT_FLAGS | VM_ACCOUNT)
+#endif
+
+#define VM_READHINTMASK			(VM_SEQ_READ | VM_RAND_READ)
+#define VM_ClearReadHint(v)		(v)->vm_flags &= ~VM_READHINTMASK
+#define VM_NormalReadHint(v)		(!((v)->vm_flags & VM_READHINTMASK))
+#define VM_SequentialReadHint(v)	((v)->vm_flags & VM_SEQ_READ)
+#define VM_RandomReadHint(v)		((v)->vm_flags & VM_RAND_READ)
 
 /* Free memory management - zoned buddy allocator.  */
 #ifndef CONFIG_FORCE_MAX_ZONEORDER
@@ -25,8 +186,6 @@ struct free_area {
 	unsigned long		*map;
 };
 
-struct pglist_data;
-
 /*
  * zone->lock and zone->lru_lock are two of the hottest locks in the kernel.
  * So add a wild amount of padding here to ensure that they fall into separate
@@ -238,6 +397,74 @@ struct zonelist {
 	struct zone *zones[MAX_NUMNODES * MAX_NR_ZONES + 1]; // NULL delimited
 };
 
+#ifndef CONFIG_DISCONTIGMEM
+
+extern struct pglist_data contig_page_data;
+#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_DISCONTIGMEM */
+
+#include <asm/mmzone.h>
+
+#if BITS_PER_LONG == 32 || defined(ARCH_HAS_ATOMIC_UNSIGNED)
+/*
+ * with 32 bit page->flags field, we reserve 8 bits for node/zone info.
+ * there are 3 zones (2 bits) and this leaves 8-2=6 bits for nodes.
+ */
+#define MAX_NODES_SHIFT		6
+#elif BITS_PER_LONG == 64
+/*
+ * with 64 bit flags field, there's plenty of room.
+ */
+#define MAX_NODES_SHIFT		10
+#endif
+
+#endif /* !CONFIG_DISCONTIGMEM */
+
+#if NODES_SHIFT > MAX_NODES_SHIFT
+#error NODES_SHIFT > MAX_NODES_SHIFT
+#endif
+
+/* There are currently 3 zones: DMA, Normal & Highmem, thus we need 2 bits */
+#define MAX_ZONES_SHIFT		2
+
+#if ZONES_SHIFT > MAX_ZONES_SHIFT
+#error ZONES_SHIFT > MAX_ZONES_SHIFT
+#endif
+
+/*
+ * The zone field is never updated after free_area_init_core()
+ * sets it, so none of the operations on it need to be atomic.
+ * We'll have up to (MAX_NUMNODES * MAX_NR_ZONES) zones total,
+ * so we use (MAX_NODES_SHIFT + MAX_ZONES_SHIFT) here to get enough bits.
+ */
+#define NODEZONE_SHIFT (sizeof(page_flags_t)*8 - MAX_NODES_SHIFT - MAX_ZONES_SHIFT)
+#define NODEZONE(node, zone)	((node << ZONES_SHIFT) | zone)
+
+static inline unsigned long page_zonenum(struct page *page)
+{
+	return (page->flags >> NODEZONE_SHIFT) & (~(~0UL << ZONES_SHIFT));
+}
+static inline unsigned long page_to_nid(struct page *page)
+{
+	return (page->flags >> (NODEZONE_SHIFT + ZONES_SHIFT));
+}
+
+static inline void set_page_zone(struct page *page, unsigned long nodezone_num)
+{
+	page->flags &= ~(~0UL << NODEZONE_SHIFT);
+	page->flags |= nodezone_num << NODEZONE_SHIFT;
+}
+
+extern struct zone *zone_table[];
+
+static inline struct zone *page_zone(struct page *page)
+{
+	return zone_table[page->flags >> NODEZONE_SHIFT];
+}
 
 /*
  * The pg_data_t structure is used in machines with CONFIG_DISCONTIGMEM
@@ -369,48 +596,9 @@ int min_free_kbytes_sysctl_handler(struc
 int lower_zone_protection_sysctl_handler(struct ctl_table *, int, struct file *,
 					void __user *, size_t *, loff_t *);
 
-#include <linux/topology.h>
 /* Returns the number of the current Node. */
 #define numa_node_id()		(cpu_to_node(smp_processor_id()))
 
-#ifndef CONFIG_DISCONTIGMEM
-
-extern struct pglist_data contig_page_data;
-#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_DISCONTIGMEM */
-
-#include <asm/mmzone.h>
-
-#if BITS_PER_LONG == 32 || defined(ARCH_HAS_ATOMIC_UNSIGNED)
-/*
- * with 32 bit page->flags field, we reserve 8 bits for node/zone info.
- * there are 3 zones (2 bits) and this leaves 8-2=6 bits for nodes.
- */
-#define MAX_NODES_SHIFT		6
-#elif BITS_PER_LONG == 64
-/*
- * with 64 bit flags field, there's plenty of room.
- */
-#define MAX_NODES_SHIFT		10
-#endif
-
-#endif /* !CONFIG_DISCONTIGMEM */
-
-#if NODES_SHIFT > MAX_NODES_SHIFT
-#error NODES_SHIFT > MAX_NODES_SHIFT
-#endif
-
-/* There are currently 3 zones: DMA, Normal & Highmem, thus we need 2 bits */
-#define MAX_ZONES_SHIFT		2
-
-#if ZONES_SHIFT > MAX_ZONES_SHIFT
-#error ZONES_SHIFT > MAX_ZONES_SHIFT
-#endif
-
 #endif /* !__ASSEMBLY__ */
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MMZONE_H */
Index: linux-mm/include/asm/mmzone.h
===================================================================
--- linux-mm.orig/include/asm/mmzone.h	2004-07-03 19:02:16.000000000 +0200
+++ linux-mm/include/asm/mmzone.h	2004-12-20 21:36:42.708848071 +0100
@@ -102,35 +102,8 @@ static inline struct pglist_data *pfn_to
 /* XXX: FIXME -- wli */
 #define kern_addr_valid(kaddr)	(0)
 
-#define pfn_to_page(pfn)						\
-({									\
-	unsigned long __pfn = pfn;					\
-	int __node  = pfn_to_nid(__pfn);				\
-	&node_mem_map(__node)[node_localnr(__pfn,__node)];		\
-})
-
-#define page_to_pfn(pg)							\
-({									\
-	struct page *__page = pg;					\
-	struct zone *__zone = page_zone(__page);			\
-	(unsigned long)(__page - __zone->zone_mem_map)			\
-		+ __zone->zone_start_pfn;				\
-})
 #define pmd_page(pmd)		(pfn_to_page(pmd_val(pmd) >> PAGE_SHIFT))
 
-#ifdef CONFIG_X86_NUMAQ            /* we have contiguous memory on NUMA-Q */
-#define pfn_valid(pfn)          ((pfn) < num_physpages)
-#else
-static inline int pfn_valid(int pfn)
-{
-	int nid = pfn_to_nid(pfn);
-
-	if (nid >= 0)
-		return (pfn < node_end_pfn(nid));
-	return 0;
-}
-#endif
-
 extern int get_memcfg_numa_flat(void );
 /*
  * This allows any one NUMA architecture to be compiled
Index: linux-mm/include/asm/pgtable.h
===================================================================
--- linux-mm.orig/include/asm/pgtable.h	2004-10-20 17:28:49.000000000 +0200
+++ linux-mm/include/asm/pgtable.h	2004-12-20 21:36:42.717846519 +0100
@@ -25,6 +25,50 @@
 #include <linux/list.h>
 #include <linux/spinlock.h>
 
+#ifndef CONFIG_DISCONTIGMEM
+
+static inline struct page *pfn_to_page(unsigned long pfn)
+{
+	return mem_map + pfn;
+}
+static inline unsigned long page_to_pfn(struct page *page)
+{
+	return page - mem_map;
+}
+
+#define pfn_valid(pfn)		((pfn) < max_mapnr)
+
+#else
+
+static inline struct page *pfn_to_page(unsigned long pfn)
+{
+	int node = pfn_to_nid(pfn);
+	return &node_mem_map(node)[node_localnr(pfn, node)];
+}
+static inline unsigned long page_to_pfn(struct page *page)
+{
+	struct zone *zone = page_zone(page);
+	return zone->zone_start_pfn + (page - zone->zone_mem_map);
+}
+
+#ifdef CONFIG_X86_NUMAQ            /* we have contiguous memory on NUMA-Q */
+#define pfn_valid(pfn)          ((pfn) < num_physpages)
+#else
+static inline int pfn_valid(int pfn)
+{
+	int nid = pfn_to_nid(pfn);
+
+	if (nid >= 0)
+		return (pfn < node_end_pfn(nid));
+	return 0;
+}
+#endif
+
+#endif /* !CONFIG_DISCONTIGMEM */
+#define virt_to_page(kaddr)	pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
+
+#define virt_addr_valid(kaddr)	pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
+
 /*
  * ZERO_PAGE is a global shared page that is always zero: used
  * for zero-mapped memory areas etc..
Index: linux-mm/include/linux/mm.h
===================================================================
--- linux-mm.orig/include/linux/mm.h	2004-12-17 12:01:55.000000000 +0100
+++ linux-mm/include/linux/mm.h	2004-12-20 21:36:42.718846346 +0100
@@ -53,114 +53,6 @@ extern int sysctl_legacy_va_layout;
  */
 
 /*
- * This struct defines a memory VMM memory area. There is one of these
- * per VM-area/task.  A VM area is any part of the process virtual memory
- * space that has a special rule for the page-fault handlers (ie a shared
- * library, the executable area etc).
- */
-struct vm_area_struct {
-	struct mm_struct * vm_mm;	/* The address space we belong to. */
-	unsigned long vm_start;		/* Our start address within vm_mm. */
-	unsigned long vm_end;		/* The first byte after our end address
-					   within vm_mm. */
-
-	/* linked list of VM areas per task, sorted by address */
-	struct vm_area_struct *vm_next;
-
-	pgprot_t vm_page_prot;		/* Access permissions of this VMA. */
-	unsigned long vm_flags;		/* Flags, listed below. */
-
-	struct rb_node vm_rb;
-
-	/*
-	 * For areas with an address space and backing store,
-	 * linkage into the address_space->i_mmap prio tree, or
-	 * linkage to the list of like vmas hanging off its node, or
-	 * linkage of vma in the address_space->i_mmap_nonlinear list.
-	 */
-	union {
-		struct {
-			struct list_head list;
-			void *parent;	/* aligns with prio_tree_node parent */
-			struct vm_area_struct *head;
-		} vm_set;
-
-		struct prio_tree_node prio_tree_node;
-	} shared;
-
-	/*
-	 * A file's MAP_PRIVATE vma can be in both i_mmap tree and anon_vma
-	 * list, after a COW of one of the file pages.  A MAP_SHARED vma
-	 * can only be in the i_mmap tree.  An anonymous MAP_PRIVATE, stack
-	 * or brk vma (with NULL file) can only be in an anon_vma list.
-	 */
-	struct list_head anon_vma_node;	/* Serialized by anon_vma->lock */
-	struct anon_vma *anon_vma;	/* Serialized by page_table_lock */
-
-	/* Function pointers to deal with this struct. */
-	struct vm_operations_struct * vm_ops;
-
-	/* Information about our backing store: */
-	unsigned long vm_pgoff;		/* Offset (within vm_file) in PAGE_SIZE
-					   units, *not* PAGE_CACHE_SIZE */
-	struct file * vm_file;		/* File we map to (can be NULL). */
-	void * vm_private_data;		/* was vm_pte (shared mem) */
-
-#ifdef CONFIG_NUMA
-	struct mempolicy *vm_policy;	/* NUMA policy for the VMA */
-#endif
-};
-
-/*
- * vm_flags..
- */
-#define VM_READ		0x00000001	/* currently active flags */
-#define VM_WRITE	0x00000002
-#define VM_EXEC		0x00000004
-#define VM_SHARED	0x00000008
-
-#define VM_MAYREAD	0x00000010	/* limits for mprotect() etc */
-#define VM_MAYWRITE	0x00000020
-#define VM_MAYEXEC	0x00000040
-#define VM_MAYSHARE	0x00000080
-
-#define VM_GROWSDOWN	0x00000100	/* general info on the segment */
-#define VM_GROWSUP	0x00000200
-#define VM_SHM		0x00000400	/* shared memory area, don't swap out */
-#define VM_DENYWRITE	0x00000800	/* ETXTBSY on write attempts.. */
-
-#define VM_EXECUTABLE	0x00001000
-#define VM_LOCKED	0x00002000
-#define VM_IO           0x00004000	/* Memory mapped I/O or similar */
-
-					/* Used by sys_madvise() */
-#define VM_SEQ_READ	0x00008000	/* App will access data sequentially */
-#define VM_RAND_READ	0x00010000	/* App will not benefit from clustered reads */
-
-#define VM_DONTCOPY	0x00020000      /* Do not copy this vma on fork */
-#define VM_DONTEXPAND	0x00040000	/* Cannot expand with mremap() */
-#define VM_RESERVED	0x00080000	/* Don't unmap it from swap_out */
-#define VM_ACCOUNT	0x00100000	/* Is a VM accounted object */
-#define VM_HUGETLB	0x00400000	/* Huge TLB Page VM */
-#define VM_NONLINEAR	0x00800000	/* Is non-linear (remap_file_pages) */
-
-#ifndef VM_STACK_DEFAULT_FLAGS		/* arch can override this */
-#define VM_STACK_DEFAULT_FLAGS VM_DATA_DEFAULT_FLAGS
-#endif
-
-#ifdef CONFIG_STACK_GROWSUP
-#define VM_STACK_FLAGS	(VM_GROWSUP | VM_STACK_DEFAULT_FLAGS | VM_ACCOUNT)
-#else
-#define VM_STACK_FLAGS	(VM_GROWSDOWN | VM_STACK_DEFAULT_FLAGS | VM_ACCOUNT)
-#endif
-
-#define VM_READHINTMASK			(VM_SEQ_READ | VM_RAND_READ)
-#define VM_ClearReadHint(v)		(v)->vm_flags &= ~VM_READHINTMASK
-#define VM_NormalReadHint(v)		(!((v)->vm_flags & VM_READHINTMASK))
-#define VM_SequentialReadHint(v)	((v)->vm_flags & VM_SEQ_READ)
-#define VM_RandomReadHint(v)		((v)->vm_flags & VM_RAND_READ)
-
-/*
  * mapping from the currently active vm_flags protection bits (the
  * low four bits) to a page protection mask..
  */
@@ -187,58 +79,6 @@ struct vm_operations_struct {
 struct mmu_gather;
 struct inode;
 
-#ifdef ARCH_HAS_ATOMIC_UNSIGNED
-typedef unsigned page_flags_t;
-#else
-typedef unsigned long page_flags_t;
-#endif
-
-/*
- * Each physical page in the system has a struct page associated with
- * it to keep track of whatever it is we are using the page for at the
- * moment. Note that we have no way to track which tasks are using
- * a page.
- */
-struct page {
-	page_flags_t flags;		/* Atomic flags, some possibly
-					 * updated asynchronously */
-	atomic_t _count;		/* Usage count, see below. */
-	atomic_t _mapcount;		/* Count of ptes mapped in mms,
-					 * to show when page is mapped
-					 * & limit reverse map searches.
-					 */
-	unsigned long private;		/* Mapping-private opaque data:
-					 * usually used for buffer_heads
-					 * if PagePrivate set; used for
-					 * swp_entry_t if PageSwapCache
-					 */
-	struct address_space *mapping;	/* If low bit clear, points to
-					 * inode address_space, or NULL.
-					 * If page mapped as anonymous
-					 * memory, low bit is set, and
-					 * it points to anon_vma object:
-					 * see PAGE_MAPPING_ANON below.
-					 */
-	pgoff_t index;			/* Our offset within mapping. */
-	struct list_head lru;		/* Pageout list, eg. active_list
-					 * protected by zone->lru_lock !
-					 */
-	/*
-	 * On machines where all RAM is mapped into kernel address space,
-	 * we can simply calculate the virtual address. On machines with
-	 * highmem some memory is mapped into kernel virtual memory
-	 * dynamically, so we need a place to store that address.
-	 * Note that this field could be 16 bits on x86 ... ;)
-	 *
-	 * Architectures with slow multiplication can define
-	 * WANT_PAGE_VIRTUAL in asm/page.h
-	 */
-#if defined(WANT_PAGE_VIRTUAL)
-	void *virtual;			/* Kernel virtual address (NULL if
-					   not kmapped, ie. highmem) */
-#endif /* WANT_PAGE_VIRTUAL */
-};
-
 /*
  * FIXME: take this include out, include page-flags.h in
  * files which need it (119 of them)
@@ -372,38 +212,6 @@ static inline void put_page(struct page 
  *   to swap space and (later) to be read back into memory.
  */
 
-/*
- * The zone field is never updated after free_area_init_core()
- * sets it, so none of the operations on it need to be atomic.
- * We'll have up to (MAX_NUMNODES * MAX_NR_ZONES) zones total,
- * so we use (MAX_NODES_SHIFT + MAX_ZONES_SHIFT) here to get enough bits.
- */
-#define NODEZONE_SHIFT (sizeof(page_flags_t)*8 - MAX_NODES_SHIFT - MAX_ZONES_SHIFT)
-#define NODEZONE(node, zone)	((node << ZONES_SHIFT) | zone)
-
-static inline unsigned long page_zonenum(struct page *page)
-{
-	return (page->flags >> NODEZONE_SHIFT) & (~(~0UL << ZONES_SHIFT));
-}
-static inline unsigned long page_to_nid(struct page *page)
-{
-	return (page->flags >> (NODEZONE_SHIFT + ZONES_SHIFT));
-}
-
-struct zone;
-extern struct zone *zone_table[];
-
-static inline struct zone *page_zone(struct page *page)
-{
-	return zone_table[page->flags >> NODEZONE_SHIFT];
-}
-
-static inline void set_page_zone(struct page *page, unsigned long nodezone_num)
-{
-	page->flags &= ~(~0UL << NODEZONE_SHIFT);
-	page->flags |= nodezone_num << NODEZONE_SHIFT;
-}
-
 #ifndef CONFIG_DISCONTIGMEM
 /* The array of struct pages - for discontigmem use pgdat->lmem_map */
 extern struct page *mem_map;
Index: linux-mm/include/asm/page.h
===================================================================
--- linux-mm.orig/include/asm/page.h	2004-12-17 12:01:48.000000000 +0100
+++ linux-mm/include/asm/page.h	2004-12-20 21:36:42.718846346 +0100
@@ -133,14 +133,6 @@ extern int sysctl_legacy_va_layout;
 #define __pa(x)			((unsigned long)(x)-PAGE_OFFSET)
 #define __va(x)			((void *)((unsigned long)(x)+PAGE_OFFSET))
 #define pfn_to_kaddr(pfn)      __va((pfn) << PAGE_SHIFT)
-#ifndef CONFIG_DISCONTIGMEM
-#define pfn_to_page(pfn)	(mem_map + (pfn))
-#define page_to_pfn(page)	((unsigned long)((page) - mem_map))
-#define pfn_valid(pfn)		((pfn) < max_mapnr)
-#endif /* !CONFIG_DISCONTIGMEM */
-#define virt_to_page(kaddr)	pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
-
-#define virt_addr_valid(kaddr)	pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
 
 #define VM_DATA_DEFAULT_FLAGS \
 	(VM_READ | VM_WRITE | \

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Roman Zippel]

Hi,

On Mon, 20 Dec 2004, Dave Hansen wrote:

> But, what do you think we should do about the current #defines in
> asm/mmzone.h, like pfn_to_page()?  Would it be feasible to put them in
> another header that can use proper functions?

It should be possible to move it to asm/pgtable.h, although asm/memory.h 
(as arm already has) would be a good alternative.
struct page can be moved to linux/mmzone.h and and asm/mmzone.h should die 
rather soon. It was mistake to have definitions of inline functions/macros 
in two different files (asm/page.h and asm/mmzone.h).
Below is a patch which moves thing around so pfn_to_page/page_to_pfn can 
become inline functions.

bye, Roman

 asm/mmzone.h   |   27 -----
 asm/page.h     |    8 -
 asm/pgtable.h  |   44 +++++++++
 linux/mm.h     |  192 ----------------------------------------
 linux/mmzone.h |  272 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 5 files changed, 274 insertions(+), 269 deletions(-)

Index: linux-mm/include/linux/mmzone.h
===================================================================
--- linux-mm.orig/include/linux/mmzone.h	2004-12-17 12:01:55.000000000 +0100
+++ linux-mm/include/linux/mmzone.h	2004-12-20 21:39:26.513594537 +0100
@@ -11,7 +11,168 @@
 #include <linux/cache.h>
 #include <linux/threads.h>
 #include <linux/numa.h>
-#include <asm/atomic.h>
+#include <linux/rbtree.h>
+#include <linux/prio_tree.h>
+
+#ifdef ARCH_HAS_ATOMIC_UNSIGNED
+typedef unsigned page_flags_t;
+#else
+typedef unsigned long page_flags_t;
+#endif
+
+/*
+ * Each physical page in the system has a struct page associated with
+ * it to keep track of whatever it is we are using the page for at the
+ * moment. Note that we have no way to track which tasks are using
+ * a page.
+ */
+struct page {
+	page_flags_t flags;		/* Atomic flags, some possibly
+					 * updated asynchronously */
+	atomic_t _count;		/* Usage count, see below. */
+	atomic_t _mapcount;		/* Count of ptes mapped in mms,
+					 * to show when page is mapped
+					 * & limit reverse map searches.
+					 */
+	unsigned long private;		/* Mapping-private opaque data:
+					 * usually used for buffer_heads
+					 * if PagePrivate set; used for
+					 * swp_entry_t if PageSwapCache
+					 */
+	struct address_space *mapping;	/* If low bit clear, points to
+					 * inode address_space, or NULL.
+					 * If page mapped as anonymous
+					 * memory, low bit is set, and
+					 * it points to anon_vma object:
+					 * see PAGE_MAPPING_ANON below.
+					 */
+	pgoff_t index;			/* Our offset within mapping. */
+	struct list_head lru;		/* Pageout list, eg. active_list
+					 * protected by zone->lru_lock !
+					 */
+	/*
+	 * On machines where all RAM is mapped into kernel address space,
+	 * we can simply calculate the virtual address. On machines with
+	 * highmem some memory is mapped into kernel virtual memory
+	 * dynamically, so we need a place to store that address.
+	 * Note that this field could be 16 bits on x86 ... ;)
+	 *
+	 * Architectures with slow multiplication can define
+	 * WANT_PAGE_VIRTUAL in asm/page.h
+	 */
+#if defined(WANT_PAGE_VIRTUAL)
+	void *virtual;			/* Kernel virtual address (NULL if
+					   not kmapped, ie. highmem) */
+#endif /* WANT_PAGE_VIRTUAL */
+};
+
+/*
+ * This struct defines a memory VMM memory area. There is one of these
+ * per VM-area/task.  A VM area is any part of the process virtual memory
+ * space that has a special rule for the page-fault handlers (ie a shared
+ * library, the executable area etc).
+ */
+struct vm_area_struct {
+	struct mm_struct * vm_mm;	/* The address space we belong to. */
+	unsigned long vm_start;		/* Our start address within vm_mm. */
+	unsigned long vm_end;		/* The first byte after our end address
+					   within vm_mm. */
+
+	/* linked list of VM areas per task, sorted by address */
+	struct vm_area_struct *vm_next;
+
+	pgprot_t vm_page_prot;		/* Access permissions of this VMA. */
+	unsigned long vm_flags;		/* Flags, listed below. */
+
+	struct rb_node vm_rb;
+
+	/*
+	 * For areas with an address space and backing store,
+	 * linkage into the address_space->i_mmap prio tree, or
+	 * linkage to the list of like vmas hanging off its node, or
+	 * linkage of vma in the address_space->i_mmap_nonlinear list.
+	 */
+	union {
+		struct {
+			struct list_head list;
+			void *parent;	/* aligns with prio_tree_node parent */
+			struct vm_area_struct *head;
+		} vm_set;
+
+		struct prio_tree_node prio_tree_node;
+	} shared;
+
+	/*
+	 * A file's MAP_PRIVATE vma can be in both i_mmap tree and anon_vma
+	 * list, after a COW of one of the file pages.  A MAP_SHARED vma
+	 * can only be in the i_mmap tree.  An anonymous MAP_PRIVATE, stack
+	 * or brk vma (with NULL file) can only be in an anon_vma list.
+	 */
+	struct list_head anon_vma_node;	/* Serialized by anon_vma->lock */
+	struct anon_vma *anon_vma;	/* Serialized by page_table_lock */
+
+	/* Function pointers to deal with this struct. */
+	struct vm_operations_struct * vm_ops;
+
+	/* Information about our backing store: */
+	unsigned long vm_pgoff;		/* Offset (within vm_file) in PAGE_SIZE
+					   units, *not* PAGE_CACHE_SIZE */
+	struct file * vm_file;		/* File we map to (can be NULL). */
+	void * vm_private_data;		/* was vm_pte (shared mem) */
+
+#ifdef CONFIG_NUMA
+	struct mempolicy *vm_policy;	/* NUMA policy for the VMA */
+#endif
+};
+
+/*
+ * vm_flags..
+ */
+#define VM_READ		0x00000001	/* currently active flags */
+#define VM_WRITE	0x00000002
+#define VM_EXEC		0x00000004
+#define VM_SHARED	0x00000008
+
+#define VM_MAYREAD	0x00000010	/* limits for mprotect() etc */
+#define VM_MAYWRITE	0x00000020
+#define VM_MAYEXEC	0x00000040
+#define VM_MAYSHARE	0x00000080
+
+#define VM_GROWSDOWN	0x00000100	/* general info on the segment */
+#define VM_GROWSUP	0x00000200
+#define VM_SHM		0x00000400	/* shared memory area, don't swap out */
+#define VM_DENYWRITE	0x00000800	/* ETXTBSY on write attempts.. */
+
+#define VM_EXECUTABLE	0x00001000
+#define VM_LOCKED	0x00002000
+#define VM_IO           0x00004000	/* Memory mapped I/O or similar */
+
+					/* Used by sys_madvise() */
+#define VM_SEQ_READ	0x00008000	/* App will access data sequentially */
+#define VM_RAND_READ	0x00010000	/* App will not benefit from clustered reads */
+
+#define VM_DONTCOPY	0x00020000      /* Do not copy this vma on fork */
+#define VM_DONTEXPAND	0x00040000	/* Cannot expand with mremap() */
+#define VM_RESERVED	0x00080000	/* Don't unmap it from swap_out */
+#define VM_ACCOUNT	0x00100000	/* Is a VM accounted object */
+#define VM_HUGETLB	0x00400000	/* Huge TLB Page VM */
+#define VM_NONLINEAR	0x00800000	/* Is non-linear (remap_file_pages) */
+
+#ifndef VM_STACK_DEFAULT_FLAGS		/* arch can override this */
+#define VM_STACK_DEFAULT_FLAGS VM_DATA_DEFAULT_FLAGS
+#endif
+
+#ifdef CONFIG_STACK_GROWSUP
+#define VM_STACK_FLAGS	(VM_GROWSUP | VM_STACK_DEFAULT_FLAGS | VM_ACCOUNT)
+#else
+#define VM_STACK_FLAGS	(VM_GROWSDOWN | VM_STACK_DEFAULT_FLAGS | VM_ACCOUNT)
+#endif
+
+#define VM_READHINTMASK			(VM_SEQ_READ | VM_RAND_READ)
+#define VM_ClearReadHint(v)		(v)->vm_flags &= ~VM_READHINTMASK
+#define VM_NormalReadHint(v)		(!((v)->vm_flags & VM_READHINTMASK))
+#define VM_SequentialReadHint(v)	((v)->vm_flags & VM_SEQ_READ)
+#define VM_RandomReadHint(v)		((v)->vm_flags & VM_RAND_READ)
 
 /* Free memory management - zoned buddy allocator.  */
 #ifndef CONFIG_FORCE_MAX_ZONEORDER
@@ -25,8 +186,6 @@ struct free_area {
 	unsigned long		*map;
 };
 
-struct pglist_data;
-
 /*
  * zone->lock and zone->lru_lock are two of the hottest locks in the kernel.
  * So add a wild amount of padding here to ensure that they fall into separate
@@ -238,6 +397,74 @@ struct zonelist {
 	struct zone *zones[MAX_NUMNODES * MAX_NR_ZONES + 1]; // NULL delimited
 };
 
+#ifndef CONFIG_DISCONTIGMEM
+
+extern struct pglist_data contig_page_data;
+#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_DISCONTIGMEM */
+
+#include <asm/mmzone.h>
+
+#if BITS_PER_LONG == 32 || defined(ARCH_HAS_ATOMIC_UNSIGNED)
+/*
+ * with 32 bit page->flags field, we reserve 8 bits for node/zone info.
+ * there are 3 zones (2 bits) and this leaves 8-2=6 bits for nodes.
+ */
+#define MAX_NODES_SHIFT		6
+#elif BITS_PER_LONG == 64
+/*
+ * with 64 bit flags field, there's plenty of room.
+ */
+#define MAX_NODES_SHIFT		10
+#endif
+
+#endif /* !CONFIG_DISCONTIGMEM */
+
+#if NODES_SHIFT > MAX_NODES_SHIFT
+#error NODES_SHIFT > MAX_NODES_SHIFT
+#endif
+
+/* There are currently 3 zones: DMA, Normal & Highmem, thus we need 2 bits */
+#define MAX_ZONES_SHIFT		2
+
+#if ZONES_SHIFT > MAX_ZONES_SHIFT
+#error ZONES_SHIFT > MAX_ZONES_SHIFT
+#endif
+
+/*
+ * The zone field is never updated after free_area_init_core()
+ * sets it, so none of the operations on it need to be atomic.
+ * We'll have up to (MAX_NUMNODES * MAX_NR_ZONES) zones total,
+ * so we use (MAX_NODES_SHIFT + MAX_ZONES_SHIFT) here to get enough bits.
+ */
+#define NODEZONE_SHIFT (sizeof(page_flags_t)*8 - MAX_NODES_SHIFT - MAX_ZONES_SHIFT)
+#define NODEZONE(node, zone)	((node << ZONES_SHIFT) | zone)
+
+static inline unsigned long page_zonenum(struct page *page)
+{
+	return (page->flags >> NODEZONE_SHIFT) & (~(~0UL << ZONES_SHIFT));
+}
+static inline unsigned long page_to_nid(struct page *page)
+{
+	return (page->flags >> (NODEZONE_SHIFT + ZONES_SHIFT));
+}
+
+static inline void set_page_zone(struct page *page, unsigned long nodezone_num)
+{
+	page->flags &= ~(~0UL << NODEZONE_SHIFT);
+	page->flags |= nodezone_num << NODEZONE_SHIFT;
+}
+
+extern struct zone *zone_table[];
+
+static inline struct zone *page_zone(struct page *page)
+{
+	return zone_table[page->flags >> NODEZONE_SHIFT];
+}
 
 /*
  * The pg_data_t structure is used in machines with CONFIG_DISCONTIGMEM
@@ -369,48 +596,9 @@ int min_free_kbytes_sysctl_handler(struc
 int lower_zone_protection_sysctl_handler(struct ctl_table *, int, struct file *,
 					void __user *, size_t *, loff_t *);
 
-#include <linux/topology.h>
 /* Returns the number of the current Node. */
 #define numa_node_id()		(cpu_to_node(smp_processor_id()))
 
-#ifndef CONFIG_DISCONTIGMEM
-
-extern struct pglist_data contig_page_data;
-#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_DISCONTIGMEM */
-
-#include <asm/mmzone.h>
-
-#if BITS_PER_LONG == 32 || defined(ARCH_HAS_ATOMIC_UNSIGNED)
-/*
- * with 32 bit page->flags field, we reserve 8 bits for node/zone info.
- * there are 3 zones (2 bits) and this leaves 8-2=6 bits for nodes.
- */
-#define MAX_NODES_SHIFT		6
-#elif BITS_PER_LONG == 64
-/*
- * with 64 bit flags field, there's plenty of room.
- */
-#define MAX_NODES_SHIFT		10
-#endif
-
-#endif /* !CONFIG_DISCONTIGMEM */
-
-#if NODES_SHIFT > MAX_NODES_SHIFT
-#error NODES_SHIFT > MAX_NODES_SHIFT
-#endif
-
-/* There are currently 3 zones: DMA, Normal & Highmem, thus we need 2 bits */
-#define MAX_ZONES_SHIFT		2
-
-#if ZONES_SHIFT > MAX_ZONES_SHIFT
-#error ZONES_SHIFT > MAX_ZONES_SHIFT
-#endif
-
 #endif /* !__ASSEMBLY__ */
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MMZONE_H */
Index: linux-mm/include/asm/mmzone.h
===================================================================
--- linux-mm.orig/include/asm/mmzone.h	2004-07-03 19:02:16.000000000 +0200
+++ linux-mm/include/asm/mmzone.h	2004-12-20 21:36:42.708848071 +0100
@@ -102,35 +102,8 @@ static inline struct pglist_data *pfn_to
 /* XXX: FIXME -- wli */
 #define kern_addr_valid(kaddr)	(0)
 
-#define pfn_to_page(pfn)						\
-({									\
-	unsigned long __pfn = pfn;					\
-	int __node  = pfn_to_nid(__pfn);				\
-	&node_mem_map(__node)[node_localnr(__pfn,__node)];		\
-})
-
-#define page_to_pfn(pg)							\
-({									\
-	struct page *__page = pg;					\
-	struct zone *__zone = page_zone(__page);			\
-	(unsigned long)(__page - __zone->zone_mem_map)			\
-		+ __zone->zone_start_pfn;				\
-})
 #define pmd_page(pmd)		(pfn_to_page(pmd_val(pmd) >> PAGE_SHIFT))
 
-#ifdef CONFIG_X86_NUMAQ            /* we have contiguous memory on NUMA-Q */
-#define pfn_valid(pfn)          ((pfn) < num_physpages)
-#else
-static inline int pfn_valid(int pfn)
-{
-	int nid = pfn_to_nid(pfn);
-
-	if (nid >= 0)
-		return (pfn < node_end_pfn(nid));
-	return 0;
-}
-#endif
-
 extern int get_memcfg_numa_flat(void );
 /*
  * This allows any one NUMA architecture to be compiled
Index: linux-mm/include/asm/pgtable.h
===================================================================
--- linux-mm.orig/include/asm/pgtable.h	2004-10-20 17:28:49.000000000 +0200
+++ linux-mm/include/asm/pgtable.h	2004-12-20 21:36:42.717846519 +0100
@@ -25,6 +25,50 @@
 #include <linux/list.h>
 #include <linux/spinlock.h>
 
+#ifndef CONFIG_DISCONTIGMEM
+
+static inline struct page *pfn_to_page(unsigned long pfn)
+{
+	return mem_map + pfn;
+}
+static inline unsigned long page_to_pfn(struct page *page)
+{
+	return page - mem_map;
+}
+
+#define pfn_valid(pfn)		((pfn) < max_mapnr)
+
+#else
+
+static inline struct page *pfn_to_page(unsigned long pfn)
+{
+	int node = pfn_to_nid(pfn);
+	return &node_mem_map(node)[node_localnr(pfn, node)];
+}
+static inline unsigned long page_to_pfn(struct page *page)
+{
+	struct zone *zone = page_zone(page);
+	return zone->zone_start_pfn + (page - zone->zone_mem_map);
+}
+
+#ifdef CONFIG_X86_NUMAQ            /* we have contiguous memory on NUMA-Q */
+#define pfn_valid(pfn)          ((pfn) < num_physpages)
+#else
+static inline int pfn_valid(int pfn)
+{
+	int nid = pfn_to_nid(pfn);
+
+	if (nid >= 0)
+		return (pfn < node_end_pfn(nid));
+	return 0;
+}
+#endif
+
+#endif /* !CONFIG_DISCONTIGMEM */
+#define virt_to_page(kaddr)	pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
+
+#define virt_addr_valid(kaddr)	pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
+
 /*
  * ZERO_PAGE is a global shared page that is always zero: used
  * for zero-mapped memory areas etc..
Index: linux-mm/include/linux/mm.h
===================================================================
--- linux-mm.orig/include/linux/mm.h	2004-12-17 12:01:55.000000000 +0100
+++ linux-mm/include/linux/mm.h	2004-12-20 21:36:42.718846346 +0100
@@ -53,114 +53,6 @@ extern int sysctl_legacy_va_layout;
  */
 
 /*
- * This struct defines a memory VMM memory area. There is one of these
- * per VM-area/task.  A VM area is any part of the process virtual memory
- * space that has a special rule for the page-fault handlers (ie a shared
- * library, the executable area etc).
- */
-struct vm_area_struct {
-	struct mm_struct * vm_mm;	/* The address space we belong to. */
-	unsigned long vm_start;		/* Our start address within vm_mm. */
-	unsigned long vm_end;		/* The first byte after our end address
-					   within vm_mm. */
-
-	/* linked list of VM areas per task, sorted by address */
-	struct vm_area_struct *vm_next;
-
-	pgprot_t vm_page_prot;		/* Access permissions of this VMA. */
-	unsigned long vm_flags;		/* Flags, listed below. */
-
-	struct rb_node vm_rb;
-
-	/*
-	 * For areas with an address space and backing store,
-	 * linkage into the address_space->i_mmap prio tree, or
-	 * linkage to the list of like vmas hanging off its node, or
-	 * linkage of vma in the address_space->i_mmap_nonlinear list.
-	 */
-	union {
-		struct {
-			struct list_head list;
-			void *parent;	/* aligns with prio_tree_node parent */
-			struct vm_area_struct *head;
-		} vm_set;
-
-		struct prio_tree_node prio_tree_node;
-	} shared;
-
-	/*
-	 * A file's MAP_PRIVATE vma can be in both i_mmap tree and anon_vma
-	 * list, after a COW of one of the file pages.  A MAP_SHARED vma
-	 * can only be in the i_mmap tree.  An anonymous MAP_PRIVATE, stack
-	 * or brk vma (with NULL file) can only be in an anon_vma list.
-	 */
-	struct list_head anon_vma_node;	/* Serialized by anon_vma->lock */
-	struct anon_vma *anon_vma;	/* Serialized by page_table_lock */
-
-	/* Function pointers to deal with this struct. */
-	struct vm_operations_struct * vm_ops;
-
-	/* Information about our backing store: */
-	unsigned long vm_pgoff;		/* Offset (within vm_file) in PAGE_SIZE
-					   units, *not* PAGE_CACHE_SIZE */
-	struct file * vm_file;		/* File we map to (can be NULL). */
-	void * vm_private_data;		/* was vm_pte (shared mem) */
-
-#ifdef CONFIG_NUMA
-	struct mempolicy *vm_policy;	/* NUMA policy for the VMA */
-#endif
-};
-
-/*
- * vm_flags..
- */
-#define VM_READ		0x00000001	/* currently active flags */
-#define VM_WRITE	0x00000002
-#define VM_EXEC		0x00000004
-#define VM_SHARED	0x00000008
-
-#define VM_MAYREAD	0x00000010	/* limits for mprotect() etc */
-#define VM_MAYWRITE	0x00000020
-#define VM_MAYEXEC	0x00000040
-#define VM_MAYSHARE	0x00000080
-
-#define VM_GROWSDOWN	0x00000100	/* general info on the segment */
-#define VM_GROWSUP	0x00000200
-#define VM_SHM		0x00000400	/* shared memory area, don't swap out */
-#define VM_DENYWRITE	0x00000800	/* ETXTBSY on write attempts.. */
-
-#define VM_EXECUTABLE	0x00001000
-#define VM_LOCKED	0x00002000
-#define VM_IO           0x00004000	/* Memory mapped I/O or similar */
-
-					/* Used by sys_madvise() */
-#define VM_SEQ_READ	0x00008000	/* App will access data sequentially */
-#define VM_RAND_READ	0x00010000	/* App will not benefit from clustered reads */
-
-#define VM_DONTCOPY	0x00020000      /* Do not copy this vma on fork */
-#define VM_DONTEXPAND	0x00040000	/* Cannot expand with mremap() */
-#define VM_RESERVED	0x00080000	/* Don't unmap it from swap_out */
-#define VM_ACCOUNT	0x00100000	/* Is a VM accounted object */
-#define VM_HUGETLB	0x00400000	/* Huge TLB Page VM */
-#define VM_NONLINEAR	0x00800000	/* Is non-linear (remap_file_pages) */
-
-#ifndef VM_STACK_DEFAULT_FLAGS		/* arch can override this */
-#define VM_STACK_DEFAULT_FLAGS VM_DATA_DEFAULT_FLAGS
-#endif
-
-#ifdef CONFIG_STACK_GROWSUP
-#define VM_STACK_FLAGS	(VM_GROWSUP | VM_STACK_DEFAULT_FLAGS | VM_ACCOUNT)
-#else
-#define VM_STACK_FLAGS	(VM_GROWSDOWN | VM_STACK_DEFAULT_FLAGS | VM_ACCOUNT)
-#endif
-
-#define VM_READHINTMASK			(VM_SEQ_READ | VM_RAND_READ)
-#define VM_ClearReadHint(v)		(v)->vm_flags &= ~VM_READHINTMASK
-#define VM_NormalReadHint(v)		(!((v)->vm_flags & VM_READHINTMASK))
-#define VM_SequentialReadHint(v)	((v)->vm_flags & VM_SEQ_READ)
-#define VM_RandomReadHint(v)		((v)->vm_flags & VM_RAND_READ)
-
-/*
  * mapping from the currently active vm_flags protection bits (the
  * low four bits) to a page protection mask..
  */
@@ -187,58 +79,6 @@ struct vm_operations_struct {
 struct mmu_gather;
 struct inode;
 
-#ifdef ARCH_HAS_ATOMIC_UNSIGNED
-typedef unsigned page_flags_t;
-#else
-typedef unsigned long page_flags_t;
-#endif
-
-/*
- * Each physical page in the system has a struct page associated with
- * it to keep track of whatever it is we are using the page for at the
- * moment. Note that we have no way to track which tasks are using
- * a page.
- */
-struct page {
-	page_flags_t flags;		/* Atomic flags, some possibly
-					 * updated asynchronously */
-	atomic_t _count;		/* Usage count, see below. */
-	atomic_t _mapcount;		/* Count of ptes mapped in mms,
-					 * to show when page is mapped
-					 * & limit reverse map searches.
-					 */
-	unsigned long private;		/* Mapping-private opaque data:
-					 * usually used for buffer_heads
-					 * if PagePrivate set; used for
-					 * swp_entry_t if PageSwapCache
-					 */
-	struct address_space *mapping;	/* If low bit clear, points to
-					 * inode address_space, or NULL.
-					 * If page mapped as anonymous
-					 * memory, low bit is set, and
-					 * it points to anon_vma object:
-					 * see PAGE_MAPPING_ANON below.
-					 */
-	pgoff_t index;			/* Our offset within mapping. */
-	struct list_head lru;		/* Pageout list, eg. active_list
-					 * protected by zone->lru_lock !
-					 */
-	/*
-	 * On machines where all RAM is mapped into kernel address space,
-	 * we can simply calculate the virtual address. On machines with
-	 * highmem some memory is mapped into kernel virtual memory
-	 * dynamically, so we need a place to store that address.
-	 * Note that this field could be 16 bits on x86 ... ;)
-	 *
-	 * Architectures with slow multiplication can define
-	 * WANT_PAGE_VIRTUAL in asm/page.h
-	 */
-#if defined(WANT_PAGE_VIRTUAL)
-	void *virtual;			/* Kernel virtual address (NULL if
-					   not kmapped, ie. highmem) */
-#endif /* WANT_PAGE_VIRTUAL */
-};
-
 /*
  * FIXME: take this include out, include page-flags.h in
  * files which need it (119 of them)
@@ -372,38 +212,6 @@ static inline void put_page(struct page 
  *   to swap space and (later) to be read back into memory.
  */
 
-/*
- * The zone field is never updated after free_area_init_core()
- * sets it, so none of the operations on it need to be atomic.
- * We'll have up to (MAX_NUMNODES * MAX_NR_ZONES) zones total,
- * so we use (MAX_NODES_SHIFT + MAX_ZONES_SHIFT) here to get enough bits.
- */
-#define NODEZONE_SHIFT (sizeof(page_flags_t)*8 - MAX_NODES_SHIFT - MAX_ZONES_SHIFT)
-#define NODEZONE(node, zone)	((node << ZONES_SHIFT) | zone)
-
-static inline unsigned long page_zonenum(struct page *page)
-{
-	return (page->flags >> NODEZONE_SHIFT) & (~(~0UL << ZONES_SHIFT));
-}
-static inline unsigned long page_to_nid(struct page *page)
-{
-	return (page->flags >> (NODEZONE_SHIFT + ZONES_SHIFT));
-}
-
-struct zone;
-extern struct zone *zone_table[];
-
-static inline struct zone *page_zone(struct page *page)
-{
-	return zone_table[page->flags >> NODEZONE_SHIFT];
-}
-
-static inline void set_page_zone(struct page *page, unsigned long nodezone_num)
-{
-	page->flags &= ~(~0UL << NODEZONE_SHIFT);
-	page->flags |= nodezone_num << NODEZONE_SHIFT;
-}
-
 #ifndef CONFIG_DISCONTIGMEM
 /* The array of struct pages - for discontigmem use pgdat->lmem_map */
 extern struct page *mem_map;
Index: linux-mm/include/asm/page.h
===================================================================
--- linux-mm.orig/include/asm/page.h	2004-12-17 12:01:48.000000000 +0100
+++ linux-mm/include/asm/page.h	2004-12-20 21:36:42.718846346 +0100
@@ -133,14 +133,6 @@ extern int sysctl_legacy_va_layout;
 #define __pa(x)			((unsigned long)(x)-PAGE_OFFSET)
 #define __va(x)			((void *)((unsigned long)(x)+PAGE_OFFSET))
 #define pfn_to_kaddr(pfn)      __va((pfn) << PAGE_SHIFT)
-#ifndef CONFIG_DISCONTIGMEM
-#define pfn_to_page(pfn)	(mem_map + (pfn))
-#define page_to_pfn(page)	((unsigned long)((page) - mem_map))
-#define pfn_valid(pfn)		((pfn) < max_mapnr)
-#endif /* !CONFIG_DISCONTIGMEM */
-#define virt_to_page(kaddr)	pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
-
-#define virt_addr_valid(kaddr)	pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
 
 #define VM_DATA_DEFAULT_FLAGS \
 	(VM_READ | VM_WRITE | \
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

On Thu, 2004-12-16 at 16:51, Roman Zippel wrote:
> Could you explain a bit more, what exactly the problem is?

Maybe I should also say that this doesn't fix any bugs.  It simply makes
the headers easier to work with.  I've been doing a lot of work in those
headers for the memory hotplug effort, and I think that this is a
worthwhile cleanup effort.  Plus, it will make my memory hotplug patches
look a little less crazy :)

-- Dave

Re: [patch] [RFC] make WANT_PAGE_VIRTUAL a config option

[Dave Hansen]

On Thu, 2004-12-16 at 16:51, Roman Zippel wrote:
> Could you explain a bit more, what exactly the problem is?

Maybe I should also say that this doesn't fix any bugs.  It simply makes
the headers easier to work with.  I've been doing a lot of work in those
headers for the memory hotplug effort, and I think that this is a
worthwhile cleanup effort.  Plus, it will make my memory hotplug patches
look a little less crazy :)

-- Dave

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