[PATCH] xen/tmem: Don't use map_domain_page for long-life-time pages.

[PATCH] xen/tmem: Don't use map_domain_page for long-life-time pages.

[Konrad Rzeszutek Wilk]

When using tmem with Xen 4.3 (and debug build) we end up with:

(XEN) Xen BUG at domain_page.c:143
(XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Not tainted ]----
(XEN) CPU:    3
(XEN) RIP:    e008:[<ffff82c4c01606a7>] map_domain_page+0x61d/0x6e1
.. 
(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)

A bit of debugging revealed that the map_domain_page and unmap_domain_page
are meant for short life-time mappings. And that those mappings are finite.
In the 2 VCPU guest we only have 32 entries and once we have exhausted those
we trigger the BUG_ON condition.

The two functions - tmh_persistent_pool_page_[get,put] are used by the xmem_pool
when xmem_pool_[alloc,free] are called. These xmem_pool_* function are wrapped
in macro and functions - the entry points are via: tmem_malloc
and tmem_page_alloc. In both cases the users are in the hypervisor and they
do not seem to suffer from using the hypervisor virtual addresses.

CC: Bob Liu <bob.liu@oracle.com>
CC: Dan Magenheimer <dan.magenheimer@oracle.com>
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/common/tmem_xen.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c
index 3a1f3c9..736a8c3 100644
--- a/xen/common/tmem_xen.c
+++ b/xen/common/tmem_xen.c
@@ -385,7 +385,7 @@ static void *tmh_persistent_pool_page_get(unsigned long size)
     if ( (pi = _tmh_alloc_page_thispool(d)) == NULL )
         return NULL;
     ASSERT(IS_VALID_PAGE(pi));
-    return __map_domain_page(pi);
+    return page_to_virt(pi);
 }
 
 static void tmh_persistent_pool_page_put(void *page_va)
@@ -393,8 +393,7 @@ static void tmh_persistent_pool_page_put(void *page_va)
     struct page_info *pi;
 
     ASSERT(IS_PAGE_ALIGNED(page_va));
-    pi = mfn_to_page(domain_page_map_to_mfn(page_va));
-    unmap_domain_page(page_va);
+    pi = mfn_to_page(virt_to_mfn(page_va));
     ASSERT(IS_VALID_PAGE(pi));
     _tmh_free_page_thispool(pi);
 }
-- 
1.7.7.6

Re: [PATCH] xen/tmem: Don't use map_domain_page for long-life-time pages.

[George Dunlap]

On 13/06/13 13:50, Konrad Rzeszutek Wilk wrote:
> When using tmem with Xen 4.3 (and debug build) we end up with:
>
> (XEN) Xen BUG at domain_page.c:143
> (XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Not tainted ]----
> (XEN) CPU:    3
> (XEN) RIP:    e008:[<ffff82c4c01606a7>] map_domain_page+0x61d/0x6e1
> ..
> (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)
>
> A bit of debugging revealed that the map_domain_page and unmap_domain_page
> are meant for short life-time mappings. And that those mappings are finite.
> In the 2 VCPU guest we only have 32 entries and once we have exhausted those
> we trigger the BUG_ON condition.
>
> The two functions - tmh_persistent_pool_page_[get,put] are used by the xmem_pool
> when xmem_pool_[alloc,free] are called. These xmem_pool_* function are wrapped
> in macro and functions - the entry points are via: tmem_malloc
> and tmem_page_alloc. In both cases the users are in the hypervisor and they
> do not seem to suffer from using the hypervisor virtual addresses.
>
> CC: Bob Liu <bob.liu@oracle.com>
> CC: Dan Magenheimer <dan.magenheimer@oracle.com>
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>   xen/common/tmem_xen.c |    5 ++---
>   1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c
> index 3a1f3c9..736a8c3 100644
> --- a/xen/common/tmem_xen.c
> +++ b/xen/common/tmem_xen.c
> @@ -385,7 +385,7 @@ static void *tmh_persistent_pool_page_get(unsigned long size)
>       if ( (pi = _tmh_alloc_page_thispool(d)) == NULL )
>           return NULL;
>       ASSERT(IS_VALID_PAGE(pi));
> -    return __map_domain_page(pi);
> +    return page_to_virt(pi);

Did I understand correctly that the map_domain_page() was required on 
 >5TiB systems, presumably because of limited virtual address space?  In 
which case this code will fail on those systems?

If that is the case, then we need to have a way to make sure tmem cannot 
be enabled on such systems.

  -George

Re: [PATCH] xen/tmem: Don't use map_domain_page for long-life-time pages.

[Konrad Rzeszutek Wilk]

On Thu, Jun 13, 2013 at 02:24:11PM +0100, George Dunlap wrote:
> On 13/06/13 13:50, Konrad Rzeszutek Wilk wrote:
> >When using tmem with Xen 4.3 (and debug build) we end up with:
> >
> >(XEN) Xen BUG at domain_page.c:143
> >(XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Not tainted ]----
> >(XEN) CPU:    3
> >(XEN) RIP:    e008:[<ffff82c4c01606a7>] map_domain_page+0x61d/0x6e1
> >..
> >(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)
> >
> >A bit of debugging revealed that the map_domain_page and unmap_domain_page
> >are meant for short life-time mappings. And that those mappings are finite.
> >In the 2 VCPU guest we only have 32 entries and once we have exhausted those
> >we trigger the BUG_ON condition.
> >
> >The two functions - tmh_persistent_pool_page_[get,put] are used by the xmem_pool
> >when xmem_pool_[alloc,free] are called. These xmem_pool_* function are wrapped
> >in macro and functions - the entry points are via: tmem_malloc
> >and tmem_page_alloc. In both cases the users are in the hypervisor and they
> >do not seem to suffer from using the hypervisor virtual addresses.
> >
> >CC: Bob Liu <bob.liu@oracle.com>
> >CC: Dan Magenheimer <dan.magenheimer@oracle.com>
> >Suggested-by: Jan Beulich <jbeulich@suse.com>
> >Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >---
> >  xen/common/tmem_xen.c |    5 ++---
> >  1 files changed, 2 insertions(+), 3 deletions(-)
> >
> >diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c
> >index 3a1f3c9..736a8c3 100644
> >--- a/xen/common/tmem_xen.c
> >+++ b/xen/common/tmem_xen.c
> >@@ -385,7 +385,7 @@ static void *tmh_persistent_pool_page_get(unsigned long size)
> >      if ( (pi = _tmh_alloc_page_thispool(d)) == NULL )
> >          return NULL;
> >      ASSERT(IS_VALID_PAGE(pi));
> >-    return __map_domain_page(pi);
> >+    return page_to_virt(pi);
> 
> Did I understand correctly that the map_domain_page() was required
> on >5TiB systems, presumably because of limited virtual address
> space?  In which case this code will fail on those systems?

Correct.
> 
> If that is the case, then we need to have a way to make sure tmem
> cannot be enabled on such systems.

Which Jan has had already done when he posted the 5TB patches.

> 
>  -George

Re: [PATCH] xen/tmem: Don't use map_domain_page for long-life-time pages.

[Josh Zhao]

2013/6/13 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
> On Thu, Jun 13, 2013 at 02:24:11PM +0100, George Dunlap wrote:
>> On 13/06/13 13:50, Konrad Rzeszutek Wilk wrote:
>> >When using tmem with Xen 4.3 (and debug build) we end up with:
>> >
>> >(XEN) Xen BUG at domain_page.c:143
>> >(XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Not tainted ]----
>> >(XEN) CPU:    3
>> >(XEN) RIP:    e008:[<ffff82c4c01606a7>] map_domain_page+0x61d/0x6e1
>> >..
>> >(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)
>> >
>> >A bit of debugging revealed that the map_domain_page and unmap_domain_page
>> >are meant for short life-time mappings. And that those mappings are finite.
>> >In the 2 VCPU guest we only have 32 entries and once we have exhausted those
>> >we trigger the BUG_ON condition.
>> >
>> >The two functions - tmh_persistent_pool_page_[get,put] are used by the xmem_pool
>> >when xmem_pool_[alloc,free] are called. These xmem_pool_* function are wrapped
>> >in macro and functions - the entry points are via: tmem_malloc
>> >and tmem_page_alloc. In both cases the users are in the hypervisor and they
>> >do not seem to suffer from using the hypervisor virtual addresses.
>> >
>> >CC: Bob Liu <bob.liu@oracle.com>
>> >CC: Dan Magenheimer <dan.magenheimer@oracle.com>
>> >Suggested-by: Jan Beulich <jbeulich@suse.com>
>> >Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> >---
>> >  xen/common/tmem_xen.c |    5 ++---
>> >  1 files changed, 2 insertions(+), 3 deletions(-)
>> >
>> >diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c
>> >index 3a1f3c9..736a8c3 100644
>> >--- a/xen/common/tmem_xen.c
>> >+++ b/xen/common/tmem_xen.c
>> >@@ -385,7 +385,7 @@ static void *tmh_persistent_pool_page_get(unsigned long size)
>> >      if ( (pi = _tmh_alloc_page_thispool(d)) == NULL )
>> >          return NULL;
>> >      ASSERT(IS_VALID_PAGE(pi));
>> >-    return __map_domain_page(pi);
>> >+    return page_to_virt(pi);
>>
>> Did I understand correctly that the map_domain_page() was required
>> on >5TiB systems, presumably because of limited virtual address
>> space?  In which case this code will fail on those systems?
>
> Correct.

I don't understand why the map_domain_page() was required on >5TiB ?


>>
>> If that is the case, then we need to have a way to make sure tmem
>> cannot be enabled on such systems.
>
> Which Jan has had already done when he posted the 5TB patches.
>
>>
>>  -George
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] xen/tmem: Don't use map_domain_page for long-life-time pages.

[Ian Campbell]

On Thu, 2013-08-22 at 18:15 +0800, Josh Zhao wrote:
> 2013/6/13 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
> > On Thu, Jun 13, 2013 at 02:24:11PM +0100, George Dunlap wrote:
> >> On 13/06/13 13:50, Konrad Rzeszutek Wilk wrote:
> >> >When using tmem with Xen 4.3 (and debug build) we end up with:
> >> >
> >> >(XEN) Xen BUG at domain_page.c:143
> >> >(XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Not tainted ]----
> >> >(XEN) CPU:    3
> >> >(XEN) RIP:    e008:[<ffff82c4c01606a7>] map_domain_page+0x61d/0x6e1
> >> >..
> >> >(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)
> >> >
> >> >A bit of debugging revealed that the map_domain_page and unmap_domain_page
> >> >are meant for short life-time mappings. And that those mappings are finite.
> >> >In the 2 VCPU guest we only have 32 entries and once we have exhausted those
> >> >we trigger the BUG_ON condition.
> >> >
> >> >The two functions - tmh_persistent_pool_page_[get,put] are used by the xmem_pool
> >> >when xmem_pool_[alloc,free] are called. These xmem_pool_* function are wrapped
> >> >in macro and functions - the entry points are via: tmem_malloc
> >> >and tmem_page_alloc. In both cases the users are in the hypervisor and they
> >> >do not seem to suffer from using the hypervisor virtual addresses.
> >> >
> >> >CC: Bob Liu <bob.liu@oracle.com>
> >> >CC: Dan Magenheimer <dan.magenheimer@oracle.com>
> >> >Suggested-by: Jan Beulich <jbeulich@suse.com>
> >> >Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >> >---
> >> >  xen/common/tmem_xen.c |    5 ++---
> >> >  1 files changed, 2 insertions(+), 3 deletions(-)
> >> >
> >> >diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c
> >> >index 3a1f3c9..736a8c3 100644
> >> >--- a/xen/common/tmem_xen.c
> >> >+++ b/xen/common/tmem_xen.c
> >> >@@ -385,7 +385,7 @@ static void *tmh_persistent_pool_page_get(unsigned long size)
> >> >      if ( (pi = _tmh_alloc_page_thispool(d)) == NULL )
> >> >          return NULL;
> >> >      ASSERT(IS_VALID_PAGE(pi));
> >> >-    return __map_domain_page(pi);
> >> >+    return page_to_virt(pi);
> >>
> >> Did I understand correctly that the map_domain_page() was required
> >> on >5TiB systems, presumably because of limited virtual address
> >> space?  In which case this code will fail on those systems?
> >
> > Correct.
> 
> I don't understand why the map_domain_page() was required on >5TiB ?

Xen on x86_64 only keeps a direct 1:1 mapping of up to 5TiB of memory.
Anything higher than that is demand mapped.

This is similar to the distinction between lowmem and highmem on x86.

Unless you require that the memory is always mapped (i.e. in the
xenheap, which may be considerably smaller that 5TiB on non-x86_64
platforms) it is correct to use the domheap allocator and
map_domain_page etc. If your domheap page ends up in the 5TiB mapping
range then the "demand map" is a nop.

Ian.

Re: [PATCH] xen/tmem: Don't use map_domain_page for long-life-time pages.

[Josh Zhao]

2013/6/13 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
> On Thu, Jun 13, 2013 at 02:24:11PM +0100, George Dunlap wrote:
>> On 13/06/13 13:50, Konrad Rzeszutek Wilk wrote:
>> >When using tmem with Xen 4.3 (and debug build) we end up with:
>> >
>> >(XEN) Xen BUG at domain_page.c:143
>> >(XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Not tainted ]----
>> >(XEN) CPU:    3
>> >(XEN) RIP:    e008:[<ffff82c4c01606a7>] map_domain_page+0x61d/0x6e1
>> >..
>> >(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)
>> >
>> >A bit of debugging revealed that the map_domain_page and unmap_domain_page
>> >are meant for short life-time mappings. And that those mappings are finite.
>> >In the 2 VCPU guest we only have 32 entries and once we have exhausted those
>> >we trigger the BUG_ON condition.
>> >
>> >The two functions - tmh_persistent_pool_page_[get,put] are used by the xmem_pool
>> >when xmem_pool_[alloc,free] are called. These xmem_pool_* function are wrapped
>> >in macro and functions - the entry points are via: tmem_malloc
>> >and tmem_page_alloc. In both cases the users are in the hypervisor and they
>> >do not seem to suffer from using the hypervisor virtual addresses.
>> >
>> >CC: Bob Liu <bob.liu@oracle.com>
>> >CC: Dan Magenheimer <dan.magenheimer@oracle.com>
>> >Suggested-by: Jan Beulich <jbeulich@suse.com>
>> >Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> >---
>> >  xen/common/tmem_xen.c |    5 ++---
>> >  1 files changed, 2 insertions(+), 3 deletions(-)
>> >
>> >diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c
>> >index 3a1f3c9..736a8c3 100644
>> >--- a/xen/common/tmem_xen.c
>> >+++ b/xen/common/tmem_xen.c
>> >@@ -385,7 +385,7 @@ static void *tmh_persistent_pool_page_get(unsigned long size)
>> >      if ( (pi = _tmh_alloc_page_thispool(d)) == NULL )
>> >          return NULL;
>> >      ASSERT(IS_VALID_PAGE(pi));
>> >-    return __map_domain_page(pi);
>> >+    return page_to_virt(pi);
>>
>> Did I understand correctly that the map_domain_page() was required
>> on >5TiB systems, presumably because of limited virtual address
>> space?  In which case this code will fail on those systems?
>
> Correct.


I don't understand why the map_domain_page() was required on >5TiB ?



>>
>> If that is the case, then we need to have a way to make sure tmem
>> cannot be enabled on such systems.
>
> Which Jan has had already done when he posted the 5TB patches.
>
>>
>>  -George
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] xen/tmem: Don't use map_domain_page for long-life-time pages.

[Ian Campbell]

On Thu, 2013-06-13 at 14:24 +0100, George Dunlap wrote:
> On 13/06/13 13:50, Konrad Rzeszutek Wilk wrote:
> > When using tmem with Xen 4.3 (and debug build) we end up with:
> >
> > (XEN) Xen BUG at domain_page.c:143
> > (XEN) ----[ Xen-4.3-unstable  x86_64  debug=y  Not tainted ]----
> > (XEN) CPU:    3
> > (XEN) RIP:    e008:[<ffff82c4c01606a7>] map_domain_page+0x61d/0x6e1
> > ..
> > (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)
> >
> > A bit of debugging revealed that the map_domain_page and unmap_domain_page
> > are meant for short life-time mappings. And that those mappings are finite.
> > In the 2 VCPU guest we only have 32 entries and once we have exhausted those
> > we trigger the BUG_ON condition.
> >
> > The two functions - tmh_persistent_pool_page_[get,put] are used by the xmem_pool
> > when xmem_pool_[alloc,free] are called. These xmem_pool_* function are wrapped
> > in macro and functions - the entry points are via: tmem_malloc
> > and tmem_page_alloc. In both cases the users are in the hypervisor and they
> > do not seem to suffer from using the hypervisor virtual addresses.

I think for this to be OK/safe you need to be sure these are Xen heap
mappings not just "in the hypervisor" (which can include domheap pages).

> >
> > CC: Bob Liu <bob.liu@oracle.com>
> > CC: Dan Magenheimer <dan.magenheimer@oracle.com>
> > Suggested-by: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > ---
> >   xen/common/tmem_xen.c |    5 ++---
> >   1 files changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c
> > index 3a1f3c9..736a8c3 100644
> > --- a/xen/common/tmem_xen.c
> > +++ b/xen/common/tmem_xen.c
> > @@ -385,7 +385,7 @@ static void *tmh_persistent_pool_page_get(unsigned long size)
> >       if ( (pi = _tmh_alloc_page_thispool(d)) == NULL )
> >           return NULL;
> >       ASSERT(IS_VALID_PAGE(pi));
> > -    return __map_domain_page(pi);
> > +    return page_to_virt(pi);
> 
> Did I understand correctly that the map_domain_page() was required on 
>  >5TiB systems, presumably because of limited virtual address space?  In 
> which case this code will fail on those systems?
> 
> If that is the case, then we need to have a way to make sure tmem cannot 
> be enabled on such systems.
> 
>   -George
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] xen/tmem: Don't use map_domain_page for long-life-time pages.

[Jan Beulich]

>>> On 13.06.13 at 15:24, George Dunlap <george.dunlap@eu.citrix.com> wrote:
> Did I understand correctly that the map_domain_page() was required on 
>  >5TiB systems, presumably because of limited virtual address space?  In 
> which case this code will fail on those systems?
> 
> If that is the case, then we need to have a way to make sure tmem cannot 
> be enabled on such systems.

Tmem is full of assumptions that all memory is mapped, and hence
one of the requirements here was to disable tmem when that
assumption is not true (until eventually tmem would get fixed).
That's, as I understand it, also why (part of) tmem has always
been disabled on x86-32.

Jan

Re: [PATCH] xen/tmem: Don't use map_domain_page for long-life-time pages.

[Ian Campbell]

On Thu, 2013-06-13 at 14:49 +0100, Jan Beulich wrote:
> >>> On 13.06.13 at 15:24, George Dunlap <george.dunlap@eu.citrix.com> wrote:
> > Did I understand correctly that the map_domain_page() was required on 
> >  >5TiB systems, presumably because of limited virtual address space?  In 
> > which case this code will fail on those systems?
> > 
> > If that is the case, then we need to have a way to make sure tmem cannot 
> > be enabled on such systems.
> 
> Tmem is full of assumptions that all memory is mapped, and hence
> one of the requirements here was to disable tmem when that
> assumption is not true (until eventually tmem would get fixed).

Isn't this change moving it further away from that fixed state though?

Ian.

Re: [PATCH] xen/tmem: Don't use map_domain_page for long-life-time pages.

[George Dunlap]

On 13/06/13 14:55, Ian Campbell wrote:
> On Thu, 2013-06-13 at 14:49 +0100, Jan Beulich wrote:
>>>>> On 13.06.13 at 15:24, George Dunlap <george.dunlap@eu.citrix.com> wrote:
>>> Did I understand correctly that the map_domain_page() was required on
>>>   >5TiB systems, presumably because of limited virtual address space?  In
>>> which case this code will fail on those systems?
>>>
>>> If that is the case, then we need to have a way to make sure tmem cannot
>>> be enabled on such systems.
>> Tmem is full of assumptions that all memory is mapped, and hence
>> one of the requirements here was to disable tmem when that
>> assumption is not true (until eventually tmem would get fixed).
> Isn't this change moving it further away from that fixed state though?

Perhaps, but it's probably the best change for 4.3 at this point.

  -George

Re: [PATCH] xen/tmem: Don't use map_domain_page for long-life-time pages.

[Jan Beulich]

>>> On 13.06.13 at 15:55, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2013-06-13 at 14:49 +0100, Jan Beulich wrote:
>> >>> On 13.06.13 at 15:24, George Dunlap <george.dunlap@eu.citrix.com> wrote:
>> > Did I understand correctly that the map_domain_page() was required on 
>> >  >5TiB systems, presumably because of limited virtual address space?  In 
>> > which case this code will fail on those systems?
>> > 
>> > If that is the case, then we need to have a way to make sure tmem cannot 
>> > be enabled on such systems.
>> 
>> Tmem is full of assumptions that all memory is mapped, and hence
>> one of the requirements here was to disable tmem when that
>> assumption is not true (until eventually tmem would get fixed).
> 
> Isn't this change moving it further away from that fixed state though?

In a way, yes. But what do you do? The code already assumes it
can use mfn_to_virt() or alike on memory taken from
alloc_domheap_pages(), so it's just being made consistently
wrong in doing it one more time. Any alternative fix would be way
more involved, as it would require adding the (un-)mapping around
the actual use points of the memory.

Jan

Re: [PATCH] xen/tmem: Don't use map_domain_page for long-life-time pages.

[Ian Campbell]

On Thu, 2013-06-13 at 15:04 +0100, Jan Beulich wrote:
> >>> On 13.06.13 at 15:55, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Thu, 2013-06-13 at 14:49 +0100, Jan Beulich wrote:
> >> >>> On 13.06.13 at 15:24, George Dunlap <george.dunlap@eu.citrix.com> wrote:
> >> > Did I understand correctly that the map_domain_page() was required on 
> >> >  >5TiB systems, presumably because of limited virtual address space?  In 
> >> > which case this code will fail on those systems?
> >> > 
> >> > If that is the case, then we need to have a way to make sure tmem cannot 
> >> > be enabled on such systems.
> >> 
> >> Tmem is full of assumptions that all memory is mapped, and hence
> >> one of the requirements here was to disable tmem when that
> >> assumption is not true (until eventually tmem would get fixed).
> > 
> > Isn't this change moving it further away from that fixed state though?
> 
> In a way, yes. But what do you do? The code already assumes it
> can use mfn_to_virt() or alike on memory taken from
> alloc_domheap_pages(), so it's just being made consistently
> wrong in doing it one more time. Any alternative fix would be way
> more involved, as it would require adding the (un-)mapping around
> the actual use points of the memory.

Yes, I'd temporarily forgotten that we needed a quick fix for 4.3. With
that in mind I suppose this is the best we can do.

Ian.