Xen devel - Feb 2010 - [PATCH] tmem: fix to 20945 "When tmem is enabled, reserve a fraction of memory"

(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 */

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

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



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

>>> 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


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

> 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

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

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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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


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

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
> 
> 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel