Allow compile with CONFIG_DEBUG_INFO=y, X86_PAE not set on gcc 3.4.5

PATCH Applied Simon Horman 4 participants 11 messages Aug 8, 2008 – Aug 11, 2008

Subsystems (2)

x86 x86 MM

lore

Share & tools

lore

Simon Horman

author

2008-08-08 05:04 UTC #0 lore raw

Hi,

It appears that "x86: preallocate and prepopulate separately" (d8d5900ef8afc562088f8470feeaf17c4747790f) introduced a minor regression. The build fails on gcc 3.4.5 if CONFIG_DEBUG_INFO=y (that is gcc is called with -g) and X86_PAE not set.

There was previously some discussion of this without resolution.
http://lkml.org/lkml/2008/7/18/250

arch/x86/mm/pgtable.c: In function `pgd_prepopulate_pmd':
arch/x86/mm/pgtable.c:222: internal compiler error: in remove_insn, at emit-rtl.c:3746 Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.

# i686-unknown-linux-gnu-gcc --version
i686-unknown-linux-gnu-gcc (GCC) 3.4.5

My investigations seem to show that gcc 3.4.5 can't cope with the following construct:

	for (i = 0; i < 0; i++)
		...

or more specifically:

	for (i = 0; i < PREALLOCTED_PMDS; i++)
		...

when PREALLOCTED_PMDS is 0. That is, when X86_PAE is not set.

This patch resolves this problem by moving the relevant code inside #define X86_PAE and providing dummy functions outside !X86_PAE.

Signed-off-by Simon Horman <[email protected]>

arch/x86/mm/pgtable.c +13 -6

Index: linux-2.6/arch/x86/mm/pgtable.c
===================================================================
@@ -141,12 +141,6 @@ void pud_populate(struct mm_struct *mm, 
 	if (mm == current->active_mm)
 		write_cr3(read_cr3());
 }
-#else  /* !CONFIG_X86_PAE */
-
-/* No need to prepopulate any pagetable entries in non-PAE modes. */
-#define PREALLOCATED_PMDS	0
-
-#endif	/* CONFIG_X86_PAE */
 
 static void free_pmds(pmd_t *pmds[])
 {

@@ -221,6 +215,19 @@ static void pgd_prepopulate_pmd(struct m
 	}
 }
 
+#else  /* !CONFIG_X86_PAE */
+
+/* No need to prepopulate any pagetable entries in non-PAE modes. */
+#define PREALLOCATED_PMDS	0
+
+static int preallocate_pmds(pmd_t *pmds[]) { return 0; }
+static void pgd_prepopulate_pmd(struct mm_struct *mm, pgd_t *pgd, pmd_t *pmds[])
+{ }
+static void free_pmds(pmd_t *pmds[]) { }
+static void pgd_mop_up_pmds(struct mm_struct *mm, pgd_t *pgdp) { }
+
+#endif	/* CONFIG_X86_PAE */
+
 pgd_t *pgd_alloc(struct mm_struct *mm)
 {
 	pgd_t *pgd;

Jeremy Fitzhardinge

2008-08-08 06:37 UTC #1 lore raw

Quoted 30 lines · Simon Horman

Hi,
It appears that "x86: preallocate and prepopulate separately" (d8d5900ef8afc562088f8470feeaf17c4747790f) introduced a minor regression. The build fails on gcc 3.4.5 if CONFIG_DEBUG_INFO=y (that is gcc is called with -g) and X86_PAE not set.
There was previously some discussion of this without resolution.
http://lkml.org/lkml/2008/7/18/250
arch/x86/mm/pgtable.c: In function `pgd_prepopulate_pmd':
arch/x86/mm/pgtable.c:222: internal compiler error: in remove_insn, at emit-rtl.c:3746 Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.
# i686-unknown-linux-gnu-gcc --version
i686-unknown-linux-gnu-gcc (GCC) 3.4.5
My investigations seem to show that gcc 3.4.5 can't cope with the following construct:
	for (i = 0; i < 0; i++)
		...
or more specifically:
	for (i = 0; i < PREALLOCTED_PMDS; i++)
		...

Quoted 7 lines

when PREALLOCTED_PMDS is 0. That is, when X86_PAE is not set.
This patch resolves this problem by moving the relevant code inside #define X86_PAE and providing dummy functions outside !X86_PAE.
Signed-off-by Simon Horman <[email protected]>

We resolved the other report by saying that gcc 3.4.4 is broken. It seems 3.4.5 is as well. Could you just update the compiler? I'd rather not have to clutter the code with more ifdefs if we can possibly avoid it.

Does putting

	if (PREALLOCATED_PMDS == 0)
		return;

before the for loop help?

Quoted 38 lines

Index: linux-2.6/arch/x86/mm/pgtable.c
===================================================================

arch/x86/mm/pgtable.c +13 -6

@@ -141,12 +141,6 @@ void pud_populate(struct mm_struct *mm, 
 	if (mm == current->active_mm)
 		write_cr3(read_cr3());
 }
-#else  /* !CONFIG_X86_PAE */
-
-/* No need to prepopulate any pagetable entries in non-PAE modes. */
-#define PREALLOCATED_PMDS	0
-
-#endif	/* CONFIG_X86_PAE */
 
 static void free_pmds(pmd_t *pmds[])
 {

@@ -221,6 +215,19 @@ static void pgd_prepopulate_pmd(struct m
 	}
 }
 
+#else  /* !CONFIG_X86_PAE */
+
+/* No need to prepopulate any pagetable entries in non-PAE modes. */
+#define PREALLOCATED_PMDS	0
+
+static int preallocate_pmds(pmd_t *pmds[]) { return 0; }
+static void pgd_prepopulate_pmd(struct mm_struct *mm, pgd_t *pgd, pmd_t *pmds[])
+{ }
+static void free_pmds(pmd_t *pmds[]) { }
+static void pgd_mop_up_pmds(struct mm_struct *mm, pgd_t *pgdp) { }
+
+#endif	/* CONFIG_X86_PAE */
+
 pgd_t *pgd_alloc(struct mm_struct *mm)
 {
 	pgd_t *pgd;

Simon Horman

author

2008-08-08 07:50 UTC #2 lore raw

Quoted 36 lines · Jeremy Fitzhardinge

Quoted 35 lines · Simon Horman
Hi,
It appears that "x86: preallocate and prepopulate separately" (d8d5900ef8afc562088f8470feeaf17c4747790f) introduced a minor regression. The build fails on gcc 3.4.5 if CONFIG_DEBUG_INFO=y (that is gcc is called with -g) and X86_PAE not set.
There was previously some discussion of this without resolution.
http://lkml.org/lkml/2008/7/18/250
arch/x86/mm/pgtable.c: In function `pgd_prepopulate_pmd':
arch/x86/mm/pgtable.c:222: internal compiler error: in remove_insn, at emit-rtl.c:3746 Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.
# i686-unknown-linux-gnu-gcc --version
i686-unknown-linux-gnu-gcc (GCC) 3.4.5
My investigations seem to show that gcc 3.4.5 can't cope with the following construct:
	for (i = 0; i < 0; i++)
		...
or more specifically:
	for (i = 0; i < PREALLOCTED_PMDS; i++)
		...
when PREALLOCTED_PMDS is 0. That is, when X86_PAE is not set.
This patch resolves this problem by moving the relevant code inside #define X86_PAE and providing dummy functions outside !X86_PAE.

Quoted 7 lines

Quoted 2 lines
Signed-off-by Simon Horman <[email protected]>
We resolved the other report by saying that gcc 3.4.4 is broken. It seems 3.4.5 is as well. Could you just update the compiler? I'd rather not have to clutter the code with more ifdefs if we can possibly avoid it.

I'm not sure that I would call that resolved.

The patch doesn't add any extra ifdefs. It just explicitly provides empty functions rather than relying on the compiler to optimise things out.

If gcc 3.4.4 and 3.4.5 really aren't valid compilers to use that is fine. But it seems rather silly that this somewhat simple change renders them unusable for this config.

Quoted 7 lines

Does putting
	if (PREALLOCATED_PMDS == 0)
		return;
before the for loop help?

Yes, it does. The code does compile with the following:

arch/x86/mm/pgtable.c +12 -0

index 557b2ab..938cfe2 100644
@@ -152,6 +152,9 @@ static void free_pmds(pmd_t *pmds[])
 {
 	int i;
 
+	if (PREALLOCATED_PMDS == 0)
+		return;
+
 	for(i = 0; i < PREALLOCATED_PMDS; i++)
 		if (pmds[i])
 			free_page((unsigned long)pmds[i]);

@@ -162,6 +165,9 @@ static int preallocate_pmds(pmd_t *pmds[])
 	int i;
 	bool failed = false;
 
+	if (PREALLOCATED_PMDS == 0)
+		return 0;
+
 	for(i = 0; i < PREALLOCATED_PMDS; i++) {
 		pmd_t *pmd = (pmd_t *)get_zeroed_page(GFP_KERNEL|__GFP_REPEAT);
 		if (pmd == NULL)

@@ -187,6 +193,9 @@ static void pgd_mop_up_pmds(struct mm_struct *mm, pgd_t *pgdp)
 {
 	int i;
 
+	if (PREALLOCATED_PMDS == 0)
+		return;
+
 	for(i = 0; i < PREALLOCATED_PMDS; i++) {
 		pgd_t pgd = pgdp[i];

@@ -207,6 +216,9 @@ static void pgd_prepopulate_pmd(struct mm_struct *mm, pgd_t *pgd, pmd_t *pmds[])
 	unsigned long addr;
 	int i;
 
+	if (PREALLOCATED_PMDS == 0)
+		return;
+
 	pud = pud_offset(pgd, 0);
 
  	for (addr = i = 0; i < PREALLOCATED_PMDS;

Adrian Bunk

2008-08-08 08:01 UTC #3 lore raw

In reply to Jeremy Fitzhardinge

Quoted 36 lines · Jeremy Fitzhardinge

Quoted 35 lines · Simon Horman
Hi,
It appears that "x86: preallocate and prepopulate separately" (d8d5900ef8afc562088f8470feeaf17c4747790f) introduced a minor regression. The build fails on gcc 3.4.5 if CONFIG_DEBUG_INFO=y (that is gcc is called with -g) and X86_PAE not set.
There was previously some discussion of this without resolution.
http://lkml.org/lkml/2008/7/18/250
arch/x86/mm/pgtable.c: In function `pgd_prepopulate_pmd':
arch/x86/mm/pgtable.c:222: internal compiler error: in remove_insn, at emit-rtl.c:3746 Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.
# i686-unknown-linux-gnu-gcc --version
i686-unknown-linux-gnu-gcc (GCC) 3.4.5
My investigations seem to show that gcc 3.4.5 can't cope with the following construct:
	for (i = 0; i < 0; i++)
		...
or more specifically:
	for (i = 0; i < PREALLOCTED_PMDS; i++)
		...
when PREALLOCTED_PMDS is 0. That is, when X86_PAE is not set.
This patch resolves this problem by moving the relevant code inside #define X86_PAE and providing dummy functions outside !X86_PAE.

Quoted 8 lines

Quoted 2 lines
Signed-off-by Simon Horman <[email protected]>
We resolved the other report by saying that gcc 3.4.4 is broken. It seems 3.4.5 is as well. Could you just update the compiler? I'd rather not have to clutter the code with more ifdefs if we can possibly avoid it.
...

CONFIG_DEBUG_INFO=y is why I never hit it before.

It does not seem to be just one dodgy Redhat compiler (as I assumed in the Bugzilla entry) but the whole 3.4 series of gcc which makes our resolution of saying that this specific compiler is broken invalid.

Considering that we officially support all compilers >= 3.2 this is just one more case where we have to add to the kernel a workaround for a broken compiler.

After all, we already have 2 or 3 workarounds for new 2.6.27-rc breakages with gcc 3.2 included or at least submitted.

cu
Adrian

Signature · 4 lines

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Jeremy Fitzhardinge

2008-08-08 15:21 UTC #4 lore raw

Quoted 6 lines · Adrian Bunk

CONFIG_DEBUG_INFO=y is why I never hit it before.
It does not seem to be just one dodgy Redhat compiler (as I assumed in the Bugzilla entry) but the whole 3.4 series of gcc which makes our resolution of saying that this specific compiler is broken invalid.
Considering that we officially support all compilers >= 3.2 this is just one more case where we have to add to the kernel a workaround for a broken compiler.

Yes. I'm happy to write off a specific rpm package, but a whole series is a different matter.

Adding

	if (PREALLOCATED_PMDS == 0)
		return;

in the appropriate places with a small comment seems like the right answer.

Adrian Bunk

2008-08-08 16:13 UTC #5 lore raw

Quoted 15 lines · Jeremy Fitzhardinge

Quoted 10 lines · Adrian Bunk
CONFIG_DEBUG_INFO=y is why I never hit it before.
It does not seem to be just one dodgy Redhat compiler (as I assumed in the Bugzilla entry) but the whole 3.4 series of gcc which makes our resolution of saying that this specific compiler is broken invalid.
Considering that we officially support all compilers >= 3.2 this is just one more case where we have to add to the kernel a workaround for a broken compiler.
Yes. I'm happy to write off a specific rpm package, but a whole series is a different matter.

Quoted 6 lines

Adding
	if (PREALLOCATED_PMDS == 0)
		return;
in the appropriate places with a small comment seems like the right answer.

The patch below you suggested fixes it for me with gcc 3.4.6.

Quoted 1 line

J

cu
Adrian

arch/x86/mm/pgtable.c +3 -0

@@ -209,6 +209,9 @@ static void pgd_prepopulate_pmd(struct mm_struct *mm, pgd_t *pgd, pmd_t *pmds[])
 
 	pud = pud_offset(pgd, 0);
 
+	if (PREALLOCATED_PMDS == 0)
+		return;
+
  	for (addr = i = 0; i < PREALLOCATED_PMDS;
 	     i++, pud++, addr += PUD_SIZE) {
 		pmd_t *pmd = pmds[i];

Jeremy Fitzhardinge

Acked-by

2008-08-08 18:01 UTC #6 lore raw

Quoted 21 lines · Adrian Bunk

Quoted 9 lines · Jeremy Fitzhardinge
Quoted 7 lines · Adrian Bunk
CONFIG_DEBUG_INFO=y is why I never hit it before.
It does not seem to be just one dodgy Redhat compiler (as I assumed in the Bugzilla entry) but the whole 3.4 series of gcc which makes our resolution of saying that this specific compiler is broken invalid.
Considering that we officially support all compilers >= 3.2 this is just one more case where we have to add to the kernel a workaround for a broken compiler.
Quoted 9 lines
Yes. I'm happy to write off a specific rpm package, but a whole series is a different matter.
Adding
	if (PREALLOCATED_PMDS == 0)
		return;
in the appropriate places with a small comment seems like the right answer.

Quoted 9 lines

The patch below you suggested fixes it for me with gcc 3.4.6.
Quoted 2 lines
J
cu
Adrian

OK. Want to submit this as a proper patch?

Acked-by Jeremy Fitzhardinge <[email protected]>

Thanks,
J

Quoted 19 lines

arch/x86/mm/pgtable.c +3 -0

@@ -209,6 +209,9 @@ static void pgd_prepopulate_pmd(struct mm_struct *mm, pgd_t *pgd, pmd_t *pmds[])
 
 	pud = pud_offset(pgd, 0);
 
+	if (PREALLOCATED_PMDS == 0)
+		return;
+
  	for (addr = i = 0; i < PREALLOCATED_PMDS;
 	     i++, pud++, addr += PUD_SIZE) {
 		pmd_t *pmd = pmds[i];

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Adrian Bunk

2008-08-08 18:37 UTC #7 lore raw

Quoted 33 lines · Jeremy Fitzhardinge

Quoted 32 lines · Adrian Bunk
Quoted 16 lines · Jeremy Fitzhardinge
Quoted 11 lines · Adrian Bunk
CONFIG_DEBUG_INFO=y is why I never hit it before.
It does not seem to be just one dodgy Redhat compiler (as I assumed in the Bugzilla entry) but the whole 3.4 series of gcc which makes our resolution of saying that this specific compiler is broken invalid.
Considering that we officially support all compilers >= 3.2 this is just one more case where we have to add to the kernel a workaround for a broken compiler.
Yes. I'm happy to write off a specific rpm package, but a whole series is a different matter.
Quoted 7 lines
Adding
	if (PREALLOCATED_PMDS == 0)
		return;
in the appropriate places with a small comment seems like the right answer.
The patch below you suggested fixes it for me with gcc 3.4.6.
Quoted 2 lines
J

Quoted 6 lines

Quoted 3 lines
cu
Adrian
OK. Want to submit this as a proper patch?
...

No, it's your patch. :-)

Quoted 3 lines

Thanks,
J
...

cu
Adrian

Signature · 4 lines

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Jeremy Fitzhardinge

2008-08-08 20:46 UTC #8 lore raw

PATCH 1/1 x86: work around gcc 3.4.x bug

gcc-3.4.x crashes when compiling pgd_prepopulate_pmd() when PREALLOCATED_PMDS == 0 and CONFIG_DEBUG_INFO is enabled. This seems to avoid the problem.

Signed-off-by Jeremy Fitzhardinge <[email protected]>

Cc Adrian Bunk <[email protected]>

Cc Simon Horman <[email protected]>

Cc Dhaval Giani <[email protected]>

---
arch/x86/mm/pgtable.c | 3 +++

1 file changed, 3 insertions(+)

===================================================================

arch/x86/mm/pgtable.c +0 -0

@@ -207,6 +207,9 @@

	unsigned long addr;
	int i;

+ if (PREALLOCATED_PMDS == 0) /* Work around gcc-3.4.x bug */
+ return;
+

	pud = pud_offset(pgd, 0);

for (addr = i = 0; i < PREALLOCATED_PMDS;

Simon Horman

author Acked-by on v1 1/1

2008-08-09 06:29 UTC #9 lore raw

Re: [PATCH] x86: work around gcc 3.4.x bug

Quoted 8 lines · Jeremy Fitzhardinge

gcc-3.4.x crashes when compiling pgd_prepopulate_pmd() when PREALLOCATED_PMDS == 0 and CONFIG_DEBUG_INFO is enabled. This seems to avoid the problem.
Signed-off-by Jeremy Fitzhardinge <[email protected]>
Cc Adrian Bunk <[email protected]>
Cc Simon Horman <[email protected]>
Cc Dhaval Giani <[email protected]>

Hi Jeremy,

This does solve the problem I was seeing on on 3.4.5.

Acked-by Simon Horman <[email protected]>

Quoted 18 lines

---
arch/x86/mm/pgtable.c | 3 +++

1 file changed, 3 insertions(+)

===================================================================

arch/x86/mm/pgtable.c +3 -0

@@ -207,6 +207,9 @@
 	unsigned long addr;
 	int i;
 
+	if (PREALLOCATED_PMDS == 0) /* Work around gcc-3.4.x bug */
+		return;
+
 	pud = pud_offset(pgd, 0);
 
 	for (addr = i = 0; i < PREALLOCATED_PMDS;

Ingo Molnar

Applied

2008-08-11 16:44 UTC #10 lore raw

Re: [PATCH] x86: work around gcc 3.4.x bug

Quoted 10 lines · Simon Horman

Quoted 8 lines · Jeremy Fitzhardinge
gcc-3.4.x crashes when compiling pgd_prepopulate_pmd() when PREALLOCATED_PMDS == 0 and CONFIG_DEBUG_INFO is enabled. This seems to avoid the problem.
Signed-off-by Jeremy Fitzhardinge <[email protected]>
Cc Adrian Bunk <[email protected]>
Cc Simon Horman <[email protected]>
Cc Dhaval Giani <[email protected]>

Quoted 5 lines

Hi Jeremy,
This does solve the problem I was seeing on on 3.4.5.
Acked-by Simon Horman <[email protected]>

applied to tip/x86/urgent - thanks for tracking this down.

Ingo