LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Re: unify pagetable accessors patch causes double fault II
       [not found] <20080114094814.GA28300@basil.nowhere.org>
@ 2008-01-14 12:56 ` Andi Kleen
  2008-01-14 13:06   ` Ingo Molnar
  0 siblings, 1 reply; 30+ messages in thread
From: Andi Kleen @ 2008-01-14 12:56 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Jeremy Fitzhardinge, mingo, tglx, linux-kernel


Subject was wrong of course -- it was a recursive oops, not a double fault.
Sorry for the inaccuracy.

Hopefully it can be fixed soon because it inhibits further testing here.

-Andi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-14 12:56 ` unify pagetable accessors patch causes double fault II Andi Kleen
@ 2008-01-14 13:06   ` Ingo Molnar
  2008-01-14 13:58     ` Andi Kleen
  0 siblings, 1 reply; 30+ messages in thread
From: Ingo Molnar @ 2008-01-14 13:06 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Jeremy Fitzhardinge, tglx, linux-kernel


* Andi Kleen <andi@firstfloor.org> wrote:

> Subject was wrong of course -- it was a recursive oops, not a double 
> fault. Sorry for the inaccuracy.
> 
> Hopefully it can be fixed soon because it inhibits further testing 
> here.

no context. Your first mail did not seem to make it to lkml. (yet)

	Ingo

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-14 13:06   ` Ingo Molnar
@ 2008-01-14 13:58     ` Andi Kleen
  2008-01-14 16:44       ` Jeremy Fitzhardinge
  2008-01-14 22:03       ` Jeremy Fitzhardinge
  0 siblings, 2 replies; 30+ messages in thread
From: Andi Kleen @ 2008-01-14 13:58 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Andi Kleen, Jeremy Fitzhardinge, tglx, linux-kernel

On Mon, Jan 14, 2008 at 02:06:20PM +0100, Ingo Molnar wrote:
> 
> * Andi Kleen <andi@firstfloor.org> wrote:
> 
> > Subject was wrong of course -- it was a recursive oops, not a double 
> > fault. Sorry for the inaccuracy.
> > 
> > Hopefully it can be fixed soon because it inhibits further testing 
> > here.
> 
> no context. Your first mail did not seem to make it to lkml. (yet)

Sorry. Not sure what happened. Here is it again:

----

This is as of 2f42671697ea9abc7d10ea7f663d6ef6e8ec6358 git-x86 HEAD:

One of my test machines here when booted with git-x86 gives a double
fault on entering user space. I bisected it down to the following commit.

commit c64ba9309275f2e89bd18adbe4d932b6ecc7eb07
Author: Jeremy Fitzhardinge <jeremy@goop.org>
Date:   Fri Jan 11 18:11:41 2008 +0100

    x86/pgtable: unify pagetable accessors
    
    Unify functions to test and set bits in pagetable entries.

This is with PAE and seems to only happen with enough RAM (6GB);
a 2GB system boots. 64bit also works.

-Andi

VFS: Mounted root (nfs filesystem).
Freeing unused kernel memory: 260k freed
boot[1151]: segfault at 00000000 ip 00000000 sp bfe5c1fc error 14
Bad page state in process 'boot'
page:c27f8000 flags:0x80080010 mapping:00000000 mapcount:0 count:0
Trying to fix it up, but a reboot is needed
Backtrace:
Pid: 1151, comm: boot Not tainted 2.6.24-rc7-gc64ba930 #10
 [<c01474eb>] bad_page+0x48/0x6f
 [<c01478de>] free_hot_cold_page+0x5b/0x148
 [<c01479e3>] __pagevec_free+0x18/0x22
 [<c014a03d>] release_pages+0x13f/0x147
 [<c0151191>] free_pgtables+0x86/0x93
 [<c01568bb>] free_pages_and_swap_cache+0x6a/0x7e
 [<c01521e1>] exit_mmap+0xa2/0xcd
 [<c011fece>] mmput+0x25/0x79
 [<c01245fa>] do_exit+0x1a9/0x5eb
 [<c0124aa7>] sys_exit_group+0x0/0xd
 [<c012bda2>] get_signal_to_deliver+0x3e3/0x405
 [<c043577c>] do_page_fault+0x0/0x6a4
 [<c01043de>] do_notify_resume+0x7d/0x64e
 [<c0122346>] printk+0x14/0x18
 [<c0435b3a>] do_page_fault+0x3be/0x6a4
 [<c0435e17>] do_page_fault+0x69b/0x6a4
 [<c043577c>] do_page_fault+0x0/0x6a4
 [<c0104d26>] work_notifysig+0x13/0x19
 =======================
Bad page state in process 'boot'
page:c27f8120 flags:0x80000000 mapping:00000000 mapcount:1 count:1
Trying to fix it up, but a reboot is needed
Backtrace:
Pid: 1153, comm: boot Tainted: G    B   2.6.24-rc7-gc64ba930 #10
 [<c01474eb>] bad_page+0x48/0x6f
 [<c0147f27>] get_page_from_freelist+0x242/0x30f
 [<c014807f>] __alloc_pages+0x67/0x2c5
 [<c014e9b5>] do_wp_page+0x20e/0x494
 [<c0150ac2>] handle_mm_fault+0x6c1/0x75a
 [<c0435a2b>] do_page_fault+0x2af/0x6a4
 [<c043577c>] do_page_fault+0x0/0x6a4
 [<c043455a>] error_code+0x72/0x78
 [<c02122aa>] __put_user_4+0x12/0x18
 [<c011e09c>] schedule_tail+0x52/0x55
 [<c0104b6e>] ret_from_fork+0x6/0x1c
 =======================
boot[1153]: segfault at 0574c985 ip b7d9488a sp bfe5bfd8 error 6
Eeek! page_mapcount(page) went negative! (-1)
  page pfn = bfc09
  page->flags = 80000060
  page->count = 1
  page->mapping = f702d091
  vma->vm_ops = 0x0
------------[ cut here ]------------
kernel BUG at /home/lsrc/git-arch-x86/linux-2.6-x86/mm/rmap.c:631!
invalid opcode: 0000 [#1] SMP 
Modules linked in:

Pid: 1153, comm: boot Tainted: G    B   (2.6.24-rc7-gc64ba930 #10)
EIP: 0060:[<c0154bd6>] EFLAGS: 00010246 CPU: 2
EIP is at page_remove_rmap+0xcc/0xe7
EAX: 00000000 EBX: c27f8120 ECX: 00000046 EDX: 00000046
ESI: f7074948 EDI: c27f8120 EBP: f7078198 ESP: f709ddfc
 DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
Process boot (pid: 1153, ti=f709c000 task=f7460000 task.ti=f709c000)
Stack: bfc09045 00000000 c014f18f 00000000 f7074948 f709de80 bfc09045 00000000 
       00000000 00000001 b7e37000 f7070010 f70468c0 c4825240 ffffffff 00000000 
       c16e0f0c 00000000 f70aadf8 003f9ed9 b7e37000 b7e37000 00000000 b7e33000 
Call Trace:
 [<c014f18f>] unmap_vmas+0x334/0x5c9
 [<c015219e>] exit_mmap+0x5f/0xcd
 [<c011fece>] mmput+0x25/0x79
 [<c01245fa>] do_exit+0x1a9/0x5eb
 [<c0124aa7>] sys_exit_group+0x0/0xd
 [<c012bda2>] get_signal_to_deliver+0x3e3/0x405
 [<c043577c>] do_page_fault+0x0/0x6a4
 [<c01043de>] do_notify_resume+0x7d/0x64e
 [<c0122346>] printk+0x14/0x18
 [<c0435b3a>] do_page_fault+0x3be/0x6a4
 [<c0435e17>] do_page_fault+0x69b/0x6a4
 [<c043577c>] do_page_fault+0x0/0x6a4
 [<c0104d26>] work_notifysig+0x13/0x19
 =======================
Code: 8b 46 44 8b 50 08 b8 e3 aa 50 c0 e8 c0 ab fe ff 8b 46 4c 85 c0 74 14 8b 40 10 85 c0 74 0d 8b 50 2c b8 01 ab 50 c0 e8 a5 ab fe ff <0f> 0b eb fe 8b 53 10 89 d8 5b 5e 83 e2 01 f7 da 83 c2 04 e9 e1 
EIP: [<c0154bd6>] page_remove_rmap+0xcc/0xe7 SS:ESP 0068:f709ddfc
---[ end trace 8cd8c46e6dae67bc ]---
Fixing recursive fault but reboot is needed!
eth0: no IPv6 routers present

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-14 13:58     ` Andi Kleen
@ 2008-01-14 16:44       ` Jeremy Fitzhardinge
  2008-01-14 16:56         ` Ingo Molnar
  2008-01-14 22:03       ` Jeremy Fitzhardinge
  1 sibling, 1 reply; 30+ messages in thread
From: Jeremy Fitzhardinge @ 2008-01-14 16:44 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Ingo Molnar, tglx, linux-kernel

Andi Kleen wrote:
> On Mon, Jan 14, 2008 at 02:06:20PM +0100, Ingo Molnar wrote:
>   
>> * Andi Kleen <andi@firstfloor.org> wrote:
>>
>>     
>>> Subject was wrong of course -- it was a recursive oops, not a double 
>>> fault. Sorry for the inaccuracy.
>>>
>>> Hopefully it can be fixed soon because it inhibits further testing 
>>> here.
>>>       
>> no context. Your first mail did not seem to make it to lkml. (yet)
>>     
>
> Sorry. Not sure what happened. Here is it again:
>
> ----
>
> This is as of 2f42671697ea9abc7d10ea7f663d6ef6e8ec6358 git-x86 HEAD:
>
> One of my test machines here when booted with git-x86 gives a double
> fault on entering user space. I bisected it down to the following commit.
>
> commit c64ba9309275f2e89bd18adbe4d932b6ecc7eb07
> Author: Jeremy Fitzhardinge <jeremy@goop.org>
> Date:   Fri Jan 11 18:11:41 2008 +0100
>
>     x86/pgtable: unify pagetable accessors
>     
>     Unify functions to test and set bits in pagetable entries.
>
> This is with PAE and seems to only happen with enough RAM (6GB);
> a 2GB system boots. 64bit also works.
>   

Thanks, I'll have a close look at it.  Presumably a pfn is getting 
truncated to 20/32-bits somewhere.

    J

> -Andi
>
> VFS: Mounted root (nfs filesystem).
> Freeing unused kernel memory: 260k freed
> boot[1151]: segfault at 00000000 ip 00000000 sp bfe5c1fc error 14
> Bad page state in process 'boot'
> page:c27f8000 flags:0x80080010 mapping:00000000 mapcount:0 count:0
> Trying to fix it up, but a reboot is needed
> Backtrace:
> Pid: 1151, comm: boot Not tainted 2.6.24-rc7-gc64ba930 #10
>  [<c01474eb>] bad_page+0x48/0x6f
>  [<c01478de>] free_hot_cold_page+0x5b/0x148
>  [<c01479e3>] __pagevec_free+0x18/0x22
>  [<c014a03d>] release_pages+0x13f/0x147
>  [<c0151191>] free_pgtables+0x86/0x93
>  [<c01568bb>] free_pages_and_swap_cache+0x6a/0x7e
>  [<c01521e1>] exit_mmap+0xa2/0xcd
>  [<c011fece>] mmput+0x25/0x79
>  [<c01245fa>] do_exit+0x1a9/0x5eb
>  [<c0124aa7>] sys_exit_group+0x0/0xd
>  [<c012bda2>] get_signal_to_deliver+0x3e3/0x405
>  [<c043577c>] do_page_fault+0x0/0x6a4
>  [<c01043de>] do_notify_resume+0x7d/0x64e
>  [<c0122346>] printk+0x14/0x18
>  [<c0435b3a>] do_page_fault+0x3be/0x6a4
>  [<c0435e17>] do_page_fault+0x69b/0x6a4
>  [<c043577c>] do_page_fault+0x0/0x6a4
>  [<c0104d26>] work_notifysig+0x13/0x19
>  =======================
> Bad page state in process 'boot'
> page:c27f8120 flags:0x80000000 mapping:00000000 mapcount:1 count:1
> Trying to fix it up, but a reboot is needed
> Backtrace:
> Pid: 1153, comm: boot Tainted: G    B   2.6.24-rc7-gc64ba930 #10
>  [<c01474eb>] bad_page+0x48/0x6f
>  [<c0147f27>] get_page_from_freelist+0x242/0x30f
>  [<c014807f>] __alloc_pages+0x67/0x2c5
>  [<c014e9b5>] do_wp_page+0x20e/0x494
>  [<c0150ac2>] handle_mm_fault+0x6c1/0x75a
>  [<c0435a2b>] do_page_fault+0x2af/0x6a4
>  [<c043577c>] do_page_fault+0x0/0x6a4
>  [<c043455a>] error_code+0x72/0x78
>  [<c02122aa>] __put_user_4+0x12/0x18
>  [<c011e09c>] schedule_tail+0x52/0x55
>  [<c0104b6e>] ret_from_fork+0x6/0x1c
>  =======================
> boot[1153]: segfault at 0574c985 ip b7d9488a sp bfe5bfd8 error 6
> Eeek! page_mapcount(page) went negative! (-1)
>   page pfn = bfc09
>   page->flags = 80000060
>   page->count = 1
>   page->mapping = f702d091
>   vma->vm_ops = 0x0
> ------------[ cut here ]------------
> kernel BUG at /home/lsrc/git-arch-x86/linux-2.6-x86/mm/rmap.c:631!
> invalid opcode: 0000 [#1] SMP 
> Modules linked in:
>
> Pid: 1153, comm: boot Tainted: G    B   (2.6.24-rc7-gc64ba930 #10)
> EIP: 0060:[<c0154bd6>] EFLAGS: 00010246 CPU: 2
> EIP is at page_remove_rmap+0xcc/0xe7
> EAX: 00000000 EBX: c27f8120 ECX: 00000046 EDX: 00000046
> ESI: f7074948 EDI: c27f8120 EBP: f7078198 ESP: f709ddfc
>  DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
> Process boot (pid: 1153, ti=f709c000 task=f7460000 task.ti=f709c000)
> Stack: bfc09045 00000000 c014f18f 00000000 f7074948 f709de80 bfc09045 00000000 
>        00000000 00000001 b7e37000 f7070010 f70468c0 c4825240 ffffffff 00000000 
>        c16e0f0c 00000000 f70aadf8 003f9ed9 b7e37000 b7e37000 00000000 b7e33000 
> Call Trace:
>  [<c014f18f>] unmap_vmas+0x334/0x5c9
>  [<c015219e>] exit_mmap+0x5f/0xcd
>  [<c011fece>] mmput+0x25/0x79
>  [<c01245fa>] do_exit+0x1a9/0x5eb
>  [<c0124aa7>] sys_exit_group+0x0/0xd
>  [<c012bda2>] get_signal_to_deliver+0x3e3/0x405
>  [<c043577c>] do_page_fault+0x0/0x6a4
>  [<c01043de>] do_notify_resume+0x7d/0x64e
>  [<c0122346>] printk+0x14/0x18
>  [<c0435b3a>] do_page_fault+0x3be/0x6a4
>  [<c0435e17>] do_page_fault+0x69b/0x6a4
>  [<c043577c>] do_page_fault+0x0/0x6a4
>  [<c0104d26>] work_notifysig+0x13/0x19
>  =======================
> Code: 8b 46 44 8b 50 08 b8 e3 aa 50 c0 e8 c0 ab fe ff 8b 46 4c 85 c0 74 14 8b 40 10 85 c0 74 0d 8b 50 2c b8 01 ab 50 c0 e8 a5 ab fe ff <0f> 0b eb fe 8b 53 10 89 d8 5b 5e 83 e2 01 f7 da 83 c2 04 e9 e1 
> EIP: [<c0154bd6>] page_remove_rmap+0xcc/0xe7 SS:ESP 0068:f709ddfc
> ---[ end trace 8cd8c46e6dae67bc ]---
> Fixing recursive fault but reboot is needed!
> eth0: no IPv6 routers present
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>   


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-14 16:44       ` Jeremy Fitzhardinge
@ 2008-01-14 16:56         ` Ingo Molnar
  2008-01-14 17:08           ` Andi Kleen
  2008-01-14 19:52           ` unify pagetable accessors patch causes double fault II Jeremy Fitzhardinge
  0 siblings, 2 replies; 30+ messages in thread
From: Ingo Molnar @ 2008-01-14 16:56 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: Andi Kleen, tglx, linux-kernel


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

>> This is with PAE and seems to only happen with enough RAM (6GB); a 
>> 2GB system boots. 64bit also works.
>
> Thanks, I'll have a close look at it.  Presumably a pfn is getting 
> truncated to 20/32-bits somewhere.

would be nice to have some debugging apparatus for bugs like this. 
Perhaps artificially add a large pfn, then convert/unconvert, then 
subtract it and expect the whole transformation to be an identity 
mapping? This way we could simulate most of the effects of >4GB RAM, 
right?

	Ingo

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-14 16:56         ` Ingo Molnar
@ 2008-01-14 17:08           ` Andi Kleen
  2008-01-14 17:18             ` unify pagetable accessors patch causes double fault III Andi Kleen
  2008-01-14 19:52           ` unify pagetable accessors patch causes double fault II Jeremy Fitzhardinge
  1 sibling, 1 reply; 30+ messages in thread
From: Andi Kleen @ 2008-01-14 17:08 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Jeremy Fitzhardinge, Andi Kleen, tglx, linux-kernel

> would be nice to have some debugging apparatus for bugs like this. 
> Perhaps artificially add a large pfn, then convert/unconvert, then 
> subtract it and expect the whole transformation to be an identity 
> mapping? This way we could simulate most of the effects of >4GB RAM, 
> right?

If you want to really test it regularly hacking KVM or Xen or Qemu 
to supply large virtual addresses is probably better. Long ago we
tested with a special BIOS that always put some memory high up
the map.

But then Addresses >3GB are pretty common now because a lot of people put in
2 2 GB DIMMs into systems (2GB DIMMs are not really that expensive anymore). 
And with memory hole remapping you get suitable large addresses then. So
it should be tested often enough.

On the other hand the whole area was always subtly broken. I yesterday 
finally tracked down 

ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt/patches/pgtable-nx

which was broken forever. What an embarassing bug :/

-Andi


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault III
  2008-01-14 17:08           ` Andi Kleen
@ 2008-01-14 17:18             ` Andi Kleen
  2008-01-14 19:00               ` Ingo Molnar
  0 siblings, 1 reply; 30+ messages in thread
From: Andi Kleen @ 2008-01-14 17:18 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Ingo Molnar, Jeremy Fitzhardinge, tglx, linux-kernel


BTW there seems to be some other instability that hits even on 64bit.
Since I updated my workstation to a recent git-x86 based kernel
(admittedly with my patches too) I had at least one hard hang already.

Previously it ran an older -rc5 based kernel without git-x86 also with most
of my patches and was stable for >23 days until a KVM oops required
a reboot.

It unfortunately was not set up to receive console at this point
and it hung in X, but I'm watching it now.

-Andi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault III
  2008-01-14 17:18             ` unify pagetable accessors patch causes double fault III Andi Kleen
@ 2008-01-14 19:00               ` Ingo Molnar
  2008-01-14 19:54                 ` Jeremy Fitzhardinge
  2008-01-14 20:15                 ` Andi Kleen
  0 siblings, 2 replies; 30+ messages in thread
From: Ingo Molnar @ 2008-01-14 19:00 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Jeremy Fitzhardinge, tglx, linux-kernel


* Andi Kleen <andi@firstfloor.org> wrote:

> BTW there seems to be some other instability that hits even on 64bit. 
> Since I updated my workstation to a recent git-x86 based kernel 
> (admittedly with my patches too) I had at least one hard hang already.
> 
> Previously it ran an older -rc5 based kernel without git-x86 also with 
> most of my patches and was stable for >23 days until a KVM oops 
> required a reboot.
> 
> It unfortunately was not set up to receive console at this point and 
> it hung in X, but I'm watching it now.

hm. Did the hanging kernel have the i387 regset fix from Roland already? 
That bug indeed manifested itself as rare desktop hangs.

other than that i know of no pending instability but the large-RAM 
bootup crash you reported.

	Ingo

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-14 16:56         ` Ingo Molnar
  2008-01-14 17:08           ` Andi Kleen
@ 2008-01-14 19:52           ` Jeremy Fitzhardinge
  1 sibling, 0 replies; 30+ messages in thread
From: Jeremy Fitzhardinge @ 2008-01-14 19:52 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Andi Kleen, tglx, linux-kernel

Ingo Molnar wrote:
> would be nice to have some debugging apparatus for bugs like this. 
> Perhaps artificially add a large pfn, then convert/unconvert, then 
> subtract it and expect the whole transformation to be an identity 
> mapping? This way we could simulate most of the effects of >4GB RAM, 
> right?

Yeah, good idea.  I'll code something up to run at pagetable init time 
or something.

    J


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault III
  2008-01-14 19:00               ` Ingo Molnar
@ 2008-01-14 19:54                 ` Jeremy Fitzhardinge
  2008-01-14 20:15                 ` Andi Kleen
  1 sibling, 0 replies; 30+ messages in thread
From: Jeremy Fitzhardinge @ 2008-01-14 19:54 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Andi Kleen, tglx, linux-kernel

Ingo Molnar wrote:
> hm. Did the hanging kernel have the i387 regset fix from Roland already? 
> That bug indeed manifested itself as rare desktop hangs.
>
> other than that i know of no pending instability but the large-RAM 
> bootup crash you reported.

I read that as Andi is having general stability problems rather than 
specific large memory ones.

I've been running x86 pretty stably on my 2G 32-PAE machine.  I had 
problems a few days ago with X crashing on startup, but I attributed 
that to the 387 problem (it went away after an x86.git update).

    J

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault III
  2008-01-14 19:00               ` Ingo Molnar
  2008-01-14 19:54                 ` Jeremy Fitzhardinge
@ 2008-01-14 20:15                 ` Andi Kleen
  1 sibling, 0 replies; 30+ messages in thread
From: Andi Kleen @ 2008-01-14 20:15 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Andi Kleen, Jeremy Fitzhardinge, tglx, linux-kernel

On Mon, Jan 14, 2008 at 08:00:04PM +0100, Ingo Molnar wrote:
> 
> * Andi Kleen <andi@firstfloor.org> wrote:
> 
> > BTW there seems to be some other instability that hits even on 64bit. 
> > Since I updated my workstation to a recent git-x86 based kernel 
> > (admittedly with my patches too) I had at least one hard hang already.
> > 
> > Previously it ran an older -rc5 based kernel without git-x86 also with 
> > most of my patches and was stable for >23 days until a KVM oops 
> > required a reboot.
> > 
> > It unfortunately was not set up to receive console at this point and 
> > it hung in X, but I'm watching it now.
> 
> hm. Did the hanging kernel have the i387 regset fix from Roland already? 
> That bug indeed manifested itself as rare desktop hangs.

Which patch was that exactly? I don't see an obvious candidate. It was
git-x86 of friday or so. I still got the whole patch, but not the change
set tip because I've updated since again.

The hang happened while I started a large java program (groupwise) and
the machine was dead as in no pings anymore.

KVM might have been running in the background but only idling (I earlier
managed to oops the 2.6.24-rc* KVM, but only with a special guest 
which was not active at that time) 

-Andi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-14 13:58     ` Andi Kleen
  2008-01-14 16:44       ` Jeremy Fitzhardinge
@ 2008-01-14 22:03       ` Jeremy Fitzhardinge
  2008-01-14 22:23         ` Andi Kleen
  1 sibling, 1 reply; 30+ messages in thread
From: Jeremy Fitzhardinge @ 2008-01-14 22:03 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Ingo Molnar, tglx, linux-kernel

Andi Kleen wrote:
> On Mon, Jan 14, 2008 at 02:06:20PM +0100, Ingo Molnar wrote:
>   
>> * Andi Kleen <andi@firstfloor.org> wrote:
>>
>>     
>>> Subject was wrong of course -- it was a recursive oops, not a double 
>>> fault. Sorry for the inaccuracy.
>>>
>>> Hopefully it can be fixed soon because it inhibits further testing 
>>> here.
>>>       
>> no context. Your first mail did not seem to make it to lkml. (yet)
>>     
>
> Sorry. Not sure what happened. Here is it again:
>
> ----
>
> This is as of 2f42671697ea9abc7d10ea7f663d6ef6e8ec6358 git-x86 HEAD:
>
> One of my test machines here when booted with git-x86 gives a double
> fault on entering user space. I bisected it down to the following commit.
>
> commit c64ba9309275f2e89bd18adbe4d932b6ecc7eb07
> Author: Jeremy Fitzhardinge <jeremy@goop.org>
> Date:   Fri Jan 11 18:11:41 2008 +0100
>
>     x86/pgtable: unify pagetable accessors
>     
>     Unify functions to test and set bits in pagetable entries.
>
> This is with PAE and seems to only happen with enough RAM (6GB);
> a 2GB system boots. 64bit also works.
>   

OK, I see the problem.  The problem is that the _PAGE_X defines are 
defined with _AC(UL, 1 << _PAGE_BIT_X), which has unsigned long type.  
This means that ~_PAGE_X also has unsigned long type, and so when cast 
to 64-bit in pte_mkX, it ends up &ing the pte with 0x00000000ffffffxxx, 
with predictable results.

The original code just used signed constants for the _PAGE_X 
definitions, which will sign-extend when cast to 64-bit, and so have the 
upper bits set when masking.  (Well, actually, the old code just 
operated on pte_low, so the problem didn't arise; however, pgtable_64.h 
also uses integers for its _PAGE_X, which has the same sign-extended 
32->64 casting property).

I'll put together a fixup patch now.

    J

> -Andi
>
> VFS: Mounted root (nfs filesystem).
> Freeing unused kernel memory: 260k freed
> boot[1151]: segfault at 00000000 ip 00000000 sp bfe5c1fc error 14
> Bad page state in process 'boot'
> page:c27f8000 flags:0x80080010 mapping:00000000 mapcount:0 count:0
> Trying to fix it up, but a reboot is needed
> Backtrace:
> Pid: 1151, comm: boot Not tainted 2.6.24-rc7-gc64ba930 #10
>  [<c01474eb>] bad_page+0x48/0x6f
>  [<c01478de>] free_hot_cold_page+0x5b/0x148
>  [<c01479e3>] __pagevec_free+0x18/0x22
>  [<c014a03d>] release_pages+0x13f/0x147
>  [<c0151191>] free_pgtables+0x86/0x93
>  [<c01568bb>] free_pages_and_swap_cache+0x6a/0x7e
>  [<c01521e1>] exit_mmap+0xa2/0xcd
>  [<c011fece>] mmput+0x25/0x79
>  [<c01245fa>] do_exit+0x1a9/0x5eb
>  [<c0124aa7>] sys_exit_group+0x0/0xd
>  [<c012bda2>] get_signal_to_deliver+0x3e3/0x405
>  [<c043577c>] do_page_fault+0x0/0x6a4
>  [<c01043de>] do_notify_resume+0x7d/0x64e
>  [<c0122346>] printk+0x14/0x18
>  [<c0435b3a>] do_page_fault+0x3be/0x6a4
>  [<c0435e17>] do_page_fault+0x69b/0x6a4
>  [<c043577c>] do_page_fault+0x0/0x6a4
>  [<c0104d26>] work_notifysig+0x13/0x19
>  =======================
> Bad page state in process 'boot'
> page:c27f8120 flags:0x80000000 mapping:00000000 mapcount:1 count:1
> Trying to fix it up, but a reboot is needed
> Backtrace:
> Pid: 1153, comm: boot Tainted: G    B   2.6.24-rc7-gc64ba930 #10
>  [<c01474eb>] bad_page+0x48/0x6f
>  [<c0147f27>] get_page_from_freelist+0x242/0x30f
>  [<c014807f>] __alloc_pages+0x67/0x2c5
>  [<c014e9b5>] do_wp_page+0x20e/0x494
>  [<c0150ac2>] handle_mm_fault+0x6c1/0x75a
>  [<c0435a2b>] do_page_fault+0x2af/0x6a4
>  [<c043577c>] do_page_fault+0x0/0x6a4
>  [<c043455a>] error_code+0x72/0x78
>  [<c02122aa>] __put_user_4+0x12/0x18
>  [<c011e09c>] schedule_tail+0x52/0x55
>  [<c0104b6e>] ret_from_fork+0x6/0x1c
>  =======================
> boot[1153]: segfault at 0574c985 ip b7d9488a sp bfe5bfd8 error 6
> Eeek! page_mapcount(page) went negative! (-1)
>   page pfn = bfc09
>   page->flags = 80000060
>   page->count = 1
>   page->mapping = f702d091
>   vma->vm_ops = 0x0
> ------------[ cut here ]------------
> kernel BUG at /home/lsrc/git-arch-x86/linux-2.6-x86/mm/rmap.c:631!
> invalid opcode: 0000 [#1] SMP 
> Modules linked in:
>
> Pid: 1153, comm: boot Tainted: G    B   (2.6.24-rc7-gc64ba930 #10)
> EIP: 0060:[<c0154bd6>] EFLAGS: 00010246 CPU: 2
> EIP is at page_remove_rmap+0xcc/0xe7
> EAX: 00000000 EBX: c27f8120 ECX: 00000046 EDX: 00000046
> ESI: f7074948 EDI: c27f8120 EBP: f7078198 ESP: f709ddfc
>  DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
> Process boot (pid: 1153, ti=f709c000 task=f7460000 task.ti=f709c000)
> Stack: bfc09045 00000000 c014f18f 00000000 f7074948 f709de80 bfc09045 00000000 
>        00000000 00000001 b7e37000 f7070010 f70468c0 c4825240 ffffffff 00000000 
>        c16e0f0c 00000000 f70aadf8 003f9ed9 b7e37000 b7e37000 00000000 b7e33000 
> Call Trace:
>  [<c014f18f>] unmap_vmas+0x334/0x5c9
>  [<c015219e>] exit_mmap+0x5f/0xcd
>  [<c011fece>] mmput+0x25/0x79
>  [<c01245fa>] do_exit+0x1a9/0x5eb
>  [<c0124aa7>] sys_exit_group+0x0/0xd
>  [<c012bda2>] get_signal_to_deliver+0x3e3/0x405
>  [<c043577c>] do_page_fault+0x0/0x6a4
>  [<c01043de>] do_notify_resume+0x7d/0x64e
>  [<c0122346>] printk+0x14/0x18
>  [<c0435b3a>] do_page_fault+0x3be/0x6a4
>  [<c0435e17>] do_page_fault+0x69b/0x6a4
>  [<c043577c>] do_page_fault+0x0/0x6a4
>  [<c0104d26>] work_notifysig+0x13/0x19
>  =======================
> Code: 8b 46 44 8b 50 08 b8 e3 aa 50 c0 e8 c0 ab fe ff 8b 46 4c 85 c0 74 14 8b 40 10 85 c0 74 0d 8b 50 2c b8 01 ab 50 c0 e8 a5 ab fe ff <0f> 0b eb fe 8b 53 10 89 d8 5b 5e 83 e2 01 f7 da 83 c2 04 e9 e1 
> EIP: [<c0154bd6>] page_remove_rmap+0xcc/0xe7 SS:ESP 0068:f709ddfc
> ---[ end trace 8cd8c46e6dae67bc ]---
> Fixing recursive fault but reboot is needed!
> eth0: no IPv6 routers present
>   


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-14 22:03       ` Jeremy Fitzhardinge
@ 2008-01-14 22:23         ` Andi Kleen
  2008-01-14 22:46           ` Jeremy Fitzhardinge
  0 siblings, 1 reply; 30+ messages in thread
From: Andi Kleen @ 2008-01-14 22:23 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: Andi Kleen, Ingo Molnar, tglx, linux-kernel

> OK, I see the problem.  The problem is that the _PAGE_X defines are 
> defined with _AC(UL, 1 << _PAGE_BIT_X), which has unsigned long type.  
> This means that ~_PAGE_X also has unsigned long type, and so when cast 
> to 64-bit in pte_mkX, it ends up &ing the pte with 0x00000000ffffffxxx, 
> with predictable results.

Actually I fixed some of that -- see the pgtable-nx patch on firstfloor -- but
it still doesn't work. Or maybe my patch was not complete.
> 
> The original code just used signed constants for the _PAGE_X 
> definitions, which will sign-extend when cast to 64-bit, and so have the 
> upper bits set when masking.  (Well, actually, the old code just 
> operated on pte_low, so the problem didn't arise; however, pgtable_64.h 
> also uses integers for its _PAGE_X, which has the same sign-extended 
> 32->64 casting property).
> 
> I'll put together a fixup patch now.

I'm leaving now but can test later.

-Andi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-14 22:23         ` Andi Kleen
@ 2008-01-14 22:46           ` Jeremy Fitzhardinge
  2008-01-15  1:05             ` Andi Kleen
  2008-01-15 13:53             ` Ingo Molnar
  0 siblings, 2 replies; 30+ messages in thread
From: Jeremy Fitzhardinge @ 2008-01-14 22:46 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Ingo Molnar, tglx, linux-kernel

Andi Kleen wrote:
>> OK, I see the problem.  The problem is that the _PAGE_X defines are 
>> defined with _AC(UL, 1 << _PAGE_BIT_X), which has unsigned long type.  
>> This means that ~_PAGE_X also has unsigned long type, and so when cast 
>> to 64-bit in pte_mkX, it ends up &ing the pte with 0x00000000ffffffxxx, 
>> with predictable results.
>>     
>
> Actually I fixed some of that -- see the pgtable-nx patch on firstfloor -- but
> it still doesn't work. Or maybe my patch was not complete.
>   

Yeah, that looks like the right sort of thing, but I wonder if there's 
other places doing an open-coded "pte_val(pte) & ~_PAGE_FOO".  My patch 
changes the definition of _PAGE_FOO so it should be OK everywhere.

>> The original code just used signed constants for the _PAGE_X 
>> definitions, which will sign-extend when cast to 64-bit, and so have the 
>> upper bits set when masking.  (Well, actually, the old code just 
>> operated on pte_low, so the problem didn't arise; however, pgtable_64.h 
>> also uses integers for its _PAGE_X, which has the same sign-extended 
>> 32->64 casting property).
>>
>> I'll put together a fixup patch now.
>>     
>
> I'm leaving now but can test later.
>   

Can you try this out?  It applies after "x86: move all asm/pgtable 
constants into one place".


Subject: x86/pgtable: fix constant sign extension problem

When the _PAGE_FOO constants are defined as (1ul << _PAGE_BIT_FOO), they
become unsigned longs.  In 32-bit PAE mode, these end up being
implicitly cast to 64-bit types when used to manipulate a pte, and
because they're unsigned the top 32-bits are 0, destroying the upper
bits of the pte.

When _PAGE_FOO constants are given a signed integer type, the cast to
64-bits will sign-extend so that the upper bits are all ones,
preserving the upper pte bits in manipulations.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Andi Kleen <ak@suse.de>

---
 include/asm-x86/pgtable.h |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

===================================================================
--- a/include/asm-x86/pgtable.h
+++ b/include/asm-x86/pgtable.h
@@ -19,18 +19,18 @@
 #define _PAGE_BIT_UNUSED3	11
 #define _PAGE_BIT_NX		63       /* No execute: only valid after cpuid check */
 
-#define _PAGE_PRESENT	(_AC(1, UL)<<_PAGE_BIT_PRESENT)
-#define _PAGE_RW	(_AC(1, UL)<<_PAGE_BIT_RW)
-#define _PAGE_USER	(_AC(1, UL)<<_PAGE_BIT_USER)
-#define _PAGE_PWT	(_AC(1, UL)<<_PAGE_BIT_PWT)
-#define _PAGE_PCD	(_AC(1, UL)<<_PAGE_BIT_PCD)
-#define _PAGE_ACCESSED	(_AC(1, UL)<<_PAGE_BIT_ACCESSED)
-#define _PAGE_DIRTY	(_AC(1, UL)<<_PAGE_BIT_DIRTY)
-#define _PAGE_PSE	(_AC(1, UL)<<_PAGE_BIT_PSE)	/* 2MB page */
-#define _PAGE_GLOBAL	(_AC(1, UL)<<_PAGE_BIT_GLOBAL)	/* Global TLB entry */
-#define _PAGE_UNUSED1	(_AC(1, UL)<<_PAGE_BIT_UNUSED1)
-#define _PAGE_UNUSED2	(_AC(1, UL)<<_PAGE_BIT_UNUSED2)
-#define _PAGE_UNUSED3	(_AC(1, UL)<<_PAGE_BIT_UNUSED3)
+#define _PAGE_PRESENT	(_AC(1, L)<<_PAGE_BIT_PRESENT)
+#define _PAGE_RW	(_AC(1, L)<<_PAGE_BIT_RW)
+#define _PAGE_USER	(_AC(1, L)<<_PAGE_BIT_USER)
+#define _PAGE_PWT	(_AC(1, L)<<_PAGE_BIT_PWT)
+#define _PAGE_PCD	(_AC(1, L)<<_PAGE_BIT_PCD)
+#define _PAGE_ACCESSED	(_AC(1, L)<<_PAGE_BIT_ACCESSED)
+#define _PAGE_DIRTY	(_AC(1, L)<<_PAGE_BIT_DIRTY)
+#define _PAGE_PSE	(_AC(1, L)<<_PAGE_BIT_PSE)	/* 2MB page */
+#define _PAGE_GLOBAL	(_AC(1, L)<<_PAGE_BIT_GLOBAL)	/* Global TLB entry */
+#define _PAGE_UNUSED1	(_AC(1, L)<<_PAGE_BIT_UNUSED1)
+#define _PAGE_UNUSED2	(_AC(1, L)<<_PAGE_BIT_UNUSED2)
+#define _PAGE_UNUSED3	(_AC(1, L)<<_PAGE_BIT_UNUSED3)
 
 #if defined(CONFIG_X86_64) || defined(CONFIG_X86_PAE)
 #define _PAGE_NX	(_AC(1, ULL) << _PAGE_BIT_NX)




^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-14 22:46           ` Jeremy Fitzhardinge
@ 2008-01-15  1:05             ` Andi Kleen
  2008-01-15  1:32               ` Jeremy Fitzhardinge
  2008-01-15 12:55               ` unify pagetable accessors patch causes double fault II Ingo Molnar
  2008-01-15 13:53             ` Ingo Molnar
  1 sibling, 2 replies; 30+ messages in thread
From: Andi Kleen @ 2008-01-15  1:05 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: Andi Kleen, Ingo Molnar, tglx, linux-kernel

On Mon, Jan 14, 2008 at 02:46:00PM -0800, Jeremy Fitzhardinge wrote:
> Andi Kleen wrote:
> >>OK, I see the problem.  The problem is that the _PAGE_X defines are 
> >>defined with _AC(UL, 1 << _PAGE_BIT_X), which has unsigned long type.  
> >>This means that ~_PAGE_X also has unsigned long type, and so when cast 
> >>to 64-bit in pte_mkX, it ends up &ing the pte with 0x00000000ffffffxxx, 
> >>with predictable results.
> >>    
> >
> >Actually I fixed some of that -- see the pgtable-nx patch on firstfloor -- 
> >but
> >it still doesn't work. Or maybe my patch was not complete.
> >  
> 
> Yeah, that looks like the right sort of thing, but I wonder if there's 
> other places doing an open-coded "pte_val(pte) & ~_PAGE_FOO".  My patch 
> changes the definition of _PAGE_FOO so it should be OK everywhere.

Yes that's probably better. I just hit it because NX bits went missing.

> 
> >>The original code just used signed constants for the _PAGE_X 
> >>definitions, which will sign-extend when cast to 64-bit, and so have the 
> >>upper bits set when masking.  (Well, actually, the old code just 
> >>operated on pte_low, so the problem didn't arise; however, pgtable_64.h 
> >>also uses integers for its _PAGE_X, which has the same sign-extended 
> >>32->64 casting property).
> >>
> >>I'll put together a fixup patch now.
> >>    
> >
> >I'm leaving now but can test later.
> >  
> 
> Can you try this out?  It applies after "x86: move all asm/pgtable 
> constants into one place".

I just applied over the whole git-x86 patchkit (tip 
2f42671697ea9abc7d10ea7f663d6ef6e8ec6358)  + the two build fixes
Unfortunately it didn't work, although the faulting loop is shorter now:

-Andi


Freeing unused kernel memory: 260k freed
mount[1159]: segfault at b7fcfe98 ip b7ec6b7a sp bfcfcdec error 7
Eeek! page_mapcount(page) went negative! (-1)
  page pfn = bfc33
  page->flags = 80000014
  page->count = 0
  page->mapping = 00000000
  vma->vm_ops = nfs_file_vm_ops+0x0/0x18
  vma->vm_ops->nopage = 0x0
  vma->vm_ops->fault = filemap_fault+0x0/0x34e
  vma->vm_file->f_op->mmap = nfs_file_mmap+0x0/0x61
------------[ cut here ]------------
kernel BUG at /home/lsrc/quilt/linux/mm/rmap.c:631!
invalid opcode: 0000 [#1] SMP
Modules linked in:

Pid: 1159, comm: mount Not tainted (2.6.24-rc7 #55)
EIP: 0060:[<c0152a32>] EFLAGS: 00010282 CPU: 3
EIP is at page_remove_rmap+0xcc/0xe7
EAX: 00000037 EBX: c27f8660 ECX: 00000046 EDX: 00000046
ESI: f7061c60 EDI: f7d87380 EBP: f7060e78 ESP: f7509df8
 DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
Process mount (pid: 1159, ti=f7508000 task=f7460540 task.ti=f7508000)
Stack: c27f8660 bfc33065 c014d166 00000000 f7061c60 f7509e80 00000000 00000000
       00000000 00000001 b7fd0000 f7555010 f7d87380 c482e240 00000000 ffffffff
       c16e0c0c 00000000 00000000 f7084df8 003d5ee2 b7fd0000 b7fd0000 00000000
Call Trace:
 [<c014d166>] unmap_vmas+0x349/0x5d1
 [<c0150006>] exit_mmap+0x5f/0xcd
 [<c011df0a>] mmput+0x25/0x79
 [<c0122636>] do_exit+0x1a9/0x5eb
 [<c0122ae3>] sys_exit_group+0x0/0xd
 [<c0129dde>] get_signal_to_deliver+0x3e3/0x405
 [<c04334b6>] do_page_fault+0x0/0x69f
 [<c0102606>] do_notify_resume+0x7d/0x64e
 [<c0120383>] printk+0x14/0x18
 [<c0433874>] do_page_fault+0x3be/0x69f
 [<c0433b4c>] do_page_fault+0x696/0x69f
 [<c01508c1>] do_munmap+0x181/0x19b
 [<c04334b6>] do_page_fault+0x0/0x69f
 [<c0102f4e>] work_notifysig+0x13/0x19
 =======================
Code: 8b 46 44 8b 50 08 b8 d2 95 50 c0 e8 9c ad fe ff 8b 46 4c 85 c0 74 14 8b 40 10 85 c0 74 0d 8b 50 2c b8 f0 95 50 c0 e8 81 ad fe ff <0f> 0b eb fe 8b 53 10 89 d8 5b 5e 83 e2 01 f7 da 83 c2 04 e9 c5
EIP: [<c0152a32>] page_remove_rmap+0xcc/0xe7 SS:ESP 0068:f7509df8
---[ end trace eb3c259ee0695fdd ]---
Fixing recursive fault but reboot is needed!
eth0: no IPv6 routers present


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-15  1:05             ` Andi Kleen
@ 2008-01-15  1:32               ` Jeremy Fitzhardinge
  2008-01-15  1:38                 ` Andi Kleen
  2008-01-15 12:55               ` unify pagetable accessors patch causes double fault II Ingo Molnar
  1 sibling, 1 reply; 30+ messages in thread
From: Jeremy Fitzhardinge @ 2008-01-15  1:32 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Ingo Molnar, tglx, linux-kernel

Andi Kleen wrote:
> On Mon, Jan 14, 2008 at 02:46:00PM -0800, Jeremy Fitzhardinge wrote:
>   
>> Andi Kleen wrote:
>>     
>>>> OK, I see the problem.  The problem is that the _PAGE_X defines are 
>>>> defined with _AC(UL, 1 << _PAGE_BIT_X), which has unsigned long type.  
>>>> This means that ~_PAGE_X also has unsigned long type, and so when cast 
>>>> to 64-bit in pte_mkX, it ends up &ing the pte with 0x00000000ffffffxxx, 
>>>> with predictable results.
>>>>    
>>>>         
>>> Actually I fixed some of that -- see the pgtable-nx patch on firstfloor -- 
>>> but
>>> it still doesn't work. Or maybe my patch was not complete.
>>>  
>>>       
>> Yeah, that looks like the right sort of thing, but I wonder if there's 
>> other places doing an open-coded "pte_val(pte) & ~_PAGE_FOO".  My patch 
>> changes the definition of _PAGE_FOO so it should be OK everywhere.
>>     
>
> Yes that's probably better. I just hit it because NX bits went missing.
>
>   
>>>> The original code just used signed constants for the _PAGE_X 
>>>> definitions, which will sign-extend when cast to 64-bit, and so have the 
>>>> upper bits set when masking.  (Well, actually, the old code just 
>>>> operated on pte_low, so the problem didn't arise; however, pgtable_64.h 
>>>> also uses integers for its _PAGE_X, which has the same sign-extended 
>>>> 32->64 casting property).
>>>>
>>>> I'll put together a fixup patch now.
>>>>    
>>>>         
>>> I'm leaving now but can test later.
>>>  
>>>       
>> Can you try this out?  It applies after "x86: move all asm/pgtable 
>> constants into one place".
>>     
>
> I just applied over the whole git-x86 patchkit (tip 
> 2f42671697ea9abc7d10ea7f663d6ef6e8ec6358)  + the two build fixes
> Unfortunately it didn't work, although the faulting loop is shorter now:
>   

Just to be clear, these two patches cause this oops to appear?  Current 
x86.git seems to be missing a pile of patches at the moment, so I 
presume you're using a version which does have my patch set?

> Freeing unused kernel memory: 260k freed
> mount[1159]: segfault at b7fcfe98 ip b7ec6b7a sp bfcfcdec error 7
> Eeek! page_mapcount(page) went negative! (-1)
>   page pfn = bfc33
>   page->flags = 80000014
>   page->count = 0
>   page->mapping = 00000000
>   vma->vm_ops = nfs_file_vm_ops+0x0/0x18
>   vma->vm_ops->nopage = 0x0
>   vma->vm_ops->fault = filemap_fault+0x0/0x34e
>   vma->vm_file->f_op->mmap = nfs_file_mmap+0x0/0x61
> ------------[ cut here ]------------
> kernel BUG at /home/lsrc/quilt/linux/mm/rmap.c:631!
> invalid opcode: 0000 [#1] SMP
> Modules linked in:
>
> Pid: 1159, comm: mount Not tainted (2.6.24-rc7 #55)
> EIP: 0060:[<c0152a32>] EFLAGS: 00010282 CPU: 3
> EIP is at page_remove_rmap+0xcc/0xe7
> EAX: 00000037 EBX: c27f8660 ECX: 00000046 EDX: 00000046
> ESI: f7061c60 EDI: f7d87380 EBP: f7060e78 ESP: f7509df8
>  DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
> Process mount (pid: 1159, ti=f7508000 task=f7460540 task.ti=f7508000)
> Stack: c27f8660 bfc33065 c014d166 00000000 f7061c60 f7509e80 00000000 00000000
>        00000000 00000001 b7fd0000 f7555010 f7d87380 c482e240 00000000 ffffffff
>        c16e0c0c 00000000 00000000 f7084df8 003d5ee2 b7fd0000 b7fd0000 00000000
> Call Trace:
>  [<c014d166>] unmap_vmas+0x349/0x5d1
>  [<c0150006>] exit_mmap+0x5f/0xcd
>  [<c011df0a>] mmput+0x25/0x79
>  [<c0122636>] do_exit+0x1a9/0x5eb
>  [<c0122ae3>] sys_exit_group+0x0/0xd
>  [<c0129dde>] get_signal_to_deliver+0x3e3/0x405
>  [<c04334b6>] do_page_fault+0x0/0x69f
>  [<c0102606>] do_notify_resume+0x7d/0x64e
>  [<c0120383>] printk+0x14/0x18
>  [<c0433874>] do_page_fault+0x3be/0x69f
>  [<c0433b4c>] do_page_fault+0x696/0x69f
>  [<c01508c1>] do_munmap+0x181/0x19b
>  [<c04334b6>] do_page_fault+0x0/0x69f
>  [<c0102f4e>] work_notifysig+0x13/0x19
>  =======================
> Code: 8b 46 44 8b 50 08 b8 d2 95 50 c0 e8 9c ad fe ff 8b 46 4c 85 c0 74 14 8b 40 10 85 c0 74 0d 8b 50 2c b8 f0 95 50 c0 e8 81 ad fe ff <0f> 0b eb fe 8b 53 10 89 d8 5b 5e 83 e2 01 f7 da 83 c2 04 e9 c5
> EIP: [<c0152a32>] page_remove_rmap+0xcc/0xe7 SS:ESP 0068:f7509df8
> ---[ end trace eb3c259ee0695fdd ]---
> Fixing recursive fault but reboot is needed!
> eth0: no IPv6 routers present
>
>   

    J

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-15  1:32               ` Jeremy Fitzhardinge
@ 2008-01-15  1:38                 ` Andi Kleen
  2008-01-15 21:03                   ` [patch] x86: lfence fix Ingo Molnar
  0 siblings, 1 reply; 30+ messages in thread
From: Andi Kleen @ 2008-01-15  1:38 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: Andi Kleen, Ingo Molnar, tglx, linux-kernel

On Mon, Jan 14, 2008 at 05:32:08PM -0800, Jeremy Fitzhardinge wrote:
> >
> >I just applied over the whole git-x86 patchkit (tip 
> >2f42671697ea9abc7d10ea7f663d6ef6e8ec6358)  + the two build fixes
> >Unfortunately it didn't work, although the faulting loop is shorter now:
> >  
> 
> Just to be clear, these two patches cause this oops to appear?  Current 

The two build fixes? Without that it just doesn't build.

> x86.git seems to be missing a pile of patches at the moment, so I 
> presume you're using a version which does have my patch set?

I just tested x86.git and then x86.git + patch you send (+ build fixes
in both cases), none of your patchkits. If you want me to test
some other patchkit please point me to it.

The problem happens with plain git-x86 (+ build fixes) and also
with git-x86 + your patch.

-Andi



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-15  1:05             ` Andi Kleen
  2008-01-15  1:32               ` Jeremy Fitzhardinge
@ 2008-01-15 12:55               ` Ingo Molnar
  2008-01-15 16:53                 ` Andi Kleen
  1 sibling, 1 reply; 30+ messages in thread
From: Ingo Molnar @ 2008-01-15 12:55 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Jeremy Fitzhardinge, tglx, linux-kernel


* Andi Kleen <andi@firstfloor.org> wrote:

> I just applied over the whole git-x86 patchkit (tip 
> 2f42671697ea9abc7d10ea7f663d6ef6e8ec6358) + the two build fixes 
> Unfortunately it didn't work, although the faulting loop is shorter 
> now:
> 
> Freeing unused kernel memory: 260k freed
> mount[1159]: segfault at b7fcfe98 ip b7ec6b7a sp bfcfcdec error 7
> Eeek! page_mapcount(page) went negative! (-1)

could you please send me the .config you are using on that kernel?

	Ingo

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-14 22:46           ` Jeremy Fitzhardinge
  2008-01-15  1:05             ` Andi Kleen
@ 2008-01-15 13:53             ` Ingo Molnar
  2008-01-15 17:16               ` Folding _PAGE_PWT into _PAGE_PCD (was Re: unify pagetable accessors patch causes double fault II) Jeremy Fitzhardinge
  2008-01-15 17:36               ` unify pagetable accessors patch causes double fault II Andi Kleen
  1 sibling, 2 replies; 30+ messages in thread
From: Ingo Molnar @ 2008-01-15 13:53 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: Andi Kleen, tglx, linux-kernel


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

> Can you try this out?  It applies after "x86: move all asm/pgtable 
> constants into one place".

and here's the patch

Subject: Re: unify pagetable accessors patch causes double fault II
From: Jeremy Fitzhardinge <jeremy@goop.org>

Andi Kleen wrote:
>> OK, I see the problem.  The problem is that the _PAGE_X defines are
>> defined with _AC(UL, 1 << _PAGE_BIT_X), which has unsigned long type.
>> This means that ~_PAGE_X also has unsigned long type, and so when cast
>> to 64-bit in pte_mkX, it ends up &ing the pte with 0x00000000ffffffxxx,
>> with predictable results.
>>
>
> Actually I fixed some of that -- see the pgtable-nx patch on firstfloor -- but
> it still doesn't work. Or maybe my patch was not complete.
>

Yeah, that looks like the right sort of thing, but I wonder if there's
other places doing an open-coded "pte_val(pte) & ~_PAGE_FOO".  My patch
changes the definition of _PAGE_FOO so it should be OK everywhere.

>> The original code just used signed constants for the _PAGE_X
>> definitions, which will sign-extend when cast to 64-bit, and so have the
>> upper bits set when masking.  (Well, actually, the old code just
>> operated on pte_low, so the problem didn't arise; however, pgtable_64.h
>> also uses integers for its _PAGE_X, which has the same sign-extended
>> 32->64 casting property).
>>
>> I'll put together a fixup patch now.
>>
>
> I'm leaving now but can test later.

and below is the fix against full x86.git#mm.

	Ingo

----------->
Subject: x86/pgtable: fix constant sign extension problem
From: Jeremy Fitzhardinge <jeremy@goop.org>

When the _PAGE_FOO constants are defined as (1ul << _PAGE_BIT_FOO), they
become unsigned longs.  In 32-bit PAE mode, these end up being
implicitly cast to 64-bit types when used to manipulate a pte, and
because they're unsigned the top 32-bits are 0, destroying the upper
bits of the pte.

When _PAGE_FOO constants are given a signed integer type, the cast to
64-bits will sign-extend so that the upper bits are all ones,
preserving the upper pte bits in manipulations.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Andi Kleen <ak@suse.de>

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 include/asm-x86/pgtable.h |   31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

===================================================================
Index: linux-x86.q/include/asm-x86/pgtable.h
===================================================================
--- linux-x86.q.orig/include/asm-x86/pgtable.h
+++ linux-x86.q/include/asm-x86/pgtable.h
@@ -19,21 +19,26 @@
 #define _PAGE_BIT_UNUSED3	11
 #define _PAGE_BIT_NX           63       /* No execute: only valid after cpuid check */
 
-#define _PAGE_PRESENT	(_AC(1, UL)<<_PAGE_BIT_PRESENT)
-#define _PAGE_RW	(_AC(1, UL)<<_PAGE_BIT_RW)
-#define _PAGE_USER	(_AC(1, UL)<<_PAGE_BIT_USER)
-#define _PAGE_PWT	(_AC(1, UL)<<_PAGE_BIT_PWT)
-#define _PAGE_PCD	((_AC(1, UL)<<_PAGE_BIT_PCD) | _PAGE_PWT)
-#define _PAGE_ACCESSED	(_AC(1, UL)<<_PAGE_BIT_ACCESSED)
-#define _PAGE_DIRTY	(_AC(1, UL)<<_PAGE_BIT_DIRTY)
-#define _PAGE_PSE	(_AC(1, UL)<<_PAGE_BIT_PSE)	/* 2MB page */
-#define _PAGE_GLOBAL	(_AC(1, UL)<<_PAGE_BIT_GLOBAL)	/* Global TLB entry */
+/*
+ * Note: we use _AC(1, L) instead of _AC(1, UL) so that we get a
+ * sign-extended value on 32-bit with all 1's in the upper word,
+ * which preserves the upper pte values on 64-bit ptes:
+ */
+#define _PAGE_PRESENT	(_AC(1, L)<<_PAGE_BIT_PRESENT)
+#define _PAGE_RW	(_AC(1, L)<<_PAGE_BIT_RW)
+#define _PAGE_USER	(_AC(1, L)<<_PAGE_BIT_USER)
+#define _PAGE_PWT	(_AC(1, L)<<_PAGE_BIT_PWT)
+#define _PAGE_PCD	((_AC(1, L)<<_PAGE_BIT_PCD) | _PAGE_PWT)
+#define _PAGE_ACCESSED	(_AC(1, L)<<_PAGE_BIT_ACCESSED)
+#define _PAGE_DIRTY	(_AC(1, L)<<_PAGE_BIT_DIRTY)
+#define _PAGE_PSE	(_AC(1, L)<<_PAGE_BIT_PSE)	/* 2MB page */
+#define _PAGE_GLOBAL	(_AC(1, L)<<_PAGE_BIT_GLOBAL)	/* Global TLB entry */
 /* We redefine PCD to be write combining. PAT bit is not used */
-#define _PAGE_WC	((_AC(1, UL)<<_PAGE_BIT_PCD))
+#define _PAGE_WC	((_AC(1, L)<<_PAGE_BIT_PCD))
 #define _PAGE_CACHE_MASK (_PAGE_PCD)
-#define _PAGE_UNUSED1	(_AC(1, UL)<<_PAGE_BIT_UNUSED1)
-#define _PAGE_UNUSED2	(_AC(1, UL)<<_PAGE_BIT_UNUSED2)
-#define _PAGE_UNUSED3	(_AC(1, UL)<<_PAGE_BIT_UNUSED3)
+#define _PAGE_UNUSED1	(_AC(1, L)<<_PAGE_BIT_UNUSED1)
+#define _PAGE_UNUSED2	(_AC(1, L)<<_PAGE_BIT_UNUSED2)
+#define _PAGE_UNUSED3	(_AC(1, L)<<_PAGE_BIT_UNUSED3)
 
 #if defined(CONFIG_X86_64) || defined(CONFIG_X86_PAE)
 #define _PAGE_NX	(_AC(1, ULL) << _PAGE_BIT_NX)

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-15 12:55               ` unify pagetable accessors patch causes double fault II Ingo Molnar
@ 2008-01-15 16:53                 ` Andi Kleen
  0 siblings, 0 replies; 30+ messages in thread
From: Andi Kleen @ 2008-01-15 16:53 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Andi Kleen, Jeremy Fitzhardinge, tglx, linux-kernel

On Tue, Jan 15, 2008 at 01:55:56PM +0100, Ingo Molnar wrote:
> 
> * Andi Kleen <andi@firstfloor.org> wrote:
> 
> > I just applied over the whole git-x86 patchkit (tip 
> > 2f42671697ea9abc7d10ea7f663d6ef6e8ec6358) + the two build fixes 
> > Unfortunately it didn't work, although the faulting loop is shorter 
> > now:
> > 
> > Freeing unused kernel memory: 260k freed
> > mount[1159]: segfault at b7fcfe98 ip b7ec6b7a sp bfcfcdec error 7
> > Eeek! page_mapcount(page) went negative! (-1)
> 
> could you please send me the .config you are using on that kernel?

http://halobates.de/config32

Essentially defconfig with CONFIG_IDE disabled and PAE enabled

-Andi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Folding _PAGE_PWT into _PAGE_PCD (was Re: unify pagetable accessors patch causes double fault II)
  2008-01-15 13:53             ` Ingo Molnar
@ 2008-01-15 17:16               ` Jeremy Fitzhardinge
  2008-01-15 17:23                 ` Andi Kleen
  2008-01-15 20:30                 ` Venki Pallipadi
  2008-01-15 17:36               ` unify pagetable accessors patch causes double fault II Andi Kleen
  1 sibling, 2 replies; 30+ messages in thread
From: Jeremy Fitzhardinge @ 2008-01-15 17:16 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Andi Kleen, tglx, linux-kernel

Ingo Molnar wrote:
> -#define _PAGE_PRESENT	(_AC(1, UL)<<_PAGE_BIT_PRESENT)
> -#define _PAGE_RW	(_AC(1, UL)<<_PAGE_BIT_RW)
> -#define _PAGE_USER	(_AC(1, UL)<<_PAGE_BIT_USER)
> -#define _PAGE_PWT	(_AC(1, UL)<<_PAGE_BIT_PWT)
> -#define _PAGE_PCD	((_AC(1, UL)<<_PAGE_BIT_PCD) | _PAGE_PWT)
>   

BTW, I just noticed that _PAGE_PWT has been folded into _PAGE_PCD.  This 
seems like a really bad idea to me, since it breaks the rule that 
_PAGE_X == 1 << _PAGE_BIT_X.  I can't think of a specific place where 
this would cause problems, but this kind of non-uniformity always ends 
up biting someone in the arse.

I think having a specific _PAGE_NOCACHE which combines these bits is a 
better approach.

    J

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Folding _PAGE_PWT into _PAGE_PCD (was Re: unify pagetable accessors patch causes double fault II)
  2008-01-15 17:16               ` Folding _PAGE_PWT into _PAGE_PCD (was Re: unify pagetable accessors patch causes double fault II) Jeremy Fitzhardinge
@ 2008-01-15 17:23                 ` Andi Kleen
  2008-01-15 17:32                   ` Jeremy Fitzhardinge
  2008-01-15 20:30                 ` Venki Pallipadi
  1 sibling, 1 reply; 30+ messages in thread
From: Andi Kleen @ 2008-01-15 17:23 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: Ingo Molnar, Andi Kleen, tglx, linux-kernel

> BTW, I just noticed that _PAGE_PWT has been folded into _PAGE_PCD.  This 
> seems like a really bad idea to me, since it breaks the rule that 
> _PAGE_X == 1 << _PAGE_BIT_X.  I can't think of a specific place where 
> this would cause problems, but this kind of non-uniformity always ends 
> up biting someone in the arse.

Agreed that it's a bad idea.

> 
> I think having a specific _PAGE_NOCACHE which combines these bits is a 
> better approach.

CPA series adds that already

+/* Needs special handling for large pages */
+#define _PAGE_CACHE     (_PAGE_PCD|_PAGE_PWT|_PAGE_PAT)
+

-Andi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Folding _PAGE_PWT into _PAGE_PCD (was Re: unify pagetable accessors patch causes double fault II)
  2008-01-15 17:23                 ` Andi Kleen
@ 2008-01-15 17:32                   ` Jeremy Fitzhardinge
  2008-01-15 17:39                     ` Andi Kleen
  0 siblings, 1 reply; 30+ messages in thread
From: Jeremy Fitzhardinge @ 2008-01-15 17:32 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Ingo Molnar, tglx, linux-kernel

Andi Kleen wrote:
>> BTW, I just noticed that _PAGE_PWT has been folded into _PAGE_PCD.  This 
>> seems like a really bad idea to me, since it breaks the rule that 
>> _PAGE_X == 1 << _PAGE_BIT_X.  I can't think of a specific place where 
>> this would cause problems, but this kind of non-uniformity always ends 
>> up biting someone in the arse.
>>     
>
> Agreed that it's a bad idea.
>
>   
>> I think having a specific _PAGE_NOCACHE which combines these bits is a 
>> better approach.
>>     
>
> CPA series adds that already
>
> +/* Needs special handling for large pages */
> +#define _PAGE_CACHE     (_PAGE_PCD|_PAGE_PWT|_PAGE_PAT)
>   

Good, but isn't the name _PAGE_CACHE misleading?  Or does it mean 
something else in the context of PAT?

    J

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-15 13:53             ` Ingo Molnar
  2008-01-15 17:16               ` Folding _PAGE_PWT into _PAGE_PCD (was Re: unify pagetable accessors patch causes double fault II) Jeremy Fitzhardinge
@ 2008-01-15 17:36               ` Andi Kleen
  2008-01-15 19:43                 ` Jeremy Fitzhardinge
  1 sibling, 1 reply; 30+ messages in thread
From: Andi Kleen @ 2008-01-15 17:36 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Jeremy Fitzhardinge, Andi Kleen, tglx, linux-kernel

On Tue, Jan 15, 2008 at 02:53:17PM +0100, Ingo Molnar wrote:
> 
> * Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> 
> > Can you try this out?  It applies after "x86: move all asm/pgtable 
> > constants into one place".
> 
> and here's the patch
> 
> Subject: Re: unify pagetable accessors patch causes double fault II
> From: Jeremy Fitzhardinge <jeremy@goop.org>

The patch does not fix the problem here. When I boot plain git-x86
upto commit 67db3027d691ff8049d19be7bc440eca0e5826d7
on my 6GB machine I still get an recursive oops on entering user space.

The only difference to earlier kernels seems to be that the problem
happens a little latter (in stty, not boot), but that might be just
coincidence.

Config http://halobates.de/config32

stty[1151]: segfault at b7f0ce98 ip b7e03b7a sp bfc31f9c error 7
Eeek! page_mapcount(page) went negative! (-1)
  page pfn = bfc12
  page->flags = 80000014
  page->count = 0
  page->mapping = 00000000
  vma->vm_ops = nfs_file_vm_ops+0x0/0x18
  vma->vm_ops->nopage = 0x0
  vma->vm_ops->fault = filemap_fault+0x0/0x34e
  vma->vm_file->f_op->mmap = nfs_file_mmap+0x0/0x61
------------[ cut here ]------------
kernel BUG at /home/lsrc/git-arch-x86/linux-2.6-x86/mm/rmap.c:631!
invalid opcode: 0000 [#1] SMP

-Andi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Folding _PAGE_PWT into _PAGE_PCD (was Re: unify pagetable accessors patch causes double fault II)
  2008-01-15 17:32                   ` Jeremy Fitzhardinge
@ 2008-01-15 17:39                     ` Andi Kleen
  0 siblings, 0 replies; 30+ messages in thread
From: Andi Kleen @ 2008-01-15 17:39 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: Andi Kleen, Ingo Molnar, tglx, linux-kernel

> Good, but isn't the name _PAGE_CACHE misleading?  Or does it mean 

It's all bits related to caching.

> something else in the context of PAT?

It could mean arbitary things with PAT -- PAT essentially allows to
reprogram these bits freely to various different modi using a mapping table.   
In the default configuration any bit set should resort in a uncacheable mode 
though.

-Andi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: unify pagetable accessors patch causes double fault II
  2008-01-15 17:36               ` unify pagetable accessors patch causes double fault II Andi Kleen
@ 2008-01-15 19:43                 ` Jeremy Fitzhardinge
  0 siblings, 0 replies; 30+ messages in thread
From: Jeremy Fitzhardinge @ 2008-01-15 19:43 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Ingo Molnar, tglx, linux-kernel

Andi Kleen wrote:
> The patch does not fix the problem here. When I boot plain git-x86
> upto commit 67db3027d691ff8049d19be7bc440eca0e5826d7
> on my 6GB machine I still get an recursive oops on entering user space.
>
> The only difference to earlier kernels seems to be that the problem
> happens a little latter (in stty, not boot), but that might be just
> coincidence.
>   

Yesterday's x86.git was missing all my unification patches, so if you 
were seeing this oops then, it's something else.   Is it still caused 
specifically by "unify pagetable accessors", or is some other change 
being problematic now?

    J

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Folding _PAGE_PWT into _PAGE_PCD (was Re: unify pagetable accessors patch causes double fault II)
  2008-01-15 17:16               ` Folding _PAGE_PWT into _PAGE_PCD (was Re: unify pagetable accessors patch causes double fault II) Jeremy Fitzhardinge
  2008-01-15 17:23                 ` Andi Kleen
@ 2008-01-15 20:30                 ` Venki Pallipadi
  2008-01-15 20:45                   ` Jeremy Fitzhardinge
  1 sibling, 1 reply; 30+ messages in thread
From: Venki Pallipadi @ 2008-01-15 20:30 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: Ingo Molnar, Andi Kleen, tglx, linux-kernel

On Tue, Jan 15, 2008 at 09:16:50AM -0800, Jeremy Fitzhardinge wrote:
> Ingo Molnar wrote:
> >-#define _PAGE_PRESENT	(_AC(1, UL)<<_PAGE_BIT_PRESENT)
> >-#define _PAGE_RW	(_AC(1, UL)<<_PAGE_BIT_RW)
> >-#define _PAGE_USER	(_AC(1, UL)<<_PAGE_BIT_USER)
> >-#define _PAGE_PWT	(_AC(1, UL)<<_PAGE_BIT_PWT)
> >-#define _PAGE_PCD	((_AC(1, UL)<<_PAGE_BIT_PCD) | _PAGE_PWT)
> >  
> 
> BTW, I just noticed that _PAGE_PWT has been folded into _PAGE_PCD.  This 
> seems like a really bad idea to me, since it breaks the rule that 
> _PAGE_X == 1 << _PAGE_BIT_X.  I can't think of a specific place where 
> this would cause problems, but this kind of non-uniformity always ends 
> up biting someone in the arse.
> 
> I think having a specific _PAGE_NOCACHE which combines these bits is a 
> better approach.
> 
>    J

How about the patch below. It defines new _PAGE_UC. One concern is drivers
continuing to use _PAGE_PCD and getting wrong attributes. May be we need to
rename _PAGE_PCD to catch those errors as well?

Thanks,
Venki

Do not fold PCD and PWT bits in _PAGE_PCD. Instead, introduce a new
_PAGE_UC which defines uncached mappings and use it in place of _PAGE_PCD.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>

Index: linux-2.6.git/arch/x86/mm/ioremap_32.c
===================================================================
--- linux-2.6.git.orig/arch/x86/mm/ioremap_32.c	2008-01-15 03:29:38.000000000 -0800
+++ linux-2.6.git/arch/x86/mm/ioremap_32.c	2008-01-15 04:42:59.000000000 -0800
@@ -173,7 +173,7 @@
 
 void __iomem *ioremap_nocache (unsigned long phys_addr, unsigned long size)
 {
-	return __ioremap(phys_addr, size, _PAGE_PCD);
+	return __ioremap(phys_addr, size, _PAGE_UC);
 }
 EXPORT_SYMBOL(ioremap_nocache);
 
Index: linux-2.6.git/arch/x86/mm/ioremap_64.c
===================================================================
--- linux-2.6.git.orig/arch/x86/mm/ioremap_64.c	2008-01-15 03:29:38.000000000 -0800
+++ linux-2.6.git/arch/x86/mm/ioremap_64.c	2008-01-15 04:43:07.000000000 -0800
@@ -150,7 +150,7 @@
 
 void __iomem *ioremap_nocache (unsigned long phys_addr, unsigned long size)
 {
-	return __ioremap(phys_addr, size, _PAGE_PCD);
+	return __ioremap(phys_addr, size, _PAGE_UC);
 }
 EXPORT_SYMBOL(ioremap_nocache);
 
Index: linux-2.6.git/arch/x86/mm/pat.c
===================================================================
--- linux-2.6.git.orig/arch/x86/mm/pat.c	2008-01-15 03:29:38.000000000 -0800
+++ linux-2.6.git/arch/x86/mm/pat.c	2008-01-15 05:01:43.000000000 -0800
@@ -64,7 +64,7 @@
 	if (smp_processor_id() && !pat_wc_enabled)
 		return;
 
-	/* Set PWT+PCD to Write-Combining. All other bits stay the same */
+	/* Set PCD to Write-Combining. All other bits stay the same */
 	/* PTE encoding used in Linux:
 	      PAT
 	      |PCD
@@ -72,7 +72,7 @@
 	      |||
 	      000 WB         default
 	      010 WC         _PAGE_WC
-	      011 UC         _PAGE_PCD
+	      011 UC         _PAGE_UC
 		PAT bit unused */
 	pat = PAT(0,WB) | PAT(1,WT) | PAT(2,WC) | PAT(3,UC) |
 	      PAT(4,WB) | PAT(5,WT) | PAT(6,WC) | PAT(7,UC);
@@ -97,7 +97,7 @@
 {
 	switch (flags & _PAGE_CACHE_MASK) {
 	case _PAGE_WC:  return "write combining";
-	case _PAGE_PCD: return "uncached";
+	case _PAGE_UC: return "uncached";
 	case 0: 	return "default";
 	default: 	return "broken";
 	}
@@ -144,7 +144,7 @@
 			if (!fattr)
 				return -EINVAL;
 			else
-				*fattr  = _PAGE_PCD;
+				*fattr  = _PAGE_UC;
 		}
 
 		return 0;
@@ -227,13 +227,13 @@
 	unsigned long flags;
 	unsigned long want_flags = 0;
 	if (file->f_flags & O_SYNC)
-		want_flags = _PAGE_PCD;
+		want_flags = _PAGE_UC;
 
 #ifdef CONFIG_X86_32
 	/*
 	 * On the PPro and successors, the MTRRs are used to set
  	 * memory types for physical addresses outside main memory,
-	 * so blindly setting PCD or PWT on those pages is wrong.
+	 * so blindly setting UC or PWT on those pages is wrong.
 	 * For Pentiums and earlier, the surround logic should disable
 	 * caching for the high addresses through the KEN pin, but
 	 * we maintain the tradition of paranoia in this code.
@@ -244,7 +244,7 @@
 		test_bit(X86_FEATURE_CYRIX_ARR, boot_cpu_data.x86_capability) ||
 		test_bit(X86_FEATURE_CENTAUR_MCR, boot_cpu_data.x86_capability)) &&
 	   offset >= __pa(high_memory))
-		want_flags = _PAGE_PCD;
+		want_flags = _PAGE_UC;
 #endif
 
 	/* ignore error because we can't handle it here */
Index: linux-2.6.git/arch/x86/pci/i386.c
===================================================================
--- linux-2.6.git.orig/arch/x86/pci/i386.c	2008-01-15 03:29:38.000000000 -0800
+++ linux-2.6.git/arch/x86/pci/i386.c	2008-01-15 05:02:12.000000000 -0800
@@ -353,7 +353,7 @@
 		 */
 		prot = pgprot_val(vma->vm_page_prot);
 		if (boot_cpu_data.x86 > 3) {
-			prot |= _PAGE_PCD;
+			prot |= _PAGE_UC;
 		}
 		vma->vm_page_prot = __pgprot(prot);
 	}
Index: linux-2.6.git/include/asm-x86/pgtable.h
===================================================================
--- linux-2.6.git.orig/include/asm-x86/pgtable.h	2008-01-15 03:29:38.000000000 -0800
+++ linux-2.6.git/include/asm-x86/pgtable.h	2008-01-15 05:11:12.000000000 -0800
@@ -28,14 +28,16 @@
 #define _PAGE_RW	(_AC(1, L)<<_PAGE_BIT_RW)
 #define _PAGE_USER	(_AC(1, L)<<_PAGE_BIT_USER)
 #define _PAGE_PWT	(_AC(1, L)<<_PAGE_BIT_PWT)
-#define _PAGE_PCD	((_AC(1, L)<<_PAGE_BIT_PCD) | _PAGE_PWT)
+#define _PAGE_PCD	(_AC(1, L)<<_PAGE_BIT_PCD) /* Do not use PCD directly */
 #define _PAGE_ACCESSED	(_AC(1, L)<<_PAGE_BIT_ACCESSED)
 #define _PAGE_DIRTY	(_AC(1, L)<<_PAGE_BIT_DIRTY)
 #define _PAGE_PSE	(_AC(1, L)<<_PAGE_BIT_PSE)	/* 2MB page */
 #define _PAGE_GLOBAL	(_AC(1, L)<<_PAGE_BIT_GLOBAL)	/* Global TLB entry */
-/* We redefine PCD to be write combining. PAT bit is not used */
-#define _PAGE_WC	((_AC(1, L)<<_PAGE_BIT_PCD))
-#define _PAGE_CACHE_MASK (_PAGE_PCD)
+/* We redefine PCD to be WC, PCD|PWT to be UC. PAT bit is not used */
+#define _PAGE_WC	(_PAGE_PCD)
+#define _PAGE_UC	(_PAGE_PCD | _PAGE_PWT)
+#define _PAGE_CACHE_MASK (_PAGE_PCD | _PAGE_PWT)
+
 #define _PAGE_UNUSED1	(_AC(1, L)<<_PAGE_BIT_UNUSED1)
 #define _PAGE_UNUSED2	(_AC(1, L)<<_PAGE_BIT_UNUSED2)
 #define _PAGE_UNUSED3	(_AC(1, L)<<_PAGE_BIT_UNUSED3)
@@ -82,9 +84,9 @@
 
 #define __PAGE_KERNEL_RO		(__PAGE_KERNEL & ~_PAGE_RW)
 #define __PAGE_KERNEL_RX		(__PAGE_KERNEL_EXEC & ~_PAGE_RW)
-#define __PAGE_KERNEL_NOCACHE		(__PAGE_KERNEL | _PAGE_PCD)
+#define __PAGE_KERNEL_NOCACHE		(__PAGE_KERNEL | _PAGE_UC)
 #define __PAGE_KERNEL_VSYSCALL		(__PAGE_KERNEL_RX | _PAGE_USER)
-#define __PAGE_KERNEL_VSYSCALL_NOCACHE	(__PAGE_KERNEL_VSYSCALL | _PAGE_PCD)
+#define __PAGE_KERNEL_VSYSCALL_NOCACHE	(__PAGE_KERNEL_VSYSCALL | _PAGE_UC)
 #define __PAGE_KERNEL_LARGE		(__PAGE_KERNEL | _PAGE_PSE)
 #define __PAGE_KERNEL_LARGE_EXEC	(__PAGE_KERNEL_EXEC | _PAGE_PSE)
 
Index: linux-2.6.git/include/asm-x86/pgtable_32.h
===================================================================
--- linux-2.6.git.orig/include/asm-x86/pgtable_32.h	2008-01-15 03:29:38.000000000 -0800
+++ linux-2.6.git/include/asm-x86/pgtable_32.h	2008-01-15 04:42:16.000000000 -0800
@@ -121,7 +121,7 @@
  * it, this is a no-op.
  */
 #define pgprot_noncached(prot)	((boot_cpu_data.x86 > 3)					  \
-				 ? (__pgprot(pgprot_val(prot) | _PAGE_PCD)) : (prot))
+				 ? (__pgprot(pgprot_val(prot) | _PAGE_UC)) : (prot))
 
 /*
  * Macro to make mark a page protection value as "write-combining".
Index: linux-2.6.git/include/asm-x86/pgtable_64.h
===================================================================
--- linux-2.6.git.orig/include/asm-x86/pgtable_64.h	2008-01-15 03:29:38.000000000 -0800
+++ linux-2.6.git/include/asm-x86/pgtable_64.h	2008-01-15 04:41:37.000000000 -0800
@@ -170,7 +170,7 @@
  * and do not allow for consecutive writes to be combined.
  */
 #define pgprot_noncached(prot) \
-	__pgprot((pgprot_val(prot) & ~_PAGE_CACHE_MASK) | _PAGE_PCD)
+	__pgprot((pgprot_val(prot) & ~_PAGE_CACHE_MASK) | _PAGE_UC)
 
 /*
  * Macro to make mark a page protection value as "write-combining".


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Folding _PAGE_PWT into _PAGE_PCD (was Re: unify pagetable accessors patch causes double fault II)
  2008-01-15 20:30                 ` Venki Pallipadi
@ 2008-01-15 20:45                   ` Jeremy Fitzhardinge
  0 siblings, 0 replies; 30+ messages in thread
From: Jeremy Fitzhardinge @ 2008-01-15 20:45 UTC (permalink / raw)
  To: Venki Pallipadi; +Cc: Ingo Molnar, Andi Kleen, tglx, linux-kernel

Venki Pallipadi wrote:
> On Tue, Jan 15, 2008 at 09:16:50AM -0800, Jeremy Fitzhardinge wrote:
>   
>> Ingo Molnar wrote:
>>     
>>> -#define _PAGE_PRESENT	(_AC(1, UL)<<_PAGE_BIT_PRESENT)
>>> -#define _PAGE_RW	(_AC(1, UL)<<_PAGE_BIT_RW)
>>> -#define _PAGE_USER	(_AC(1, UL)<<_PAGE_BIT_USER)
>>> -#define _PAGE_PWT	(_AC(1, UL)<<_PAGE_BIT_PWT)
>>> -#define _PAGE_PCD	((_AC(1, UL)<<_PAGE_BIT_PCD) | _PAGE_PWT)
>>>  
>>>       
>> BTW, I just noticed that _PAGE_PWT has been folded into _PAGE_PCD.  This 
>> seems like a really bad idea to me, since it breaks the rule that 
>> _PAGE_X == 1 << _PAGE_BIT_X.  I can't think of a specific place where 
>> this would cause problems, but this kind of non-uniformity always ends 
>> up biting someone in the arse.
>>
>> I think having a specific _PAGE_NOCACHE which combines these bits is a 
>> better approach.
>>
>>    J
>>     
>
> How about the patch below. It defines new _PAGE_UC. One concern is drivers
> continuing to use _PAGE_PCD and getting wrong attributes. May be we need to
> rename _PAGE_PCD to catch those errors as well?
>   

Sure, looks fine.  I would have said that _NOCACHE matches current usage 
better, but if it makes more sense to have _UC and _WC then that's fine 
with me.

I guess renaming _PAGE_BIT_PCD to _PAGE_BIT__PCD and the corresponding 
_PAGE__PCD might be reasonable if you think there's a chance of new 
misusers appearing (I guess something like out of tree DRI/proprietary 
patches are a source of that).

    J


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [patch] x86: lfence fix
  2008-01-15  1:38                 ` Andi Kleen
@ 2008-01-15 21:03                   ` Ingo Molnar
  2008-01-16  0:44                     ` Andi Kleen
  0 siblings, 1 reply; 30+ messages in thread
From: Ingo Molnar @ 2008-01-15 21:03 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Jeremy Fitzhardinge, tglx, linux-kernel


* Andi Kleen <andi@firstfloor.org> wrote:

> I just tested x86.git and then x86.git + patch you send (+ build fixes 
> in both cases), none of your patchkits. If you want me to test some 
> other patchkit please point me to it.
> 
> The problem happens with plain git-x86 (+ build fixes) and also with 
> git-x86 + your patch.

i was held up from debugging your problem by a bug introduced by you 
into x86.git ;-) See the fix below.

	Ingo

----------------------------->
Subject: x86: lfence fix
From: Ingo Molnar <mingo@elte.hu>

LFENCE is available on XMM2 or higher Intel CPUs - not XMM or higher...

this caused boot failures on pre-XMM2 CPUs.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/cpu/intel.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-x86.q/arch/x86/kernel/cpu/intel.c
===================================================================
--- linux-x86.q.orig/arch/x86/kernel/cpu/intel.c
+++ linux-x86.q/arch/x86/kernel/cpu/intel.c
@@ -206,7 +206,7 @@ static void __cpuinit init_intel(struct 
 	}
 #endif
 
-	if (cpu_has_xmm)
+	if (cpu_has_xmm2)
 		set_bit(X86_FEATURE_LFENCE_RDTSC, c->x86_capability);
 	if (c->x86 == 15) {
 		set_bit(X86_FEATURE_P4, c->x86_capability);

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [patch] x86: lfence fix
  2008-01-15 21:03                   ` [patch] x86: lfence fix Ingo Molnar
@ 2008-01-16  0:44                     ` Andi Kleen
  0 siblings, 0 replies; 30+ messages in thread
From: Andi Kleen @ 2008-01-16  0:44 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Andi Kleen, Jeremy Fitzhardinge, tglx, linux-kernel

On Tue, Jan 15, 2008 at 10:03:21PM +0100, Ingo Molnar wrote:
> 
> * Andi Kleen <andi@firstfloor.org> wrote:
> 
> > I just tested x86.git and then x86.git + patch you send (+ build fixes 
> > in both cases), none of your patchkits. If you want me to test some 
> > other patchkit please point me to it.
> > 
> > The problem happens with plain git-x86 (+ build fixes) and also with 
> > git-x86 + your patch.
> 
> i was held up from debugging your problem by a bug introduced by you 
> into x86.git ;-) See the fix below.

Thanks.

-Andi


^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2008-01-16  0:41 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20080114094814.GA28300@basil.nowhere.org>
2008-01-14 12:56 ` unify pagetable accessors patch causes double fault II Andi Kleen
2008-01-14 13:06   ` Ingo Molnar
2008-01-14 13:58     ` Andi Kleen
2008-01-14 16:44       ` Jeremy Fitzhardinge
2008-01-14 16:56         ` Ingo Molnar
2008-01-14 17:08           ` Andi Kleen
2008-01-14 17:18             ` unify pagetable accessors patch causes double fault III Andi Kleen
2008-01-14 19:00               ` Ingo Molnar
2008-01-14 19:54                 ` Jeremy Fitzhardinge
2008-01-14 20:15                 ` Andi Kleen
2008-01-14 19:52           ` unify pagetable accessors patch causes double fault II Jeremy Fitzhardinge
2008-01-14 22:03       ` Jeremy Fitzhardinge
2008-01-14 22:23         ` Andi Kleen
2008-01-14 22:46           ` Jeremy Fitzhardinge
2008-01-15  1:05             ` Andi Kleen
2008-01-15  1:32               ` Jeremy Fitzhardinge
2008-01-15  1:38                 ` Andi Kleen
2008-01-15 21:03                   ` [patch] x86: lfence fix Ingo Molnar
2008-01-16  0:44                     ` Andi Kleen
2008-01-15 12:55               ` unify pagetable accessors patch causes double fault II Ingo Molnar
2008-01-15 16:53                 ` Andi Kleen
2008-01-15 13:53             ` Ingo Molnar
2008-01-15 17:16               ` Folding _PAGE_PWT into _PAGE_PCD (was Re: unify pagetable accessors patch causes double fault II) Jeremy Fitzhardinge
2008-01-15 17:23                 ` Andi Kleen
2008-01-15 17:32                   ` Jeremy Fitzhardinge
2008-01-15 17:39                     ` Andi Kleen
2008-01-15 20:30                 ` Venki Pallipadi
2008-01-15 20:45                   ` Jeremy Fitzhardinge
2008-01-15 17:36               ` unify pagetable accessors patch causes double fault II Andi Kleen
2008-01-15 19:43                 ` Jeremy Fitzhardinge

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).