Powerpc - Include pagemap.h in asm/powerpc/tlb.h

Powerpc - Include pagemap.h in asm/powerpc/tlb.h

[Mathieu Desnoyers]

Powerpc - Include pagemap.h in asm/powerpc/tlb.h

Fixes this powerpc build error in 2.6.22-rc6-mm1 for powerpc 64 :

In file included from include2/asm/tlb.h:60,
                 from /home/compudj/git/linux-2.6-lttng/arch/powerpc/mm/init_64.
c:56:
/home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h: In function 'tlb_fl
ush_mmu':
/home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h:76: error: implicit 
declaration of function 'release_pages'
/home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h: In function 'tlb_re
move_page':
/home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h:105: error: implicit
 declaration of function 'page_cache_release'
make[2]: *** [arch/powerpc/mm/init_64.o] Error 1

release_pages is declared in linux/pagemap.h, but cannot be included in
linux/swap.h because of a sparc related comment:

/* only sparc can not include linux/pagemap.h in this file
 * so leave page_cache_release and release_pages undeclared... */
#define free_page_and_swap_cache(page) \
        page_cache_release(page)
#define free_pages_and_swap_cache(pages, nr) \
        release_pages((pages), (nr), 0);

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: linuxppc-dev@ozlabs.org
CC: Paul Mackerras <paulus@samba.org>
---
 include/asm-powerpc/tlb.h |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6-lttng/include/asm-powerpc/tlb.h
===================================================================
--- linux-2.6-lttng.orig/include/asm-powerpc/tlb.h	2007-07-13 11:30:54.000000000 -0400
+++ linux-2.6-lttng/include/asm-powerpc/tlb.h	2007-07-13 11:31:22.000000000 -0400
@@ -23,6 +23,8 @@
 #include <asm/mmu.h>
 #endif
 
+#include <linux/pagemap.h>
+
 struct mmu_gather;
 
 #define tlb_start_vma(tlb, vma)	do { } while (0)
-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: Powerpc - Include pagemap.h in asm/powerpc/tlb.h

[Andrew Morton]

On Fri, 13 Jul 2007 21:20:33 -0400 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:

> Powerpc - Include pagemap.h in asm/powerpc/tlb.h
> 
> Fixes this powerpc build error in 2.6.22-rc6-mm1 for powerpc 64 :
> 
> In file included from include2/asm/tlb.h:60,
>                  from /home/compudj/git/linux-2.6-lttng/arch/powerpc/mm/init_64.
> c:56:
> /home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h: In function 'tlb_fl
> ush_mmu':
> /home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h:76: error: implicit 
> declaration of function 'release_pages'
> /home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h: In function 'tlb_re
> move_page':
> /home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h:105: error: implicit
>  declaration of function 'page_cache_release'

You have some wordwrapping going on there.

> make[2]: *** [arch/powerpc/mm/init_64.o] Error 1
> 
> release_pages is declared in linux/pagemap.h, but cannot be included in
> linux/swap.h because of a sparc related comment:
> 
> /* only sparc can not include linux/pagemap.h in this file
>  * so leave page_cache_release and release_pages undeclared... */
> #define free_page_and_swap_cache(page) \
>         page_cache_release(page)
> #define free_pages_and_swap_cache(pages, nr) \
>         release_pages((pages), (nr), 0);

It's always a worry when this happens.  What change made us need this
inclusion?  How come you're hitting it but I (and test.kernel.org, at least)
did not?  How come so few other architectures include pagemap.h from
asm/tlb.h?  Why do header files get into such a mess?


> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> CC: linuxppc-dev@ozlabs.org
> CC: Paul Mackerras <paulus@samba.org>
> ---
>  include/asm-powerpc/tlb.h |    2 ++
>  1 file changed, 2 insertions(+)
> 
> Index: linux-2.6-lttng/include/asm-powerpc/tlb.h
> ===================================================================
> --- linux-2.6-lttng.orig/include/asm-powerpc/tlb.h	2007-07-13 11:30:54.000000000 -0400
> +++ linux-2.6-lttng/include/asm-powerpc/tlb.h	2007-07-13 11:31:22.000000000 -0400
> @@ -23,6 +23,8 @@
>  #include <asm/mmu.h>
>  #endif
>  
> +#include <linux/pagemap.h>
> +
>  struct mmu_gather;
>  

Oh well.  I queued it up for someone else to worry over ;)

Re: Powerpc - Include pagemap.h in asm/powerpc/tlb.h

[Kumar Gala]

On Jul 19, 2007, at 12:30 AM, Andrew Morton wrote:

> On Fri, 13 Jul 2007 21:20:33 -0400 Mathieu Desnoyers  
> <mathieu.desnoyers@polymtl.ca> wrote:
>
>> Powerpc - Include pagemap.h in asm/powerpc/tlb.h
>>
>> Fixes this powerpc build error in 2.6.22-rc6-mm1 for powerpc 64 :
>>
>> In file included from include2/asm/tlb.h:60,
>>                  from /home/compudj/git/linux-2.6-lttng/arch/ 
>> powerpc/mm/init_64.
>> c:56:
>> /home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h: In  
>> function 'tlb_fl
>> ush_mmu':
>> /home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h:76:  
>> error: implicit
>> declaration of function 'release_pages'
>> /home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h: In  
>> function 'tlb_re
>> move_page':
>> /home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h:105:  
>> error: implicit
>>  declaration of function 'page_cache_release'
>
> You have some wordwrapping going on there.
>
>> make[2]: *** [arch/powerpc/mm/init_64.o] Error 1
>>
>> release_pages is declared in linux/pagemap.h, but cannot be  
>> included in
>> linux/swap.h because of a sparc related comment:
>>
>> /* only sparc can not include linux/pagemap.h in this file
>>  * so leave page_cache_release and release_pages undeclared... */
>> #define free_page_and_swap_cache(page) \
>>         page_cache_release(page)
>> #define free_pages_and_swap_cache(pages, nr) \
>>         release_pages((pages), (nr), 0);
>
> It's always a worry when this happens.  What change made us need this
> inclusion?  How come you're hitting it but I (and test.kernel.org,  
> at least)
> did not?  How come so few other architectures include pagemap.h from
> asm/tlb.h?  Why do header files get into such a mess?
>
>
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
>> CC: linuxppc-dev@ozlabs.org
>> CC: Paul Mackerras <paulus@samba.org>
>> ---
>>  include/asm-powerpc/tlb.h |    2 ++
>>  1 file changed, 2 insertions(+)
>>
>> Index: linux-2.6-lttng/include/asm-powerpc/tlb.h
>> ===================================================================
>> --- linux-2.6-lttng.orig/include/asm-powerpc/tlb.h	2007-07-13  
>> 11:30:54.000000000 -0400
>> +++ linux-2.6-lttng/include/asm-powerpc/tlb.h	2007-07-13  
>> 11:31:22.000000000 -0400
>> @@ -23,6 +23,8 @@
>>  #include <asm/mmu.h>
>>  #endif
>>
>> +#include <linux/pagemap.h>
>> +
>>  struct mmu_gather;
>>
>
> Oh well.  I queued it up for someone else to worry over ;)

Andrew,

Are you sending this to linus directly or should this go via paul and  
me?

- k

Re: Powerpc - Include pagemap.h in asm/powerpc/tlb.h

[Andrew Morton]

On Thu, 19 Jul 2007 14:55:23 -0500
Kumar Gala <galak@kernel.crashing.org> wrote:

> 
> On Jul 19, 2007, at 12:30 AM, Andrew Morton wrote:
> 
> > On Fri, 13 Jul 2007 21:20:33 -0400 Mathieu Desnoyers  
> > <mathieu.desnoyers@polymtl.ca> wrote:
> >
> >> Powerpc - Include pagemap.h in asm/powerpc/tlb.h
> >>
> >> Fixes this powerpc build error in 2.6.22-rc6-mm1 for powerpc 64 :
> >>
> >> In file included from include2/asm/tlb.h:60,
> >>                  from /home/compudj/git/linux-2.6-lttng/arch/ 
> >> powerpc/mm/init_64.
> >> c:56:
> >> /home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h: In  
> >> function 'tlb_fl
> >> ush_mmu':
> >> /home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h:76:  
> >> error: implicit
> >> declaration of function 'release_pages'
> >> /home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h: In  
> >> function 'tlb_re
> >> move_page':
> >> /home/compudj/git/linux-2.6-lttng/include/asm-generic/tlb.h:105:  
> >> error: implicit
> >>  declaration of function 'page_cache_release'
> >
> > You have some wordwrapping going on there.
> >
> >> make[2]: *** [arch/powerpc/mm/init_64.o] Error 1
> >>
> >> release_pages is declared in linux/pagemap.h, but cannot be  
> >> included in
> >> linux/swap.h because of a sparc related comment:
> >>
> >> /* only sparc can not include linux/pagemap.h in this file
> >>  * so leave page_cache_release and release_pages undeclared... */
> >> #define free_page_and_swap_cache(page) \
> >>         page_cache_release(page)
> >> #define free_pages_and_swap_cache(pages, nr) \
> >>         release_pages((pages), (nr), 0);
> >
> > It's always a worry when this happens.  What change made us need this
> > inclusion?  How come you're hitting it but I (and test.kernel.org,  
> > at least)
> > did not?  How come so few other architectures include pagemap.h from
> > asm/tlb.h?  Why do header files get into such a mess?
> >
> >
> >> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> >> CC: linuxppc-dev@ozlabs.org
> >> CC: Paul Mackerras <paulus@samba.org>
> >> ---
> >>  include/asm-powerpc/tlb.h |    2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> Index: linux-2.6-lttng/include/asm-powerpc/tlb.h
> >> ===================================================================
> >> --- linux-2.6-lttng.orig/include/asm-powerpc/tlb.h	2007-07-13  
> >> 11:30:54.000000000 -0400
> >> +++ linux-2.6-lttng/include/asm-powerpc/tlb.h	2007-07-13  
> >> 11:31:22.000000000 -0400
> >> @@ -23,6 +23,8 @@
> >>  #include <asm/mmu.h>
> >>  #endif
> >>
> >> +#include <linux/pagemap.h>
> >> +
> >>  struct mmu_gather;
> >>
> >
> > Oh well.  I queued it up for someone else to worry over ;)
> 
> Andrew,
> 
> Are you sending this to linus directly or should this go via paul and  
> me?
> 

I queued it up for Paul.  I can send it over to Linus today if we have a
reason for that.  It's just that I dont understand the patch: does it fix
-mm-only breakage?  Does it fix something which already got fixed, or what?

One might think "gee, it's trivial, just slam it in", but these nested
includes are often not-trivial and something else can explode as a result.

Re: Powerpc - Include pagemap.h in asm/powerpc/tlb.h

[Kumar Gala]

>> Andrew,
>>
>> Are you sending this to linus directly or should this go via paul and
>> me?
>>
>
> I queued it up for Paul.  I can send it over to Linus today if we  
> have a
> reason for that.  It's just that I dont understand the patch: does  
> it fix
> -mm-only breakage?  Does it fix something which already got fixed,  
> or what?

I'm seeing the same breakage in my tree.

> One might think "gee, it's trivial, just slam it in", but these nested
> includes are often not-trivial and something else can explode as a  
> result.

I'll do a git-bisect and see if I can find the patch that causes this  
breakage so we know what's going on.

- k

Re: Powerpc - Include pagemap.h in asm/powerpc/tlb.h

[Andrew Morton]

On Thu, 19 Jul 2007 16:04:38 -0500
Kumar Gala <galak@kernel.crashing.org> wrote:

> >> Andrew,
> >>
> >> Are you sending this to linus directly or should this go via paul and
> >> me?
> >>
> >
> > I queued it up for Paul.  I can send it over to Linus today if we  
> > have a
> > reason for that.  It's just that I dont understand the patch: does  
> > it fix
> > -mm-only breakage?  Does it fix something which already got fixed,  
> > or what?
> 
> I'm seeing the same breakage in my tree.

Oh, OK.  Let's just merge it.

> > One might think "gee, it's trivial, just slam it in", but these nested
> > includes are often not-trivial and something else can explode as a  
> > result.
> 
> I'll do a git-bisect and see if I can find the patch that causes this  
> breakage so we know what's going on.

eh, don't bother.

Re: Powerpc - Include pagemap.h in asm/powerpc/tlb.h

[Mathieu Desnoyers]

* Andrew Morton (akpm@linux-foundation.org) wrote:
> On Thu, 19 Jul 2007 16:04:38 -0500
> Kumar Gala <galak@kernel.crashing.org> wrote:
> 
> > >> Andrew,
> > >>
> > >> Are you sending this to linus directly or should this go via paul and
> > >> me?
> > >>
> > >
> > > I queued it up for Paul.  I can send it over to Linus today if we  
> > > have a
> > > reason for that.  It's just that I dont understand the patch: does  
> > > it fix
> > > -mm-only breakage?  Does it fix something which already got fixed,  
> > > or what?
> > 
> > I'm seeing the same breakage in my tree.
> 
> Oh, OK.  Let's just merge it.
> 
> > > One might think "gee, it's trivial, just slam it in", but these nested
> > > includes are often not-trivial and something else can explode as a  
> > > result.
> > 
> > I'll do a git-bisect and see if I can find the patch that causes this  
> > breakage so we know what's going on.
> 
> eh, don't bother.

Hrm, I still think it's kind of interesting to see what happens there.

So the sane way to make sure it's fixed would be to turn the macros
free_page_and_swap_cache() and free_pages_and_swap_cache() into an
inline function (just to make sure the compiler complains), but that
would require to include linux/pagemap.h from linux/swap.h. But let's
just see what happens on sparc if we include linux/pagemap.h from
swap.h.

I just tested compiling on sparc, here is the result:


  /opt/crosstool/gcc-4.1.1-glibc-2.3.6/sparc-unknown-linux-gnu/bin/sparc-unknown-linux-gnu-gcc -Wp,-MD,init/.main.o.d  -nostdinc -isystem /opt/crosstool/gcc-4.1.1-glibc-2.3.6/sparc-unknown-linux-gnu/lib/gcc/sparc-unknown-linux-gnu/4.1.1/include -D__KERNEL__ -Iinclude -Iinclude2 -I/home/compudj/git/linux-2.6-lttng/include -include include/linux/autoconf.h -I/home/compudj/git/linux-2.6-lttng/init -Iinit -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -Werror-implicit-function-declaration -Os -m32 -pipe -mno-fpu -fcall-used-g5 -fcall-used-g7 -fomit-frame-pointer -fno-stack-protector -Wdeclaration-after-statement -Wno-pointer-sign  -D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(main)"  -D"KBUILD_MODNAME=KBUILD_STR(main)" -c -o init/.tmp_main.o /home/compudj/git/linux-2.6-lttng/init/main.c
In file included from /home/compudj/git/linux-2.6-lttng/include/linux/highmem.h:24,
                 from /home/compudj/git/linux-2.6-lttng/include/linux/pagemap.h:10,
                 from /home/compudj/git/linux-2.6-lttng/include/linux/swap.h:9,
                 from include2/asm/pgtable.h:15,
                 from /home/compudj/git/linux-2.6-lttng/include/linux/mm.h:38,
                 from /home/compudj/git/linux-2.6-lttng/include/linux/rmap.h:9,
                 from /home/compudj/git/linux-2.6-lttng/init/main.c:49:
include2/asm/highmem.h: In function 'kmap':
include2/asm/highmem.h:60: error: implicit declaration of function 'PageHighMem'
include2/asm/highmem.h:61: error: implicit declaration of function 'page_address'
include2/asm/highmem.h:61: warning: return makes pointer from integer without a cast
In file included from /home/compudj/git/linux-2.6-lttng/include/linux/swap.h:9,
                 from include2/asm/pgtable.h:15,
                 from /home/compudj/git/linux-2.6-lttng/include/linux/mm.h:38,
                 from /home/compudj/git/linux-2.6-lttng/include/linux/rmap.h:9,
                 from /home/compudj/git/linux-2.6-lttng/init/main.c:49:
/home/compudj/git/linux-2.6-lttng/include/linux/pagemap.h: In function 'linear_page_index':
/home/compudj/git/linux-2.6-lttng/include/linux/pagemap.h:153: error: dereferencing pointer to incomplete type
/home/compudj/git/linux-2.6-lttng/include/linux/pagemap.h:154: error: dereferencing pointer to incomplete type
/home/compudj/git/linux-2.6-lttng/include/linux/pagemap.h: In function 'lock_page':
/home/compudj/git/linux-2.6-lttng/include/linux/pagemap.h:168: error: implicit declaration of function 'TestSetPageLocked'
/home/compudj/git/linux-2.6-lttng/include/linux/pagemap.h: In function 'wait_on_page_locked':
/home/compudj/git/linux-2.6-lttng/include/linux/pagemap.h:198: error: implicit declaration of function 'PageLocked'
/home/compudj/git/linux-2.6-lttng/include/linux/pagemap.h:199: error: 'PG_locked' undeclared (first use in this function)
/home/compudj/git/linux-2.6-lttng/include/linux/pagemap.h:199: error: (Each undeclared identifier is reported only once
/home/compudj/git/linux-2.6-lttng/include/linux/pagemap.h:199: error: for each function it appears in.)
/home/compudj/git/linux-2.6-lttng/include/linux/pagemap.h: In function 'wait_on_page_writeback':
/home/compudj/git/linux-2.6-lttng/include/linux/pagemap.h:207: error: implicit declaration of function 'PageWriteback'
/home/compudj/git/linux-2.6-lttng/include/linux/pagemap.h:208: error: 'PG_writeback' undeclared (first use in this function)
In file included from /home/compudj/git/linux-2.6-lttng/include/linux/rmap.h:9,
                 from /home/compudj/git/linux-2.6-lttng/init/main.c:49:
/home/compudj/git/linux-2.6-lttng/include/linux/mm.h: At top level:
/home/compudj/git/linux-2.6-lttng/include/linux/mm.h:605: error: conflicting types for 'page_address'
include2/asm/highmem.h:61: error: previous implicit declaration of 'page_address' was here

PageHighMem is declared in linux/page-flags.h, which is included in
linux/mm.h with the following comment:

/*
 * FIXME: take this include out, include page-flags.h in
 * files which need it (119 of them)
 */
#include <linux/page-flags.h>

(linux/mm.h is included from linux/highmem.h)

Actually, we get a circular inclusion there:

In file included from /home/compudj/git/linux-2.6-lttng/include/linux/highmem.h:24,
                 from /home/compudj/git/linux-2.6-lttng/include/linux/pagemap.h:10,
                 from /home/compudj/git/linux-2.6-lttng/include/linux/swap.h:9,
                 from include2/asm/pgtable.h:15,
                 from /home/compudj/git/linux-2.6-lttng/include/linux/mm.h:38,
                 from /home/compudj/git/linux-2.6-lttng/include/linux/rmap.h:9,
                 from /home/compudj/git/linux-2.6-lttng/init/main.c:49:


mm.h includes asm-sparc/pgtable.h includes linux/swap.h includes
linux/pagemap.h (which I have added myself) includes linux/highmem.h
includes mm.h.....

Is it me or it all looks like a fubarish mess ? :(

Why is asm-sparc/pgtable.h including linux/swap.h ? Perharps the sparc
devs will be able to enlighten us...

Mathieu

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: Powerpc - Include pagemap.h in asm/powerpc/tlb.h

[Kumar Gala]

On Jul 19, 2007, at 4:30 PM, Andrew Morton wrote:

> On Thu, 19 Jul 2007 16:04:38 -0500
> Kumar Gala <galak@kernel.crashing.org> wrote:
>
>>>> Andrew,
>>>>
>>>> Are you sending this to linus directly or should this go via  
>>>> paul and
>>>> me?
>>>>
>>>
>>> I queued it up for Paul.  I can send it over to Linus today if we
>>> have a
>>> reason for that.  It's just that I dont understand the patch: does
>>> it fix
>>> -mm-only breakage?  Does it fix something which already got fixed,
>>> or what?
>>
>> I'm seeing the same breakage in my tree.
>
> Oh, OK.  Let's just merge it.

Hmm, depending on the config it goes away.

Lets hold off on this for a little while so understand exactly what's  
going on.

>>> One might think "gee, it's trivial, just slam it in", but these  
>>> nested
>>> includes are often not-trivial and something else can explode as a
>>> result.
>>
>> I'll do a git-bisect and see if I can find the patch that causes this
>> breakage so we know what's going on.
>
> eh, don't bother.

I think I'll poke around a little since it clearly has some config  
dependancy.

- k