All of lore.kernel.org
 help / color / mirror / Atom feed
* Xen 4.3 + tmem =  Xen BUG at domain_page.c:143
@ 2013-06-11 13:45 konrad wilk
  2013-06-11 14:46 ` Jan Beulich
  0 siblings, 1 reply; 28+ messages in thread
From: konrad wilk @ 2013-06-11 13:45 UTC (permalink / raw)
  To: xen-devel

This is a fairly simple test and it does work with Xen 4.2.


# xl info
host                   : tst035.dumpdata.com
release                : 3.10.0-rc5upstream-00438-g335262d-dirty
version       3f00:17bae3ff:00000000:00000001:00000000
virt_caps              : hvm hvm_directio
total_memory           : 8016
free_memory            : 5852
sharing_freed_memory   : 0
sharing_used_memory    : 0
outstanding_claims     : 0
free_cpus              : 0
xen_major              : 4
xen_minor              : 3
xen_extra              : -unstable
xen_caps               : xen-3.0-x86_64 xen-3.0-x86_32p hvm-3.0-x86_32 
hvm-3.0-x86_32p hvm-3.0-x86_64
xen_scheduler          : credit
xen_pagesize           : 4096
platform_params        : virt_start=0xffff800000000000
xen_changeset          : Mon Jun 10 14:42:51 2013 +0200 git:44434f3-dirty
xen_commandline        : com1=115200,8n1 tmem=1 dom0_mem=max:2G 
cpufreq=xen:performance,verbose noreboot console=com1,vga loglvl=all 
guest_loglvl=all
cc_compiler            : gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2)
cc_compile_by          : konrad
cc_compile_domain      : (none)
cc_compile_date        : Mon Jun 10 17:01:43 EDT 2013
xend_config_format     : 4
#
   133.475684] xen-blkback:(backend_changed:585) .
Jun 11 13:39:58 tst035 logger: /etc/xen/scripts/block: add 
XENBUS_PATH=backend/vbd/1/51712
[  133.477018] xen-blkback:(xen_vbd_create:421) Successful creation of 
handle=ca00 (dom=1)
[  133.477018] .
[  133.479632] xen-blkback:(frontend_changed:665) Initialising.
mapping kernel into physical memory
about to get started...
Jun 11 13:39:59 tst035 logger: /etc/xen/scripts/vif-bridge: online 
type_if=vif XENBUS_PATH=backend/vif/1/0
[  133.635819] device vif1.0 entered promiscuous mode
[  133.639363] IPv6: ADDRCONF(NETDEV_UP): vif1.0: link is not ready
Jun 11 13:39:59 tst035 logger: /etc/xen/scripts/vif-bridge: Successful 
vif-bridge online for vif1.0, bridge switch.
Jun 11 13:39:59 tst035 logger: /etc/xen/scripts/vif-bridge: Writing 
backend/vif/1/0/hotplug-status connected to xenstore.
[  135.864732] IPv6: ADDRCONF(NETDEV_CHANGE): vif1.0: link becomes ready
[  135.865760] switch: port 2(vif1.0) entered forward[  135.965777] 
xen-blkback:(frontend_changed:665) Initialised.
[  135.966711] xen-blkback:(connect_ring:820) /local/domain/1/d 
persistent grants
[  135.968942] xen-blkback:(connect:734) /local/domain/1/device/vbd/51712.
[  135.981089] xen-blkback:(frontend_changed:665) Connected.
... snip..
[  140.441073] xen-blkback: grant 38 added to the tree of persistent 
grants, using 28/1056
[  140.441640] xen-blkback: grant 39 added to the tree of persistent 
grants, using 29/1056
[  140.442284] xen-blkback: grant 40 added to the tree of persistent 
grants, using 30/1056
[  140.442840] xen-blkback: grant 41 added to the tree of persistent 
grants, using 31/1056
[  140.443389] xen-blkback: grant 42 added to the tree of persistent 
grants, using 32/1056
[  140.443920] xen-blkback: grant 43 added to the tree of persistent 
grants, using 33/1056
[  140.444449] xen-blkback: grant 44 added to the tree of persistent 
grants, using 34/1056
(XEN) tmem: initializing tmem capability for domid=1...<G><2>ok
(XEN) tmem: allocating persistent-private tmem pool for 
domid=1...<G><2>pool_id=0
[  150.879132] switch: port 2(vif1.0) entered forwarding state

(XEN) Xen BUG at domain_page.c:143
(XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Not tainted ]----
(XEN) CPU:    0
(XEN) RIP:    e008:[<ffff82c4c0160461>] map_domain_page+0x450/0x514
(XEN) RFLAGS: 0000000000010046   CONTEXT: hypervisor
(XEN) rax: 0000000000000020   rbx: ffff8300c68f9000   rcx: 0000000000000000
(XEN) rdx: 0000000000000020   rsi: 0000000000000020   rdi: 0000000000000000
(XEN) rbp: ffff82c4c02c7cc8   rsp: ffff82c4c02c7c88   r8: ffff820060001000
(XEN) r9:  00000000ffffffff   r10: ffff820060006000   r11: 0000000000000000
(XEN) r12: ffff83022e1bb000   r13: 00000000001ebcdc   r14: 0000000000000020
(XEN) r15: 0000000000000004   cr0: 0000000080050033   cr4: 00000000000426f0
(XEN) cr3: 0000000209541000   cr2: ffff88002b683fd0
(XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
(XEN) Xen stack trace from rsp=ffff82c4c02c7c88:
(XEN)    ffff83022e1bb2d8 0000000000000286 ffff82c4c012760a ffff83022e1bb000
(XEN)    ffff82e003d79b80 ffff82c4c02c7d60 00000000001ebcdc 0000000000000000
(XEN)    ffff82c4c02c7d38 ffff82c4c01373de ffff82c4c0127b6b ffffffffffffffff
(XEN)    00000000c02c7d38 ffff82c4c02c7d58 ffff83022e1bb2d8 0000000000000286
(XEN)    0000000000000027 0000000000000000 0000000000001000 0000000000000000
(XEN)    0000000000000000 00000000001ebcdc ffff82c4c02c7d98 ffff82c4c01377c4
(XEN)    0000000000000000 ffff820040014000 ffff82e003d79b80 00000000001ebcdc
(XEN)    ffff82c4c02c7d98 ffff830210ecf390 00000000fffffff4 ffff820040010010
(XEN)    ffff82004001cf50 ffff83022e1bcc90 ffff82c4c02c7e18 ffff82c4c0135929
(XEN)    ffff82c4c02c7db8 ffff82004001cf50 0000000000000000 00000000001ebcdc
(XEN)    0000000000000000 0000000000000000 0000e8a200000000 ffff82c4c02c7e00
(XEN)    ffff82c4c02c7e18 ffff83022e1bcc90 ffff830210ecf390 0000000000000000
(XEN)    0000000000000001 000000000000009a ffff82c4c02c7ef8 ffff82c4c0136510
(XEN)    0000002700001000 0000000000000000 ffff82c4c02c7e90 97c4284effffffc2
(XEN)    ffff82c4c02c7e68 ffff82c4c015719d ffff82c4c0127b09 0000000000000000
(XEN)    ffff82c4c02c7e88 ffff82c4c018c13c ffff82c4c0319100 ffff82c4c02c7f18
(XEN)    0000000000000004 0000000000000001 0000000000000000 0000000000000000
(XEN)    000000000000e8a2 0000000000000000 00000000001ebcdc 000000000000e030
(XEN)    0000000000000246 ffff8300c68f9000 0000000000000000 0000000000000000
(XEN)    0000000000000001 0000000000000000 00007d3b3fd380c7 ffff82c4c02236db
(XEN) Xen call trace:
(XEN)    [<ffff82c4c0160461>] map_domain_page+0x450/0x514
(XEN)    [<ffff82c4c01373de>] cli_get_page+0x15e/0x17b
(XEN)    [<ffff82c4c01377c4>] tmh_copy_from_client+0x150/0x284
(XEN)    [<ffff82c4c0135929>] do_tmem_put+0x323/0x5c4
(XEN)    [<ffff82c4c0136510>] do_tmem_op+0x5a0/0xbd0
(XEN)    [<ffff82c4c02236db>] syscall_enter+0xeb/0x145
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Xen BUG at domain_page.c:143
(XEN) ****************************************
(XEN)
(XEN) Manual reset required ('noreboot' specified)

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

* Re: Xen 4.3 + tmem =  Xen BUG at domain_page.c:143
  2013-06-11 13:45 Xen 4.3 + tmem = Xen BUG at domain_page.c:143 konrad wilk
@ 2013-06-11 14:46 ` Jan Beulich
  2013-06-11 15:30   ` konrad wilk
  0 siblings, 1 reply; 28+ messages in thread
From: Jan Beulich @ 2013-06-11 14:46 UTC (permalink / raw)
  To: konrad wilk; +Cc: xen-devel

>>> On 11.06.13 at 15:45, konrad wilk <konrad.wilk@oracle.com> wrote:
> This is a fairly simple test and it does work with Xen 4.2.
> [...]
> (XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Not tainted ]----
> (XEN) CPU:    0
> (XEN) RIP:    e008:[<ffff82c4c0160461>] map_domain_page+0x450/0x514
> (XEN) RFLAGS: 0000000000010046   CONTEXT: hypervisor
> (XEN) rax: 0000000000000020   rbx: ffff8300c68f9000   rcx: 0000000000000000
> (XEN) rdx: 0000000000000020   rsi: 0000000000000020   rdi: 0000000000000000
> (XEN) rbp: ffff82c4c02c7cc8   rsp: ffff82c4c02c7c88   r8: ffff820060001000
> (XEN) r9:  00000000ffffffff   r10: ffff820060006000   r11: 0000000000000000
> (XEN) r12: ffff83022e1bb000   r13: 00000000001ebcdc   r14: 0000000000000020
> (XEN) r15: 0000000000000004   cr0: 0000000080050033   cr4: 00000000000426f0
> (XEN) cr3: 0000000209541000   cr2: ffff88002b683fd0
> (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
> (XEN) Xen stack trace from rsp=ffff82c4c02c7c88:
> (XEN)    ffff83022e1bb2d8 0000000000000286 ffff82c4c012760a ffff83022e1bb000
> (XEN)    ffff82e003d79b80 ffff82c4c02c7d60 00000000001ebcdc 0000000000000000
> (XEN)    ffff82c4c02c7d38 ffff82c4c01373de ffff82c4c0127b6b ffffffffffffffff
> (XEN)    00000000c02c7d38 ffff82c4c02c7d58 ffff83022e1bb2d8 0000000000000286
> (XEN)    0000000000000027 0000000000000000 0000000000001000 0000000000000000
> (XEN)    0000000000000000 00000000001ebcdc ffff82c4c02c7d98 ffff82c4c01377c4
> (XEN)    0000000000000000 ffff820040014000 ffff82e003d79b80 00000000001ebcdc
> (XEN)    ffff82c4c02c7d98 ffff830210ecf390 00000000fffffff4 ffff820040010010
> (XEN)    ffff82004001cf50 ffff83022e1bcc90 ffff82c4c02c7e18 ffff82c4c0135929
> (XEN)    ffff82c4c02c7db8 ffff82004001cf50 0000000000000000 00000000001ebcdc
> (XEN)    0000000000000000 0000000000000000 0000e8a200000000 ffff82c4c02c7e00
> (XEN)    ffff82c4c02c7e18 ffff83022e1bcc90 ffff830210ecf390 0000000000000000
> (XEN)    0000000000000001 000000000000009a ffff82c4c02c7ef8 ffff82c4c0136510
> (XEN)    0000002700001000 0000000000000000 ffff82c4c02c7e90 97c4284effffffc2
> (XEN)    ffff82c4c02c7e68 ffff82c4c015719d ffff82c4c0127b09 0000000000000000
> (XEN)    ffff82c4c02c7e88 ffff82c4c018c13c ffff82c4c0319100 ffff82c4c02c7f18
> (XEN)    0000000000000004 0000000000000001 0000000000000000 0000000000000000
> (XEN)    000000000000e8a2 0000000000000000 00000000001ebcdc 000000000000e030
> (XEN)    0000000000000246 ffff8300c68f9000 0000000000000000 0000000000000000
> (XEN)    0000000000000001 0000000000000000 00007d3b3fd380c7 ffff82c4c02236db
> (XEN) Xen call trace:
> (XEN)    [<ffff82c4c0160461>] map_domain_page+0x450/0x514
> (XEN)    [<ffff82c4c01373de>] cli_get_page+0x15e/0x17b
> (XEN)    [<ffff82c4c01377c4>] tmh_copy_from_client+0x150/0x284
> (XEN)    [<ffff82c4c0135929>] do_tmem_put+0x323/0x5c4
> (XEN)    [<ffff82c4c0136510>] do_tmem_op+0x5a0/0xbd0
> (XEN)    [<ffff82c4c02236db>] syscall_enter+0xeb/0x145
> (XEN)
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) Xen BUG at domain_page.c:143
> (XEN) ****************************************

For one, you won't see this with debug=n.

And then it seems quite likely that tmem shows behavior that I'm
unaware of, and hence I may have broken it with the 16Tb
support patches - I suspect it simply drives the hypervisor out of
domain page mapping resources. After all, x86-64 didn't do any
such mapping yet in 4.2.

But tmem being unsupported due to the still pending security
audit makes this a low priority issue anyway. And as you may
or may not recall, it is being disabled for systems with more
than 5Tb too. So quite a bit of work on the tmem side...

Jan

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

* Re: Xen 4.3 + tmem =  Xen BUG at domain_page.c:143
  2013-06-11 14:46 ` Jan Beulich
@ 2013-06-11 15:30   ` konrad wilk
  2013-06-11 15:56     ` George Dunlap
  2013-06-11 16:38     ` Jan Beulich
  0 siblings, 2 replies; 28+ messages in thread
From: konrad wilk @ 2013-06-11 15:30 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel

[-- Attachment #1: Type: text/plain, Size: 8711 bytes --]


On 6/11/2013 10:46 AM, Jan Beulich wrote:
>>>> On 11.06.13 at 15:45, konrad wilk <konrad.wilk@oracle.com> wrote:
>> This is a fairly simple test and it does work with Xen 4.2.
>> [...]
>> (XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Not tainted ]----
>> (XEN) CPU:    0
>> (XEN) RIP:    e008:[<ffff82c4c0160461>] map_domain_page+0x450/0x514
>> (XEN) RFLAGS: 0000000000010046   CONTEXT: hypervisor
>> (XEN) rax: 0000000000000020   rbx: ffff8300c68f9000   rcx: 0000000000000000
>> (XEN) rdx: 0000000000000020   rsi: 0000000000000020   rdi: 0000000000000000
>> (XEN) rbp: ffff82c4c02c7cc8   rsp: ffff82c4c02c7c88   r8: ffff820060001000
>> (XEN) r9:  00000000ffffffff   r10: ffff820060006000   r11: 0000000000000000
>> (XEN) r12: ffff83022e1bb000   r13: 00000000001ebcdc   r14: 0000000000000020
>> (XEN) r15: 0000000000000004   cr0: 0000000080050033   cr4: 00000000000426f0
>> (XEN) cr3: 0000000209541000   cr2: ffff88002b683fd0
>> (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
>> (XEN) Xen stack trace from rsp=ffff82c4c02c7c88:
>> (XEN)    ffff83022e1bb2d8 0000000000000286 ffff82c4c012760a ffff83022e1bb000
>> (XEN)    ffff82e003d79b80 ffff82c4c02c7d60 00000000001ebcdc 0000000000000000
>> (XEN)    ffff82c4c02c7d38 ffff82c4c01373de ffff82c4c0127b6b ffffffffffffffff
>> (XEN)    00000000c02c7d38 ffff82c4c02c7d58 ffff83022e1bb2d8 0000000000000286
>> (XEN)    0000000000000027 0000000000000000 0000000000001000 0000000000000000
>> (XEN)    0000000000000000 00000000001ebcdc ffff82c4c02c7d98 ffff82c4c01377c4
>> (XEN)    0000000000000000 ffff820040014000 ffff82e003d79b80 00000000001ebcdc
>> (XEN)    ffff82c4c02c7d98 ffff830210ecf390 00000000fffffff4 ffff820040010010
>> (XEN)    ffff82004001cf50 ffff83022e1bcc90 ffff82c4c02c7e18 ffff82c4c0135929
>> (XEN)    ffff82c4c02c7db8 ffff82004001cf50 0000000000000000 00000000001ebcdc
>> (XEN)    0000000000000000 0000000000000000 0000e8a200000000 ffff82c4c02c7e00
>> (XEN)    ffff82c4c02c7e18 ffff83022e1bcc90 ffff830210ecf390 0000000000000000
>> (XEN)    0000000000000001 000000000000009a ffff82c4c02c7ef8 ffff82c4c0136510
>> (XEN)    0000002700001000 0000000000000000 ffff82c4c02c7e90 97c4284effffffc2
>> (XEN)    ffff82c4c02c7e68 ffff82c4c015719d ffff82c4c0127b09 0000000000000000
>> (XEN)    ffff82c4c02c7e88 ffff82c4c018c13c ffff82c4c0319100 ffff82c4c02c7f18
>> (XEN)    0000000000000004 0000000000000001 0000000000000000 0000000000000000
>> (XEN)    000000000000e8a2 0000000000000000 00000000001ebcdc 000000000000e030
>> (XEN)    0000000000000246 ffff8300c68f9000 0000000000000000 0000000000000000
>> (XEN)    0000000000000001 0000000000000000 00007d3b3fd380c7 ffff82c4c02236db
>> (XEN) Xen call trace:
>> (XEN)    [<ffff82c4c0160461>] map_domain_page+0x450/0x514
>> (XEN)    [<ffff82c4c01373de>] cli_get_page+0x15e/0x17b
>> (XEN)    [<ffff82c4c01377c4>] tmh_copy_from_client+0x150/0x284
>> (XEN)    [<ffff82c4c0135929>] do_tmem_put+0x323/0x5c4
>> (XEN)    [<ffff82c4c0136510>] do_tmem_op+0x5a0/0xbd0
>> (XEN)    [<ffff82c4c02236db>] syscall_enter+0xeb/0x145
>> (XEN)
>> (XEN)
>> (XEN) ****************************************
>> (XEN) Panic on CPU 0:
>> (XEN) Xen BUG at domain_page.c:143
>> (XEN) ****************************************
> For one, you won't see this with debug=n.
>
> And then it seems quite likely that tmem shows behavior that I'm
> unaware of, and hence I may have broken it with the 16Tb
> support patches - I suspect it simply drives the hypervisor out of
> domain page mapping resources. After all, x86-64 didn't do any
> such mapping yet in 4.2.

I think this is a more subtle bug.
I applied a debug patch (see attached) and with the help of it and the logs:

(XEN) domain_page.c:160:d1 mfn (1ebe96) -> 6 idx: 32(i:1,j:0), branch:1
(XEN) domain_page.c:166:d1 [0] idx=26, mfn=0x1ebcd8, refcnt: 0
(XEN) domain_page.c:166:d1 [1] idx=12, mfn=0x1ebcd9, refcnt: 0
(XEN) domain_page.c:166:d1 [2] idx=2, mfn=0x210e9a, refcnt: 0
(XEN) domain_page.c:166:d1 [3] idx=14, mfn=0x210e9b, refcnt: 0
(XEN) domain_page.c:166:d1 [4] idx=7, mfn=0x210e9c, refcnt: 0
(XEN) domain_page.c:166:d1 [5] idx=10, mfn=0x210e9d, refcnt: 0
(XEN) domain_page.c:166:d1 [6] idx=5, mfn=0x210e9e, refcnt: 0
(XEN) domain_page.c:166:d1 [7] idx=13, mfn=0x1ebe97, refcnt: 0
(XEN) Xen BUG at domain_page.c:169
(XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Not tainted ]----
(XEN) CPU:    3
(XEN) RIP:    e008:[<ffff82c4c01606a7>] map_domain_page+0x61d/0x6e1
(XEN) RFLAGS: 0000000000010046   CONTEXT: hypervisor
(XEN) rax: 0000000000000000   rbx: ffff8300c68f9000   rcx: 0000000000000000
(XEN) rdx: ffff8302125b2020   rsi: 000000000000000a   rdi: ffff82c4c027a6e8
(XEN) rbp: ffff8302125afcc8   rsp: ffff8302125afc48   r8: 0000000000000004
(XEN) r9:  0000000000000004   r10: 0000000000000004   r11: 0000000000000001
(XEN) r12: ffff83022e2ef000   r13: 00000000001ebe96   r14: 0000000000000020
(XEN) r15: ffff8300c68f9080   cr0: 0000000080050033   cr4: 00000000000426f0
(XEN) cr3: 0000000209541000   cr2: ffffffffff600400
(XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
(XEN) Xen stack trace from rsp=ffff8302125afc48:
(XEN)    00000000001ebe97 0000000000000000 0000000000000000 ffff830200000001
(XEN)    ffff8302125afcc8 ffff82c400000000 00000000001ebe97 000000080000000d
(XEN)    ffff83022e2ef2d8 0000000000000286 ffff82c4c0127b6b ffff83022e2ef000
(XEN)    ffff82e003d7d2c0 ffff8302125afd60 00000000001ebe96 0000000000000000
(XEN)    ffff8302125afd38 ffff82c4c01373de 0000000000000000 ffffffffffffffff
(XEN)    0000000000000001 ffff8302125afd58 ffff83022e2ef2d8 0000000000000286
(XEN)    0000000000000027 0000000000000000 0000000000001000 0000000000000000
(XEN)    0000000000000000 00000000001ebe96 ffff8302125afd98 ffff82c4c01377c4
(XEN)    0000000000000000 ffff820040017000 ffff82e003d7d2c0 00000000001ebe96
(XEN)    ffff8302125afd98 ffff830210ecf390 00000000fffffff4 ffff820040009010
(XEN)    ffff820040000f50 ffff83022e2f0c90 ffff8302125afe18 ffff82c4c0135929
(XEN)    000000160000001e ffff820040000f50 0000000000000000 00000000001ebe96
(XEN)    0000000000000000 0000000000000000 0000a2f6125afe28 ffff8302125afe00
(XEN)    0000001675f02b51 ffff83022e2f0c90 ffff830210ecf390 0000000000000000
(XEN)    0000000000000001 0000000000000065 ffff8302125afef8 ffff82c4c0136510
(XEN)    ffff830200001000 0000000000000000 ffff8302125afe90 255ece02125b2040
(XEN)    00000003125afe68 00000016742667d1 ffff8302125b2100 0000003d52299000
(XEN)    ffff8300c68f9000 0000000001c9c380 ffff8302125b2100 ffff8302125b1808
(XEN)    0000000000000004 0000000000000004 0000000000000000 0000000000000000
(XEN)    000000000000a2f6 0000000000000000 00000000001ebe96 ffff82c4c0126e77
(XEN) Xen call trace:
(XEN)    [<ffff82c4c01606a7>] map_domain_page+0x61d/0x6e1
(XEN)    [<ffff82c4c01373de>] cli_get_page+0x15e/0x17b
(XEN)    [<ffff82c4c01377c4>] tmh_copy_from_client+0x150/0x284
(XEN)    [<ffff82c4c0135929>] do_tmem_put+0x323/0x5c4
(XEN)    [<ffff82c4c0136510>] do_tmem_op+0x5a0/0xbd0
(XEN)    [<ffff82c4c022391b>] syscall_enter+0xeb/0x145
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 3:
(XEN) Xen BUG at domain_page.c:169
(XEN) ****************************************
(XEN)
(XEN) Manual reset required ('noreboot' specified)

It looks as if the path that is taken is:

110     idx = find_next_zero_bit(dcache->inuse, dcache->entries, 
dcache->cursor);
111     if ( unlikely(idx >= dcache->entries) )
112     {

115         /* /First/, clean the garbage map and update the inuse list. */
116         for ( i = 0; i < BITS_TO_LONGS(dcache->entries); i++ )
117         {
118             dcache->inuse[i] &= ~xchg(&dcache->garbage[i], 0);
119             accum |= ~dcache->inuse[i];

Here computes the accum
120         }
121
122         if ( accum )
123             idx = find_first_zero_bit(dcache->inuse, dcache->entries)

Ok, finds the idx (32),
124         else
125         {
.. does not go here.
142         }
143         BUG_ON(idx >= dcache->entries);

And hits the BUG_ON().

But I am not sure if that is appropriate. Perhaps the BUG_ON was meant 
as a check
for the loop (lines 128 ->  141) - in case it looped around and never 
found an empty place.
But if that is the condition then that would also look suspect as it 
might have found an
empty hash entry and the idx would still end up being 32.


>
> But tmem being unsupported due to the still pending security
> audit makes this a low priority issue anyway. And as you may
I think the discussion on whether a fix should go in Xen 4.3 (or the 
stable releases)
is something we can discuss when a patch has been fabricated.
> or may not recall, it is being disabled for systems with more
> than 5Tb too. So quite a bit of work on the tmem side...
>
> Jan
>


[-- Attachment #2: xen-domain_page.patch --]
[-- Type: text/x-patch, Size: 3651 bytes --]

diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
index efda6af..7c61adc 100644
--- a/xen/arch/x86/domain_page.c
+++ b/xen/arch/x86/domain_page.c
@@ -59,12 +59,12 @@ void __init mapcache_override_current(struct vcpu *v)
 void *map_domain_page(unsigned long mfn)
 {
     unsigned long flags;
-    unsigned int idx, i;
+    unsigned int idx, i, j = 0;
     struct vcpu *v;
     struct mapcache_domain *dcache;
     struct mapcache_vcpu *vcache;
     struct vcpu_maphash_entry *hashent;
-
+    int branch = 0;
 #ifdef NDEBUG
     if ( mfn <= PFN_DOWN(__pa(HYPERVISOR_VIRT_END - 1)) )
         return mfn_to_virt(mfn);
@@ -119,27 +119,53 @@ void *map_domain_page(unsigned long mfn)
             accum |= ~dcache->inuse[i];
         }
 
-        if ( accum )
+        if ( accum ) {
             idx = find_first_zero_bit(dcache->inuse, dcache->entries);
+            branch |= 1;
+        }
         else
         {
+            branch |= 2;
             /* Replace a hash entry instead. */
             i = MAPHASH_HASHFN(mfn);
             do {
                 hashent = &vcache->hash[i];
                 if ( hashent->idx != MAPHASHENT_NOTINUSE && !hashent->refcnt )
                 {
+                    branch |= 4;
                     idx = hashent->idx;
                     ASSERT(l1e_get_pfn(MAPCACHE_L1ENT(idx)) == hashent->mfn);
                     l1e_write(&MAPCACHE_L1ENT(idx), l1e_empty());
                     hashent->idx = MAPHASHENT_NOTINUSE;
                     hashent->mfn = ~0UL;
+                    if (idx >= dcache->entries) {
+                        branch |= 8;
+                        gdprintk(XENLOG_INFO, "mfn (%lx) -> %ld idx (iter:%d)\n", mfn,  MAPHASH_HASHFN(mfn), j);
+
+                        for (i = 0; i < MAPHASH_ENTRIES;i++) {
+                            hashent = &vcache->hash[i];
+
+                            gdprintk(XENLOG_INFO, "[%d] idx=%d, mfn=0x%lx, refcnt: %d\n",
+                                    i, hashent->idx, hashent->mfn, hashent->refcnt);
+                        }
+                    }
                     break;
                 }
                 if ( ++i == MAPHASH_ENTRIES )
                     i = 0;
+                j++;
             } while ( i != MAPHASH_HASHFN(mfn) );
         }
+        if (idx >= dcache->entries) {
+           gdprintk(XENLOG_INFO, "mfn (%lx) -> %ld idx: %d(i:%d,j:%d), branch:%x\n", mfn,  MAPHASH_HASHFN(mfn), idx,  i, j, branch);
+
+           for (i = 0; i < MAPHASH_ENTRIES;i++) {
+                    hashent = &vcache->hash[i];
+
+                    gdprintk(XENLOG_INFO, "[%d] idx=%d, mfn=0x%lx, refcnt: %d\n",
+                                    i, hashent->idx, hashent->mfn, hashent->refcnt);
+           }
+        }
         BUG_ON(idx >= dcache->entries);
 
         /* /Second/, flush TLBs. */
@@ -254,6 +280,7 @@ int mapcache_domain_init(struct domain *d)
                  2 * PFN_UP(BITS_TO_LONGS(MAPCACHE_ENTRIES) * sizeof(long))) >
                  MAPCACHE_VIRT_START + (PERDOMAIN_SLOT_MBYTES << 20));
     bitmap_pages = PFN_UP(BITS_TO_LONGS(MAPCACHE_ENTRIES) * sizeof(long));
+    gdprintk(XENLOG_INFO, "domain bitmap pages: %d\n", bitmap_pages);
     dcache->inuse = (void *)MAPCACHE_VIRT_END + PAGE_SIZE;
     dcache->garbage = dcache->inuse +
                       (bitmap_pages + 1) * PAGE_SIZE / sizeof(long);
@@ -276,6 +303,7 @@ int mapcache_vcpu_init(struct vcpu *v)
     if ( is_hvm_vcpu(v) || !dcache->inuse )
         return 0;
 
+    gdprintk(XENLOG_INFO, "ents: %d, entries: %d\n", ents, dcache->entries);
     if ( ents > dcache->entries )
     {
         /* Populate page tables. */

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-06-11 15:30   ` konrad wilk
@ 2013-06-11 15:56     ` George Dunlap
  2013-06-11 16:38     ` Jan Beulich
  1 sibling, 0 replies; 28+ messages in thread
From: George Dunlap @ 2013-06-11 15:56 UTC (permalink / raw)
  To: konrad wilk; +Cc: Jan Beulich, xen-devel

On Tue, Jun 11, 2013 at 4:30 PM, konrad wilk <konrad.wilk@oracle.com> wrote:
> I think this is a more subtle bug.
> I applied a debug patch (see attached) and with the help of it and the logs:
>
> (XEN) domain_page.c:160:d1 mfn (1ebe96) -> 6 idx: 32(i:1,j:0), branch:1
> (XEN) domain_page.c:166:d1 [0] idx=26, mfn=0x1ebcd8, refcnt: 0
> (XEN) domain_page.c:166:d1 [1] idx=12, mfn=0x1ebcd9, refcnt: 0
> (XEN) domain_page.c:166:d1 [2] idx=2, mfn=0x210e9a, refcnt: 0
> (XEN) domain_page.c:166:d1 [3] idx=14, mfn=0x210e9b, refcnt: 0
> (XEN) domain_page.c:166:d1 [4] idx=7, mfn=0x210e9c, refcnt: 0
> (XEN) domain_page.c:166:d1 [5] idx=10, mfn=0x210e9d, refcnt: 0
> (XEN) domain_page.c:166:d1 [6] idx=5, mfn=0x210e9e, refcnt: 0
> (XEN) domain_page.c:166:d1 [7] idx=13, mfn=0x1ebe97, refcnt: 0
> (XEN) Xen BUG at domain_page.c:169
>
> (XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Not tainted ]----
> (XEN) CPU:    3
> (XEN) RIP:    e008:[<ffff82c4c01606a7>] map_domain_page+0x61d/0x6e1
>
> (XEN) RFLAGS: 0000000000010046   CONTEXT: hypervisor
> (XEN) rax: 0000000000000000   rbx: ffff8300c68f9000   rcx: 0000000000000000
> (XEN) rdx: ffff8302125b2020   rsi: 000000000000000a   rdi: ffff82c4c027a6e8
> (XEN) rbp: ffff8302125afcc8   rsp: ffff8302125afc48   r8: 0000000000000004
> (XEN) r9:  0000000000000004   r10: 0000000000000004   r11: 0000000000000001
> (XEN) r12: ffff83022e2ef000   r13: 00000000001ebe96   r14: 0000000000000020
> (XEN) r15: ffff8300c68f9080   cr0: 0000000080050033   cr4: 00000000000426f0
> (XEN) cr3: 0000000209541000   cr2: ffffffffff600400
>
> (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
> (XEN) Xen stack trace from rsp=ffff8302125afc48:
> (XEN)    00000000001ebe97 0000000000000000 0000000000000000 ffff830200000001
> (XEN)    ffff8302125afcc8 ffff82c400000000 00000000001ebe97 000000080000000d
> (XEN)    ffff83022e2ef2d8 0000000000000286 ffff82c4c0127b6b ffff83022e2ef000
> (XEN)    ffff82e003d7d2c0 ffff8302125afd60 00000000001ebe96 0000000000000000
> (XEN)    ffff8302125afd38 ffff82c4c01373de 0000000000000000 ffffffffffffffff
> (XEN)    0000000000000001 ffff8302125afd58 ffff83022e2ef2d8 0000000000000286
>
> (XEN)    0000000000000027 0000000000000000 0000000000001000 0000000000000000
> (XEN)    0000000000000000 00000000001ebe96 ffff8302125afd98 ffff82c4c01377c4
> (XEN)    0000000000000000 ffff820040017000 ffff82e003d7d2c0 00000000001ebe96
> (XEN)    ffff8302125afd98 ffff830210ecf390 00000000fffffff4 ffff820040009010
> (XEN)    ffff820040000f50 ffff83022e2f0c90 ffff8302125afe18 ffff82c4c0135929
> (XEN)    000000160000001e ffff820040000f50 0000000000000000 00000000001ebe96
> (XEN)    0000000000000000 0000000000000000 0000a2f6125afe28 ffff8302125afe00
> (XEN)    0000001675f02b51 ffff83022e2f0c90 ffff830210ecf390 0000000000000000
> (XEN)    0000000000000001 0000000000000065 ffff8302125afef8 ffff82c4c0136510
> (XEN)    ffff830200001000 0000000000000000 ffff8302125afe90 255ece02125b2040
> (XEN)    00000003125afe68 00000016742667d1 ffff8302125b2100 0000003d52299000
> (XEN)    ffff8300c68f9000 0000000001c9c380 ffff8302125b2100 ffff8302125b1808
> (XEN)    0000000000000004 0000000000000004 0000000000000000 0000000000000000
> (XEN)    000000000000a2f6 0000000000000000 00000000001ebe96 ffff82c4c0126e77
> (XEN) Xen call trace:
> (XEN)    [<ffff82c4c01606a7>] map_domain_page+0x61d/0x6e1
>
> (XEN)    [<ffff82c4c01373de>] cli_get_page+0x15e/0x17b
> (XEN)    [<ffff82c4c01377c4>] tmh_copy_from_client+0x150/0x284
> (XEN)    [<ffff82c4c0135929>] do_tmem_put+0x323/0x5c4
> (XEN)    [<ffff82c4c0136510>] do_tmem_op+0x5a0/0xbd0
> (XEN)    [<ffff82c4c022391b>] syscall_enter+0xeb/0x145
>
> (XEN)
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 3:
> (XEN) Xen BUG at domain_page.c:169
>
> (XEN) ****************************************
> (XEN)
> (XEN) Manual reset required ('noreboot' specified)
>
> It looks as if the path that is taken is:
>
> 110     idx = find_next_zero_bit(dcache->inuse, dcache->entries,
> dcache->cursor);
> 111     if ( unlikely(idx >= dcache->entries) )
> 112     {
>
> 115         /* /First/, clean the garbage map and update the inuse list. */
> 116         for ( i = 0; i < BITS_TO_LONGS(dcache->entries); i++ )
> 117         {
> 118             dcache->inuse[i] &= ~xchg(&dcache->garbage[i], 0);
> 119             accum |= ~dcache->inuse[i];
>
> Here computes the accum
> 120         }
> 121
> 122         if ( accum )
> 123             idx = find_first_zero_bit(dcache->inuse, dcache->entries)
>
> Ok, finds the idx (32),
> 124         else
> 125         {
> .. does not go here.
> 142         }
> 143         BUG_ON(idx >= dcache->entries);
>
> And hits the BUG_ON().
>
> But I am not sure if that is appropriate. Perhaps the BUG_ON was meant as a
> check
> for the loop (lines 128 ->  141) - in case it looped around and never found
> an empty place.
> But if that is the condition then that would also look suspect as it might
> have found an
> empty hash entry and the idx would still end up being 32.

Right -- it is really curious that "accum |= ~dcache->inuse[x]"
managed to be non-zero, while find_first_zero_bit() goes off the end
(as it seems).

It seems like you should add a printk in the first loop:
   if(~dcache->inuse[i]) printk(...);

Also, I don't think you've printed what dcache->entries is -- is it 32?

 -George

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

* Re: Xen 4.3 + tmem =  Xen BUG at domain_page.c:143
  2013-06-11 15:30   ` konrad wilk
  2013-06-11 15:56     ` George Dunlap
@ 2013-06-11 16:38     ` Jan Beulich
  2013-06-11 17:30       ` konrad wilk
  2013-06-11 18:52       ` konrad wilk
  1 sibling, 2 replies; 28+ messages in thread
From: Jan Beulich @ 2013-06-11 16:38 UTC (permalink / raw)
  To: konrad wilk; +Cc: xen-devel

>>> On 11.06.13 at 17:30, konrad wilk <konrad.wilk@oracle.com> wrote:
> I think this is a more subtle bug.
> I applied a debug patch (see attached) and with the help of it and the logs:
> 
> (XEN) domain_page.c:160:d1 mfn (1ebe96) -> 6 idx: 32(i:1,j:0), branch:1
> (XEN) domain_page.c:166:d1 [0] idx=26, mfn=0x1ebcd8, refcnt: 0
> (XEN) domain_page.c:166:d1 [1] idx=12, mfn=0x1ebcd9, refcnt: 0
> (XEN) domain_page.c:166:d1 [2] idx=2, mfn=0x210e9a, refcnt: 0
> (XEN) domain_page.c:166:d1 [3] idx=14, mfn=0x210e9b, refcnt: 0
> (XEN) domain_page.c:166:d1 [4] idx=7, mfn=0x210e9c, refcnt: 0
> (XEN) domain_page.c:166:d1 [5] idx=10, mfn=0x210e9d, refcnt: 0
> (XEN) domain_page.c:166:d1 [6] idx=5, mfn=0x210e9e, refcnt: 0
> (XEN) domain_page.c:166:d1 [7] idx=13, mfn=0x1ebe97, refcnt: 0
> (XEN) Xen BUG at domain_page.c:169
> (XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Not tainted ]----
> (XEN) CPU:    3
> (XEN) RIP:    e008:[<ffff82c4c01606a7>] map_domain_page+0x61d/0x6e1
> (XEN) RFLAGS: 0000000000010046   CONTEXT: hypervisor
> (XEN) rax: 0000000000000000   rbx: ffff8300c68f9000   rcx: 0000000000000000
> (XEN) rdx: ffff8302125b2020   rsi: 000000000000000a   rdi: ffff82c4c027a6e8
> (XEN) rbp: ffff8302125afcc8   rsp: ffff8302125afc48   r8: 0000000000000004
> (XEN) r9:  0000000000000004   r10: 0000000000000004   r11: 0000000000000001
> (XEN) r12: ffff83022e2ef000   r13: 00000000001ebe96   r14: 0000000000000020
> (XEN) r15: ffff8300c68f9080   cr0: 0000000080050033   cr4: 00000000000426f0
> (XEN) cr3: 0000000209541000   cr2: ffffffffff600400
> (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
> (XEN) Xen stack trace from rsp=ffff8302125afc48:
> (XEN)    00000000001ebe97 0000000000000000 0000000000000000 ffff830200000001
> (XEN)    ffff8302125afcc8 ffff82c400000000 00000000001ebe97 000000080000000d
> (XEN)    ffff83022e2ef2d8 0000000000000286 ffff82c4c0127b6b ffff83022e2ef000
> (XEN)    ffff82e003d7d2c0 ffff8302125afd60 00000000001ebe96 0000000000000000
> (XEN)    ffff8302125afd38 ffff82c4c01373de 0000000000000000 ffffffffffffffff
> (XEN)    0000000000000001 ffff8302125afd58 ffff83022e2ef2d8 0000000000000286
> (XEN)    0000000000000027 0000000000000000 0000000000001000 0000000000000000
> (XEN)    0000000000000000 00000000001ebe96 ffff8302125afd98 ffff82c4c01377c4
> (XEN)    0000000000000000 ffff820040017000 ffff82e003d7d2c0 00000000001ebe96
> (XEN)    ffff8302125afd98 ffff830210ecf390 00000000fffffff4 ffff820040009010
> (XEN)    ffff820040000f50 ffff83022e2f0c90 ffff8302125afe18 ffff82c4c0135929
> (XEN)    000000160000001e ffff820040000f50 0000000000000000 00000000001ebe96
> (XEN)    0000000000000000 0000000000000000 0000a2f6125afe28 ffff8302125afe00
> (XEN)    0000001675f02b51 ffff83022e2f0c90 ffff830210ecf390 0000000000000000
> (XEN)    0000000000000001 0000000000000065 ffff8302125afef8 ffff82c4c0136510
> (XEN)    ffff830200001000 0000000000000000 ffff8302125afe90 255ece02125b2040
> (XEN)    00000003125afe68 00000016742667d1 ffff8302125b2100 0000003d52299000
> (XEN)    ffff8300c68f9000 0000000001c9c380 ffff8302125b2100 ffff8302125b1808
> (XEN)    0000000000000004 0000000000000004 0000000000000000 0000000000000000
> (XEN)    000000000000a2f6 0000000000000000 00000000001ebe96 ffff82c4c0126e77
> (XEN) Xen call trace:
> (XEN)    [<ffff82c4c01606a7>] map_domain_page+0x61d/0x6e1
> (XEN)    [<ffff82c4c01373de>] cli_get_page+0x15e/0x17b
> (XEN)    [<ffff82c4c01377c4>] tmh_copy_from_client+0x150/0x284
> (XEN)    [<ffff82c4c0135929>] do_tmem_put+0x323/0x5c4
> (XEN)    [<ffff82c4c0136510>] do_tmem_op+0x5a0/0xbd0
> (XEN)    [<ffff82c4c022391b>] syscall_enter+0xeb/0x145
> (XEN)
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 3:
> (XEN) Xen BUG at domain_page.c:169
> (XEN) ****************************************
> (XEN)
> (XEN) Manual reset required ('noreboot' specified)
> 
> It looks as if the path that is taken is:
> 
> 110     idx = find_next_zero_bit(dcache->inuse, dcache->entries, 
> dcache->cursor);
> 111     if ( unlikely(idx >= dcache->entries) )
> 112     {
> 
> 115         /* /First/, clean the garbage map and update the inuse list. */
> 116         for ( i = 0; i < BITS_TO_LONGS(dcache->entries); i++ )
> 117         {
> 118             dcache->inuse[i] &= ~xchg(&dcache->garbage[i], 0);
> 119             accum |= ~dcache->inuse[i];
> 
> Here computes the accum
> 120         }
> 121
> 122         if ( accum )
> 123             idx = find_first_zero_bit(dcache->inuse, dcache->entries)
> 
> Ok, finds the idx (32),
> 124         else
> 125         {
> .. does not go here.
> 142         }
> 143         BUG_ON(idx >= dcache->entries);
> 
> And hits the BUG_ON().
> 
> But I am not sure if that is appropriate. Perhaps the BUG_ON was meant 
> as a check
> for the loop (lines 128 ->  141) - in case it looped around and never 
> found an empty place.
> But if that is the condition then that would also look suspect as it 
> might have found an
> empty hash entry and the idx would still end up being 32.

The BUG_ON() here is definitely valid - a few lines down, after the
enclosing if(), we use it in ways that requires this to not have
triggered. It basically tells you whether an in range idx was found,
which apparently isn't the case here.

As I think George already pointed out - printing accum here would
be quite useful: It should have at least one of the low 32 bits set,
given that dcache->entries must be at most 32 according to the
data you already got logged.

Jan

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

* Re: Xen 4.3 + tmem =  Xen BUG at domain_page.c:143
  2013-06-11 16:38     ` Jan Beulich
@ 2013-06-11 17:30       ` konrad wilk
  2013-06-11 18:52       ` konrad wilk
  1 sibling, 0 replies; 28+ messages in thread
From: konrad wilk @ 2013-06-11 17:30 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel

[-- Attachment #1: Type: text/plain, Size: 9316 bytes --]


On 6/11/2013 12:38 PM, Jan Beulich wrote:
>>>> On 11.06.13 at 17:30, konrad wilk <konrad.wilk@oracle.com> wrote:
>> I think this is a more subtle bug.
>> I applied a debug patch (see attached) and with the help of it and the logs:
>>
>> (XEN) domain_page.c:160:d1 mfn (1ebe96) -> 6 idx: 32(i:1,j:0), branch:1
>> (XEN) domain_page.c:166:d1 [0] idx=26, mfn=0x1ebcd8, refcnt: 0
>> (XEN) domain_page.c:166:d1 [1] idx=12, mfn=0x1ebcd9, refcnt: 0
>> (XEN) domain_page.c:166:d1 [2] idx=2, mfn=0x210e9a, refcnt: 0
>> (XEN) domain_page.c:166:d1 [3] idx=14, mfn=0x210e9b, refcnt: 0
>> (XEN) domain_page.c:166:d1 [4] idx=7, mfn=0x210e9c, refcnt: 0
>> (XEN) domain_page.c:166:d1 [5] idx=10, mfn=0x210e9d, refcnt: 0
>> (XEN) domain_page.c:166:d1 [6] idx=5, mfn=0x210e9e, refcnt: 0
>> (XEN) domain_page.c:166:d1 [7] idx=13, mfn=0x1ebe97, refcnt: 0
>> (XEN) Xen BUG at domain_page.c:169
>> (XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Not tainted ]----
>> (XEN) CPU:    3
>> (XEN) RIP:    e008:[<ffff82c4c01606a7>] map_domain_page+0x61d/0x6e1
>> (XEN) RFLAGS: 0000000000010046   CONTEXT: hypervisor
>> (XEN) rax: 0000000000000000   rbx: ffff8300c68f9000   rcx: 0000000000000000
>> (XEN) rdx: ffff8302125b2020   rsi: 000000000000000a   rdi: ffff82c4c027a6e8
>> (XEN) rbp: ffff8302125afcc8   rsp: ffff8302125afc48   r8: 0000000000000004
>> (XEN) r9:  0000000000000004   r10: 0000000000000004   r11: 0000000000000001
>> (XEN) r12: ffff83022e2ef000   r13: 00000000001ebe96   r14: 0000000000000020
>> (XEN) r15: ffff8300c68f9080   cr0: 0000000080050033   cr4: 00000000000426f0
>> (XEN) cr3: 0000000209541000   cr2: ffffffffff600400
>> (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
>> (XEN) Xen stack trace from rsp=ffff8302125afc48:
>> (XEN)    00000000001ebe97 0000000000000000 0000000000000000 ffff830200000001
>> (XEN)    ffff8302125afcc8 ffff82c400000000 00000000001ebe97 000000080000000d
>> (XEN)    ffff83022e2ef2d8 0000000000000286 ffff82c4c0127b6b ffff83022e2ef000
>> (XEN)    ffff82e003d7d2c0 ffff8302125afd60 00000000001ebe96 0000000000000000
>> (XEN)    ffff8302125afd38 ffff82c4c01373de 0000000000000000 ffffffffffffffff
>> (XEN)    0000000000000001 ffff8302125afd58 ffff83022e2ef2d8 0000000000000286
>> (XEN)    0000000000000027 0000000000000000 0000000000001000 0000000000000000
>> (XEN)    0000000000000000 00000000001ebe96 ffff8302125afd98 ffff82c4c01377c4
>> (XEN)    0000000000000000 ffff820040017000 ffff82e003d7d2c0 00000000001ebe96
>> (XEN)    ffff8302125afd98 ffff830210ecf390 00000000fffffff4 ffff820040009010
>> (XEN)    ffff820040000f50 ffff83022e2f0c90 ffff8302125afe18 ffff82c4c0135929
>> (XEN)    000000160000001e ffff820040000f50 0000000000000000 00000000001ebe96
>> (XEN)    0000000000000000 0000000000000000 0000a2f6125afe28 ffff8302125afe00
>> (XEN)    0000001675f02b51 ffff83022e2f0c90 ffff830210ecf390 0000000000000000
>> (XEN)    0000000000000001 0000000000000065 ffff8302125afef8 ffff82c4c0136510
>> (XEN)    ffff830200001000 0000000000000000 ffff8302125afe90 255ece02125b2040
>> (XEN)    00000003125afe68 00000016742667d1 ffff8302125b2100 0000003d52299000
>> (XEN)    ffff8300c68f9000 0000000001c9c380 ffff8302125b2100 ffff8302125b1808
>> (XEN)    0000000000000004 0000000000000004 0000000000000000 0000000000000000
>> (XEN)    000000000000a2f6 0000000000000000 00000000001ebe96 ffff82c4c0126e77
>> (XEN) Xen call trace:
>> (XEN)    [<ffff82c4c01606a7>] map_domain_page+0x61d/0x6e1
>> (XEN)    [<ffff82c4c01373de>] cli_get_page+0x15e/0x17b
>> (XEN)    [<ffff82c4c01377c4>] tmh_copy_from_client+0x150/0x284
>> (XEN)    [<ffff82c4c0135929>] do_tmem_put+0x323/0x5c4
>> (XEN)    [<ffff82c4c0136510>] do_tmem_op+0x5a0/0xbd0
>> (XEN)    [<ffff82c4c022391b>] syscall_enter+0xeb/0x145
>> (XEN)
>> (XEN)
>> (XEN) ****************************************
>> (XEN) Panic on CPU 3:
>> (XEN) Xen BUG at domain_page.c:169
>> (XEN) ****************************************
>> (XEN)
>> (XEN) Manual reset required ('noreboot' specified)
>>
>> It looks as if the path that is taken is:
>>
>> 110     idx = find_next_zero_bit(dcache->inuse, dcache->entries,
>> dcache->cursor);
>> 111     if ( unlikely(idx >= dcache->entries) )
>> 112     {
>>
>> 115         /* /First/, clean the garbage map and update the inuse list. */
>> 116         for ( i = 0; i < BITS_TO_LONGS(dcache->entries); i++ )
>> 117         {
>> 118             dcache->inuse[i] &= ~xchg(&dcache->garbage[i], 0);
>> 119             accum |= ~dcache->inuse[i];
>>
>> Here computes the accum
>> 120         }
>> 121
>> 122         if ( accum )
>> 123             idx = find_first_zero_bit(dcache->inuse, dcache->entries)
>>
>> Ok, finds the idx (32),
>> 124         else
>> 125         {
>> .. does not go here.
>> 142         }
>> 143         BUG_ON(idx >= dcache->entries);
>>
>> And hits the BUG_ON().
>>
>> But I am not sure if that is appropriate. Perhaps the BUG_ON was meant
>> as a check
>> for the loop (lines 128 ->  141) - in case it looped around and never
>> found an empty place.
>> But if that is the condition then that would also look suspect as it
>> might have found an
>> empty hash entry and the idx would still end up being 32.
> The BUG_ON() here is definitely valid - a few lines down, after the
> enclosing if(), we use it in ways that requires this to not have
> triggered. It basically tells you whether an in range idx was found,
> which apparently isn't the case here.
>
> As I think George already pointed out - printing accum here would
> be quite useful: It should have at least one of the low 32 bits set,
> given that dcache->entries must be at most 32 according to the
> data you already got logged.

Of course, here is the new log (and the debug attachment)

(XEN) domain_page.c:122:d1 [0]: ffffffff, idx: 32
(XEN) domain_page.c:167:d1 mfn (1eba98) -> 0 idx: 32(i:1,j:0), branch:9 
0xffffffff00000000
(XEN) domain_page.c:173:d1 [0] idx=0, mfn=0x182790, refcnt: 0
(XEN) domain_page.c:173:d1 [1] idx=29, mfn=0x1946f9, refcnt: 0
(XEN) domain_page.c:173:d1 [2] idx=15, mfn=0x1946fa, refcnt: 0
(XEN) domain_page.c:173:d1 [3] idx=11, mfn=0x1946fb, refcnt: 0
(XEN) domain_page.c:173:d1 [4] idx=17, mfn=0x1946fc, refcnt: 0
(XEN) domain_page.c:173:d1 [5] idx=21, mfn=0x1946fd, refcnt: 0
(XEN) domain_page.c:173:d1 [6] idx=10, mfn=0x180296, refcnt: 0
(XEN) domain_page.c:173:d1 [7] idx=4, mfn=0x180297, refcnt: 0
(XEN) Xen BUG at domain_page.c:176
(XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Not tainted ]----
(XEN) CPU:    3
(XEN) RIP:    e008:[<ffff82c4c0160742>] map_domain_page+0x6b8/0x77c
(XEN) RFLAGS: 0000000000010046   CONTEXT: hypervisor
(XEN) rax: 0000000000000000   rbx: ffff8300c68f9000   rcx: 0000000000000000
(XEN) rdx: ffff83020e84c020   rsi: 000000000000000a   rdi: ffff82c4c027a6e8
(XEN) rbp: ffff83020e847cc8   rsp: ffff83020e847c28   r8: 0000000000000004
(XEN) r9:  0000000000000004   r10: 0000000000000004   r11: 0000000000000001
(XEN) r12: ffff83022d815000   r13: 00000000001eba98   r14: 0000000000000020
(XEN) r15: ffff8300c68f9080   cr0: 0000000080050033   cr4: 00000000000426f0
(XEN) cr3: 000000019c644000   cr2: ffff88000ef124b0
(XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
(XEN) Xen stack trace from rsp=ffff83020e847c28:
(XEN)    0000000000180297 0000000000000000 ffff830200000000 0000000000000009
(XEN)    ffffffff00000000 ffff82c4c0116542 0000000000000297 ffffffff00000000
(XEN)    ffff830200000020 00000000c01714f6 0000000000180297 0000000800000004
(XEN)    ffff83022d8152d8 0000000000000286 ffff82c4c012760a ffff83022d815000
(XEN)    ffff82e003d75300 ffff83020e847d60 00000000001eba98 0000000000000000
(XEN)    ffff83020e847d38 ffff82c4c01373de 0000000000000000 ffffffffffffffff
(XEN)    0000000000000001 ffff83020e847d58 ffff83022d8152d8 0000000000000286
(XEN)    0000000000000027 0000000000000000 0000000000001000 0000000000000000
(XEN)    0000000000000000 00000000001eba98 ffff83020e847d98 ffff82c4c01377c4
(XEN)    0000000000000000 ffff82004001a000 ffff82e003d75300 00000000001eba98
(XEN)    ffff83020e847d98 ffff83020354f390 00000000fffffff4 ffff820040002010
(XEN)    ffff820040001580 ffff83022d816c90 ffff83020e847e18 ffff82c4c0135929
(XEN)    ffff83020e847db8 ffff820040001580 0000000000000000 00000000001eba98
(XEN)    0000000000000000 0000000000000000 000001f200000000 ffff83020e847e00
(XEN)    ffff83020e847e18 ffff83022d816c90 ffff83020354f390 0000000000000000
(XEN)    0000000000000001 0000000000000091 ffff83020e847ef8 ffff82c4c0136510
(XEN)    ffff830200001000 0000000000000000 ffff83020e847e90 bbbc0ca3c027bba0
(XEN)    ffff82c4c027bba0 ffff82c4c02e0000 0000000000000002 ffff83020e847e78
(XEN)    ffff82c4c0127b09 ffff82c4c027bba0 ffff83020e847e98 ffff82c4c01299af
(XEN)    0000000000000004 0000000000000005 0000000000000000 0000000000000000
(XEN) Xen call trace:
(XEN)    [<ffff82c4c0160742>] map_domain_page+0x6b8/0x77c
(XEN)    [<ffff82c4c01373de>] cli_get_page+0x15e/0x17b
(XEN)    [<ffff82c4c01377c4>] tmh_copy_from_client+0x150/0x284
(XEN)    [<ffff82c4c0135929>] do_tmem_put+0x323/0x5c4
(XEN)    [<ffff82c4c0136510>] do_tmem_op+0x5a0/0xbd0
(XEN)    [<ffff82c4c02239bb>] syscall_enter+0xeb/0x145
(XEN)
\a(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 3:
(XEN) Xen BUG at domain_page.c:176
(XEN) ****************************************
(XEN)
(XEN) Manual reset required ('noreboot' specified)


[-- Attachment #2: xen-domain_page-v2.patch --]
[-- Type: text/x-patch, Size: 4427 bytes --]

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 3f0b262..bc6b437 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -3,6 +3,7 @@
 # If you change any of these configuration options then you must
 # 'make clean' before rebuilding.
 #
+debug := y
 verbose       ?= n
 perfc         ?= n
 perfc_arrays  ?= n
diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
index efda6af..9ad6193 100644
--- a/xen/arch/x86/domain_page.c
+++ b/xen/arch/x86/domain_page.c
@@ -59,12 +59,12 @@ void __init mapcache_override_current(struct vcpu *v)
 void *map_domain_page(unsigned long mfn)
 {
     unsigned long flags;
-    unsigned int idx, i;
+    unsigned int idx, i, j = 0;
     struct vcpu *v;
     struct mapcache_domain *dcache;
     struct mapcache_vcpu *vcache;
     struct vcpu_maphash_entry *hashent;
-
+    int branch = 0;
 #ifdef NDEBUG
     if ( mfn <= PFN_DOWN(__pa(HYPERVISOR_VIRT_END - 1)) )
         return mfn_to_virt(mfn);
@@ -116,30 +116,63 @@ void *map_domain_page(unsigned long mfn)
         for ( i = 0; i < BITS_TO_LONGS(dcache->entries); i++ )
         {
             dcache->inuse[i] &= ~xchg(&dcache->garbage[i], 0);
+            if (v->domain->domain_id) {
+                if (~dcache->inuse[i]) {
+                    gdprintk(XENLOG_INFO, "[%d]: %lx, idx: %d\n", i, dcache->inuse[i],
+                             find_first_zero_bit(dcache->inuse, dcache->entries));
+                    branch |= 8;
+                }
+            }
             accum |= ~dcache->inuse[i];
         }
 
-        if ( accum )
+        if ( accum ) {
             idx = find_first_zero_bit(dcache->inuse, dcache->entries);
+            branch |= 1;
+        }
         else
         {
+            branch |= 2;
             /* Replace a hash entry instead. */
             i = MAPHASH_HASHFN(mfn);
             do {
                 hashent = &vcache->hash[i];
                 if ( hashent->idx != MAPHASHENT_NOTINUSE && !hashent->refcnt )
                 {
+                    branch |= 4;
                     idx = hashent->idx;
                     ASSERT(l1e_get_pfn(MAPCACHE_L1ENT(idx)) == hashent->mfn);
                     l1e_write(&MAPCACHE_L1ENT(idx), l1e_empty());
                     hashent->idx = MAPHASHENT_NOTINUSE;
                     hashent->mfn = ~0UL;
+                    if (idx >= dcache->entries) {
+                        branch |= 8;
+                        gdprintk(XENLOG_INFO, "mfn (%lx) -> %ld idx (iter:%d)\n", mfn,  MAPHASH_HASHFN(mfn), j);
+
+                        for (i = 0; i < MAPHASH_ENTRIES;i++) {
+                            hashent = &vcache->hash[i];
+
+                            gdprintk(XENLOG_INFO, "[%d] idx=%d, mfn=0x%lx, refcnt: %d\n",
+                                    i, hashent->idx, hashent->mfn, hashent->refcnt);
+                        }
+                    }
                     break;
                 }
                 if ( ++i == MAPHASH_ENTRIES )
                     i = 0;
+                j++;
             } while ( i != MAPHASH_HASHFN(mfn) );
         }
+        if (idx >= dcache->entries) {
+           gdprintk(XENLOG_INFO, "mfn (%lx) -> %ld idx: %d(i:%d,j:%d), branch:%x 0x%lx\n", mfn,  MAPHASH_HASHFN(mfn), idx,  i, j, branch, accum);
+
+           for (i = 0; i < MAPHASH_ENTRIES;i++) {
+                    hashent = &vcache->hash[i];
+
+                    gdprintk(XENLOG_INFO, "[%d] idx=%d, mfn=0x%lx, refcnt: %d\n",
+                                    i, hashent->idx, hashent->mfn, hashent->refcnt);
+           }
+        }
         BUG_ON(idx >= dcache->entries);
 
         /* /Second/, flush TLBs. */
@@ -254,6 +287,7 @@ int mapcache_domain_init(struct domain *d)
                  2 * PFN_UP(BITS_TO_LONGS(MAPCACHE_ENTRIES) * sizeof(long))) >
                  MAPCACHE_VIRT_START + (PERDOMAIN_SLOT_MBYTES << 20));
     bitmap_pages = PFN_UP(BITS_TO_LONGS(MAPCACHE_ENTRIES) * sizeof(long));
+    gdprintk(XENLOG_INFO, "domain bitmap pages: %d\n", bitmap_pages);
     dcache->inuse = (void *)MAPCACHE_VIRT_END + PAGE_SIZE;
     dcache->garbage = dcache->inuse +
                       (bitmap_pages + 1) * PAGE_SIZE / sizeof(long);
@@ -276,6 +310,7 @@ int mapcache_vcpu_init(struct vcpu *v)
     if ( is_hvm_vcpu(v) || !dcache->inuse )
         return 0;
 
+    gdprintk(XENLOG_INFO, "ents: %d, entries: %d\n", ents, dcache->entries);
     if ( ents > dcache->entries )
     {
         /* Populate page tables. */

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

* Re: Xen 4.3 + tmem =  Xen BUG at domain_page.c:143
  2013-06-11 16:38     ` Jan Beulich
  2013-06-11 17:30       ` konrad wilk
@ 2013-06-11 18:52       ` konrad wilk
  2013-06-11 21:06         ` konrad wilk
  2013-06-12 11:00         ` George Dunlap
  1 sibling, 2 replies; 28+ messages in thread
From: konrad wilk @ 2013-06-11 18:52 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel

[-- Attachment #1: Type: text/plain, Size: 1803 bytes --]


> The BUG_ON() here is definitely valid - a few lines down, after the
> enclosing if(), we use it in ways that requires this to not have
> triggered. It basically tells you whether an in range idx was found,
> which apparently isn't the case here.
>
> As I think George already pointed out - printing accum here would
> be quite useful: It should have at least one of the low 32 bits set,
> given that dcache->entries must be at most 32 according to the
> data you already got logged.

With extra debugging (see attached patch)

(XEN) domain_page.c:125:d1 mfn: 1eb483, [0]: bffff1ff, 
~ffffffff40000e00, idx: 9 garbage: 40000e00, inuse: ffffffff
(XEN) domain_page.c:125:d1 mfn: 1eb480, [0]: fdbfffff, 
~ffffffff02400000, idx: 22 garbage: 2400000, inuse: ffffffff
(XEN) domain_page.c:125:d1 mfn: 2067ca, [0]: fffff7ff, 
~ffffffff00000800, idx: 11 garbage: 800, inuse: ffffffff
(XEN) domain_page.c:125:d1 mfn: 183642, [0]: ffffffff, 
~ffffffff00000000, idx: 32 garbage: 0, inuse: ffffffff
(XEN) domain_page.c:170:d1 mfn (183642) -> 2 idx: 32(i:1,j:0), branch:9 
0xffffffff00000000
(XEN) domain_page.c:176:d1 [0] idx=13, mfn=0x203b00, refcnt: 0
(XEN) domain_page.c:176:d1 [1] idx=25, mfn=0x1839e1, refcnt: 0
(XEN) domain_page.c:176:d1 [2] idx=3, mfn=0x1824d2, refcnt: 0
(XEN) domain_page.c:176:d1 [3] idx=5, mfn=0x1eb48b, refcnt: 0
(XEN) domain_page.c:176:d1 [4] idx=28, mfn=0x203b04, refcnt: 0
(XEN) domain_page.c:176:d1 [5] idx=0, mfn=0x1eb485, refcnt: 0
(XEN) domain_page.c:176:d1 [6] idx=30, mfn=0x203afe, refcnt: 0
(XEN) domain_page.c:176:d1 [7] idx=20, mfn=0x203aff, refcnt: 0

And that does point the picture that we have exhausted the full 32 
entries of mapcache.

Now off to find out who is holding them and why. Aren't these operations 
(map/unmap domain_page) suppose to be shortlived?
> Jan


[-- Attachment #2: xen-domain_page-v3.patch --]
[-- Type: text/x-patch, Size: 4717 bytes --]

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 3f0b262..bc6b437 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -3,6 +3,7 @@
 # If you change any of these configuration options then you must
 # 'make clean' before rebuilding.
 #
+debug := y
 verbose       ?= n
 perfc         ?= n
 perfc_arrays  ?= n
diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
index efda6af..af63d76 100644
--- a/xen/arch/x86/domain_page.c
+++ b/xen/arch/x86/domain_page.c
@@ -59,12 +59,12 @@ void __init mapcache_override_current(struct vcpu *v)
 void *map_domain_page(unsigned long mfn)
 {
     unsigned long flags;
-    unsigned int idx, i;
+    unsigned int idx, i, j = 0;
     struct vcpu *v;
     struct mapcache_domain *dcache;
     struct mapcache_vcpu *vcache;
     struct vcpu_maphash_entry *hashent;
-
+    int branch = 0;
 #ifdef NDEBUG
     if ( mfn <= PFN_DOWN(__pa(HYPERVISOR_VIRT_END - 1)) )
         return mfn_to_virt(mfn);
@@ -115,31 +115,67 @@ void *map_domain_page(unsigned long mfn)
         /* /First/, clean the garbage map and update the inuse list. */
         for ( i = 0; i < BITS_TO_LONGS(dcache->entries); i++ )
         {
+            unsigned long garbage = dcache->garbage[i];
+            unsigned long _inuse = dcache->inuse[i];
+            barrier();
             dcache->inuse[i] &= ~xchg(&dcache->garbage[i], 0);
+            if (v->domain->domain_id) {
+                if (~dcache->inuse[i]) {
+                    gdprintk(XENLOG_INFO, "mfn: %lx, [%d]: %lx, ~%lx, idx: %d garbage: %lx, inuse: %lx\n", mfn, i, dcache->inuse[i], ~dcache->inuse[i],
+                             find_first_zero_bit(dcache->inuse, dcache->entries), garbage, _inuse);
+                    branch |= 8;
+                }
+            }
             accum |= ~dcache->inuse[i];
         }
 
-        if ( accum )
+        if ( accum ) {
             idx = find_first_zero_bit(dcache->inuse, dcache->entries);
+            branch |= 1;
+        }
         else
         {
+            branch |= 2;
             /* Replace a hash entry instead. */
             i = MAPHASH_HASHFN(mfn);
             do {
                 hashent = &vcache->hash[i];
                 if ( hashent->idx != MAPHASHENT_NOTINUSE && !hashent->refcnt )
                 {
+                    branch |= 4;
                     idx = hashent->idx;
                     ASSERT(l1e_get_pfn(MAPCACHE_L1ENT(idx)) == hashent->mfn);
                     l1e_write(&MAPCACHE_L1ENT(idx), l1e_empty());
                     hashent->idx = MAPHASHENT_NOTINUSE;
                     hashent->mfn = ~0UL;
+                    if (idx >= dcache->entries) {
+                        branch |= 8;
+                        gdprintk(XENLOG_INFO, "mfn (%lx) -> %ld idx (iter:%d)\n", mfn,  MAPHASH_HASHFN(mfn), j);
+
+                        for (i = 0; i < MAPHASH_ENTRIES;i++) {
+                            hashent = &vcache->hash[i];
+
+                            gdprintk(XENLOG_INFO, "[%d] idx=%d, mfn=0x%lx, refcnt: %d\n",
+                                    i, hashent->idx, hashent->mfn, hashent->refcnt);
+                        }
+                    }
                     break;
                 }
                 if ( ++i == MAPHASH_ENTRIES )
                     i = 0;
+                j++;
             } while ( i != MAPHASH_HASHFN(mfn) );
         }
+        if (idx >= dcache->entries) {
+           gdprintk(XENLOG_INFO, "mfn (%lx) -> %ld idx: %d(i:%d,j:%d), branch:%x 0x%lx\n", mfn,  MAPHASH_HASHFN(mfn), idx,  i, j, branch, accum);
+
+           for (i = 0; i < MAPHASH_ENTRIES;i++) {
+                    hashent = &vcache->hash[i];
+
+                    gdprintk(XENLOG_INFO, "[%d] idx=%d, mfn=0x%lx, refcnt: %d\n",
+                                    i, hashent->idx, hashent->mfn, hashent->refcnt);
+           }
+        }
         BUG_ON(idx >= dcache->entries);
 
         /* /Second/, flush TLBs. */
@@ -254,6 +290,7 @@ int mapcache_domain_init(struct domain *d)
                  2 * PFN_UP(BITS_TO_LONGS(MAPCACHE_ENTRIES) * sizeof(long))) >
                  MAPCACHE_VIRT_START + (PERDOMAIN_SLOT_MBYTES << 20));
     bitmap_pages = PFN_UP(BITS_TO_LONGS(MAPCACHE_ENTRIES) * sizeof(long));
+    gdprintk(XENLOG_INFO, "domain bitmap pages: %d\n", bitmap_pages);
     dcache->inuse = (void *)MAPCACHE_VIRT_END + PAGE_SIZE;
     dcache->garbage = dcache->inuse +
                       (bitmap_pages + 1) * PAGE_SIZE / sizeof(long);
@@ -276,6 +313,7 @@ int mapcache_vcpu_init(struct vcpu *v)
     if ( is_hvm_vcpu(v) || !dcache->inuse )
         return 0;
 
+    gdprintk(XENLOG_INFO, "ents: %d, entries: %d\n", ents, dcache->entries);
     if ( ents > dcache->entries )
     {
         /* Populate page tables. */

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

* Re: Xen 4.3 + tmem =  Xen BUG at domain_page.c:143
  2013-06-11 18:52       ` konrad wilk
@ 2013-06-11 21:06         ` konrad wilk
  2013-06-12  6:38           ` Jan Beulich
  2013-06-12 11:00         ` George Dunlap
  1 sibling, 1 reply; 28+ messages in thread
From: konrad wilk @ 2013-06-11 21:06 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel

[-- Attachment #1: Type: text/plain, Size: 4830 bytes --]


On 6/11/2013 2:52 PM, konrad wilk wrote:
>
>> The BUG_ON() here is definitely valid - a few lines down, after the
>> enclosing if(), we use it in ways that requires this to not have
>> triggered. It basically tells you whether an in range idx was found,
>> which apparently isn't the case here.
>>
>> As I think George already pointed out - printing accum here would
>> be quite useful: It should have at least one of the low 32 bits set,
>> given that dcache->entries must be at most 32 according to the
>> data you already got logged.
>
> With extra debugging (see attached patch)
>
> (XEN) domain_page.c:125:d1 mfn: 1eb483, [0]: bffff1ff, 
> ~ffffffff40000e00, idx: 9 garbage: 40000e00, inuse: ffffffff
> (XEN) domain_page.c:125:d1 mfn: 1eb480, [0]: fdbfffff, 
> ~ffffffff02400000, idx: 22 garbage: 2400000, inuse: ffffffff
> (XEN) domain_page.c:125:d1 mfn: 2067ca, [0]: fffff7ff, 
> ~ffffffff00000800, idx: 11 garbage: 800, inuse: ffffffff
> (XEN) domain_page.c:125:d1 mfn: 183642, [0]: ffffffff, 
> ~ffffffff00000000, idx: 32 garbage: 0, inuse: ffffffff
> (XEN) domain_page.c:170:d1 mfn (183642) -> 2 idx: 32(i:1,j:0), 
> branch:9 0xffffffff00000000
> (XEN) domain_page.c:176:d1 [0] idx=13, mfn=0x203b00, refcnt: 0
> (XEN) domain_page.c:176:d1 [1] idx=25, mfn=0x1839e1, refcnt: 0
> (XEN) domain_page.c:176:d1 [2] idx=3, mfn=0x1824d2, refcnt: 0
> (XEN) domain_page.c:176:d1 [3] idx=5, mfn=0x1eb48b, refcnt: 0
> (XEN) domain_page.c:176:d1 [4] idx=28, mfn=0x203b04, refcnt: 0
> (XEN) domain_page.c:176:d1 [5] idx=0, mfn=0x1eb485, refcnt: 0
> (XEN) domain_page.c:176:d1 [6] idx=30, mfn=0x203afe, refcnt: 0
> (XEN) domain_page.c:176:d1 [7] idx=20, mfn=0x203aff, refcnt: 0
>
> And that does point the picture that we have exhausted the full 32 
> entries of mapcache.
>
> Now off to find out who is holding them and why. Aren't these 
> operations (map/unmap domain_page) suppose to be shortlived?

And found the culprit. With some EIP logging:

(XEN) domain_page.c:214:d1 [0] mfn=0x1ff67a idx=0, mfn=0x1ff67a, refcnt: 
0 [EIP=0]
(XEN) domain_page.c:216:d1 [1] mfn=18fef2, [EIP=0]
(XEN) domain_page.c:216:d1 [2] mfn=1eb518, 
[tmh_persistent_pool_page_get+0x26d/0x2d8]
(XEN) domain_page.c:216:d1 [3] mfn=170a08, 
[tmh_persistent_pool_page_get+0x26d/0x2d8]
(XEN) domain_page.c:216:d1 [4] mfn=18feef, [EIP=0]
(XEN) domain_page.c:216:d1 [5] mfn=1eb4c8, 
[tmh_persistent_pool_page_get+0x26d/0x2d8]
(XEN) domain_page.c:216:d1 [6] mfn=202699, 
[tmh_persistent_pool_page_get+0x26d/0x2d8]
(XEN) domain_page.c:216:d1 [7] mfn=18fef0, [EIP=0]
(XEN) domain_page.c:216:d1 [8] mfn=0, [EIP=0]
(XEN) domain_page.c:214:d1 [9] mfn=0x18e7ed idx=9, mfn=0x18e7ed, refcnt: 
0 [EIP=0]
(XEN) domain_page.c:214:d1 [10] mfn=0x18f629 idx=10, mfn=0x18f629, 
refcnt: 0 [EIP=0]
(XEN) domain_page.c:216:d1 [11] mfn=1eb47e, 
[tmh_persistent_pool_page_get+0x26d/0x2d8]
(XEN) domain_page.c:214:d1 [12] mfn=0x18209c idx=12, mfn=0x18209c, 
refcnt: 0 [EIP=0]
(XEN) domain_page.c:216:d1 [13] mfn=18fef5, [EIP=0]
(XEN) domain_page.c:214:d1 [14] mfn=0x18f62b idx=14, mfn=0x18f62b, 
refcnt: 0 [EIP=0]
(XEN) domain_page.c:216:d1 [15] mfn=1eb459, 
[tmh_persistent_pool_page_get+0x26d/0x2d8]
(XEN) domain_page.c:216:d1 [16] mfn=1eb512, 
[tmh_persistent_pool_page_get+0x26d/0x2d8]
(XEN) domain_page.c:216:d1 [17] mfn=170d2b, 
[tmh_persistent_pool_page_get+0x26d/0x2d8]
(XEN) domain_page.c:216:d1 [18] mfn=20272b, 
[tmh_persistent_pool_page_get+0x26d/0x2d8]
(XEN) domain_page.c:216:d1 [19] mfn=16c22c, 
[tmh_persistent_pool_page_get+0x26d/0x2d8]
(XEN) domain_page.c:216:d1 [20] mfn=18fef4, [EIP=0]
(XEN) domain_page.c:216:d1 [21] mfn=18e7e9, [EIP=0]
(XEN) domain_page.c:216:d1 [22] mfn=18feee, [EIP=0]
(XEN) domain_page.c:216:d1 [23] mfn=1eb4a3, 
[tmh_persistent_pool_page_get+0x26d/0x2d8]
(XEN) domain_page.c:216:d1 [24] mfn=18fef3, [EIP=0]
(XEN) domain_page.c:214:d1 [25] mfn=0x18f62f idx=25, mfn=0x18f62f, 
refcnt: 0 [EIP=0]
(XEN) domain_page.c:216:d1 [26] mfn=18ff02, [__get_page_type+0x1001/0x146a]
(XEN) domain_page.c:214:d1 [27] mfn=0x18fefe idx=27, mfn=0x18fefe, 
refcnt: 0 [EIP=0]
(XEN) domain_page.c:216:d1 [28] mfn=18ff00, [__get_page_type+0xcc3/0x146a]
(XEN) domain_page.c:216:d1 [29] mfn=0, [EIP=0]
(XEN) domain_page.c:214:d1 [30] mfn=0x18f628 idx=30, mfn=0x18f628, 
refcnt: 0 [EIP=0]
(XEN) domain_page.c:216:d1 [31] mfn=1eb4ed, 
[tmh_persistent_pool_page_get+0x26d/0x2d8]

And a brief look at the code it looks as any calls to the xmalloc_pool 
code ends up
calling map_domain_page. Since most of the tmem code is using the pool 
to store
guest pages (looking briefly at tmem_malloc), this would explain why we 
ran out of
32 slots. Especially as we don't free them until the guest puts the 
persistent pages back.

The fix.. well, not yet here but I think it would be mostly 
concentrating around
tmem code.

Thanks for suggestion on looking at the accum value.

[-- Attachment #2: xen-domain_page-v4.patch --]
[-- Type: text/x-patch, Size: 6936 bytes --]

diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
index efda6af..d297037 100644
--- a/xen/arch/x86/domain_page.c
+++ b/xen/arch/x86/domain_page.c
@@ -14,6 +14,7 @@
 #include <asm/current.h>
 #include <asm/flushtlb.h>
 #include <asm/hardirq.h>
+#include <xen/symbols.h>
 
 static struct vcpu *__read_mostly override;
 
@@ -59,12 +60,12 @@ void __init mapcache_override_current(struct vcpu *v)
 void *map_domain_page(unsigned long mfn)
 {
     unsigned long flags;
-    unsigned int idx, i;
+    unsigned int idx, i, j = 0;
     struct vcpu *v;
     struct mapcache_domain *dcache;
     struct mapcache_vcpu *vcache;
     struct vcpu_maphash_entry *hashent;
-
+    int branch = 0;
 #ifdef NDEBUG
     if ( mfn <= PFN_DOWN(__pa(HYPERVISOR_VIRT_END - 1)) )
         return mfn_to_virt(mfn);
@@ -115,31 +116,102 @@ void *map_domain_page(unsigned long mfn)
         /* /First/, clean the garbage map and update the inuse list. */
         for ( i = 0; i < BITS_TO_LONGS(dcache->entries); i++ )
         {
+            unsigned long garbage = dcache->garbage[i];
+            unsigned long _inuse = dcache->inuse[i];
+            barrier();
             dcache->inuse[i] &= ~xchg(&dcache->garbage[i], 0);
+            if (v->domain->domain_id) {
+                if (~dcache->inuse[i]) {
+                    gdprintk(XENLOG_INFO, "mfn: %lx, [%d]: %lx->%lx, garbage: %lx -> ~%lx, idx: %d,  ~garbage: %lx \n", mfn, i, _inuse, dcache->inuse[i], garbage, ~dcache->inuse[i],
+                             find_first_zero_bit(dcache->inuse, dcache->entries),~garbage);
+                    branch |= 8;
+                }
+            }
             accum |= ~dcache->inuse[i];
         }
 
-        if ( accum )
+        if ( accum ) {
             idx = find_first_zero_bit(dcache->inuse, dcache->entries);
+            branch |= 1;
+        }
         else
         {
+            branch |= 2;
             /* Replace a hash entry instead. */
             i = MAPHASH_HASHFN(mfn);
             do {
                 hashent = &vcache->hash[i];
                 if ( hashent->idx != MAPHASHENT_NOTINUSE && !hashent->refcnt )
                 {
+                    branch |= 4;
                     idx = hashent->idx;
                     ASSERT(l1e_get_pfn(MAPCACHE_L1ENT(idx)) == hashent->mfn);
                     l1e_write(&MAPCACHE_L1ENT(idx), l1e_empty());
                     hashent->idx = MAPHASHENT_NOTINUSE;
                     hashent->mfn = ~0UL;
+                    if (idx >= dcache->entries) {
+                        branch |= 8;
+                        gdprintk(XENLOG_INFO, "mfn (%lx) -> %ld idx (iter:%d)\n", mfn,  MAPHASH_HASHFN(mfn), j);
+
+                        for (i = 0; i < MAPHASH_ENTRIES;i++) {
+                            hashent = &vcache->hash[i];
+
+                            gdprintk(XENLOG_INFO, "[%d] idx=%d, mfn=0x%lx, refcnt: %d\n",
+                                    i, hashent->idx, hashent->mfn, hashent->refcnt);
+                        }
+                    }
                     break;
                 }
                 if ( ++i == MAPHASH_ENTRIES )
                     i = 0;
+                j++;
             } while ( i != MAPHASH_HASHFN(mfn) );
         }
+        if (idx >= dcache->entries) {
+            unsigned long _mfn;
+            const char *name;
+            unsigned long offset, size;
+
+            static char namebuf[KSYM_NAME_LEN+1];
+            #define BUFFER_SIZE sizeof("%s+%#lx/%#lx [%s]") + KSYM_NAME_LEN + \
+			2*(BITS_PER_LONG*3/10) + 1
+            static char buffer[BUFFER_SIZE];
+
+
+
+           gdprintk(XENLOG_INFO, "mfn (%lx) -> %ld idx: %d(i:%d,j:%d), branch:%x 0x%lx\n", mfn,  MAPHASH_HASHFN(mfn), idx,  i, j, branch, accum);
+
+           for (i = 0; i < dcache->entries; i++) {
+             int in_mapcache = 0;
+            _mfn = l1e_get_pfn(MAPCACHE_L1ENT(i));
+             hashent = NULL;
+
+             for (j = 0; j < MAPHASH_ENTRIES;j++) {
+                    hashent = &vcache->hash[j];
+
+                    if (hashent->mfn == _mfn) {
+                            in_mapcache = 1;
+                            break;
+                    }
+             }
+            if (i <= 32 && dcache->ips[i]) {
+                    name = symbols_lookup(dcache->ips[i], &size, &offset, namebuf);
+
+                    if (!name)
+                        snprintf(buffer, BUFFER_SIZE, "0x%lx", dcache->ips[i]);
+                    else
+                        snprintf(buffer, BUFFER_SIZE, "%s+%#lx/%#lx", name, offset, size);
+            } else
+                  snprintf(buffer, BUFFER_SIZE, "EIP=0");
+
+             if (in_mapcache)
+                    gdprintk(XENLOG_INFO, "[%d] mfn=0x%lx idx=%d, mfn=0x%lx, refcnt: %d [%s]\n",
+                                    i, _mfn, hashent->idx, hashent->mfn, hashent->refcnt, buffer);
+             else
+                    gdprintk(XENLOG_INFO, "[%d] mfn=%lx, [%s]\n", i, _mfn, buffer);
+           }
+
+        }
         BUG_ON(idx >= dcache->entries);
 
         /* /Second/, flush TLBs. */
@@ -152,6 +224,9 @@ void *map_domain_page(unsigned long mfn)
     set_bit(idx, dcache->inuse);
     dcache->cursor = idx + 1;
 
+    if (v->domain->domain_id && idx <= 32)
+        dcache->ips[idx] = (unsigned long)__builtin_return_address(0);
+
     spin_unlock(&dcache->lock);
 
     l1e_write(&MAPCACHE_L1ENT(idx), l1e_from_pfn(mfn, __PAGE_HYPERVISOR));
@@ -215,7 +290,8 @@ void unmap_domain_page(const void *ptr)
         /* /Second/, mark as garbage. */
         set_bit(idx, dcache->garbage);
     }
-
+    if (v->domain->domain_id && idx <= 32)
+        dcache->ips[idx] = 0;
     local_irq_restore(flags);
 }
 
@@ -254,6 +330,7 @@ int mapcache_domain_init(struct domain *d)
                  2 * PFN_UP(BITS_TO_LONGS(MAPCACHE_ENTRIES) * sizeof(long))) >
                  MAPCACHE_VIRT_START + (PERDOMAIN_SLOT_MBYTES << 20));
     bitmap_pages = PFN_UP(BITS_TO_LONGS(MAPCACHE_ENTRIES) * sizeof(long));
+    gdprintk(XENLOG_INFO, "domain bitmap pages: %d\n", bitmap_pages);
     dcache->inuse = (void *)MAPCACHE_VIRT_END + PAGE_SIZE;
     dcache->garbage = dcache->inuse +
                       (bitmap_pages + 1) * PAGE_SIZE / sizeof(long);
@@ -276,6 +353,7 @@ int mapcache_vcpu_init(struct vcpu *v)
     if ( is_hvm_vcpu(v) || !dcache->inuse )
         return 0;
 
+    gdprintk(XENLOG_INFO, "ents: %d, entries: %d\n", ents, dcache->entries);
     if ( ents > dcache->entries )
     {
         /* Populate page tables. */
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index d79464d..5389ea9 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -67,6 +67,7 @@ struct mapcache_domain {
     /* Which mappings are in use, and which are garbage to reap next epoch? */
     unsigned long *inuse;
     unsigned long *garbage;
+    unsigned long ips[33];
 };
 
 int mapcache_domain_init(struct domain *);

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

* Re: Xen 4.3 + tmem =  Xen BUG at domain_page.c:143
  2013-06-11 21:06         ` konrad wilk
@ 2013-06-12  6:38           ` Jan Beulich
  0 siblings, 0 replies; 28+ messages in thread
From: Jan Beulich @ 2013-06-12  6:38 UTC (permalink / raw)
  To: konrad wilk; +Cc: xen-devel

>>> On 11.06.13 at 23:06, konrad wilk <konrad.wilk@oracle.com> wrote:
> On 6/11/2013 2:52 PM, konrad wilk wrote:
>> And that does point the picture that we have exhausted the full 32 
>> entries of mapcache.
>>
>> Now off to find out who is holding them and why. Aren't these 
>> operations (map/unmap domain_page) suppose to be shortlived?

Yes, they are.

> And found the culprit. With some EIP logging:
> [...]
> (XEN) domain_page.c:216:d1 [31] mfn=1eb4ed, 
> [tmh_persistent_pool_page_get+0x26d/0x2d8]
> 
> And a brief look at the code it looks as any calls to the xmalloc_pool 
> code ends up
> calling map_domain_page. Since most of the tmem code is using the pool 
> to store
> guest pages (looking briefly at tmem_malloc), this would explain why we 
> ran out of
> 32 slots. Especially as we don't free them until the guest puts the 
> persistent pages back.

Yes, this is (and never was) a valid use model for
map_domain_page(). What's really odd is the respective difference
between tmh_mempool_page_get() (using page_to_virt() on the
result of tmh_alloc_page()) and tmh_persistent_pool_page_get()
(using __map_domain_page() on what _tmh_alloc_page_thispool()
returned), while both allocation functions end up calling
alloc_domheap_page(). With Dan no longer around, it may be
hard to understand the reasons behind this brokenness.

As tmem gets disabled anyway when there is memory not covered
by the 1:1 mapping, switching tmh_persistent_pool_page_get() to
use page_to_virt() would appear to be the obvious immediate
solution. Re-enabling it on such huge memory systems is going to
require a re-design anyway afaict.

Jan

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-06-11 18:52       ` konrad wilk
  2013-06-11 21:06         ` konrad wilk
@ 2013-06-12 11:00         ` George Dunlap
  2013-06-12 11:15           ` Processed: " xen
                             ` (2 more replies)
  1 sibling, 3 replies; 28+ messages in thread
From: George Dunlap @ 2013-06-12 11:00 UTC (permalink / raw)
  To: konrad wilk; +Cc: Jan Beulich, xen-devel

create ^
title it map_domain_page second-stage emergency fallback path never taken
thanks

On Tue, Jun 11, 2013 at 7:52 PM, konrad wilk <konrad.wilk@oracle.com> wrote:
>
>> The BUG_ON() here is definitely valid - a few lines down, after the
>> enclosing if(), we use it in ways that requires this to not have
>> triggered. It basically tells you whether an in range idx was found,
>> which apparently isn't the case here.
>>
>> As I think George already pointed out - printing accum here would
>> be quite useful: It should have at least one of the low 32 bits set,
>> given that dcache->entries must be at most 32 according to the
>> data you already got logged.
>
>
> With extra debugging (see attached patch)
>
> (XEN) domain_page.c:125:d1 mfn: 1eb483, [0]: bffff1ff, ~ffffffff40000e00,
> idx: 9 garbage: 40000e00, inuse: ffffffff
> (XEN) domain_page.c:125:d1 mfn: 1eb480, [0]: fdbfffff, ~ffffffff02400000,
> idx: 22 garbage: 2400000, inuse: ffffffff
> (XEN) domain_page.c:125:d1 mfn: 2067ca, [0]: fffff7ff, ~ffffffff00000800,
> idx: 11 garbage: 800, inuse: ffffffff
> (XEN) domain_page.c:125:d1 mfn: 183642, [0]: ffffffff, ~ffffffff00000000,
> idx: 32 garbage: 0, inuse: ffffffff

So regardless of the fact that tmem is obviously holding what are
supposed to be short-term references for so long, there is something
that seems not quite right about this failure path.

It looks like the algorithm is:
1. Clean the garbage map and update the inuse list
2. If anything has been cleaned up, use the first not-inuse entry
3. Otherwise, do something else ("replace a hash entry" -- not sure
exactly what that means).

What we see above is that this failure path succeeds three times, but
fails the fourth time: there are, in fact, no zero entries after the
garbage clean-up; however, because "inuse" is 32-bit (effectively) and
"accum" is 64-bit, ~inuse always has bits 32-63 set, and so will
always return true and never fall back to the "something else"

This is probably not something we need to fix for 4.3, but we should
put it on our to-do list.

 -George

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

* Processed: Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-06-12 11:00         ` George Dunlap
@ 2013-06-12 11:15           ` xen
  2013-06-12 11:37           ` George Dunlap
  2013-06-12 12:12           ` Jan Beulich
  2 siblings, 0 replies; 28+ messages in thread
From: xen @ 2013-06-12 11:15 UTC (permalink / raw)
  To: George Dunlap, xen-devel

Processing commands for xen@bugs.xenproject.org:

> create ^
Created new bug #14 rooted at `<51B7720B.10607@oracle.com>'
Title: `Re: [Xen-devel] Xen 4.3 + tmem = Xen BUG at domain_page.c:143'
> title it map_domain_page second-stage emergency fallback path never taken
Set title for #14 to `map_domain_page second-stage emergency fallback path never taken'
> thanks
Finished processing.

Modified/created Bugs:
 - 14: http://bugs.xenproject.org/xen/bug/14 (new)

---
Xen Hypervisor Bug Tracker
See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on reporting bugs
Contact xen-bugs-owner@bugs.xenproject.org with any infrastructure issues

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-06-12 11:00         ` George Dunlap
  2013-06-12 11:15           ` Processed: " xen
@ 2013-06-12 11:37           ` George Dunlap
  2013-06-12 12:46             ` Jan Beulich
  2013-06-12 14:13             ` Konrad Rzeszutek Wilk
  2013-06-12 12:12           ` Jan Beulich
  2 siblings, 2 replies; 28+ messages in thread
From: George Dunlap @ 2013-06-12 11:37 UTC (permalink / raw)
  To: konrad wilk; +Cc: Jan Beulich, xen-devel

[-- Attachment #1: Type: text/plain, Size: 332 bytes --]

On Wed, Jun 12, 2013 at 12:00 PM, George Dunlap
<George.Dunlap@eu.citrix.com> wrote:
> This is probably not something we need to fix for 4.3, but we should
> put it on our to-do list.

Konrad,

Could you try the attached patch with your debug patch, to see if it
successfully falls back to the "replace a map hash" patch?

 -George

[-- Attachment #2: fix-map_domain_page-fallback.diff --]
[-- Type: application/octet-stream, Size: 2110 bytes --]

commit 4f24632a52bba54487e2a5ce53144b55cd37bd56
Author: George Dunlap <george.dunlap@eu.citrix.com>
Commit: George Dunlap <george.dunlap@eu.citrix.com>

    xen: Fix map_domain_page check for garbage collection
    
    When map_domain_page() finds that it has reached the maximum number of
    entries, it is supposed to first do garbage collection, and if that
    fails, try to replace a hash entry.
    
    However, a bug in the code that checks to see if something was
    successfully cleaned made it think that it has always cleaned
    something, and so it never fell back to replacing a hash entry.  The
    problem is that if the number of valid "inuse" bits doesn't fill up
    the whole word, then the empty bits at the top will always be zero,
    which will set the accumulator to a non-zero value.
    
    This patch caculates how many valid "inuse" bits are left, and masks
    out invalid bits.
    
    Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
    CC: Jan Beulich <JBeulich@suse.com>
    CC: Konrad Wilk <konrad.wilk@oracle.com>

diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
index efda6af..bebeff8 100644
--- a/xen/arch/x86/domain_page.c
+++ b/xen/arch/x86/domain_page.c
@@ -110,13 +110,19 @@ void *map_domain_page(unsigned long mfn)
     idx = find_next_zero_bit(dcache->inuse, dcache->entries, dcache->cursor);
     if ( unlikely(idx >= dcache->entries) )
     {
-        unsigned long accum = 0;
+        unsigned long accum = 0, mask;
+        int entries_rem;
 
+        entries_rem = dcache->entries;
         /* /First/, clean the garbage map and update the inuse list. */
         for ( i = 0; i < BITS_TO_LONGS(dcache->entries); i++ )
         {
+            ASSERT(entries_rem>0);
             dcache->inuse[i] &= ~xchg(&dcache->garbage[i], 0);
-            accum |= ~dcache->inuse[i];
+            /* Only accumulate valid "inuse" bits */
+            mask = (entries_rem >= 64) ? ~0UL: ((1<<entries_rem)-1);
+            accum |= (~dcache->inuse[i]) & mask;
+            entries_rem -= BITS_PER_LONG;
         }
 
         if ( accum )

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-06-12 11:00         ` George Dunlap
  2013-06-12 11:15           ` Processed: " xen
  2013-06-12 11:37           ` George Dunlap
@ 2013-06-12 12:12           ` Jan Beulich
  2013-06-12 13:16             ` George Dunlap
  2013-06-12 15:11             ` Keir Fraser
  2 siblings, 2 replies; 28+ messages in thread
From: Jan Beulich @ 2013-06-12 12:12 UTC (permalink / raw)
  To: George Dunlap, konrad wilk; +Cc: xen-devel

>>> On 12.06.13 at 13:00, George Dunlap <George.Dunlap@eu.citrix.com> wrote:
> create ^
> title it map_domain_page second-stage emergency fallback path never taken
> thanks
> 
> On Tue, Jun 11, 2013 at 7:52 PM, konrad wilk <konrad.wilk@oracle.com> wrote:
>>
>>> The BUG_ON() here is definitely valid - a few lines down, after the
>>> enclosing if(), we use it in ways that requires this to not have
>>> triggered. It basically tells you whether an in range idx was found,
>>> which apparently isn't the case here.
>>>
>>> As I think George already pointed out - printing accum here would
>>> be quite useful: It should have at least one of the low 32 bits set,
>>> given that dcache->entries must be at most 32 according to the
>>> data you already got logged.
>>
>>
>> With extra debugging (see attached patch)
>>
>> (XEN) domain_page.c:125:d1 mfn: 1eb483, [0]: bffff1ff, ~ffffffff40000e00,
>> idx: 9 garbage: 40000e00, inuse: ffffffff
>> (XEN) domain_page.c:125:d1 mfn: 1eb480, [0]: fdbfffff, ~ffffffff02400000,
>> idx: 22 garbage: 2400000, inuse: ffffffff
>> (XEN) domain_page.c:125:d1 mfn: 2067ca, [0]: fffff7ff, ~ffffffff00000800,
>> idx: 11 garbage: 800, inuse: ffffffff
>> (XEN) domain_page.c:125:d1 mfn: 183642, [0]: ffffffff, ~ffffffff00000000,
>> idx: 32 garbage: 0, inuse: ffffffff
> 
> So regardless of the fact that tmem is obviously holding what are
> supposed to be short-term references for so long, there is something
> that seems not quite right about this failure path.
> 
> It looks like the algorithm is:
> 1. Clean the garbage map and update the inuse list
> 2. If anything has been cleaned up, use the first not-inuse entry
> 3. Otherwise, do something else ("replace a hash entry" -- not sure
> exactly what that means).
> 
> What we see above is that this failure path succeeds three times, but
> fails the fourth time: there are, in fact, no zero entries after the
> garbage clean-up; however, because "inuse" is 32-bit (effectively) and
> "accum" is 64-bit, ~inuse always has bits 32-63 set, and so will
> always return true and never fall back to the "something else"

Right, that's what occurred to me too yesterday, but the again
I knew I had seen this code path executed. Now that I look again,
I think I understand why: All of my Dom0-s and typical DomU-s
have a vCPU count divisible by 4, and with MAPCACHE_VCPU_ENTRIES
being 16, the full unsigned long would always be used.

> This is probably not something we need to fix for 4.3, but we should
> put it on our to-do list.

Actually I think we should fix this right away.

Jan

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-06-12 11:37           ` George Dunlap
@ 2013-06-12 12:46             ` Jan Beulich
  2013-06-12 14:13             ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 28+ messages in thread
From: Jan Beulich @ 2013-06-12 12:46 UTC (permalink / raw)
  To: George Dunlap, konrad wilk; +Cc: xen-devel

>>> On 12.06.13 at 13:37, George Dunlap <George.Dunlap@eu.citrix.com> wrote:
> On Wed, Jun 12, 2013 at 12:00 PM, George Dunlap
> <George.Dunlap@eu.citrix.com> wrote:
>> This is probably not something we need to fix for 4.3, but we should
>> put it on our to-do list.
> 
> Could you try the attached patch with your debug patch, to see if it
> successfully falls back to the "replace a map hash" patch?

This could be a fix, but I'd prefer to do this with a change to an
entirely cold code path, i.e. by setting the unused bits in the last
word in mapcache_vcpu_init().

Jan

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-06-12 12:12           ` Jan Beulich
@ 2013-06-12 13:16             ` George Dunlap
  2013-06-12 13:27               ` Jan Beulich
  2013-06-12 15:11             ` Keir Fraser
  1 sibling, 1 reply; 28+ messages in thread
From: George Dunlap @ 2013-06-12 13:16 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, konrad wilk

On 12/06/13 13:12, Jan Beulich wrote:
>>>> On 12.06.13 at 13:00, George Dunlap <George.Dunlap@eu.citrix.com> wrote:
>> create ^
>> title it map_domain_page second-stage emergency fallback path never taken
>> thanks
>>
>> On Tue, Jun 11, 2013 at 7:52 PM, konrad wilk <konrad.wilk@oracle.com> wrote:
>>>> The BUG_ON() here is definitely valid - a few lines down, after the
>>>> enclosing if(), we use it in ways that requires this to not have
>>>> triggered. It basically tells you whether an in range idx was found,
>>>> which apparently isn't the case here.
>>>>
>>>> As I think George already pointed out - printing accum here would
>>>> be quite useful: It should have at least one of the low 32 bits set,
>>>> given that dcache->entries must be at most 32 according to the
>>>> data you already got logged.
>>>
>>> With extra debugging (see attached patch)
>>>
>>> (XEN) domain_page.c:125:d1 mfn: 1eb483, [0]: bffff1ff, ~ffffffff40000e00,
>>> idx: 9 garbage: 40000e00, inuse: ffffffff
>>> (XEN) domain_page.c:125:d1 mfn: 1eb480, [0]: fdbfffff, ~ffffffff02400000,
>>> idx: 22 garbage: 2400000, inuse: ffffffff
>>> (XEN) domain_page.c:125:d1 mfn: 2067ca, [0]: fffff7ff, ~ffffffff00000800,
>>> idx: 11 garbage: 800, inuse: ffffffff
>>> (XEN) domain_page.c:125:d1 mfn: 183642, [0]: ffffffff, ~ffffffff00000000,
>>> idx: 32 garbage: 0, inuse: ffffffff
>> So regardless of the fact that tmem is obviously holding what are
>> supposed to be short-term references for so long, there is something
>> that seems not quite right about this failure path.
>>
>> It looks like the algorithm is:
>> 1. Clean the garbage map and update the inuse list
>> 2. If anything has been cleaned up, use the first not-inuse entry
>> 3. Otherwise, do something else ("replace a hash entry" -- not sure
>> exactly what that means).
>>
>> What we see above is that this failure path succeeds three times, but
>> fails the fourth time: there are, in fact, no zero entries after the
>> garbage clean-up; however, because "inuse" is 32-bit (effectively) and
>> "accum" is 64-bit, ~inuse always has bits 32-63 set, and so will
>> always return true and never fall back to the "something else"
> Right, that's what occurred to me too yesterday, but the again
> I knew I had seen this code path executed. Now that I look again,
> I think I understand why: All of my Dom0-s and typical DomU-s
> have a vCPU count divisible by 4, and with MAPCACHE_VCPU_ENTRIES
> being 16, the full unsigned long would always be used.
>
>> This is probably not something we need to fix for 4.3, but we should
>> put it on our to-do list.
> Actually I think we should fix this right away.

How often is the second path taken in practice?

And, you said this doesn't happen with debug=n builds -- why not exactly?

I'm trying to assess the actual risk of not fixing it, vs the risk of 
fixing it.

  -George

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-06-12 13:16             ` George Dunlap
@ 2013-06-12 13:27               ` Jan Beulich
  0 siblings, 0 replies; 28+ messages in thread
From: Jan Beulich @ 2013-06-12 13:27 UTC (permalink / raw)
  To: George Dunlap; +Cc: konrad wilk, xen-devel

>>> On 12.06.13 at 15:16, George Dunlap <george.dunlap@eu.citrix.com> wrote:
> On 12/06/13 13:12, Jan Beulich wrote:
>>>>> On 12.06.13 at 13:00, George Dunlap <George.Dunlap@eu.citrix.com> wrote:
>>> create ^
>>> title it map_domain_page second-stage emergency fallback path never taken
>>> thanks
>>>
>>> On Tue, Jun 11, 2013 at 7:52 PM, konrad wilk <konrad.wilk@oracle.com> wrote:
>>>>> The BUG_ON() here is definitely valid - a few lines down, after the
>>>>> enclosing if(), we use it in ways that requires this to not have
>>>>> triggered. It basically tells you whether an in range idx was found,
>>>>> which apparently isn't the case here.
>>>>>
>>>>> As I think George already pointed out - printing accum here would
>>>>> be quite useful: It should have at least one of the low 32 bits set,
>>>>> given that dcache->entries must be at most 32 according to the
>>>>> data you already got logged.
>>>>
>>>> With extra debugging (see attached patch)
>>>>
>>>> (XEN) domain_page.c:125:d1 mfn: 1eb483, [0]: bffff1ff, ~ffffffff40000e00,
>>>> idx: 9 garbage: 40000e00, inuse: ffffffff
>>>> (XEN) domain_page.c:125:d1 mfn: 1eb480, [0]: fdbfffff, ~ffffffff02400000,
>>>> idx: 22 garbage: 2400000, inuse: ffffffff
>>>> (XEN) domain_page.c:125:d1 mfn: 2067ca, [0]: fffff7ff, ~ffffffff00000800,
>>>> idx: 11 garbage: 800, inuse: ffffffff
>>>> (XEN) domain_page.c:125:d1 mfn: 183642, [0]: ffffffff, ~ffffffff00000000,
>>>> idx: 32 garbage: 0, inuse: ffffffff
>>> So regardless of the fact that tmem is obviously holding what are
>>> supposed to be short-term references for so long, there is something
>>> that seems not quite right about this failure path.
>>>
>>> It looks like the algorithm is:
>>> 1. Clean the garbage map and update the inuse list
>>> 2. If anything has been cleaned up, use the first not-inuse entry
>>> 3. Otherwise, do something else ("replace a hash entry" -- not sure
>>> exactly what that means).
>>>
>>> What we see above is that this failure path succeeds three times, but
>>> fails the fourth time: there are, in fact, no zero entries after the
>>> garbage clean-up; however, because "inuse" is 32-bit (effectively) and
>>> "accum" is 64-bit, ~inuse always has bits 32-63 set, and so will
>>> always return true and never fall back to the "something else"
>> Right, that's what occurred to me too yesterday, but the again
>> I knew I had seen this code path executed. Now that I look again,
>> I think I understand why: All of my Dom0-s and typical DomU-s
>> have a vCPU count divisible by 4, and with MAPCACHE_VCPU_ENTRIES
>> being 16, the full unsigned long would always be used.
>>
>>> This is probably not something we need to fix for 4.3, but we should
>>> put it on our to-do list.
>> Actually I think we should fix this right away.
> 
> How often is the second path taken in practice?

On non-debug builds, not at all except on systems with more than
5Tb (as all of the map_domain_page() code).

Once domain page mappings are needed, this depends on the use
pattern of the function. In any case this is going to be way more
frequent than the one time per-vCPU setup.

> And, you said this doesn't happen with debug=n builds -- why not exactly?

That was with the user (tmem) in mind: For <= 5Tb systems, as
said above map_domain_page() has a short cut. And for >5Tb
systems tmem gets turned off.

But any other users of the function could still run into this on
huge memory systems, and with this being one of the listed new
features of 4.3 I think we should fix it.

Jan

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-06-12 11:37           ` George Dunlap
  2013-06-12 12:46             ` Jan Beulich
@ 2013-06-12 14:13             ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 28+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-06-12 14:13 UTC (permalink / raw)
  To: George Dunlap; +Cc: Jan Beulich, xen-devel

[-- Attachment #1: Type: text/plain, Size: 7132 bytes --]

On Wed, Jun 12, 2013 at 12:37:53PM +0100, George Dunlap wrote:
> On Wed, Jun 12, 2013 at 12:00 PM, George Dunlap
> <George.Dunlap@eu.citrix.com> wrote:
> > This is probably not something we need to fix for 4.3, but we should
> > put it on our to-do list.
> 
> Konrad,
> 
> Could you try the attached patch with your debug patch, to see if it
> successfully falls back to the "replace a map hash" patch?

It does, but I believe it falls in the BUG_ON scenario where
the idx = 32 and the BUG_ON gets hit. Please see the serial log:

(attached is the debug patch + your patch)

(XEN) domain_page.c:137:d1 mfn: 1eb533, [0]: ff000000->ff000000, garbage: 0 -> ~0 (mask: 0), idx: 0,  ~garbage: ffffffffffffffff 
(XEN) domain_page.c:137:d1 mfn: 1eb864, [0]: ff000000->ff000000, garbage: 0 -> ~0 (mask: 0), idx: 0,  ~garbage: ffffffffffffffff 
(XEN) domain_page.c:137:d1 mfn: 1eb862, [0]: ff000000->ff000000, garbage: 0 -> ~0 (mask: 0), idx: 0,  ~garbage: ffffffffffffffff 
(XEN) domain_page.c:137:d1 mfn: 1eb531, [0]: ff000000->ff000000, garbage: 0 -> ~0 (mask: 0), idx: 0,  ~garbage: ffffffffffffffff 
(XEN) domain_page.c:137:d1 mfn: 1ebd14, [0]: ff000000->ff000000, garbage: 0 -> ~0 (mask: 0), idx: 0,  ~garbage: ffffffffffffffff 
(XEN) domain_page.c:137:d1 mfn: 18e8bb, [0]: ff000000->ff000000, garbage: 0 -> ~0 (mask: 0), idx: 0,  ~garbage: ffffffffffffffff 
(XEN) domain_page.c:137:d1 mfn: 2062e2, [0]: ff000000->ff000000, garbage: 0 -> ~0 (mask: 0), idx: 0,  ~garbage: ffffffffffffffff 
(XEN) domain_page.c:137:d1 mfn: 1eb525, [0]: ff000000->ff000000, garbage: 0 -> ~0 (mask: 0), idx: 0,  ~garbage: ffffffffffffffff 
(XEN) domain_page.c:137:d1 mfn: 20653e, [0]: ff000000->ff000000, garbage: 0 -> ~0 (mask: 0), idx: 0,  ~garbage: ffffffffffffffff 
(XEN) domain_page.c:137:d1 mfn: 1ed3a6, [0]: ff000000->ff000000, garbage: 0 -> ~0 (mask: 0), idx: 0,  ~garbage: ffffffffffffffff 

(XEN) domain_page.c:137:d1 mfn: 1ed3a6, [0]: ff000000->7f000000, garbage: 80000000 -> ~0 (mask: 0), idx: 0,  ~garbage: ffffffff7fffffff 
(XEN) domain_page.c:137:d1 mfn: 1eb525, [0]: ff000000->df000000, garbage: 20000000 -> ~0 (mask: 0), idx: 0,  ~garbage: ffffffffdfffffff 
(XEN) domain_page.c:137:d1 mfn: 1ed3a4, [0]: df000000->df000000, garbage: 0 -> ~0 (mask: 0), idx: 0,  ~garbage: ffffffffffffffff 
 

BusyBox v1.14.3 (2013-06-10 16:30:12 EDT) built-in shell (ash)
Enter 'help' for a list of built-in commands.

# 
(XEN) domain_page.c:137:d1 mfn: 18e8c8, [0]: ff000000->7f000000, garbage: 80000000 -> ~0 (mask: 0), idx: 0,  ~garbage: ffffffff7fffffff 

(XEN) domain_page.c:137:d1 mfn: 1eb531, [0]: ff000000->ef000000, garbage: 10000000 -> ~0 (mask: 0), idx: 0,  ~garbage: ffffffffeb531) -> 1 idx: 32(i:1,j:8), branch:a accum: 0x0
(XEN) domain_page.c:222:d1 [0] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [1] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [2] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [3] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [4] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [5] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [6] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [7] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [8] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [9] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [10] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [11] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [12] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [13] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [14] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [15] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [16] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [17] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [18] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [19] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [20] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [21] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [22] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [23] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [24] mfn=1eb531, [EIP=0]
(XEN) domain_page.c:222:d1 [25] mfn=2062e2, [EIP=0]
(XEN) domain_page.c:222:d1 [26] mfn=18e8bb, [EIP=0]
(XEN) domain_page.c:222:d1 [27] mfn=18e8c8, [EIP=0]
(XEN) domain_page.c:222:d1 [28] mfn=0, [EIP=0]
(XEN) domain_page.c:222:d1 [29] mfn=1ed3a5, [EIP=0]
(XEN) domain_page.c:222:d1 [30] mfn=1ed3a6, [EIP=0]
(XEN) domain_page.c:222:d1 [31] mfn=1ebc14, [EIP=0]
(XEN) Xen BUG at domain_page.c:226
(XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Tainted:    C ]----
(XEN) CPU:    0
(XEN) RIP:    e008:[<ffff82c4c01609bf>] map_domain_page+0x920/0xa03
(XEN) RFLAGS: 0000000000010046   CONTEXT: hypervisor
(XEN) rax: 0000000000000000   rbx: ffff8300c68f8000   rcx: 0000000000000000
(XEN) rdx: ffff82c4c03191a0   rsi: 000000000000000a   rdi: ffff82c4c027a6e8
(XEN) rbp: ffff82c4c02c7e58   rsp: ffff82c4c02c7d78   r8:  0000000000000004
(XEN) r9:  0000000000000004   r10: 0000000000000004   r11: 0000000000000001
(XEN) r12: ffff83022d7b7000   r13: 00000000001eb531   r14: 0000000000000020
(XEN) r15: 0000000000000020   cr0: 000000008005003b   cr4: 00000000000426f0
(XEN) cr3: 000000020653e000   cr2: 00007f808ece33e0
(XEN) ds: 002b   es: 002b   fs: 0000   gs: 0000   ss: e010   cs: e008
(XEN) Xen stack trace from rsp=ffff82c4c02c7d78:
(XEN)    ffff82c4c02f0de0 0000000000000001 0000000000000008 000000000000000a
(XEN)    0000000000000000 ffff830000000000 ffffffffefffffff 0000000000000000
(XEN)    0000000000000000 0000002000000000 00000000ff000000 00000000001eb531
(XEN)    ffff82c4c02c7e20 0000000000000020 ffff82c4c02c7e18 ffff8300c68f83c8
(XEN)    0000000ac02c7f08 ffff82c400000000 ffff83022d7b72d8 0000000000000286
(XEN)    ffff82c4c03191c0 0000000000000002 0000000900000000 ffff83022d7b7000
(XEN)    ffff83022d7b7000 ffff8300c68f8000 0000000000000000 ffff88003f904000
(XEN)    ffff82c4c02c7ee8 ffff82c4c017a242 ffffffff81039c19 00000000000426f0
(XEN)    80100001ebc14065 ffff82c4c02c0000 ffffffffffffffff ffff82c4c02c7ed0
(XEN)    ffff82c4c0126e77 ffff82c4c02e0000 ffff82c4c02c00e7 00000000001eb531
(XEN)    ffffffffffffffff ffff8300c68f8000 0000000000000000 ffff88003e29fe40
(XEN)    ffff88003e29fee0 0000000000000080 ffff82c4c02c7ef8 ffff82c4c017a50f
(XEN)    00007d3b3fd380c7 ffff82c4c0223c4b ffffffff810011ca 000000000000000e
(XEN)    ffff88003e27f800 ffff88003f90c740 0000000000000001 ffff88003f911640
(XEN)    ffff88003e29fe30 ffff88003f904000 0000000000000286 0000000000000000
(XEN)    ffff880000000000 00003ffffffff000 000000000000000e ffffffff810011ca
(XEN)    0000000000000000 80100001ebc14065 ffff88003f904000 0001010000000000
(XEN)    ffffffff810011ca 000000000000e033 0000000000000286 ffff88003e29fdf8
(XEN)    000000000000e02b 0000000000000000 0000000000000000 0000000000000000
(XEN)    0000000000000000 0000000000000000 ffff8300c68f8000 0000000000000000
(XEN) Xen call trace:
(XEN)    [<ffff82c4c01609bf>] map_domain_page+0x920/0xa03
(XEN)    [<ffff82c4c017a242>] __do_update_va_mapping+0xd6/0x316
(XEN)    [<ffff82c4c017a50f>] do_update_va_mapping+0x1e/0x22
(XEN)    [<ffff82c4c0223c4b>] syscall_enter+0xeb/0x145
(XEN)    
(XEN) 
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Xen BUG at domain_page.c:226
(XEN) ****************************************
(XEN) 
(XEN) Manual reset required ('noreboot' specified)

[-- Attachment #2: xen-domain_page-v5.patch --]
[-- Type: text/plain, Size: 7487 bytes --]

diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
index efda6af..d6eb783 100644
--- a/xen/arch/x86/domain_page.c
+++ b/xen/arch/x86/domain_page.c
@@ -14,6 +14,7 @@
 #include <asm/current.h>
 #include <asm/flushtlb.h>
 #include <asm/hardirq.h>
+#include <xen/symbols.h>
 
 static struct vcpu *__read_mostly override;
 
@@ -59,12 +60,12 @@ void __init mapcache_override_current(struct vcpu *v)
 void *map_domain_page(unsigned long mfn)
 {
     unsigned long flags;
-    unsigned int idx, i;
+    unsigned int idx, i, j = 0;
     struct vcpu *v;
     struct mapcache_domain *dcache;
     struct mapcache_vcpu *vcache;
     struct vcpu_maphash_entry *hashent;
-
+    int branch = 0;
 #ifdef NDEBUG
     if ( mfn <= PFN_DOWN(__pa(HYPERVISOR_VIRT_END - 1)) )
         return mfn_to_virt(mfn);
@@ -110,36 +111,118 @@ void *map_domain_page(unsigned long mfn)
     idx = find_next_zero_bit(dcache->inuse, dcache->entries, dcache->cursor);
     if ( unlikely(idx >= dcache->entries) )
     {
-        unsigned long accum = 0;
+        unsigned long accum = 0, mask;
+        int entries_rem;
 
+        entries_rem = dcache->entries;
         /* /First/, clean the garbage map and update the inuse list. */
         for ( i = 0; i < BITS_TO_LONGS(dcache->entries); i++ )
         {
+            unsigned long garbage = dcache->garbage[i];
+            unsigned long _inuse = dcache->inuse[i];
+
+            ASSERT(entries_rem > 0);
+
+            barrier();
+
             dcache->inuse[i] &= ~xchg(&dcache->garbage[i], 0);
-            accum |= ~dcache->inuse[i];
+            mask = (entries_rem >= 64) ? ~0UL : ((1 << entries_rem) - 1);
+            accum |= (~dcache->inuse[i]) & mask;
+            entries_rem -= BITS_PER_LONG;
+
+            if (v->domain->domain_id) {
+                if (~dcache->inuse[i]) {
+                    gdprintk(XENLOG_INFO, "mfn: %lx, [%d]: %lx->%lx, garbage: %lx -> ~%lx (mask: %lx), idx: %d,  ~garbage: %lx \n",
+                             mfn, i, _inuse, dcache->inuse[i], garbage, accum, mask,
+                             find_first_zero_bit(dcache->inuse, dcache->entries),~garbage);
+                    branch |= 8;
+                }
+            }
         }
 
-        if ( accum )
+        if ( accum ) {
             idx = find_first_zero_bit(dcache->inuse, dcache->entries);
+            branch |= 1;
+        }
         else
         {
+            branch |= 2;
             /* Replace a hash entry instead. */
             i = MAPHASH_HASHFN(mfn);
             do {
                 hashent = &vcache->hash[i];
                 if ( hashent->idx != MAPHASHENT_NOTINUSE && !hashent->refcnt )
                 {
+                    branch |= 4;
                     idx = hashent->idx;
                     ASSERT(l1e_get_pfn(MAPCACHE_L1ENT(idx)) == hashent->mfn);
                     l1e_write(&MAPCACHE_L1ENT(idx), l1e_empty());
                     hashent->idx = MAPHASHENT_NOTINUSE;
                     hashent->mfn = ~0UL;
+                    if (idx >= dcache->entries) {
+                        branch |= 8;
+                        gdprintk(XENLOG_INFO, "mfn (%lx) -> %ld idx (iter:%d)\n", mfn,  MAPHASH_HASHFN(mfn), j);
+
+                        for (i = 0; i < MAPHASH_ENTRIES;i++) {
+                            hashent = &vcache->hash[i];
+
+                            gdprintk(XENLOG_INFO, "[%d] idx=%d, mfn=0x%lx, refcnt: %d\n",
+                                    i, hashent->idx, hashent->mfn, hashent->refcnt);
+                        }
+                    }
                     break;
                 }
                 if ( ++i == MAPHASH_ENTRIES )
                     i = 0;
+                j++;
             } while ( i != MAPHASH_HASHFN(mfn) );
         }
+        if (idx >= dcache->entries) {
+            unsigned long _mfn;
+            const char *name;
+            unsigned long offset, size;
+
+            static char namebuf[KSYM_NAME_LEN+1];
+            #define BUFFER_SIZE sizeof("%s+%#lx/%#lx [%s]") + KSYM_NAME_LEN + \
+			2*(BITS_PER_LONG*3/10) + 1
+            static char buffer[BUFFER_SIZE];
+
+
+
+           gdprintk(XENLOG_INFO, "mfn (%lx) -> %ld idx: %d(i:%d,j:%d), branch:%x accum: 0x%lx\n",
+                    mfn,  MAPHASH_HASHFN(mfn), idx,  i, j, branch, accum);
+
+           for (i = 0; i < dcache->entries; i++) {
+             int in_mapcache = 0;
+            _mfn = l1e_get_pfn(MAPCACHE_L1ENT(i));
+             hashent = NULL;
+
+             for (j = 0; j < MAPHASH_ENTRIES;j++) {
+                    hashent = &vcache->hash[j];
+
+                    if (hashent->mfn == _mfn) {
+                            in_mapcache = 1;
+                            break;
+                    }
+             }
+            if (i <= 32 && dcache->ips[i]) {
+                    name = symbols_lookup(dcache->ips[i], &size, &offset, namebuf);
+
+                    if (!name)
+                        snprintf(buffer, BUFFER_SIZE, "0x%lx", dcache->ips[i]);
+                    else
+                        snprintf(buffer, BUFFER_SIZE, "%s+%#lx/%#lx", name, offset, size);
+            } else
+                  snprintf(buffer, BUFFER_SIZE, "EIP=0");
+
+             if (in_mapcache)
+                    gdprintk(XENLOG_INFO, "[%d] mfn=0x%lx idx=%d, mfn=0x%lx, refcnt: %d [%s]\n",
+                                    i, _mfn, hashent->idx, hashent->mfn, hashent->refcnt, buffer);
+             else
+                    gdprintk(XENLOG_INFO, "[%d] mfn=%lx, [%s]\n", i, _mfn, buffer);
+           }
+
+        }
         BUG_ON(idx >= dcache->entries);
 
         /* /Second/, flush TLBs. */
@@ -152,6 +235,9 @@ void *map_domain_page(unsigned long mfn)
     set_bit(idx, dcache->inuse);
     dcache->cursor = idx + 1;
 
+    if (v->domain->domain_id && idx <= 32)
+        dcache->ips[idx] = (unsigned long)__builtin_return_address(0);
+
     spin_unlock(&dcache->lock);
 
     l1e_write(&MAPCACHE_L1ENT(idx), l1e_from_pfn(mfn, __PAGE_HYPERVISOR));
@@ -215,7 +301,8 @@ void unmap_domain_page(const void *ptr)
         /* /Second/, mark as garbage. */
         set_bit(idx, dcache->garbage);
     }
-
+    if (v->domain->domain_id && idx <= 32)
+        dcache->ips[idx] = 0;
     local_irq_restore(flags);
 }
 
@@ -254,6 +341,7 @@ int mapcache_domain_init(struct domain *d)
                  2 * PFN_UP(BITS_TO_LONGS(MAPCACHE_ENTRIES) * sizeof(long))) >
                  MAPCACHE_VIRT_START + (PERDOMAIN_SLOT_MBYTES << 20));
     bitmap_pages = PFN_UP(BITS_TO_LONGS(MAPCACHE_ENTRIES) * sizeof(long));
+    gdprintk(XENLOG_INFO, "domain bitmap pages: %d\n", bitmap_pages);
     dcache->inuse = (void *)MAPCACHE_VIRT_END + PAGE_SIZE;
     dcache->garbage = dcache->inuse +
                       (bitmap_pages + 1) * PAGE_SIZE / sizeof(long);
@@ -276,6 +364,7 @@ int mapcache_vcpu_init(struct vcpu *v)
     if ( is_hvm_vcpu(v) || !dcache->inuse )
         return 0;
 
+    gdprintk(XENLOG_INFO, "ents: %d, entries: %d\n", ents, dcache->entries);
     if ( ents > dcache->entries )
     {
         /* Populate page tables. */
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index d79464d..5389ea9 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -67,6 +67,7 @@ struct mapcache_domain {
     /* Which mappings are in use, and which are garbage to reap next epoch? */
     unsigned long *inuse;
     unsigned long *garbage;
+    unsigned long ips[33];
 };
 
 int mapcache_domain_init(struct domain *);

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-06-12 12:12           ` Jan Beulich
  2013-06-12 13:16             ` George Dunlap
@ 2013-06-12 15:11             ` Keir Fraser
  2013-06-12 15:27               ` Keir Fraser
  2013-06-12 15:48               ` Jan Beulich
  1 sibling, 2 replies; 28+ messages in thread
From: Keir Fraser @ 2013-06-12 15:11 UTC (permalink / raw)
  To: Jan Beulich, George Dunlap, konrad wilk; +Cc: xen-devel

On 12/06/2013 13:12, "Jan Beulich" <JBeulich@suse.com> wrote:

>> What we see above is that this failure path succeeds three times, but
>> fails the fourth time: there are, in fact, no zero entries after the
>> garbage clean-up; however, because "inuse" is 32-bit (effectively) and
>> "accum" is 64-bit, ~inuse always has bits 32-63 set, and so will
>> always return true and never fall back to the "something else"
> 
> Right, that's what occurred to me too yesterday, but the again
> I knew I had seen this code path executed. Now that I look again,
> I think I understand why: All of my Dom0-s and typical DomU-s
> have a vCPU count divisible by 4, and with MAPCACHE_VCPU_ENTRIES
> being 16, the full unsigned long would always be used.

Why are we so tight on MAPCACHE_VCPU_ENTRIES? Why not say double that number
and get rid of the accum and the 'replace a hash entry instead' logic
instead? We never used to have it, and it's kind of extra complication and a
bit gross.

 -- Keir

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-06-12 15:11             ` Keir Fraser
@ 2013-06-12 15:27               ` Keir Fraser
  2013-06-12 15:54                 ` Jan Beulich
  2013-06-12 15:48               ` Jan Beulich
  1 sibling, 1 reply; 28+ messages in thread
From: Keir Fraser @ 2013-06-12 15:27 UTC (permalink / raw)
  To: Jan Beulich, George Dunlap, konrad wilk; +Cc: xen-devel

On 12/06/2013 16:11, "Keir Fraser" <keir.xen@gmail.com> wrote:

> On 12/06/2013 13:12, "Jan Beulich" <JBeulich@suse.com> wrote:
> 
>>> What we see above is that this failure path succeeds three times, but
>>> fails the fourth time: there are, in fact, no zero entries after the
>>> garbage clean-up; however, because "inuse" is 32-bit (effectively) and
>>> "accum" is 64-bit, ~inuse always has bits 32-63 set, and so will
>>> always return true and never fall back to the "something else"
>> 
>> Right, that's what occurred to me too yesterday, but the again
>> I knew I had seen this code path executed. Now that I look again,
>> I think I understand why: All of my Dom0-s and typical DomU-s
>> have a vCPU count divisible by 4, and with MAPCACHE_VCPU_ENTRIES
>> being 16, the full unsigned long would always be used.
> 
> Why are we so tight on MAPCACHE_VCPU_ENTRIES? Why not say double that number
> and get rid of the accum and the 'replace a hash entry instead' logic
> instead? We never used to have it, and it's kind of extra complication and a
> bit gross.

Could even pull MAPHASH_ENTRIES into config.h and explicitly add it to
MAPCACHE_VCPU_ENTRIES. That would be neat would it not? And I believe we
have space in the mapcache's perdomain slot to enlarge MAPCACHE_VCPU_ENTRIES
in this way (i.e., from 16 to 24).

 -- Keir


>  -- Keir
> 
> 

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-06-12 15:11             ` Keir Fraser
  2013-06-12 15:27               ` Keir Fraser
@ 2013-06-12 15:48               ` Jan Beulich
  2013-06-12 17:26                 ` Keir Fraser
  1 sibling, 1 reply; 28+ messages in thread
From: Jan Beulich @ 2013-06-12 15:48 UTC (permalink / raw)
  To: Keir Fraser; +Cc: George Dunlap, konrad wilk, xen-devel

>>> On 12.06.13 at 17:11, Keir Fraser <keir.xen@gmail.com> wrote:
> On 12/06/2013 13:12, "Jan Beulich" <JBeulich@suse.com> wrote:
> 
>>> What we see above is that this failure path succeeds three times, but
>>> fails the fourth time: there are, in fact, no zero entries after the
>>> garbage clean-up; however, because "inuse" is 32-bit (effectively) and
>>> "accum" is 64-bit, ~inuse always has bits 32-63 set, and so will
>>> always return true and never fall back to the "something else"
>> 
>> Right, that's what occurred to me too yesterday, but the again
>> I knew I had seen this code path executed. Now that I look again,
>> I think I understand why: All of my Dom0-s and typical DomU-s
>> have a vCPU count divisible by 4, and with MAPCACHE_VCPU_ENTRIES
>> being 16, the full unsigned long would always be used.
> 
> Why are we so tight on MAPCACHE_VCPU_ENTRIES? Why not say double that number
> and get rid of the accum and the 'replace a hash entry instead' logic
> instead? We never used to have it, and it's kind of extra complication and a
> bit gross.

First of all, doubling the entries is not an argument for dropping
that code - the old 32-bit implementation really would have
needed this too from a theoretical perspective: The number of
readily available (garbage) entries is bounded by
MAPCACHE_VCPU_ENTRIES - MAPHASH_ENTRIES (because the
hash entries actively block getting treated as garbage). Adding
MAPHASH_ENTRIES into the calculation of MAPCACHE_VCPU_ENTRIES
would result in header dependency problems (I tried this when I
re-activated that code, as ideally we would want to set this value
to what we anticipate we might need _plus_ MAPHASH_ENTRIES).

Consequently the code should be prepared to recover entries
from the hash anyway (this is particularly relevant when
MAPCACHE_VCPU_ENTRIES <= MAPHASH_ENTRIES - while that's
not the case currently, if someone decided to double the latter
it would be).

Finally, the VA range for this already is an order-17 block (with
the inuse and garbage maps added at the end, i.e. it's slightly
above 512M), so doubling would require the code to be
adjusted to handle a single per-domain block covering two per-
domain slots (each slot covering 1Gb).

Jan

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-06-12 15:27               ` Keir Fraser
@ 2013-06-12 15:54                 ` Jan Beulich
  0 siblings, 0 replies; 28+ messages in thread
From: Jan Beulich @ 2013-06-12 15:54 UTC (permalink / raw)
  To: Keir Fraser; +Cc: George Dunlap, konrad wilk, xen-devel

>>> On 12.06.13 at 17:27, Keir Fraser <keir.xen@gmail.com> wrote:
> On 12/06/2013 16:11, "Keir Fraser" <keir.xen@gmail.com> wrote:
> 
>> On 12/06/2013 13:12, "Jan Beulich" <JBeulich@suse.com> wrote:
>> 
>>>> What we see above is that this failure path succeeds three times, but
>>>> fails the fourth time: there are, in fact, no zero entries after the
>>>> garbage clean-up; however, because "inuse" is 32-bit (effectively) and
>>>> "accum" is 64-bit, ~inuse always has bits 32-63 set, and so will
>>>> always return true and never fall back to the "something else"
>>> 
>>> Right, that's what occurred to me too yesterday, but the again
>>> I knew I had seen this code path executed. Now that I look again,
>>> I think I understand why: All of my Dom0-s and typical DomU-s
>>> have a vCPU count divisible by 4, and with MAPCACHE_VCPU_ENTRIES
>>> being 16, the full unsigned long would always be used.
>> 
>> Why are we so tight on MAPCACHE_VCPU_ENTRIES? Why not say double that number
>> and get rid of the accum and the 'replace a hash entry instead' logic
>> instead? We never used to have it, and it's kind of extra complication and a
>> bit gross.
> 
> Could even pull MAPHASH_ENTRIES into config.h and explicitly add it to
> MAPCACHE_VCPU_ENTRIES. That would be neat would it not? And I believe we
> have space in the mapcache's perdomain slot to enlarge MAPCACHE_VCPU_ENTRIES
> in this way (i.e., from 16 to 24).

Yes, this would an option, but I very much dislike pulling further
stuff not belonging there into config.h - this define really best
lives side by side with its related definitions.

While I just checked the code and think that this not being a power
of two should be fine, I'd nevertheless be careful with such a
change.

Jan

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-06-12 15:48               ` Jan Beulich
@ 2013-06-12 17:26                 ` Keir Fraser
  2013-07-05 16:56                   ` George Dunlap
  0 siblings, 1 reply; 28+ messages in thread
From: Keir Fraser @ 2013-06-12 17:26 UTC (permalink / raw)
  To: Jan Beulich; +Cc: George Dunlap, Konrad Rzeszutek Wilk, xen-devel

On 12/06/2013 16:48, "Jan Beulich" <JBeulich@suse.com> wrote:

>> Why are we so tight on MAPCACHE_VCPU_ENTRIES? Why not say double that number
>> and get rid of the accum and the 'replace a hash entry instead' logic
>> instead? We never used to have it, and it's kind of extra complication and a
>> bit gross.
> 
> First of all, doubling the entries is not an argument for dropping
> that code - the old 32-bit implementation really would have
> needed this too from a theoretical perspective: The number of
> readily available (garbage) entries is bounded by
> MAPCACHE_VCPU_ENTRIES - MAPHASH_ENTRIES (because the
> hash entries actively block getting treated as garbage).

So? We have control over both MAPCACHE_VCPU_ENTRUES and MAPHASH_ENTRIES. We
can make these somewhat arbitrary constants big

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-06-12 17:26                 ` Keir Fraser
@ 2013-07-05 16:56                   ` George Dunlap
  2013-07-08  8:58                     ` Jan Beulich
  0 siblings, 1 reply; 28+ messages in thread
From: George Dunlap @ 2013-07-05 16:56 UTC (permalink / raw)
  To: Keir Fraser; +Cc: xen-devel, Jan Beulich

On Wed, Jun 12, 2013 at 6:26 PM, Keir Fraser <keir.xen@gmail.com> wrote:
> On 12/06/2013 16:48, "Jan Beulich" <JBeulich@suse.com> wrote:
>
>>> Why are we so tight on MAPCACHE_VCPU_ENTRIES? Why not say double that number
>>> and get rid of the accum and the 'replace a hash entry instead' logic
>>> instead? We never used to have it, and it's kind of extra complication and a
>>> bit gross.
>>
>> First of all, doubling the entries is not an argument for dropping
>> that code - the old 32-bit implementation really would have
>> needed this too from a theoretical perspective: The number of
>> readily available (garbage) entries is bounded by
>> MAPCACHE_VCPU_ENTRIES - MAPHASH_ENTRIES (because the
>> hash entries actively block getting treated as garbage).
>
> So? We have control over both MAPCACHE_VCPU_ENTRUES and MAPHASH_ENTRIES. We
> can make these somewhat arbitrary constants big

So is this bug fixed now?  Can we close it in the bug tracker?

 -George

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-07-05 16:56                   ` George Dunlap
@ 2013-07-08  8:58                     ` Jan Beulich
  2013-07-08  9:07                       ` George Dunlap
  0 siblings, 1 reply; 28+ messages in thread
From: Jan Beulich @ 2013-07-08  8:58 UTC (permalink / raw)
  To: George Dunlap; +Cc: Keir Fraser, xen-devel

>>> On 05.07.13 at 18:56, George Dunlap <George.Dunlap@eu.citrix.com> wrote:
> On Wed, Jun 12, 2013 at 6:26 PM, Keir Fraser <keir.xen@gmail.com> wrote:
>> On 12/06/2013 16:48, "Jan Beulich" <JBeulich@suse.com> wrote:
>>
>>>> Why are we so tight on MAPCACHE_VCPU_ENTRIES? Why not say double that number
>>>> and get rid of the accum and the 'replace a hash entry instead' logic
>>>> instead? We never used to have it, and it's kind of extra complication and a
>>>> bit gross.
>>>
>>> First of all, doubling the entries is not an argument for dropping
>>> that code - the old 32-bit implementation really would have
>>> needed this too from a theoretical perspective: The number of
>>> readily available (garbage) entries is bounded by
>>> MAPCACHE_VCPU_ENTRIES - MAPHASH_ENTRIES (because the
>>> hash entries actively block getting treated as garbage).
>>
>> So? We have control over both MAPCACHE_VCPU_ENTRUES and MAPHASH_ENTRIES. We
>> can make these somewhat arbitrary constants big
> 
> So is this bug fixed now?  Can we close it in the bug tracker?

The bug itself got fixed, but the number of hash entries didn't
get increased so far (and as previously indicated I'm not
intending to do so; Keir indicated he might).

Jan

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-07-08  8:58                     ` Jan Beulich
@ 2013-07-08  9:07                       ` George Dunlap
  2013-07-08  9:15                         ` Processed: " xen
  2013-07-08  9:25                         ` George Dunlap
  0 siblings, 2 replies; 28+ messages in thread
From: George Dunlap @ 2013-07-08  9:07 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Keir Fraser, xen-devel

close ^
thanks

On Mon, Jul 8, 2013 at 9:58 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 05.07.13 at 18:56, George Dunlap <George.Dunlap@eu.citrix.com> wrote:
>> On Wed, Jun 12, 2013 at 6:26 PM, Keir Fraser <keir.xen@gmail.com> wrote:
>>> On 12/06/2013 16:48, "Jan Beulich" <JBeulich@suse.com> wrote:
>>>
>>>>> Why are we so tight on MAPCACHE_VCPU_ENTRIES? Why not say double that number
>>>>> and get rid of the accum and the 'replace a hash entry instead' logic
>>>>> instead? We never used to have it, and it's kind of extra complication and a
>>>>> bit gross.
>>>>
>>>> First of all, doubling the entries is not an argument for dropping
>>>> that code - the old 32-bit implementation really would have
>>>> needed this too from a theoretical perspective: The number of
>>>> readily available (garbage) entries is bounded by
>>>> MAPCACHE_VCPU_ENTRIES - MAPHASH_ENTRIES (because the
>>>> hash entries actively block getting treated as garbage).
>>>
>>> So? We have control over both MAPCACHE_VCPU_ENTRUES and MAPHASH_ENTRIES. We
>>> can make these somewhat arbitrary constants big
>>
>> So is this bug fixed now?  Can we close it in the bug tracker?
>
> The bug itself got fixed, but the number of hash entries didn't
> get increased so far (and as previously indicated I'm not
> intending to do so; Keir indicated he might).
>
> Jan
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

* Processed: Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-07-08  9:07                       ` George Dunlap
@ 2013-07-08  9:15                         ` xen
  2013-07-08  9:25                         ` George Dunlap
  1 sibling, 0 replies; 28+ messages in thread
From: xen @ 2013-07-08  9:15 UTC (permalink / raw)
  To: George Dunlap, xen-devel

Processing commands for xen@bugs.xenproject.org:

> close ^
Command failed: Cannot parse arguments at /srv/xen-devel-bugs/lib/emesinae/control.pl line 325, <M> line 41.
Stop processing here.

---
Xen Hypervisor Bug Tracker
See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on reporting bugs
Contact xen-bugs-owner@bugs.xenproject.org with any infrastructure issues

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

* Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-07-08  9:07                       ` George Dunlap
  2013-07-08  9:15                         ` Processed: " xen
@ 2013-07-08  9:25                         ` George Dunlap
  2013-07-08  9:30                           ` Processed: " xen
  1 sibling, 1 reply; 28+ messages in thread
From: George Dunlap @ 2013-07-08  9:25 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Keir Fraser, xen-devel

close 14
thanks

On Mon, Jul 8, 2013 at 10:07 AM, George Dunlap
<George.Dunlap@eu.citrix.com> wrote:
> close ^
> thanks
>
> On Mon, Jul 8, 2013 at 9:58 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 05.07.13 at 18:56, George Dunlap <George.Dunlap@eu.citrix.com> wrote:
>>> On Wed, Jun 12, 2013 at 6:26 PM, Keir Fraser <keir.xen@gmail.com> wrote:
>>>> On 12/06/2013 16:48, "Jan Beulich" <JBeulich@suse.com> wrote:
>>>>
>>>>>> Why are we so tight on MAPCACHE_VCPU_ENTRIES? Why not say double that number
>>>>>> and get rid of the accum and the 'replace a hash entry instead' logic
>>>>>> instead? We never used to have it, and it's kind of extra complication and a
>>>>>> bit gross.
>>>>>
>>>>> First of all, doubling the entries is not an argument for dropping
>>>>> that code - the old 32-bit implementation really would have
>>>>> needed this too from a theoretical perspective: The number of
>>>>> readily available (garbage) entries is bounded by
>>>>> MAPCACHE_VCPU_ENTRIES - MAPHASH_ENTRIES (because the
>>>>> hash entries actively block getting treated as garbage).
>>>>
>>>> So? We have control over both MAPCACHE_VCPU_ENTRUES and MAPHASH_ENTRIES. We
>>>> can make these somewhat arbitrary constants big
>>>
>>> So is this bug fixed now?  Can we close it in the bug tracker?
>>
>> The bug itself got fixed, but the number of hash entries didn't
>> get increased so far (and as previously indicated I'm not
>> intending to do so; Keir indicated he might).
>>
>> Jan
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel

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

* Processed: Re: Xen 4.3 + tmem = Xen BUG at domain_page.c:143
  2013-07-08  9:25                         ` George Dunlap
@ 2013-07-08  9:30                           ` xen
  0 siblings, 0 replies; 28+ messages in thread
From: xen @ 2013-07-08  9:30 UTC (permalink / raw)
  To: George Dunlap, xen-devel

Processing commands for xen@bugs.xenproject.org:

> close 14
Closing bug #14
> thanks
Finished processing.

Modified/created Bugs:
 - 14: http://bugs.xenproject.org/xen/bug/14

---
Xen Hypervisor Bug Tracker
See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on reporting bugs
Contact xen-bugs-owner@bugs.xenproject.org with any infrastructure issues

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

end of thread, other threads:[~2013-07-08  9:30 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-11 13:45 Xen 4.3 + tmem = Xen BUG at domain_page.c:143 konrad wilk
2013-06-11 14:46 ` Jan Beulich
2013-06-11 15:30   ` konrad wilk
2013-06-11 15:56     ` George Dunlap
2013-06-11 16:38     ` Jan Beulich
2013-06-11 17:30       ` konrad wilk
2013-06-11 18:52       ` konrad wilk
2013-06-11 21:06         ` konrad wilk
2013-06-12  6:38           ` Jan Beulich
2013-06-12 11:00         ` George Dunlap
2013-06-12 11:15           ` Processed: " xen
2013-06-12 11:37           ` George Dunlap
2013-06-12 12:46             ` Jan Beulich
2013-06-12 14:13             ` Konrad Rzeszutek Wilk
2013-06-12 12:12           ` Jan Beulich
2013-06-12 13:16             ` George Dunlap
2013-06-12 13:27               ` Jan Beulich
2013-06-12 15:11             ` Keir Fraser
2013-06-12 15:27               ` Keir Fraser
2013-06-12 15:54                 ` Jan Beulich
2013-06-12 15:48               ` Jan Beulich
2013-06-12 17:26                 ` Keir Fraser
2013-07-05 16:56                   ` George Dunlap
2013-07-08  8:58                     ` Jan Beulich
2013-07-08  9:07                       ` George Dunlap
2013-07-08  9:15                         ` Processed: " xen
2013-07-08  9:25                         ` George Dunlap
2013-07-08  9:30                           ` Processed: " xen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.