All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] increase initial memory reservation for stubdom based HVM domains
@ 2009-01-15 16:46 Stefano Stabellini
  2009-01-15 17:04 ` Keir Fraser
  0 siblings, 1 reply; 8+ messages in thread
From: Stefano Stabellini @ 2009-01-15 16:46 UTC (permalink / raw)
  To: xen-devel

Hi all,
this patch requests 32 additional MB of free RAM from dom0 when an HVM
domain is started, if the device model for the domain is provided by a
stubdom.
This way there is no risk that a stubdom fails to populate the videoram
because the RAM freed for the videoram by dom0 has already been used to
create the stubdom.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff -r 10a8fae412c5 tools/python/xen/xend/image.py
--- a/tools/python/xen/xend/image.py	Wed Jan 14 13:43:17 2009 +0000
+++ b/tools/python/xen/xend/image.py	Thu Jan 15 16:33:26 2009 +0000
@@ -838,6 +838,7 @@
         HVMImageHandler.configure(self, vmConfig)
         self.vhpt = int(vmConfig['platform'].get('vhpt',  0))
         self.vramsize = int(vmConfig['platform'].get('videoram',4)) * 1024
+        self.use_stubdom = (vmConfig['platform'].get('device_model').find('stubdom-dm') >= 0)
 
     def buildDomain(self):
         xc.nvram_init(self.vm.getName(), self.vm.getDomid())
@@ -853,6 +854,8 @@
         extra_pages = 1024 + 5
         mem_kb += extra_pages * page_kb
         mem_kb += self.vramsize
+        if self.use_stubdom :
+            mem_kb += 32 * 1024 
         return mem_kb
 
     def getRequiredInitialReservation(self):
@@ -888,6 +891,7 @@
         HVMImageHandler.configure(self, vmConfig)
         self.pae = int(vmConfig['platform'].get('pae',  0))
         self.vramsize = int(vmConfig['platform'].get('videoram',4)) * 1024
+        self.use_stubdom = (vmConfig['platform'].get('device_model').find('stubdom-dm') >= 0)
 
     def buildDomain(self):
         xc.hvm_set_param(self.vm.getDomid(), HVM_PARAM_PAE_ENABLED, self.pae)
@@ -896,7 +900,10 @@
         return rc
 
     def getRequiredAvailableMemory(self, mem_kb):
-        return mem_kb + self.vramsize
+        mem_kb += self.vramsize
+        if self.use_stubdom :
+            mem_kb += 32 * 1024
+        return mem_kb
 
     def getRequiredInitialReservation(self):
         return self.vm.getMemoryTarget()

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

* Re: [PATCH] increase initial memory reservation for stubdom based HVM domains
  2009-01-15 16:46 [PATCH] increase initial memory reservation for stubdom based HVM domains Stefano Stabellini
@ 2009-01-15 17:04 ` Keir Fraser
  2009-01-15 17:08   ` Stefano Stabellini
  2009-01-15 17:42   ` Samuel Thibault
  0 siblings, 2 replies; 8+ messages in thread
From: Keir Fraser @ 2009-01-15 17:04 UTC (permalink / raw)
  To: Stefano Stabellini, xen-devel

On 15/01/2009 16:46, "Stefano Stabellini" <stefano.stabellini@eu.citrix.com>
wrote:

> this patch requests 32 additional MB of free RAM from dom0 when an HVM
> domain is started, if the device model for the domain is provided by a
> stubdom.
> This way there is no risk that a stubdom fails to populate the videoram
> because the RAM freed for the videoram by dom0 has already been used to
> create the stubdom.

Does this mean you're giving up on 'properly' fixing the auto ballooner? :-)

At least the +32 needs a comment. Perhaps abstraction into a function or
variable as well as a comment. Is the value 32 as arbitrary as it seems?

 -- Keir

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

* Re: [PATCH] increase initial memory reservation for stubdom based HVM domains
  2009-01-15 17:04 ` Keir Fraser
@ 2009-01-15 17:08   ` Stefano Stabellini
  2009-01-15 17:34     ` Keir Fraser
  2009-01-15 17:42   ` Samuel Thibault
  1 sibling, 1 reply; 8+ messages in thread
From: Stefano Stabellini @ 2009-01-15 17:08 UTC (permalink / raw)
  To: Keir Fraser; +Cc: xen-devel

Keir Fraser wrote:

> On 15/01/2009 16:46, "Stefano Stabellini" <stefano.stabellini@eu.citrix.com>
> wrote:
> 
>> this patch requests 32 additional MB of free RAM from dom0 when an HVM
>> domain is started, if the device model for the domain is provided by a
>> stubdom.
>> This way there is no risk that a stubdom fails to populate the videoram
>> because the RAM freed for the videoram by dom0 has already been used to
>> create the stubdom.
> 
> Does this mean you're giving up on 'properly' fixing the auto ballooner? :-)
> 
> At least the +32 needs a comment. Perhaps abstraction into a function or
> variable as well as a comment. Is the value 32 as arbitrary as it seems?
> 

This fix is not as bad as it seems: I am only increasing the memory
freed by the autoballoner when a stubdom based HVM domain is created, in
order to also take into account the memory used by the stubdom.
In facts 32MB is exactly the size currently needed by a stubdom, it can
probably be reduced, but I preferred making one step at a time.
The memory actually used to create the HVM domain is still the same as
before.

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

* Re: [PATCH] increase initial memory reservation for stubdom based HVM domains
  2009-01-15 17:08   ` Stefano Stabellini
@ 2009-01-15 17:34     ` Keir Fraser
  2009-01-15 17:44       ` Samuel Thibault
  0 siblings, 1 reply; 8+ messages in thread
From: Keir Fraser @ 2009-01-15 17:34 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel

On 15/01/2009 17:08, "Stefano Stabellini" <stefano.stabellini@eu.citrix.com>
wrote:

> This fix is not as bad as it seems: I am only increasing the memory
> freed by the autoballoner when a stubdom based HVM domain is created, in
> order to also take into account the memory used by the stubdom.
> In facts 32MB is exactly the size currently needed by a stubdom, it can
> probably be reduced, but I preferred making one step at a time.
> The memory actually used to create the HVM domain is still the same as
> before.

Doesn't stubdom itself cause the auto-ballooner to be run? Could stubdom add
on the extra headroom of the guest its servicing when it requests
ballooning? I don't know whether that would be clearer/cleaner than your
current patch...

 -- Keir

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

* Re: [PATCH] increase initial memory reservation for stubdom based HVM domains
  2009-01-15 17:04 ` Keir Fraser
  2009-01-15 17:08   ` Stefano Stabellini
@ 2009-01-15 17:42   ` Samuel Thibault
  1 sibling, 0 replies; 8+ messages in thread
From: Samuel Thibault @ 2009-01-15 17:42 UTC (permalink / raw)
  To: Keir Fraser; +Cc: xen-devel, Stefano Stabellini

Keir Fraser, le Thu 15 Jan 2009 17:04:36 +0000, a écrit :
> On 15/01/2009 16:46, "Stefano Stabellini" <stefano.stabellini@eu.citrix.com>
> wrote:
> > this patch requests 32 additional MB of free RAM from dom0 when an HVM
> > domain is started, if the device model for the domain is provided by a
> > stubdom.
> > This way there is no risk that a stubdom fails to populate the videoram
> > because the RAM freed for the videoram by dom0 has already been used to
> > create the stubdom.
> 
> Does this mean you're giving up on 'properly' fixing the auto ballooner? :-)
> 
> At least the +32 needs a comment. Perhaps abstraction into a function or
> variable as well as a comment. Is the value 32 as arbitrary as it seems?

32 comes from stubdom/stubdom-dm. It is a bit arbitrary in that it may
not be enough if you add a lot of network cards etc. but should usually
be fine.

Samuel

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

* Re: [PATCH] increase initial memory reservation for stubdom based HVM domains
  2009-01-15 17:34     ` Keir Fraser
@ 2009-01-15 17:44       ` Samuel Thibault
  2009-01-15 17:59         ` Stefano Stabellini
  0 siblings, 1 reply; 8+ messages in thread
From: Samuel Thibault @ 2009-01-15 17:44 UTC (permalink / raw)
  To: Keir Fraser; +Cc: xen-devel, Stefano Stabellini

Keir Fraser, le Thu 15 Jan 2009 17:34:26 +0000, a écrit :
> Doesn't stubdom itself cause the auto-ballooner to be run?

Yes. (and the issue was that here the auto-ballooner doesn't know that
qemu hasn't taken the 8MB extra ballooned memory for video memory yet)

> Could stubdom add on the extra headroom of the guest its servicing
> when it requests ballooning? I don't know whether that would be
> clearer/cleaner than your current patch...

I don't know how that could be done, but it seems better to me.

Samuel

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

* Re: [PATCH] increase initial memory reservation for stubdom based HVM domains
  2009-01-15 17:44       ` Samuel Thibault
@ 2009-01-15 17:59         ` Stefano Stabellini
  2009-01-15 18:34           ` Trolle Selander
  0 siblings, 1 reply; 8+ messages in thread
From: Stefano Stabellini @ 2009-01-15 17:59 UTC (permalink / raw)
  To: Samuel Thibault, Keir Fraser, Stefano Stabellini, xen-devel

Samuel Thibault wrote:

> Keir Fraser, le Thu 15 Jan 2009 17:34:26 +0000, a écrit :
>> Doesn't stubdom itself cause the auto-ballooner to be run?
> 
> Yes. (and the issue was that here the auto-ballooner doesn't know that
> qemu hasn't taken the 8MB extra ballooned memory for video memory yet)
> 
>> Could stubdom add on the extra headroom of the guest its servicing
>> when it requests ballooning? I don't know whether that would be
>> clearer/cleaner than your current patch...
> 
> I don't know how that could be done, but it seems better to me.
> 

It can be done: instead of requesting 16MB for the videoram (or whatever
is set in the config file) when we create the HVM domain, we request
them when we create the stubdom.
It also seems to me that this approach is cleaner, I'll try to come up
with a patch.

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

* Re: [PATCH] increase initial memory reservation for stubdom based HVM domains
  2009-01-15 17:59         ` Stefano Stabellini
@ 2009-01-15 18:34           ` Trolle Selander
  0 siblings, 0 replies; 8+ messages in thread
From: Trolle Selander @ 2009-01-15 18:34 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel


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

For what it's worth, the discussion on qemu-devel is leaning towards not
having a hardcoded limit to the amount of assignable vram, and that more
than xres * yres * 4 bytes or vram can in fact be useful for a guest. We can
of course still enforce a limit - I'd raise it to 32 Megs, taking
double-buffering into consideration. Regardless of the arguments on
qemu-devel, i think the practical use of any more vram than that is
questionable to say the least - but depending on how closely we want to stay
in sync with upstream qemu this should perhaps the at least be taken into
consideration.

-- Trolle

On Thu, Jan 15, 2009 at 12:59 PM, Stefano Stabellini <
stefano.stabellini@eu.citrix.com> wrote:

> Samuel Thibault wrote:
>
> > Keir Fraser, le Thu 15 Jan 2009 17:34:26 +0000, a écrit :
> >> Doesn't stubdom itself cause the auto-ballooner to be run?
> >
> > Yes. (and the issue was that here the auto-ballooner doesn't know that
> > qemu hasn't taken the 8MB extra ballooned memory for video memory yet)
> >
> >> Could stubdom add on the extra headroom of the guest its servicing
> >> when it requests ballooning? I don't know whether that would be
> >> clearer/cleaner than your current patch...
> >
> > I don't know how that could be done, but it seems better to me.
> >
>
> It can be done: instead of requesting 16MB for the videoram (or whatever
> is set in the config file) when we create the HVM domain, we request
> them when we create the stubdom.
> It also seems to me that this approach is cleaner, I'll try to come up
> with a patch.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

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

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

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

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

end of thread, other threads:[~2009-01-15 18:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-15 16:46 [PATCH] increase initial memory reservation for stubdom based HVM domains Stefano Stabellini
2009-01-15 17:04 ` Keir Fraser
2009-01-15 17:08   ` Stefano Stabellini
2009-01-15 17:34     ` Keir Fraser
2009-01-15 17:44       ` Samuel Thibault
2009-01-15 17:59         ` Stefano Stabellini
2009-01-15 18:34           ` Trolle Selander
2009-01-15 17:42   ` Samuel Thibault

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.