[ This looks non-urgent to me ]
Without linux/pagemap.h, asm-generic/tlb.h is missing declarations of
page_cache_release and release_pages.
Signed-off-by: Jeff Dike <[email protected]>
--
include/asm-generic/tlb.h | 1 +
1 file changed, 1 insertion(+)
Index: linux-2.6.22/include/asm-generic/tlb.h
===================================================================
--- linux-2.6.22.orig/include/asm-generic/tlb.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.22/include/asm-generic/tlb.h 2007-08-22 17:29:45.000000000 -0400
@@ -13,6 +13,7 @@
#ifndef _ASM_GENERIC__TLB_H
#define _ASM_GENERIC__TLB_H
+#include <linux/pagemap.h>
#include <linux/swap.h>
#include <asm/pgalloc.h>
#include <asm/tlbflush.h>
On Friday 24 August 2007 1:46:16 pm Jeff Dike wrote:
> [ This looks non-urgent to me ]
>
> Without linux/pagemap.h, asm-generic/tlb.h is missing declarations of
> page_cache_release and release_pages.
It may be non-urgent to you, but it still broke my build. :)
Acked-by: Rob Landley <[email protected]>
Rob
--
"One of my most productive days was throwing away 1000 lines of code."
- Ken Thompson.
On Fri, Aug 24, 2007 at 03:51:41PM -0500, Rob Landley wrote:
> It may be non-urgent to you, but it still broke my build. :)
>
> Acked-by: Rob Landley <[email protected]>
Heh, it doesn't seem like -stable material to me, but if there aren't
any screams while it sits in -mm, maybe it's 2.6.23 material.
Jeff
--
Work email - jdike at linux dot intel dot com
On Fri, 24 Aug 2007 14:46:16 -0400
Jeff Dike <[email protected]> wrote:
> [ This looks non-urgent to me ]
>
> Without linux/pagemap.h, asm-generic/tlb.h is missing declarations of
> page_cache_release and release_pages.
>
> Signed-off-by: Jeff Dike <[email protected]>
> --
> include/asm-generic/tlb.h | 1 +
> 1 file changed, 1 insertion(+)
>
> Index: linux-2.6.22/include/asm-generic/tlb.h
> ===================================================================
> --- linux-2.6.22.orig/include/asm-generic/tlb.h 2007-07-08 19:32:17.000000000 -0400
> +++ linux-2.6.22/include/asm-generic/tlb.h 2007-08-22 17:29:45.000000000 -0400
> @@ -13,6 +13,7 @@
> #ifndef _ASM_GENERIC__TLB_H
> #define _ASM_GENERIC__TLB_H
>
> +#include <linux/pagemap.h>
> #include <linux/swap.h>
> #include <asm/pgalloc.h>
> #include <asm/tlbflush.h>
This is worrisome. If you look at pagemap.h, it includes a pile of things
which could easily themsleves try to include tlb.h via some path or
another. I fear that this patch will cause explosions with some config
and/or architecture.
If you like, tlb.h is a low-level sort of thing whereas pagemap.h is a
higher-level VFS thing. It is more appropriate that pagemap.h be including
tlb.h.
So I think a better fix would be better, but I'm not able to suggest what,
as there is little detail about the failure here and I can find no mention
of page_cache_release and release_pages in asm-generic/tlb.h. Presumably
they're getting pulled in via some macro and then instantiated in some
inline function or something?
On Fri, Aug 24, 2007 at 05:20:27PM -0700, Andrew Morton wrote:
> This is worrisome. If you look at pagemap.h, it includes a pile of things
> which could easily themsleves try to include tlb.h via some path or
> another. I fear that this patch will cause explosions with some config
> and/or architecture.
I agree, I looked at the includes and had the same thoughts. I sent
it in because there is a problem here, and it fixes a UML config
without noticably breaking anything else.
> So I think a better fix would be better, but I'm not able to suggest what,
> as there is little detail about the failure here and I can find no mention
> of page_cache_release and release_pages in asm-generic/tlb.h. Presumably
> they're getting pulled in via some macro and then instantiated in some
> inline function or something?
Yes, it's straightforward -
tlb.h:
static inline void
tlb_flush_mmu(struct mmu_gather *tlb, unsigned long start, unsigned long end)
{
...
free_pages_and_swap_cache(tlb->pages, tlb->nr);
...
}
swap.h:
#define free_pages_and_swap_cache(pages, nr) \
release_pages((pages), (nr), 0);
Feel free to drop it, and I'll look at this some more.
Jeff
--
Work email - jdike at linux dot intel dot com
On Fri, 24 Aug 2007 22:15:33 -0400 Jeff Dike <[email protected]> wrote:
> > So I think a better fix would be better, but I'm not able to suggest what,
> > as there is little detail about the failure here and I can find no mention
> > of page_cache_release and release_pages in asm-generic/tlb.h. Presumably
> > they're getting pulled in via some macro and then instantiated in some
> > inline function or something?
>
> Yes, it's straightforward -
>
> tlb.h:
>
> static inline void
> tlb_flush_mmu(struct mmu_gather *tlb, unsigned long start, unsigned long end)
> {
> ...
> free_pages_and_swap_cache(tlb->pages, tlb->nr);
> ...
> }
>
> swap.h:
> #define free_pages_and_swap_cache(pages, nr) \
> release_pages((pages), (nr), 0);
Oh that damn thing again. It's a regular source of include breakage.
I suppose we could just uninline tlb_flush_mmu().
On Friday 24 August 2007 7:20:27 pm Andrew Morton wrote:
> On Fri, 24 Aug 2007 14:46:16 -0400
>
> Jeff Dike <[email protected]> wrote:
> > [ This looks non-urgent to me ]
> >
> > Without linux/pagemap.h, asm-generic/tlb.h is missing declarations of
> > page_cache_release and release_pages.
> >
> > Signed-off-by: Jeff Dike <[email protected]>
> > --
> > include/asm-generic/tlb.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > Index: linux-2.6.22/include/asm-generic/tlb.h
> > ===================================================================
> > --- linux-2.6.22.orig/include/asm-generic/tlb.h 2007-07-08
> > 19:32:17.000000000 -0400 +++
> > linux-2.6.22/include/asm-generic/tlb.h 2007-08-22 17:29:45.000000000
> > -0400 @@ -13,6 +13,7 @@
> > #ifndef _ASM_GENERIC__TLB_H
> > #define _ASM_GENERIC__TLB_H
> >
> > +#include <linux/pagemap.h>
> > #include <linux/swap.h>
> > #include <asm/pgalloc.h>
> > #include <asm/tlbflush.h>
>
> This is worrisome. If you look at pagemap.h, it includes a pile of things
> which could easily themsleves try to include tlb.h via some path or
> another. I fear that this patch will cause explosions with some config
> and/or architecture.
>
> If you like, tlb.h is a low-level sort of thing whereas pagemap.h is a
> higher-level VFS thing. It is more appropriate that pagemap.h be including
> tlb.h.
>
>
> So I think a better fix would be better, but I'm not able to suggest what,
> as there is little detail about the failure here and I can find no mention
> of page_cache_release and release_pages in asm-generic/tlb.h.
To reproduce it, do this in -rc3:
cat > mini.conf << EOF
CONFIG_MODE_SKAS=y
CONFIG_BINFMT_ELF=y
CONFIG_HOSTFS=y
CONFIG_SYSCTL=y
CONFIG_STDERR_CONSOLE=y
CONFIG_UNIX98_PTYS=y
CONFIG_BLK_DEV_LOOP=y
CONFIG_LBD=y
CONFIG_EXT2_FS=y
CONFIG_PROC_FS=y
EOF
make ARCH=um allnoconfig KCONFIG_ALLCONFIG=mini.conf
make ARCH=um
Rob
(P.S. I note that in order for CONFIG_BLK_DEV_LOOP to actually trigger work
now, I have to add CONFIG_BLK_DEV=y to the above, which I didn't have to do
in 2.6.22 or in any previous version all the way back to 2.6.12. Not a major
regression, but still a bit of a surprise. That said, the above is what
triggered the break for me.)
--
"One of my most productive days was throwing away 1000 lines of code."
- Ken Thompson.
On Friday 24 August 2007 5:46:51 pm Jeff Dike wrote:
> On Fri, Aug 24, 2007 at 03:51:41PM -0500, Rob Landley wrote:
> > It may be non-urgent to you, but it still broke my build. :)
> >
> > Acked-by: Rob Landley <[email protected]>
>
> Heh, it doesn't seem like -stable material to me, but if there aren't
> any screams while it sits in -mm, maybe it's 2.6.23 material.
I believe 2.6.22 built for me without the patch. It was 2.6.23-rc3 that
needed it.
(I have a bad head cold right now so I'm not going back to confirm this
belief. I am, instead, making tea.)
> Jeff
Rob
--
"One of my most productive days was throwing away 1000 lines of code."
- Ken Thompson.