Skip to contentSkip to conversation

include linux/pagemap.h in asm-generic/tlb.h

PATCH In review Jeff Dike 3 participants 8 messages Aug 24, 2007 – Aug 26, 2007

Subsystems (2)
Generic include/asm header files MMU gather and TLB invalidation
lore
Share & tools
lore

Jeff Dike

author
#0 lore raw

PATCH 1/1 include linux/pagemap.h in asm-generic/tlb.h

[ 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(+)
include/asm-generic/tlb.h +1 -0
Index: linux-2.6.22/include/asm-generic/tlb.h
===================================================================
@@ -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>
Acked-by on v1 1/1
#1 lore raw
Quoted 4 lines · Jeff Dike

[ 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

Signature · 2 lines
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.
author
#2 lore raw
Quoted 3 lines · Rob Landley

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

Signature · 1 line
Work email - jdike at linux dot intel dot com
#3 lore raw
Quoted 7 lines · Jeff Dike
Quoted 3 lines · Rob Landley

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

Quoted 1 line

Jeff

Rob

Signature · 2 lines
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.
maintainer
#4 lore raw
In reply to Jeff Dike
Quoted 15 lines · 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
===================================================================

include/asm-generic/tlb.h line 19

Quoted 7 lines
include/asm-generic/tlb.h +1 -0
@@ -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?

author
#5 lore raw
Quoted 4 lines · Andrew Morton

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.

Quoted 5 lines

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

Signature · 1 line
Work email - jdike at linux dot intel dot com
maintainer
#6 lore raw
Quoted 18 lines · On Fri, 24 Aug 2007 22:15:33 -0400 Jeff Dike <[email protected]> wrote:
Quoted 5 lines

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);
	...
}
Quoted 3 lines

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

#7 lore raw
In reply to Andrew Morton
Quoted 32 lines · Andrew Morton

On Fri, 24 Aug 2007 14:46:16 -0400

Quoted 19 lines · Jeff Dike

[ 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

Quoted 4 lines

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

Quoted 8 lines

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

Signature · 2 lines
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.