Re: [PATCH 0 of 4] x86: some more patches

[Ingo Molnar <mingo@elte.hu>]

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> Ingo Molnar wrote:
>> unfortunately they dont solve it:
>>
>>  [   92.042586] Freeing unused kernel memory: 328k freed
>>  [   92.091838] khelper used greatest stack depth: 6244 bytes left
>>  [   92.281738] init[1]: segfault at 00000004 ip 49471cbb sp bff8dbb0 error 4
>>  [   92.288761] init[1]: segfault at 00000004 ip 49471cbb sp bff8dbb0 error 4
>>  [   92.295763] init[1]: segfault at 00000004 ip 49471cbb sp bff8dbb0 error 4
>>  [...]
>>  [   97.312484] printk: 611046 messages suppressed.
>>
>> 32-bit, PAE and RAM above 4GB seems to be enough to trigger that bug.
>>   
>
> Bum.  We established that once you fix the accessors+sign extension 
> bug, there's another bug somewhere later on in the series.  Any chance 
> you could move "x86/pgtable: fix constant sign extension problem" to 
> be just after "x86/pgtable: unify pagetable accessors" and bisect the 
> following patches?

after a day of debugging i finally tracked it down to another 
unsigned-type and masking mismatch on PAE, caused by:

  Subject: x86: unify pgtable accessors which use supported_pte_mask
  From: Jeremy Fitzhardinge <jeremy@goop.org>

the fix is below.

I have to say, i'm less than impressed about the structure of that 
patch. The patch too was way too coarse for easy bisection. You did 
multiple nontrivial steps in the same patch.

I'm not going to apply such all-in-one patches to x86.git anymore, and 
will start straight with another patch you sent yesterday:

  Subject: x86: refactor mmu ops in paravirt.h
  2 files changed, 129 insertions(+), 88 deletions(-)

If anyone finds a problem in that patch we'll have a hard time figuring 
out what went wrong. Please split that patch up properly. To demonstrate 
the kind of splitup we need for such patches, i've done a sample splitup 
of the following patch in x86.git:

  Subject: x86/pgtable: unify pagetable accessors
  From: Jeremy Fitzhardinge <jeremy@goop.org>

and turned it into 6 easy-to-review and easy-to-validate patches. Each 
step is _provably trivial_, and even if something goes wrong, it's easy 
to figure out where the problem comes from. At each step, document in 
the patch description whether the patch is intended to cause any code 
changes or not.

	Ingo

Index: linux-x86.q/include/asm-x86/page.h
===================================================================
--- linux-x86.q.orig/include/asm-x86/page.h
+++ linux-x86.q/include/asm-x86/page.h
@@ -11,7 +11,7 @@
 #ifdef __KERNEL__
 
 #define PHYSICAL_PAGE_MASK	(PAGE_MASK & __PHYSICAL_MASK)
-#define PTE_MASK		PHYSICAL_PAGE_MASK
+#define PTE_MASK		(_AT(long, PHYSICAL_PAGE_MASK))
 
 #define LARGE_PAGE_SIZE		(_AC(1,UL) << PMD_SHIFT)
 #define LARGE_PAGE_MASK		(~(LARGE_PAGE_SIZE-1))