* Linux Kernel Mailing List <[email protected]> wrote:
> Gitweb: http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=2f569afd9ced9ebec9a6eb3dbf6f83429be0a7b4
> Commit: 2f569afd9ced9ebec9a6eb3dbf6f83429be0a7b4
> Parent: 13214adf738abc92b0a00c0763fd3be79eebaa7c
> Author: Martin Schwidefsky <[email protected]>
> AuthorDate: Fri Feb 8 04:22:04 2008 -0800
> Committer: Linus Torvalds <[email protected]>
> CommitDate: Fri Feb 8 09:22:42 2008 -0800
>
> CONFIG_HIGHPTE vs. sub-page page tables.
this patch broke the 32-bit x86 build after just 2 randconfig
iterations:
In file included from include/linux/memcontrol.h:25,
from include/linux/swap.h:9,
from include/linux/suspend.h:8,
from arch/x86/kernel/asm-offsets_32.c:12,
from arch/x86/kernel/asm-offsets.c:3:
include/linux/mm.h:1151: error: expected declaration specifiers or '...' before 'pgtable_t'
config attached. It seems all !PAE 32-bit kernel builds are broken due
to this. The patch below fixes it.
Ingo
------------------->
Subject: x86: pgtable_t build fix
From: Ingo Molnar <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
include/asm-x86/page_32.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
Index: linux/include/asm-x86/page_32.h
===================================================================
--- linux.orig/include/asm-x86/page_32.h
+++ linux/include/asm-x86/page_32.h
@@ -49,11 +49,13 @@ typedef unsigned long phys_addr_t;
typedef union { pteval_t pte, pte_low; } pte_t;
-typedef struct page *pgtable_t;
-
#endif /* __ASSEMBLY__ */
#endif /* CONFIG_X86_PAE */
+#ifndef __ASSEMBLY__
+typedef struct page *pgtable_t;
+#endif
+
#ifdef CONFIG_HUGETLB_PAGE
#define HAVE_ARCH_HUGETLB_UNMAPPED_AREA
#endif
* Ingo Molnar <[email protected]> wrote:
> config attached. It seems all !PAE 32-bit kernel builds are broken due
> to this. The patch below fixes it.
ok, the patch was against x86.git which had some other changes in this
area - the one below applies to vanilla -git and fixes the bug.
Ingo
----------------->
Subject: x86: fix pgtable_t build breakage
From: Ingo Molnar <[email protected]>
fix build breakage caused by commit 2f569afd9ced9ebec9.
Signed-off-by: Ingo Molnar <[email protected]>
---
include/asm-x86/page_32.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
Index: linux-x86.q/include/asm-x86/page_32.h
===================================================================
--- linux-x86.q.orig/include/asm-x86/page_32.h
+++ linux-x86.q/include/asm-x86/page_32.h
@@ -50,11 +50,13 @@ typedef unsigned long phys_addr_t;
typedef union { pteval_t pte, pte_low; } pte_t;
typedef pte_t boot_pte_t;
-typedef struct page *pgtable_t;
-
#endif /* __ASSEMBLY__ */
#endif /* CONFIG_X86_PAE */
+#ifndef __ASSEMBLY__
+typedef struct page *pgtable_t;
+#endif
+
#ifdef CONFIG_HUGETLB_PAGE
#define HAVE_ARCH_HUGETLB_UNMAPPED_AREA
#endif
On Sat, 2008-02-09 at 00:15 +0100, Ingo Molnar wrote:
> > config attached. It seems all !PAE 32-bit kernel builds are broken due
> > to this. The patch below fixes it.
>
> ok, the patch was against x86.git which had some other changes in this
> area - the one below applies to vanilla -git and fixes the bug.
Thanks Ingo for taking care of it. I already feared that we'll have some
more fallout from this patch although it has been in -mm for quite a
while. The real testing starts after a patch hit Linus tree.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
* Martin Schwidefsky <[email protected]> wrote:
> On Sat, 2008-02-09 at 00:15 +0100, Ingo Molnar wrote:
> > > config attached. It seems all !PAE 32-bit kernel builds are broken due
> > > to this. The patch below fixes it.
> >
> > ok, the patch was against x86.git which had some other changes in
> > this area - the one below applies to vanilla -git and fixes the bug.
>
> Thanks Ingo for taking care of it. I already feared that we'll have
> some more fallout from this patch although it has been in -mm for
> quite a while. The real testing starts after a patch hit Linus tree.
i think the worst is over already and i'm reasonably sure that there are
no more bugs in it - this _is_ a 1:1 patch after all, so in theory the
worst side-effect should be build breakages due to include file
spaghetti. The window for this particular breakage was just 256 commits,
that's OK i think.
If you want less stress next time around you might want to consider
pushing such patches via individual architectures, so that it can all be
shaken out (and such build bugs are found quickly) and pushed via the
architecture trees. (Even such a patch that changes the number of
cross-arch function arguments and introduces a new type can be
architectured in a way to make it per arch.)
Ingo
On Sat, 2008-02-09 at 11:37 +0100, Ingo Molnar wrote:
> i think the worst is over already and i'm reasonably sure that there are
> no more bugs in it - this _is_ a 1:1 patch after all, so in theory the
> worst side-effect should be build breakages due to include file
> spaghetti. The window for this particular breakage was just 256 commits,
> that's OK i think.
Except for the breakage of all nommu architectures .. they need the
pgtable_t as well due to the pte_fn_t type.
> If you want less stress next time around you might want to consider
> pushing such patches via individual architectures, so that it can all be
> shaken out (and such build bugs are found quickly) and pushed via the
> architecture trees. (Even such a patch that changes the number of
> cross-arch function arguments and introduces a new type can be
> architectured in a way to make it per arch.)
I'll try that with the __pte_free_tlb macro to inline conversion patch.
This one is really driving me nuts, the dependencies of the macros in
asm/pgalloc, asm/tlb.h and asm-generic/tlb.h for the different archs are
pure evil.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
On Feb 9, 2008 5:56 AM, Martin Schwidefsky <[email protected]> wrote:
> On Sat, 2008-02-09 at 11:37 +0100, Ingo Molnar wrote:
> > i think the worst is over already and i'm reasonably sure that there are
> > no more bugs in it - this _is_ a 1:1 patch after all, so in theory the
> > worst side-effect should be build breakages due to include file
> > spaghetti. The window for this particular breakage was just 256 commits,
> > that's OK i think.
>
> Except for the breakage of all nommu architectures .. they need the
> pgtable_t as well due to the pte_fn_t type.
so why wasnt this in the original patch ? why do no-mmu arches have
to add the pgtable_t typedefs themselves ?
-mike
On Sat, 2008-02-09 at 12:56 -0500, Mike Frysinger wrote:
> On Feb 9, 2008 5:56 AM, Martin Schwidefsky <[email protected]> wrote:
> > On Sat, 2008-02-09 at 11:37 +0100, Ingo Molnar wrote:
> > > i think the worst is over already and i'm reasonably sure that there are
> > > no more bugs in it - this _is_ a 1:1 patch after all, so in theory the
> > > worst side-effect should be build breakages due to include file
> > > spaghetti. The window for this particular breakage was just 256 commits,
> > > that's OK i think.
> >
> > Except for the breakage of all nommu architectures .. they need the
> > pgtable_t as well due to the pte_fn_t type.
>
> so why wasnt this in the original patch ? why do no-mmu arches have
> to add the pgtable_t typedefs themselves ?
I do cross-compiles for some architectures but not all. None of the
nommu architectures are covered (I should change that). I actually did
cross compile m68knommu for the first versions of the patch, that didn't
went to well because of the standard m68k compiler. The pte_fn_t change
has been added after the test compile for m68knommu. This is how is
slipped through my fingers. The problem wasn't noticed either while the
patch has been aging in Andrews -mm tree.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
On Feb 10, 2008 4:17 AM, Martin Schwidefsky <[email protected]> wrote:
> On Sat, 2008-02-09 at 12:56 -0500, Mike Frysinger wrote:
> > On Feb 9, 2008 5:56 AM, Martin Schwidefsky <[email protected]> wrote:
> > > On Sat, 2008-02-09 at 11:37 +0100, Ingo Molnar wrote:
> > > > i think the worst is over already and i'm reasonably sure that there are
> > > > no more bugs in it - this _is_ a 1:1 patch after all, so in theory the
> > > > worst side-effect should be build breakages due to include file
> > > > spaghetti. The window for this particular breakage was just 256 commits,
> > > > that's OK i think.
> > >
> > > Except for the breakage of all nommu architectures .. they need the
> > > pgtable_t as well due to the pte_fn_t type.
> >
> > so why wasnt this in the original patch ? why do no-mmu arches have
> > to add the pgtable_t typedefs themselves ?
>
> I do cross-compiles for some architectures but not all. None of the
> nommu architectures are covered (I should change that). I actually did
> cross compile m68knommu for the first versions of the patch, that didn't
> went to well because of the standard m68k compiler. The pte_fn_t change
> has been added after the test compile for m68knommu. This is how is
> slipped through my fingers. The problem wasn't noticed either while the
> patch has been aging in Andrews -mm tree.
i guess my point was more: the pgtable_t typdef is new therefore it
must be defined for every architecture. your ability to directly
cross-compile and/or test a subset is great, but posting a change that
is know for a fact to break arches you didnt update seems like a bad
idea. even if you just included the obvious-but-not-compile-tested
changes and included the linux-arch@vger alias instead would have been
better than nothing
-mike
On Sun, 2008-02-10 at 04:25 -0500, Mike Frysinger wrote:
> i guess my point was more: the pgtable_t typdef is new therefore it
> must be defined for every architecture. your ability to directly
> cross-compile and/or test a subset is great, but posting a change that
> is know for a fact to break arches you didnt update seems like a bad
> idea. even if you just included the obvious-but-not-compile-tested
> changes and included the linux-arch@vger alias instead would have been
> better than nothing
> -mike
The patch was posted multiple times on linux-arch and it has been part
of -mm for 3 months. Plenty of time for the arch maintainers to notice.
And without the pte_pfn_t change it would compile on a nommu
architecture even without the typedef. That is why I didn't add the new
typedef to the nommu archs. Which turned out to be a mistake after the
pte_pfn_t change has been added but the problem is fixed with the patch
sent yesterday, isn't?
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
On Feb 10, 2008 4:41 AM, Martin Schwidefsky <[email protected]> wrote:
> On Sun, 2008-02-10 at 04:25 -0500, Mike Frysinger wrote:
> > i guess my point was more: the pgtable_t typdef is new therefore it
> > must be defined for every architecture. your ability to directly
> > cross-compile and/or test a subset is great, but posting a change that
> > is know for a fact to break arches you didnt update seems like a bad
> > idea. even if you just included the obvious-but-not-compile-tested
> > changes and included the linux-arch@vger alias instead would have been
> > better than nothing
>
> The patch was posted multiple times on linux-arch and it has been part
> of -mm for 3 months. Plenty of time for the arch maintainers to notice.
> And without the pte_pfn_t change it would compile on a nommu
> architecture even without the typedef. That is why I didn't add the new
> typedef to the nommu archs.
generally things are posted to linux-arch for the arch maintainers to
review. the person posting the changes does all the footwork to make
sure no one is left behind since they're the ones proposing the
change. it isnt "hey, unless you do something, your arch is going to
be broken, sorry".
> Which turned out to be a mistake after the
> pte_pfn_t change has been added but the problem is fixed with the patch
> sent yesterday, isn't?
yes, the patch you posted for the remaining arches should fix the
arches left broken.
-mike