2.6.28 and balloon driver

2.6.28 and balloon driver

[Dan Magenheimer]

I am playing with 2.6.28 on xen.  Nice!  Thanks Jeremy!

But I have a minor gripe...

The balloon driver now is accessed via a sysfs file, for example:

# SIZE=`cat /sys/devices/system/xen_memory/xen_memory0/target_kb`
# echo $SIZE
# echo $SIZE > /sys/devices/system/xen_memory/xen_memory0/target_kb

SIZE does indeed get current memory size in kbytes, but
if one tries to pass SIZE (or slightly smaller value) back,
all hell breaks loose because:

1) Despite the name, the value written must be in bytes, not kbytes
2) There is no "safety minimum", so writing the same value back
   actually reduces memory by a factor of 1024!

I realize behavior (1) is backwards-compatible with the existing
/proc/xen/balloon behavior, but at least that filename doesn't
imply a unit size.

For (2), as sysadmins grow comfortable with the "safety minimum"
that's been implemented in upstream xen now for nearly a year,
(users can do:

# echo 0 > /proc/xen/balloon

and it still works), some people upgrading to a 2.6.28 kernel
are in for a nasty surprise.

<gripe off>

More on 2.6.28 and balloon driver

[Dan Magenheimer]

Can't prove it yet, but it appears that when 2.6.28
is ballooned to use less memory, the pages aren't
actually returned to the domain's available pool
of memory in Xen.

I've got some code (to be posted soon) that does
an alloc_domheap_pages(d,0,0) after domain d balloons
to use less memory.  The code works with linux-2.6.18-xen
and linux-2.6.27-xen, but not with 2.6.28... the
allocate fails.

Any good reason why this might be different?  Or should
I dig for a bug / code difference?

Also, I am still on xen 3.3.0+ so if this was fixed
later, please let me know.

Dan

Re: 2.6.28 and balloon driver

[Ian Campbell]

On Tue, 2009-01-06 at 23:34 +0000, Dan Magenheimer wrote:
> I am playing with 2.6.28 on xen.  Nice!  Thanks Jeremy!
> 
> But I have a minor gripe...
> 
> The balloon driver now is accessed via a sysfs file, for example:
> 
> # SIZE=`cat /sys/devices/system/xen_memory/xen_memory0/target_kb`
> # echo $SIZE
> # echo $SIZE > /sys/devices/system/xen_memory/xen_memory0/target_kb
> 
> SIZE does indeed get current memory size in kbytes, but
> if one tries to pass SIZE (or slightly smaller value) back,
> all hell breaks loose because:
> 
> 1) Despite the name, the value written must be in bytes, not kbytes

Looks like the write function uses memparse which understands the M, K
etc suffixes and defaults to bytes. The ability to say balloon to <n>M
is quite nice but for the sake of consistency with the name it's
probably preferable to just treat the value as a number of Kbytes.

> 2) There is no "safety minimum", so writing the same value back
>    actually reduces memory by a factor of 1024!

I guess this just needs porting forward.

Can you provide patches for both these issues?

Ian.

> 
> I realize behavior (1) is backwards-compatible with the existing
> /proc/xen/balloon behavior, but at least that filename doesn't
> imply a unit size.
> 
> For (2), as sysadmins grow comfortable with the "safety minimum"
> that's been implemented in upstream xen now for nearly a year,
> (users can do:
> 
> # echo 0 > /proc/xen/balloon
> 
> and it still works), some people upgrading to a 2.6.28 kernel
> are in for a nasty surprise.
> 
> <gripe off>
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: 2.6.28 and balloon driver

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] 2.6.28 and balloon driver"):
> Looks like the write function uses memparse which understands the M, K
> etc suffixes and defaults to bytes. The ability to say balloon to <n>M
> is quite nice but for the sake of consistency with the name it's
> probably preferable to just treat the value as a number of Kbytes.

Since you're changing the name anyway, can't we change the users of
the file to understand the new units too ?

Ie, I think we should have
  /sys/devices/system/xen_memory/xen_memory0/targe
which when read produces a number of bytes and which uses the default
memparse function.  If nothing else this is more likely to be
something upstream mind less.

Ian.

RE: More on 2.6.28 and balloon driver

[Dan Magenheimer]

Hi Jeremy --

It appears that in the upstream balloon driver,
the call to HYPERVISOR_update_va_mapping is missing
from decrease_reservation.  I think as a result,
the balloon driver is eating memory but not
releasing it to Xen, thus rendering the balloon
driver essentially useless.  (Can be observed via xentop.)

Is this just a bug/oversight, or was there maybe
a problem pushing this code upstream?

Thanks,
Dan

P.S. Also please see related balloon driver thread
on xen-devel yesterday (Jan 06).

> -----Original Message-----
> From: Dan Magenheimer 
> Sent: Tuesday, January 06, 2009 5:53 PM
> To: Xen-Devel (E-mail)
> Subject: [Xen-devel] More on 2.6.28 and balloon driver
> 
> 
> Can't prove it yet, but it appears that when 2.6.28
> is ballooned to use less memory, the pages aren't
> actually returned to the domain's available pool
> of memory in Xen.
> 
> I've got some code (to be posted soon) that does
> an alloc_domheap_pages(d,0,0) after domain d balloons
> to use less memory.  The code works with linux-2.6.18-xen
> and linux-2.6.27-xen, but not with 2.6.28... the
> allocate fails.
> 
> Any good reason why this might be different?  Or should
> I dig for a bug / code difference?
> 
> Also, I am still on xen 3.3.0+ so if this was fixed
> later, please let me know.
> 
> Dan
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 
>

RE: 2.6.28 and balloon driver

[Dan Magenheimer]

> Since you're changing the name anyway, can't we change the users of
> the file to understand the new units too ?

The patch should be easy, it's the change to backwards compatibility
that should be discussed, both for xen distros and now for upstream
Linux as well.  I suspect this isn't used much (yet) but don't know.

Writing bytes to a sysfs entry called target_kb is a bug
I think.  Is there precedence for having one sysfs entry for
reading and another for writing (the same value)?

>> 2) There is no "safety minimum", so writing the same value back
>>    actually reduces memory by a factor of 1024!

> I guess this just needs porting forward.

I vaguely recalled and so went and found this post from Jeremy:
http://lists.xensource.com/archives/html/xen-devel/2008-04/msg00144.html
(so Jeremy cc'ed)

So is it better to have the existing static model, or none
at all, or discuss and implement something better?

One option might be to have a writable sysfs min-mem which defaults
to the computed-from-maxmem value (from Jan's patch) but can
be overridden**.  That way, just changing target won't accidentally
cause OOMs but more aggressive ballooning can also be done.

Dan

** and bytes? kb? mb? :-)

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Wednesday, January 07, 2009 4:42 AM
> To: Ian Campbell
> Cc: Dan Magenheimer; Xen-Devel (E-mail)
> Subject: Re: [Xen-devel] 2.6.28 and balloon driver
> 
> 
> Ian Campbell writes ("Re: [Xen-devel] 2.6.28 and balloon driver"):
> > Looks like the write function uses memparse which 
> understands the M, K
> > etc suffixes and defaults to bytes. The ability to say 
> balloon to <n>M
> > is quite nice but for the sake of consistency with the name it's
> > probably preferable to just treat the value as a number of Kbytes.
> 
> Since you're changing the name anyway, can't we change the users of
> the file to understand the new units too ?
> 
> Ie, I think we should have
>   /sys/devices/system/xen_memory/xen_memory0/targe
> which when read produces a number of bytes and which uses the default
> memparse function.  If nothing else this is more likely to be
> something upstream mind less.
> 
> Ian.
> 

On Tue, 2009-01-06 at 23:34 +0000, Dan Magenheimer wrote:
> I am playing with 2.6.28 on xen.  Nice!  Thanks Jeremy!
> 
> But I have a minor gripe...
> 
> The balloon driver now is accessed via a sysfs file, for example:
> 
> # SIZE=`cat /sys/devices/system/xen_memory/xen_memory0/target_kb`
> # echo $SIZE
> # echo $SIZE > /sys/devices/system/xen_memory/xen_memory0/target_kb
> 
> SIZE does indeed get current memory size in kbytes, but
> if one tries to pass SIZE (or slightly smaller value) back,
> all hell breaks loose because:
> 
> 1) Despite the name, the value written must be in bytes, not kbytes

Looks like the write function uses memparse which understands the M, K
etc suffixes and defaults to bytes. The ability to say balloon to <n>M
is quite nice but for the sake of consistency with the name it's
probably preferable to just treat the value as a number of Kbytes.

> 2) There is no "safety minimum", so writing the same value back
>    actually reduces memory by a factor of 1024!

I guess this just needs porting forward.

Can you provide patches for both these issues?

Ian.

> 
> I realize behavior (1) is backwards-compatible with the existing
> /proc/xen/balloon behavior, but at least that filename doesn't
> imply a unit size.
> 
> For (2), as sysadmins grow comfortable with the "safety minimum"
> that's been implemented in upstream xen now for nearly a year,
> (users can do:
> 
> # echo 0 > /proc/xen/balloon
> 
> and it still works), some people upgrading to a 2.6.28 kernel
> are in for a nasty surprise.
> 
> <gripe off>

Re: 2.6.28 and balloon driver

[Trolle Selander]

[-- Attachment #1.1: Type: text/plain, Size: 4602 bytes --]

On 1/7/09, Dan Magenheimer <dan.magenheimer@oracle.com> wrote:
>
> > Since you're changing the name anyway, can't we change the users of
> > the file to understand the new units too ?
>
>
> The patch should be easy, it's the change to backwards compatibility
> that should be discussed, both for xen distros and now for upstream
> Linux as well.  I suspect this isn't used much (yet) but don't know.
>
> Writing bytes to a sysfs entry called target_kb is a bug
> I think.  Is there precedence for having one sysfs entry for
> reading and another for writing (the same value)?
>
>
> >> 2) There is no "safety minimum", so writing the same value back
> >>    actually reduces memory by a factor of 1024!
>
> > I guess this just needs porting forward.
>
>
> I vaguely recalled and so went and found this post from Jeremy:
> http://lists.xensource.com/archives/html/xen-devel/2008-04/msg00144.html
> (so Jeremy cc'ed)
>
> So is it better to have the existing static model, or none
> at all, or discuss and implement something better?
>
> One option might be to have a writable sysfs min-mem which defaults
> to the computed-from-maxmem value (from Jan's patch) but can
> be overridden**.  That way, just changing target won't accidentally
> cause OOMs but more aggressive ballooning can also be done.


I think that's definitely the right thing, and I'd just started
familiarizing myself with the balloon driver source with the intent to add
that functionality. An appliance or "instant-on" vm starting from a saved
snapshot state would want to aggressively balloon memory down to the
absolute minimum before saving and then balloon on restore. The
computed-from-maxmem min-mem value doesn't work very well for this use case.

-- Trolle

Dan
>
> ** and bytes? kb? mb? :-)
>
>
> > -----Original Message-----
> > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > Sent: Wednesday, January 07, 2009 4:42 AM
> > To: Ian Campbell
> > Cc: Dan Magenheimer; Xen-Devel (E-mail)
> > Subject: Re: [Xen-devel] 2.6.28 and balloon driver
> >
> >
> > Ian Campbell writes ("Re: [Xen-devel] 2.6.28 and balloon driver"):
> > > Looks like the write function uses memparse which
> > understands the M, K
> > > etc suffixes and defaults to bytes. The ability to say
> > balloon to <n>M
> > > is quite nice but for the sake of consistency with the name it's
> > > probably preferable to just treat the value as a number of Kbytes.
> >
> > Since you're changing the name anyway, can't we change the users of
> > the file to understand the new units too ?
> >
> > Ie, I think we should have
> >   /sys/devices/system/xen_memory/xen_memory0/targe
> > which when read produces a number of bytes and which uses the default
> > memparse function.  If nothing else this is more likely to be
> > something upstream mind less.
> >
> > Ian.
> >
>
>
> On Tue, 2009-01-06 at 23:34 +0000, Dan Magenheimer wrote:
> > I am playing with 2.6.28 on xen.  Nice!  Thanks Jeremy!
> >
> > But I have a minor gripe...
> >
> > The balloon driver now is accessed via a sysfs file, for example:
> >
> > # SIZE=`cat /sys/devices/system/xen_memory/xen_memory0/target_kb`
> > # echo $SIZE
> > # echo $SIZE > /sys/devices/system/xen_memory/xen_memory0/target_kb
> >
> > SIZE does indeed get current memory size in kbytes, but
> > if one tries to pass SIZE (or slightly smaller value) back,
> > all hell breaks loose because:
> >
> > 1) Despite the name, the value written must be in bytes, not kbytes
>
>
> Looks like the write function uses memparse which understands the M, K
> etc suffixes and defaults to bytes. The ability to say balloon to <n>M
> is quite nice but for the sake of consistency with the name it's
> probably preferable to just treat the value as a number of Kbytes.
>
>
> > 2) There is no "safety minimum", so writing the same value back
> >    actually reduces memory by a factor of 1024!
>
> I guess this just needs porting forward.
>
> Can you provide patches for both these issues?
>
> Ian.
>
> >
> > I realize behavior (1) is backwards-compatible with the existing
> > /proc/xen/balloon behavior, but at least that filename doesn't
> > imply a unit size.
> >
> > For (2), as sysadmins grow comfortable with the "safety minimum"
> > that's been implemented in upstream xen now for nearly a year,
> > (users can do:
> >
> > # echo 0 > /proc/xen/balloon
> >
> > and it still works), some people upgrading to a 2.6.28 kernel
> > are in for a nasty surprise.
> >
> > <gripe off>
>
> _______________________________________________
>
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

[-- Attachment #1.2: Type: text/html, Size: 6015 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

RE: More on 2.6.28 and balloon driver

[Dan Magenheimer]

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

> It appears that in the upstream balloon driver,
> the call to HYPERVISOR_update_va_mapping is missing
> from decrease_reservation.  I think as a result,
> the balloon driver is eating memory but not
> releasing it to Xen, thus rendering the balloon
> driver essentially useless.  (Can be observed via xentop.)

Limited testing, but it appears that adding it
back in with this simple patch to linux-2.6.28
makes ballooning work properly.

As the code is lifted from the original Xen balloon driver,
I'm not sure I need this, but:

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

Dan

P.S. Attachement is same, provided in case my mailer
messes up this >80-char-width patch.


--- linux-2.6.28/drivers/xen/balloon.c.orig	2009-01-07 12:35:48.000000000 -0700
+++ linux-2.6.28/drivers/xen/balloon.c	2009-01-07 12:36:06.000000000 -0700
@@ -296,6 +296,11 @@
 		frame_list[i] = pfn_to_mfn(pfn);
 
 		scrub_page(page);
+
+		ret = HYPERVISOR_update_va_mapping(
+			(unsigned long)__va(pfn << PAGE_SHIFT),
+			__pte_ma(0), 0);
+		BUG_ON(ret);
 	}
 
 	/* Ensure that ballooned highmem pages don't have kmaps. */

[-- Attachment #2: balloon.patch --]
[-- Type: application/octet-stream, Size: 425 bytes --]

--- linux-2.6.28/drivers/xen/balloon.c.orig	2009-01-07 12:35:48.000000000 -0700
+++ linux-2.6.28/drivers/xen/balloon.c	2009-01-07 12:36:06.000000000 -0700
@@ -296,6 +296,11 @@
 		frame_list[i] = pfn_to_mfn(pfn);
 
 		scrub_page(page);
+
+		ret = HYPERVISOR_update_va_mapping(
+			(unsigned long)__va(pfn << PAGE_SHIFT),
+			__pte_ma(0), 0);
+		BUG_ON(ret);
 	}
 
 	/* Ensure that ballooned highmem pages don't have kmaps. */

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

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

Re: More on 2.6.28 and balloon driver

[Jeremy Fitzhardinge]

Dan Magenheimer wrote:
>> It appears that in the upstream balloon driver,
>> the call to HYPERVISOR_update_va_mapping is missing
>> from decrease_reservation.  I think as a result,
>> the balloon driver is eating memory but not
>> releasing it to Xen, thus rendering the balloon
>> driver essentially useless.  (Can be observed via xentop.)
>>     
>
> Limited testing, but it appears that adding it
> back in with this simple patch to linux-2.6.28
> makes ballooning work properly.
>   

Thanks.  That's a pretty embarrassing oversight :/

> As the code is lifted from the original Xen balloon driver,
> I'm not sure I need this, but:
>
> Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
>   

Yeah, the S-o-b line is a way to keep track of who a patch went via.  
It's "I assert that I'm allowed to post this patch".  In theory you'd 
include the previous S-o-b lines (if any), but a reference to the source 
repo/version would be enough in this case.

    J

> Dan
>
> P.S. Attachement is same, provided in case my mailer
> messes up this >80-char-width patch.
>
>
> --- linux-2.6.28/drivers/xen/balloon.c.orig	2009-01-07 12:35:48.000000000 -0700
> +++ linux-2.6.28/drivers/xen/balloon.c	2009-01-07 12:36:06.000000000 -0700
> @@ -296,6 +296,11 @@
>  		frame_list[i] = pfn_to_mfn(pfn);
>  
>  		scrub_page(page);
> +
> +		ret = HYPERVISOR_update_va_mapping(
> +			(unsigned long)__va(pfn << PAGE_SHIFT),
> +			__pte_ma(0), 0);
> +		BUG_ON(ret);
>  	}
>  
>  	/* Ensure that ballooned highmem pages don't have kmaps. */
> ------------------------------------------------------------------------
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>