[PATCH] tmem: fix to 20945 "When tmem is enabled, reserve a fraction of memory"

[PATCH] tmem: fix to 20945 "When tmem is enabled, reserve a fraction of memory"

[Dan Magenheimer]

(Sorry, I was testing with this tweak to my previously posted
patch but had neglected to post it before the patch was taken.)

With tmem enabled, when available memory is scarce, don't allow
order==0 pages to be taken from the "midsize alloc zone" but
DO attempt to relinquish a page from tmem.  Else many
things fail when tmem has absorbed nearly all system memory.

Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>

diff -r 364001067e26 xen/common/page_alloc.c
--- a/xen/common/page_alloc.c	Tue Feb 16 11:55:21 2010 +0000
+++ b/xen/common/page_alloc.c	Tue Feb 16 10:19:39 2010 -0700
@@ -311,9 +311,13 @@ static struct page_info *alloc_heap_page
      * TMEM: When available memory is scarce, allow only mid-size allocations
      * to avoid worst of fragmentation issues.
      */
-    if ( opt_tmem && ((order == 0) || (order >= 9)) &&
-         (total_avail_pages <= midsize_alloc_zone_pages) )
-        goto fail;
+    if ( opt_tmem && (total_avail_pages <= midsize_alloc_zone_pages) )
+    {
+        if ( order == 0)
+            goto try_tmem;
+        if ( order >= 9)
+            goto fail;
+    }
 
     /*
      * Start with requested node, but exhaust all node memory in requested 
@@ -341,6 +345,7 @@ static struct page_info *alloc_heap_page
     }
 
     /* Try to free memory from tmem */
+try_tmem:
     if ( (pg = tmem_relinquish_pages(order,memflags)) != NULL )
     {
         /* reassigning an already allocated anonymous heap page */

Re: [PATCH] tmem: fix to 20945 "When tmem is enabled, reserve a fraction of memory"

[Keir Fraser]

On 16/02/2010 18:30, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:

> -    if ( opt_tmem && ((order == 0) || (order >= 9)) &&
> -         (total_avail_pages <= midsize_alloc_zone_pages) )
> -        goto fail;
> +    if ( opt_tmem && (total_avail_pages <= midsize_alloc_zone_pages) )
> +    {
> +        if ( order == 0)
> +            goto try_tmem;
> +        if ( order >= 9)
> +            goto fail;

Why not try_tmem in the case that order>=9, too, rather than fail outright?

 -- Keir

Re: [PATCH] tmem: fix to 20945 "When tmem is enabled, reserve a fraction of memory"

[Jan Beulich]

>>> Keir Fraser <keir.fraser@eu.citrix.com> 17.02.10 13:10 >>>
>On 16/02/2010 18:30, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:
>
>> -    if ( opt_tmem && ((order == 0) || (order >= 9)) &&
>> -         (total_avail_pages <= midsize_alloc_zone_pages) )
>> -        goto fail;
>> +    if ( opt_tmem && (total_avail_pages <= midsize_alloc_zone_pages) )
>> +    {
>> +        if ( order == 0)
>> +            goto try_tmem;
>> +        if ( order >= 9)
>> +            goto fail;
>
>Why not try_tmem in the case that order>=9, too, rather than fail outright?

It could be done that way, but wouldn't have any effect, as tmem
doesn't even try to relinquish any memory when order > 0.

Jan

RE: [PATCH] tmem: fix to 20945 "When tmem is enabled, reserve a fraction of memory"

[Dan Magenheimer]

> From: Jan Beulich [mailto:JBeulich@novell.com]
> Subject: Re: [PATCH] tmem: fix to 20945 "When tmem is enabled, reserve
> a fraction of memory"
> 
> >>> Keir Fraser <keir.fraser@eu.citrix.com> 17.02.10 13:10 >>>
> >On 16/02/2010 18:30, "Dan Magenheimer" <dan.magenheimer@oracle.com>
> wrote:
> >
> >> +        if ( order == 0)
> >> +            goto try_tmem;
> >> +        if ( order >= 9)
> >> +            goto fail;
> >
> >Why not try_tmem in the case that order>=9, too, rather than fail
> outright?
> 
> It could be done that way, but wouldn't have any effect, as tmem
> doesn't even try to relinquish any memory when order > 0.

Correct.  To explain (if anyone is interested):

Tmem maintains queues of order==0 pages internally because
if a page is released to the xenheap/domheap, it must be scrubbed.
But tmem is highly likely to use the page again (and SOON).
If tmem immediately reclaims the page, the scrubbing is wasted
cycles.  But if it does not and some other xenheap/domheap allocation
obtains the page, the contents of an unscrubbed page could
reveal data from another domain so would be a potential
security hole.

When a domain is being created, a large number of pages
may be (scrubbed and) transferred from tmem to Xen/domheap.
While this transfer, in combination with the "buddying"
in xenheap/domheap, may result in some order>0 chunks of
memory, there is no guarantee that it will.

I considered adding some kind of "buddying" to tmem's "free"
pages (and the interface to tmem_relinquish_pages() from
alloc_heap_pages() allows for an order>0 to be requested),
but due to fragmentation it would only rarely have any
value, especially for order>1, so I never implemented it.

So, in the end, the real solution is to change any allocations
in Xen, at least any allocations that occur after dom0 is
running, to no longer require order>0 allocations.

Dan

RE: [PATCH] tmem: fix to 20945 "When tmem is enabled, reserve a fraction of memory"

[Dan Magenheimer]

Hi Keir --

Hmmm... one other consequence of the change you made to the patch
(as checked in as 20955 in staging) is that every attempt to allocate
any 2MB page for a new domain (when memory is scarce) will result
in a complaint printk'ed from tmem_relinquish_pages() before
the domain builder falls back to order==0 pages.  This
verbosity is probably not desirable in a product, though
it may be very desirable with debug enabled as we track
down other order>0 allocations.

Changing back to the "goto fail" avoids the verbosity without
losing the debug capability.

Dan

> -----Original Message-----
> From: Dan Magenheimer
> Sent: Wednesday, February 17, 2010 8:13 AM
> To: Jan Beulich; Keir Fraser
> Cc: xen-devel@lists.xensource.com
> Subject: RE: [PATCH] tmem: fix to 20945 "When tmem is enabled, reserve
> a fraction of memory"
> 
> > From: Jan Beulich [mailto:JBeulich@novell.com]
> > Subject: Re: [PATCH] tmem: fix to 20945 "When tmem is enabled,
> reserve
> > a fraction of memory"
> >
> > >>> Keir Fraser <keir.fraser@eu.citrix.com> 17.02.10 13:10 >>>
> > >On 16/02/2010 18:30, "Dan Magenheimer" <dan.magenheimer@oracle.com>
> > wrote:
> > >
> > >> +        if ( order == 0)
> > >> +            goto try_tmem;
> > >> +        if ( order >= 9)
> > >> +            goto fail;
> > >
> > >Why not try_tmem in the case that order>=9, too, rather than fail
> > outright?
> >
> > It could be done that way, but wouldn't have any effect, as tmem
> > doesn't even try to relinquish any memory when order > 0.
> 
> Correct.  To explain (if anyone is interested):
> 
> Tmem maintains queues of order==0 pages internally because
> if a page is released to the xenheap/domheap, it must be scrubbed.
> But tmem is highly likely to use the page again (and SOON).
> If tmem immediately reclaims the page, the scrubbing is wasted
> cycles.  But if it does not and some other xenheap/domheap allocation
> obtains the page, the contents of an unscrubbed page could
> reveal data from another domain so would be a potential
> security hole.
> 
> When a domain is being created, a large number of pages
> may be (scrubbed and) transferred from tmem to Xen/domheap.
> While this transfer, in combination with the "buddying"
> in xenheap/domheap, may result in some order>0 chunks of
> memory, there is no guarantee that it will.
> 
> I considered adding some kind of "buddying" to tmem's "free"
> pages (and the interface to tmem_relinquish_pages() from
> alloc_heap_pages() allows for an order>0 to be requested),
> but due to fragmentation it would only rarely have any
> value, especially for order>1, so I never implemented it.
> 
> So, in the end, the real solution is to change any allocations
> in Xen, at least any allocations that occur after dom0 is
> running, to no longer require order>0 allocations.
> 
> Dan

Re: [PATCH] tmem: fix to 20945 "When tmem is enabled, reserve a fraction of memory"

[Keir Fraser]

If you don't want verbosity in the failed-allocation path, remove your
printk. Notice non-tmem code doesn't make a noise in this case.
tmem_relinquish_pages() takes an order parameter, and the normal path
through the allocator unconditionally calls it. Hence it doesn't make sense
to logically separate order=0 from order>=9 in this case either, from the
p.o.v. of the caller.

 -- Keir

On 17/02/2010 21:49, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:

> Hi Keir --
> 
> Hmmm... one other consequence of the change you made to the patch
> (as checked in as 20955 in staging) is that every attempt to allocate
> any 2MB page for a new domain (when memory is scarce) will result
> in a complaint printk'ed from tmem_relinquish_pages() before
> the domain builder falls back to order==0 pages.  This
> verbosity is probably not desirable in a product, though
> it may be very desirable with debug enabled as we track
> down other order>0 allocations.
> 
> Changing back to the "goto fail" avoids the verbosity without
> losing the debug capability.
> 
> Dan
> 
>> -----Original Message-----
>> From: Dan Magenheimer
>> Sent: Wednesday, February 17, 2010 8:13 AM
>> To: Jan Beulich; Keir Fraser
>> Cc: xen-devel@lists.xensource.com
>> Subject: RE: [PATCH] tmem: fix to 20945 "When tmem is enabled, reserve
>> a fraction of memory"
>> 
>>> From: Jan Beulich [mailto:JBeulich@novell.com]
>>> Subject: Re: [PATCH] tmem: fix to 20945 "When tmem is enabled,
>> reserve
>>> a fraction of memory"
>>> 
>>>>>> Keir Fraser <keir.fraser@eu.citrix.com> 17.02.10 13:10 >>>
>>>> On 16/02/2010 18:30, "Dan Magenheimer" <dan.magenheimer@oracle.com>
>>> wrote:
>>>> 
>>>>> +        if ( order == 0)
>>>>> +            goto try_tmem;
>>>>> +        if ( order >= 9)
>>>>> +            goto fail;
>>>> 
>>>> Why not try_tmem in the case that order>=9, too, rather than fail
>>> outright?
>>> 
>>> It could be done that way, but wouldn't have any effect, as tmem
>>> doesn't even try to relinquish any memory when order > 0.
>> 
>> Correct.  To explain (if anyone is interested):
>> 
>> Tmem maintains queues of order==0 pages internally because
>> if a page is released to the xenheap/domheap, it must be scrubbed.
>> But tmem is highly likely to use the page again (and SOON).
>> If tmem immediately reclaims the page, the scrubbing is wasted
>> cycles.  But if it does not and some other xenheap/domheap allocation
>> obtains the page, the contents of an unscrubbed page could
>> reveal data from another domain so would be a potential
>> security hole.
>> 
>> When a domain is being created, a large number of pages
>> may be (scrubbed and) transferred from tmem to Xen/domheap.
>> While this transfer, in combination with the "buddying"
>> in xenheap/domheap, may result in some order>0 chunks of
>> memory, there is no guarantee that it will.
>> 
>> I considered adding some kind of "buddying" to tmem's "free"
>> pages (and the interface to tmem_relinquish_pages() from
>> alloc_heap_pages() allows for an order>0 to be requested),
>> but due to fragmentation it would only rarely have any
>> value, especially for order>1, so I never implemented it.
>> 
>> So, in the end, the real solution is to change any allocations
>> in Xen, at least any allocations that occur after dom0 is
>> running, to no longer require order>0 allocations.
>> 
>> Dan

RE: [PATCH] tmem: fix to 20945 "When tmem is enabled, reserve a fraction of memory"

[Dan Magenheimer]

Hmmm... OK, would you mind putting an #ifndef NDEBUG around
the printk("...failing order...)" in tmem_relinquish_pages()
for now then?  Cleaning this up properly is too much surgery
until post-4.0.

(Do you want an officially submitted patch?)

> -----Original Message-----
> From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
> Sent: Thursday, February 18, 2010 12:38 AM
> To: Dan Magenheimer; Jan Beulich
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [PATCH] tmem: fix to 20945 "When tmem is enabled, reserve
> a fraction of memory"
> 
> If you don't want verbosity in the failed-allocation path, remove your
> printk. Notice non-tmem code doesn't make a noise in this case.
> tmem_relinquish_pages() takes an order parameter, and the normal path
> through the allocator unconditionally calls it. Hence it doesn't make
> sense
> to logically separate order=0 from order>=9 in this case either, from
> the
> p.o.v. of the caller.
> 
>  -- Keir
> 
> On 17/02/2010 21:49, "Dan Magenheimer" <dan.magenheimer@oracle.com>
> wrote:
> 
> > Hi Keir --
> >
> > Hmmm... one other consequence of the change you made to the patch
> > (as checked in as 20955 in staging) is that every attempt to allocate
> > any 2MB page for a new domain (when memory is scarce) will result
> > in a complaint printk'ed from tmem_relinquish_pages() before
> > the domain builder falls back to order==0 pages.  This
> > verbosity is probably not desirable in a product, though
> > it may be very desirable with debug enabled as we track
> > down other order>0 allocations.
> >
> > Changing back to the "goto fail" avoids the verbosity without
> > losing the debug capability.
> >
> > Dan
> >
> >> -----Original Message-----
> >> From: Dan Magenheimer
> >> Sent: Wednesday, February 17, 2010 8:13 AM
> >> To: Jan Beulich; Keir Fraser
> >> Cc: xen-devel@lists.xensource.com
> >> Subject: RE: [PATCH] tmem: fix to 20945 "When tmem is enabled,
> reserve
> >> a fraction of memory"
> >>
> >>> From: Jan Beulich [mailto:JBeulich@novell.com]
> >>> Subject: Re: [PATCH] tmem: fix to 20945 "When tmem is enabled,
> >> reserve
> >>> a fraction of memory"
> >>>
> >>>>>> Keir Fraser <keir.fraser@eu.citrix.com> 17.02.10 13:10 >>>
> >>>> On 16/02/2010 18:30, "Dan Magenheimer"
> <dan.magenheimer@oracle.com>
> >>> wrote:
> >>>>
> >>>>> +        if ( order == 0)
> >>>>> +            goto try_tmem;
> >>>>> +        if ( order >= 9)
> >>>>> +            goto fail;
> >>>>
> >>>> Why not try_tmem in the case that order>=9, too, rather than fail
> >>> outright?
> >>>
> >>> It could be done that way, but wouldn't have any effect, as tmem
> >>> doesn't even try to relinquish any memory when order > 0.
> >>
> >> Correct.  To explain (if anyone is interested):
> >>
> >> Tmem maintains queues of order==0 pages internally because
> >> if a page is released to the xenheap/domheap, it must be scrubbed.
> >> But tmem is highly likely to use the page again (and SOON).
> >> If tmem immediately reclaims the page, the scrubbing is wasted
> >> cycles.  But if it does not and some other xenheap/domheap
> allocation
> >> obtains the page, the contents of an unscrubbed page could
> >> reveal data from another domain so would be a potential
> >> security hole.
> >>
> >> When a domain is being created, a large number of pages
> >> may be (scrubbed and) transferred from tmem to Xen/domheap.
> >> While this transfer, in combination with the "buddying"
> >> in xenheap/domheap, may result in some order>0 chunks of
> >> memory, there is no guarantee that it will.
> >>
> >> I considered adding some kind of "buddying" to tmem's "free"
> >> pages (and the interface to tmem_relinquish_pages() from
> >> alloc_heap_pages() allows for an order>0 to be requested),
> >> but due to fragmentation it would only rarely have any
> >> value, especially for order>1, so I never implemented it.
> >>
> >> So, in the end, the real solution is to change any allocations
> >> in Xen, at least any allocations that occur after dom0 is
> >> running, to no longer require order>0 allocations.
> >>
> >> Dan
> 
>