[PATCH] Prevent changing a memory size of Domain-0 even if users make a careless mistake

[PATCH] Prevent changing a memory size of Domain-0 even if users make a careless mistake

[Masaki Kanno]

[-- Attachment #1: Mail message body --]
[-- Type: text/plain, Size: 1106 bytes --]

Hi,

If users accidentally change a memory size of Domain-0 to very small 
memory size by xm mem-set command, users will be not able to operate 
Domain-0.  I think that Domain-0 is important for Xen, so I'd like to 
prevent the accident by xm mem-set command. 

This patch prevents changing the memory size of Domain-0 by xm mem-set 
command if specified memory size is smaller than dom0-min-mem in 
xend-config.sxp. 

 # cat /etc/xen/xend-config.sxp | grep "(dom0-min-mem"
 (dom0-min-mem 384)
 # xm list Domain-0
 Name                                        ID   Mem VCPUs      State   Time(s)
 Domain-0                                     0   941     2     r-----     62.6
 # xm mem-set Domain-0 1
 Error: memory_dynamic_max cannot be changed for safety reasons
 Usage: xm mem-set <Domain> <Mem>
 
 Set the current memory usage for a domain.
 # xm list Domain-0
 Name                                        ID   Mem VCPUs      State   Time(s)
 Domain-0                                     0   941     2     r-----     63.6


Signed-off-by: Masaki Kanno <kanno.masaki@jp.fujitsu.com>

Best regards,
 Kan


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

diff -r db943e8d1051 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py	Tue Apr 01 10:09:33 2008 +0100
+++ b/tools/python/xen/xend/XendDomainInfo.py	Fri Apr 04 13:43:55 2008 +0900
@@ -986,6 +986,14 @@ class XendDomainInfo:
                   self.info['name_label'], str(self.domid), target)
         
         MiB = 1024 * 1024
+
+        if self.domid == 0:
+            dom0_min_mem = xoptions.get_dom0_min_mem()
+            memory_cur = self.get_memory_dynamic_max() / MiB
+            if target < memory_cur:
+                if dom0_min_mem == 0 or dom0_min_mem > target:
+                    raise XendError("memory_dynamic_max cannot be changed for safety reasons")
+
         self._safe_set_memory('memory_dynamic_min', target * MiB)
         self._safe_set_memory('memory_dynamic_max', target * MiB)
 

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

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

Re: [PATCH] Prevent changing a memory size of Domain-0 even if users make a careless mistake

[Keir Fraser]

I think it's fair to just say "Don't do that then, you idiot!". :-)

 -- Keir

On 4/4/08 11:06, "Masaki Kanno" <kanno.masaki@jp.fujitsu.com> wrote:

> Hi,
> 
> If users accidentally change a memory size of Domain-0 to very small
> memory size by xm mem-set command, users will be not able to operate
> Domain-0.  I think that Domain-0 is important for Xen, so I'd like to
> prevent the accident by xm mem-set command.
> 
> This patch prevents changing the memory size of Domain-0 by xm mem-set
> command if specified memory size is smaller than dom0-min-mem in
> xend-config.sxp. 
> 
>  # cat /etc/xen/xend-config.sxp | grep "(dom0-min-mem"
>  (dom0-min-mem 384)
>  # xm list Domain-0
>  Name                                        ID   Mem VCPUs      State
> Time(s)
>  Domain-0                                     0   941     2     r-----
> 62.6
>  # xm mem-set Domain-0 1
>  Error: memory_dynamic_max cannot be changed for safety reasons
>  Usage: xm mem-set <Domain> <Mem>
>  
>  Set the current memory usage for a domain.
>  # xm list Domain-0
>  Name                                        ID   Mem VCPUs      State
> Time(s)
>  Domain-0                                     0   941     2     r-----
> 63.6
> 
> 
> Signed-off-by: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
> 
> Best regards,
>  Kan
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] Prevent changing a memory size ofDomain-0 even if users make a careless mistake

[Masaki Kanno]

I as the vendor cannot say it... (^^;)

 Kan

Fri, 04 Apr 2008 11:13:33 +0100, Keir Fraser wrote:

>I think it's fair to just say "Don't do that then, you idiot!". :-)
>
> -- Keir
>
>On 4/4/08 11:06, "Masaki Kanno" <kanno.masaki@jp.fujitsu.com> wrote:
>
>> Hi,
>> 
>> If users accidentally change a memory size of Domain-0 to very small
>> memory size by xm mem-set command, users will be not able to operate
>> Domain-0.  I think that Domain-0 is important for Xen, so I'd like to
>> prevent the accident by xm mem-set command.
>> 
>> This patch prevents changing the memory size of Domain-0 by xm mem-set
>> command if specified memory size is smaller than dom0-min-mem in
>> xend-config.sxp. 
>> 
>>  # cat /etc/xen/xend-config.sxp | grep "(dom0-min-mem"
>>  (dom0-min-mem 384)
>>  # xm list Domain-0
>>  Name                                        ID   Mem VCPUs      State
>> Time(s)
>>  Domain-0                                     0   941     2     r-----
>> 62.6
>>  # xm mem-set Domain-0 1
>>  Error: memory_dynamic_max cannot be changed for safety reasons
>>  Usage: xm mem-set <Domain> <Mem>
>>  
>>  Set the current memory usage for a domain.
>>  # xm list Domain-0
>>  Name                                        ID   Mem VCPUs      State
>> Time(s)
>>  Domain-0                                     0   941     2     r-----
>> 63.6
>> 
>> 
>> Signed-off-by: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
>> 
>> Best regards,
>>  Kan
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>
>
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel

Re: [PATCH] Prevent changing a memory size of Domain-0 even if users make a careless mistake

[Mark Williamson]

> I think it's fair to just say "Don't do that then, you idiot!". :-)

I think if we have a "dom0-min-mem" parameter then we may as well use it 
consistently.  We already use it to prevent event quite determined 
foot-shooting attempts (like starting up too many domains!).

Seems consistent to apply the rule whenever an xm command might balloon dom0.

Kan, a comment on your patch:


diff -r db943e8d1051 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py   Tue Apr 01 10:09:33 2008 +0100
+++ b/tools/python/xen/xend/XendDomainInfo.py   Fri Apr 04 13:43:55 2008 +0900
@@ -986,6 +986,14 @@ class XendDomainInfo:
                   self.info['name_label'], str(self.domid), target)
         
         MiB = 1024 * 1024
+
+        if self.domid == 0:
+            dom0_min_mem = xoptions.get_dom0_min_mem()
+            memory_cur = self.get_memory_dynamic_max() / MiB
+            if target < memory_cur:
+                if dom0_min_mem == 0 or dom0_min_mem > target:

If I recall correctly, the current meaning of dom0_min_mem set to 0 is "No 
limiting", so I think you can just remove that check to be consistent with 
existing behaviour.

Cheers,
Mark

+                    raise XendError("memory_dynamic_max cannot be changed for 
safety reasons")
+
         self._safe_set_memory('memory_dynamic_min', target * MiB)
         self._safe_set_memory('memory_dynamic_max', target * MiB)
 


>  -- Keir
>
> On 4/4/08 11:06, "Masaki Kanno" <kanno.masaki@jp.fujitsu.com> wrote:
> > Hi,
> >
> > If users accidentally change a memory size of Domain-0 to very small
> > memory size by xm mem-set command, users will be not able to operate
> > Domain-0.  I think that Domain-0 is important for Xen, so I'd like to
> > prevent the accident by xm mem-set command.
> >
> > This patch prevents changing the memory size of Domain-0 by xm mem-set
> > command if specified memory size is smaller than dom0-min-mem in
> > xend-config.sxp.
> >
> >  # cat /etc/xen/xend-config.sxp | grep "(dom0-min-mem"
> >  (dom0-min-mem 384)
> >  # xm list Domain-0
> >  Name                                        ID   Mem VCPUs      State
> > Time(s)
> >  Domain-0                                     0   941     2     r-----
> > 62.6
> >  # xm mem-set Domain-0 1
> >  Error: memory_dynamic_max cannot be changed for safety reasons
> >  Usage: xm mem-set <Domain> <Mem>
> >
> >  Set the current memory usage for a domain.
> >  # xm list Domain-0
> >  Name                                        ID   Mem VCPUs      State
> > Time(s)
> >  Domain-0                                     0   941     2     r-----
> > 63.6
> >
> >
> > Signed-off-by: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
> >
> > Best regards,
> >  Kan
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel



-- 
Push Me Pull You - Distributed SCM tool (http://www.cl.cam.ac.uk/~maw48/pmpu/)

Re: [PATCH] Prevent changing a memory size of Domain-0even if users make a careless mistake

[Masaki Kanno]

Sat, 5 Apr 2008 19:36:07 +0100, Mark Williamson wrote:

>> I think it's fair to just say "Don't do that then, you idiot!". :-)
>
>I think if we have a "dom0-min-mem" parameter then we may as well use it 
>consistently.  We already use it to prevent event quite determined 
>foot-shooting attempts (like starting up too many domains!).
>
>Seems consistent to apply the rule whenever an xm command might balloon dom0.
>
>Kan, a comment on your patch:
>
>
>diff -r db943e8d1051 tools/python/xen/xend/XendDomainInfo.py
>--- a/tools/python/xen/xend/XendDomainInfo.py   Tue Apr 01 10:09:33 2008 +
>0100
>+++ b/tools/python/xen/xend/XendDomainInfo.py   Fri Apr 04 13:43:55 2008 +
>0900
>@@ -986,6 +986,14 @@ class XendDomainInfo:
>                   self.info['name_label'], str(self.domid), target)
>         
>         MiB = 1024 * 1024
>+
>+        if self.domid == 0:
>+            dom0_min_mem = xoptions.get_dom0_min_mem()
>+            memory_cur = self.get_memory_dynamic_max() / MiB
>+            if target < memory_cur:
>+                if dom0_min_mem == 0 or dom0_min_mem > target:
>
>If I recall correctly, the current meaning of dom0_min_mem set to 0 is "No 
>limiting", so I think you can just remove that check to be consistent with 
>existing behaviour.

Hi Mark,

Is it?  As far as I know, the current meaning of dom0-min-mem set to 0 
is "No ballooning".  There are the following comments about dom0-min-mem 
in xend-config.sxp. 

# Dom0 will balloon out when needed to free memory for domU.
# dom0-min-mem is the lowest memory level (in MB) dom0 will get down to.
# If dom0-min-mem=0, dom0 will never balloon out.

Best regards,
 Kan

>
>Cheers,
>Mark
>
>+                    raise XendError("memory_dynamic_max cannot be changed 
>for 
>safety reasons")
>+
>         self._safe_set_memory('memory_dynamic_min', target * MiB)
>         self._safe_set_memory('memory_dynamic_max', target * MiB)
> 
>
>
>>  -- Keir
>>
>> On 4/4/08 11:06, "Masaki Kanno" <kanno.masaki@jp.fujitsu.com> wrote:
>> > Hi,
>> >
>> > If users accidentally change a memory size of Domain-0 to very small
>> > memory size by xm mem-set command, users will be not able to operate
>> > Domain-0.  I think that Domain-0 is important for Xen, so I'd like to
>> > prevent the accident by xm mem-set command.
>> >
>> > This patch prevents changing the memory size of Domain-0 by xm mem-set
>> > command if specified memory size is smaller than dom0-min-mem in
>> > xend-config.sxp.
>> >
>> >  # cat /etc/xen/xend-config.sxp | grep "(dom0-min-mem"
>> >  (dom0-min-mem 384)
>> >  # xm list Domain-0
>> >  Name                                        ID   Mem VCPUs      State
>> > Time(s)
>> >  Domain-0                                     0   941     2     r-----
>> > 62.6
>> >  # xm mem-set Domain-0 1
>> >  Error: memory_dynamic_max cannot be changed for safety reasons
>> >  Usage: xm mem-set <Domain> <Mem>
>> >
>> >  Set the current memory usage for a domain.
>> >  # xm list Domain-0
>> >  Name                                        ID   Mem VCPUs      State
>> > Time(s)
>> >  Domain-0                                     0   941     2     r-----
>> > 63.6
>> >
>> >
>> > Signed-off-by: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
>> >
>> > Best regards,
>> >  Kan
>> >
>> > _______________________________________________
>> > Xen-devel mailing list
>> > Xen-devel@lists.xensource.com
>> > http://lists.xensource.com/xen-devel
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>
>
>
>-- 
>Push Me Pull You - Distributed SCM tool (http://www.cl.cam.ac.uk/~maw48/
>pmpu/)
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel

Re: [PATCH] Prevent changing a memory size of Domain-0even if users make a careless mistake

[Mark Williamson]

> Is it?  As far as I know, the current meaning of dom0-min-mem set to 0
> is "No ballooning".  There are the following comments about dom0-min-mem
> in xend-config.sxp.
>
> # Dom0 will balloon out when needed to free memory for domU.
> # dom0-min-mem is the lowest memory level (in MB) dom0 will get down to.
> # If dom0-min-mem=0, dom0 will never balloon out.

Sorry, think I hadn't had enough coffee!  You're right of course.  Plus, your 
suggested behaviour is more consistent.

On the face of it, disabling ballooning like this seems inconvenient.  
However, I'd think that if disabling auto-ballooning, the administrator has 
probably set their dom0 footprint manually on the Xen command line, so they 
won't want to mem-set it anyhow.

Admins who want to be able to mess about arbitrarily with memory can still put 
dom0-min-mem=1.  I think dom0-min-mem should really /mean/ dom0-min-mem as 
far as the control tools are concerned.  If we want to disable autoballooning 
independently of mem-set, we could add a simple boolean option.

Cheers,
Mark

> Best regards,
>  Kan
>
> >Cheers,
> >Mark
> >
> >+                    raise XendError("memory_dynamic_max cannot be changed
> >for
> >safety reasons")
> >+
> >         self._safe_set_memory('memory_dynamic_min', target * MiB)
> >         self._safe_set_memory('memory_dynamic_max', target * MiB)
> >
> >>  -- Keir
> >>
> >> On 4/4/08 11:06, "Masaki Kanno" <kanno.masaki@jp.fujitsu.com> wrote:
> >> > Hi,
> >> >
> >> > If users accidentally change a memory size of Domain-0 to very small
> >> > memory size by xm mem-set command, users will be not able to operate
> >> > Domain-0.  I think that Domain-0 is important for Xen, so I'd like to
> >> > prevent the accident by xm mem-set command.
> >> >
> >> > This patch prevents changing the memory size of Domain-0 by xm mem-set
> >> > command if specified memory size is smaller than dom0-min-mem in
> >> > xend-config.sxp.
> >> >
> >> >  # cat /etc/xen/xend-config.sxp | grep "(dom0-min-mem"
> >> >  (dom0-min-mem 384)
> >> >  # xm list Domain-0
> >> >  Name                                        ID   Mem VCPUs      State
> >> > Time(s)
> >> >  Domain-0                                     0   941     2     r-----
> >> > 62.6
> >> >  # xm mem-set Domain-0 1
> >> >  Error: memory_dynamic_max cannot be changed for safety reasons
> >> >  Usage: xm mem-set <Domain> <Mem>
> >> >
> >> >  Set the current memory usage for a domain.
> >> >  # xm list Domain-0
> >> >  Name                                        ID   Mem VCPUs      State
> >> > Time(s)
> >> >  Domain-0                                     0   941     2     r-----
> >> > 63.6
> >> >
> >> >
> >> > Signed-off-by: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
> >> >
> >> > Best regards,
> >> >  Kan
> >> >
> >> > _______________________________________________
> >> > Xen-devel mailing list
> >> > Xen-devel@lists.xensource.com
> >> > http://lists.xensource.com/xen-devel
> >>
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@lists.xensource.com
> >> http://lists.xensource.com/xen-devel
> >
> >--
> >Push Me Pull You - Distributed SCM tool (http://www.cl.cam.ac.uk/~maw48/
> >pmpu/)
> >
> >_______________________________________________
> >Xen-devel mailing list
> >Xen-devel@lists.xensource.com
> >http://lists.xensource.com/xen-devel



-- 
Push Me Pull You - Distributed SCM tool (http://www.cl.cam.ac.uk/~maw48/pmpu/)

Re: [PATCH] Prevent changing a memory size of Domain-0even if users make a careless mistake

[Masaki Kanno]

Mon, 7 Apr 2008 01:56:02 +0100, Mark Williamson wrote:

>> Is it?  As far as I know, the current meaning of dom0-min-mem set to 0
>> is "No ballooning".  There are the following comments about dom0-min-mem
>> in xend-config.sxp.
>>
>> # Dom0 will balloon out when needed to free memory for domU.
>> # dom0-min-mem is the lowest memory level (in MB) dom0 will get down to.
>> # If dom0-min-mem=0, dom0 will never balloon out.
>
>Sorry, think I hadn't had enough coffee!  You're right of course.  Plus, 
>your 
>suggested behaviour is more consistent.
>
>On the face of it, disabling ballooning like this seems inconvenient.  
>However, I'd think that if disabling auto-ballooning, the administrator has 
>probably set their dom0 footprint manually on the Xen command line, so they 
>won't want to mem-set it anyhow.
>
>Admins who want to be able to mess about arbitrarily with memory can still 
>put 
>dom0-min-mem=1.  I think dom0-min-mem should really /mean/ dom0-min-mem as 
>far as the control tools are concerned.  If we want to disable 
>autoballooning 
>independently of mem-set, we could add a simple boolean option.

Hi Mark,

Thanks for your suggestion.  It is good suggestion.
I will write a patch according to your suggestion.

Best regards,
 Kan

>
>Cheers,
>Mark
>
>> Best regards,
>>  Kan
>>
>> >Cheers,
>> >Mark
>> >
>> >+                    raise XendError("memory_dynamic_max cannot be changed
>> >for
>> >safety reasons")
>> >+
>> >         self._safe_set_memory('memory_dynamic_min', target * MiB)
>> >         self._safe_set_memory('memory_dynamic_max', target * MiB)
>> >
>> >>  -- Keir
>> >>
>> >> On 4/4/08 11:06, "Masaki Kanno" <kanno.masaki@jp.fujitsu.com> wrote:
>> >> > Hi,
>> >> >
>> >> > If users accidentally change a memory size of Domain-0 to very small
>> >> > memory size by xm mem-set command, users will be not able to operate
>> >> > Domain-0.  I think that Domain-0 is important for Xen, so I'd like to
>> >> > prevent the accident by xm mem-set command.
>> >> >
>> >> > This patch prevents changing the memory size of Domain-0 by xm mem-set
>> >> > command if specified memory size is smaller than dom0-min-mem in
>> >> > xend-config.sxp.
>> >> >
>> >> >  # cat /etc/xen/xend-config.sxp | grep "(dom0-min-mem"
>> >> >  (dom0-min-mem 384)
>> >> >  # xm list Domain-0
>> >> >  Name                                        ID   Mem VCPUs      State
>> >> > Time(s)
>> >> >  Domain-0                                     0   941     2     r-----
>> >> > 62.6
>> >> >  # xm mem-set Domain-0 1
>> >> >  Error: memory_dynamic_max cannot be changed for safety reasons
>> >> >  Usage: xm mem-set <Domain> <Mem>
>> >> >
>> >> >  Set the current memory usage for a domain.
>> >> >  # xm list Domain-0
>> >> >  Name                                        ID   Mem VCPUs      State
>> >> > Time(s)
>> >> >  Domain-0                                     0   941     2     r-----
>> >> > 63.6
>> >> >
>> >> >
>> >> > Signed-off-by: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
>> >> >
>> >> > Best regards,
>> >> >  Kan
>> >> >
>> >> > _______________________________________________
>> >> > Xen-devel mailing list
>> >> > Xen-devel@lists.xensource.com
>> >> > http://lists.xensource.com/xen-devel
>> >>
>> >> _______________________________________________
>> >> Xen-devel mailing list
>> >> Xen-devel@lists.xensource.com
>> >> http://lists.xensource.com/xen-devel
>> >
>> >--
>> >Push Me Pull You - Distributed SCM tool (http://www.cl.cam.ac.uk/~maw48/
>> >pmpu/)
>> >
>> >_______________________________________________
>> >Xen-devel mailing list
>> >Xen-devel@lists.xensource.com
>> >http://lists.xensource.com/xen-devel
>
>
>
>-- 
>Push Me Pull You - Distributed SCM tool (http://www.cl.cam.ac.uk/~maw48/
>pmpu/)

Re: [PATCH] Prevent changing a memory size of Domain-0 evenif users make a careless mistake

[Jan Beulich]

>>> Masaki Kanno <kanno.masaki@jp.fujitsu.com> 04.04.08 12:06 >>>
>If users accidentally change a memory size of Domain-0 to very small 
>memory size by xm mem-set command, users will be not able to operate 
>Domain-0.  I think that Domain-0 is important for Xen, so I'd like to 
>prevent the accident by xm mem-set command. 

Each domain, in my opinion, should also be able to protect itself from
being ballooned down too much. We have been carrying a respective
patch for quite a while. Since originally it wasn't written by me, I never
tried to push it. Nevertheless, I'm showing it below to see whether
others would think it makes sense.

Jan

From: ksrinivasan@novell.com 
Subject: Don't allow ballooning down a domain below a reasonable limit.
References: 172482

Reasonable is hard to judge; we don't want to disallow small domains.
But the system needs a reasonable amount of memory to perform its
duties, set up tables, etc. If on the other hand, the admin is able
to set up and boot up correctly a very small domain, there's no point
in forcing it to be larger.
We end up with some kind of logarithmic function, approximated.

Memory changes are logged, so making domains too small should at least
result in a trace.

Signed-off-by: Kurt Garloff <garloff@suse.de>

Index: head-2008-02-20/drivers/xen/balloon/balloon.c
===================================================================
--- head-2008-02-20.orig/drivers/xen/balloon/balloon.c	2008-02-20 10:32:43.000000000 +0100
+++ head-2008-02-20/drivers/xen/balloon/balloon.c	2008-02-20 10:40:54.000000000 +0100
@@ -194,6 +194,42 @@ static unsigned long current_target(void
 	return target;
 }
 
+static unsigned long minimum_target(void)
+{
+	unsigned long min_pages;
+	unsigned long curr_pages = current_target();
+#ifndef CONFIG_XEN
+#define max_pfn totalram_pages
+#endif
+
+#define MB2PAGES(mb) ((mb) << (20 - PAGE_SHIFT))
+	/* Simple continuous piecewiese linear function:
+	 *  max MiB -> min MiB	gradient
+	 *       0	   0
+	 *      16	  16
+	 *      32	  24
+	 *     128	  72	(1/2)
+	 *     512 	 168	(1/4)
+	 *    2048	 360	(1/8)
+	 *    8192	 552	(1/32)
+	 *   32768	1320
+	 *  131072	4392
+	 */
+	if (max_pfn < MB2PAGES(128))
+		min_pages = MB2PAGES(8) + (max_pfn >> 1);
+	else if (max_pfn < MB2PAGES(512))
+		min_pages = MB2PAGES(40) + (max_pfn >> 2);
+	else if (max_pfn < MB2PAGES(2048))
+		min_pages = MB2PAGES(104) + (max_pfn >> 3);
+	else
+		min_pages = MB2PAGES(296) + (max_pfn >> 5);
+#undef MB2PAGES
+
+	/* Don't enforce growth */
+	return min_pages < curr_pages ? min_pages : curr_pages;
+#undef max_pfn
+}
+
 static int increase_reservation(unsigned long nr_pages)
 {
 	unsigned long  pfn, i, flags;
@@ -382,6 +418,17 @@ static void balloon_process(struct work_
 /* Resets the Xen limit, sets new target, and kicks off processing. */
 void balloon_set_new_target(unsigned long target)
 {
+	/* First make sure that we are not lowering the value below the
+	 * "minimum".
+	 */
+	unsigned long min_pages = minimum_target();
+
+	if (target < min_pages)
+		target = min_pages;
+
+	printk(KERN_INFO "Setting mem allocation to %lu kiB\n",
+	       PAGES2KB(target));
+
 	/* No need for lock. Not read-modify-write updates. */
 	bs.hard_limit   = ~0UL;
 	bs.target_pages = target;

Re: [PATCH] Prevent changing a memory size of Domain-0 evenif users make a careless mistake

[Keir Fraser]

This at least I have a bit more time for. It's trying to pick a minimum
below which only bad things can happen. This is a plausible thing to try for
when you know the details of the specific operating system (which of course
you do in this case, since it's implemented in an OS driver). What I don't
like about Masaki's patch is that it's very specific, it abuses a
configuration variable that actually has its own meaning specific to the
auto-ballooning logic, and also is frankly an unwarranted and possibly
unwanted expression of policy inside xend.

 -- Keir

On 4/4/08 11:20, "Jan Beulich" <jbeulich@novell.com> wrote:

>>>> Masaki Kanno <kanno.masaki@jp.fujitsu.com> 04.04.08 12:06 >>>
>> If users accidentally change a memory size of Domain-0 to very small
>> memory size by xm mem-set command, users will be not able to operate
>> Domain-0.  I think that Domain-0 is important for Xen, so I'd like to
>> prevent the accident by xm mem-set command.
> 
> Each domain, in my opinion, should also be able to protect itself from
> being ballooned down too much. We have been carrying a respective
> patch for quite a while. Since originally it wasn't written by me, I never
> tried to push it. Nevertheless, I'm showing it below to see whether
> others would think it makes sense.
> 
> Jan
> 
> From: ksrinivasan@novell.com
> Subject: Don't allow ballooning down a domain below a reasonable limit.
> References: 172482
> 
> Reasonable is hard to judge; we don't want to disallow small domains.
> But the system needs a reasonable amount of memory to perform its
> duties, set up tables, etc. If on the other hand, the admin is able
> to set up and boot up correctly a very small domain, there's no point
> in forcing it to be larger.
> We end up with some kind of logarithmic function, approximated.
> 
> Memory changes are logged, so making domains too small should at least
> result in a trace.
> 
> Signed-off-by: Kurt Garloff <garloff@suse.de>
> 
> Index: head-2008-02-20/drivers/xen/balloon/balloon.c
> ===================================================================
> --- head-2008-02-20.orig/drivers/xen/balloon/balloon.c 2008-02-20
> 10:32:43.000000000 +0100
> +++ head-2008-02-20/drivers/xen/balloon/balloon.c 2008-02-20
> 10:40:54.000000000 +0100
> @@ -194,6 +194,42 @@ static unsigned long current_target(void
> return target;
>  }
>  
> +static unsigned long minimum_target(void)
> +{
> + unsigned long min_pages;
> + unsigned long curr_pages = current_target();
> +#ifndef CONFIG_XEN
> +#define max_pfn totalram_pages
> +#endif
> +
> +#define MB2PAGES(mb) ((mb) << (20 - PAGE_SHIFT))
> + /* Simple continuous piecewiese linear function:
> +  *  max MiB -> min MiB gradient
> +  *       0    0
> +  *      16   16
> +  *      32   24
> +  *     128   72 (1/2)
> +  *     512   168 (1/4)
> +  *    2048  360 (1/8)
> +  *    8192  552 (1/32)
> +  *   32768 1320
> +  *  131072 4392
> +  */
> + if (max_pfn < MB2PAGES(128))
> +  min_pages = MB2PAGES(8) + (max_pfn >> 1);
> + else if (max_pfn < MB2PAGES(512))
> +  min_pages = MB2PAGES(40) + (max_pfn >> 2);
> + else if (max_pfn < MB2PAGES(2048))
> +  min_pages = MB2PAGES(104) + (max_pfn >> 3);
> + else
> +  min_pages = MB2PAGES(296) + (max_pfn >> 5);
> +#undef MB2PAGES
> +
> + /* Don't enforce growth */
> + return min_pages < curr_pages ? min_pages : curr_pages;
> +#undef max_pfn
> +}
> +
>  static int increase_reservation(unsigned long nr_pages)
>  {
> unsigned long  pfn, i, flags;
> @@ -382,6 +418,17 @@ static void balloon_process(struct work_
>  /* Resets the Xen limit, sets new target, and kicks off processing. */
>  void balloon_set_new_target(unsigned long target)
>  {
> + /* First make sure that we are not lowering the value below the
> +  * "minimum".
> +  */
> + unsigned long min_pages = minimum_target();
> +
> + if (target < min_pages)
> +  target = min_pages;
> +
> + printk(KERN_INFO "Setting mem allocation to %lu kiB\n",
> +        PAGES2KB(target));
> +
> /* No need for lock. Not read-modify-write updates. */
> bs.hard_limit   = ~0UL;
> bs.target_pages = target;
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] Prevent changing a memory size ofDomain-0 evenif users make a careless mistake

[Jan Beulich]

>>> Keir Fraser <keir.fraser@eu.citrix.com> 04.04.08 12:27 >>>
>This at least I have a bit more time for. It's trying to pick a minimum
>below which only bad things can happen. This is a plausible thing to try for
>when you know the details of the specific operating system (which of course
>you do in this case, since it's implemented in an OS driver). What I don't
>like about Masaki's patch is that it's very specific, it abuses a
>configuration variable that actually has its own meaning specific to the
>auto-ballooning logic, and also is frankly an unwarranted and possibly
>unwanted expression of policy inside xend.

So any chance you'd take it if either of the original authors or I submit
it with a formal Signed-Off-By line?

Jan

Re: [PATCH] Prevent changing a memory size ofDomain-0 evenif users make a careless mistake

[Keir Fraser]

On 4/4/08 11:50, "Jan Beulich" <jbeulich@novell.com> wrote:

>>>> Keir Fraser <keir.fraser@eu.citrix.com> 04.04.08 12:27 >>>
>> This at least I have a bit more time for. It's trying to pick a minimum
>> below which only bad things can happen. This is a plausible thing to try for
>> when you know the details of the specific operating system (which of course
>> you do in this case, since it's implemented in an OS driver). What I don't
>> like about Masaki's patch is that it's very specific, it abuses a
>> configuration variable that actually has its own meaning specific to the
>> auto-ballooning logic, and also is frankly an unwarranted and possibly
>> unwanted expression of policy inside xend.
> 
> So any chance you'd take it if either of the original authors or I submit
> it with a formal Signed-Off-By line?

Yes.

 -- Keir

Re: [PATCH] Prevent changing a memory size of Domain-0 evenif users make a careless mistake

[Mark Williamson]

> This at least I have a bit more time for. It's trying to pick a minimum
> below which only bad things can happen. This is a plausible thing to try
> for when you know the details of the specific operating system (which of
> course you do in this case, since it's implemented in an OS driver).

Agreed, ideally all domains ought to protect themselves from an insanely low 
mem-set request (although since these generally come from the administrator, 
some kind of failure feedback would be good...  but that's another issue).

> What I 
> don't like about Masaki's patch is that it's very specific, it abuses a
> configuration variable that actually has its own meaning specific to the
> auto-ballooning logic, and also is frankly an unwarranted and possibly
> unwanted expression of policy inside xend.

Well, dom0-min-mem was originally added for autoballooning, but it seems more 
logical to have it applied whenever a potential ballooning operation is 
requested.  IMO the original behaviour was inconsistent and this fixes it.

I don't think it's unreasonable for an administrator to expect "dom0-min-mem" 
to set the minimum threshold for dom0 memory that should be allowed by the 
commands, rather than just for some of them.  I also think that Kan's 
proposed behaviour is unlikely to inconvenience anyone but may save a few 
people a nasty surprise.

FWIW, I'd argue for both approaches since I see them as complimentary.

Cheers,
Mark

>  -- Keir
>
> On 4/4/08 11:20, "Jan Beulich" <jbeulich@novell.com> wrote:
> >>>> Masaki Kanno <kanno.masaki@jp.fujitsu.com> 04.04.08 12:06 >>>
> >>
> >> If users accidentally change a memory size of Domain-0 to very small
> >> memory size by xm mem-set command, users will be not able to operate
> >> Domain-0.  I think that Domain-0 is important for Xen, so I'd like to
> >> prevent the accident by xm mem-set command.
> >
> > Each domain, in my opinion, should also be able to protect itself from
> > being ballooned down too much. We have been carrying a respective
> > patch for quite a while. Since originally it wasn't written by me, I
> > never tried to push it. Nevertheless, I'm showing it below to see whether
> > others would think it makes sense.
> >
> > Jan
> >
> > From: ksrinivasan@novell.com
> > Subject: Don't allow ballooning down a domain below a reasonable limit.
> > References: 172482
> >
> > Reasonable is hard to judge; we don't want to disallow small domains.
> > But the system needs a reasonable amount of memory to perform its
> > duties, set up tables, etc. If on the other hand, the admin is able
> > to set up and boot up correctly a very small domain, there's no point
> > in forcing it to be larger.
> > We end up with some kind of logarithmic function, approximated.
> >
> > Memory changes are logged, so making domains too small should at least
> > result in a trace.
> >
> > Signed-off-by: Kurt Garloff <garloff@suse.de>
> >
> > Index: head-2008-02-20/drivers/xen/balloon/balloon.c
> > ===================================================================
> > --- head-2008-02-20.orig/drivers/xen/balloon/balloon.c 2008-02-20
> > 10:32:43.000000000 +0100
> > +++ head-2008-02-20/drivers/xen/balloon/balloon.c 2008-02-20
> > 10:40:54.000000000 +0100
> > @@ -194,6 +194,42 @@ static unsigned long current_target(void
> > return target;
> >  }
> >
> > +static unsigned long minimum_target(void)
> > +{
> > + unsigned long min_pages;
> > + unsigned long curr_pages = current_target();
> > +#ifndef CONFIG_XEN
> > +#define max_pfn totalram_pages
> > +#endif
> > +
> > +#define MB2PAGES(mb) ((mb) << (20 - PAGE_SHIFT))
> > + /* Simple continuous piecewiese linear function:
> > +  *  max MiB -> min MiB gradient
> > +  *       0    0
> > +  *      16   16
> > +  *      32   24
> > +  *     128   72 (1/2)
> > +  *     512   168 (1/4)
> > +  *    2048  360 (1/8)
> > +  *    8192  552 (1/32)
> > +  *   32768 1320
> > +  *  131072 4392
> > +  */
> > + if (max_pfn < MB2PAGES(128))
> > +  min_pages = MB2PAGES(8) + (max_pfn >> 1);
> > + else if (max_pfn < MB2PAGES(512))
> > +  min_pages = MB2PAGES(40) + (max_pfn >> 2);
> > + else if (max_pfn < MB2PAGES(2048))
> > +  min_pages = MB2PAGES(104) + (max_pfn >> 3);
> > + else
> > +  min_pages = MB2PAGES(296) + (max_pfn >> 5);
> > +#undef MB2PAGES
> > +
> > + /* Don't enforce growth */
> > + return min_pages < curr_pages ? min_pages : curr_pages;
> > +#undef max_pfn
> > +}
> > +
> >  static int increase_reservation(unsigned long nr_pages)
> >  {
> > unsigned long  pfn, i, flags;
> > @@ -382,6 +418,17 @@ static void balloon_process(struct work_
> >  /* Resets the Xen limit, sets new target, and kicks off processing. */
> >  void balloon_set_new_target(unsigned long target)
> >  {
> > + /* First make sure that we are not lowering the value below the
> > +  * "minimum".
> > +  */
> > + unsigned long min_pages = minimum_target();
> > +
> > + if (target < min_pages)
> > +  target = min_pages;
> > +
> > + printk(KERN_INFO "Setting mem allocation to %lu kiB\n",
> > +        PAGES2KB(target));
> > +
> > /* No need for lock. Not read-modify-write updates. */
> > bs.hard_limit   = ~0UL;
> > bs.target_pages = target;
> >
> >
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel



-- 
Push Me Pull You - Distributed SCM tool (http://www.cl.cam.ac.uk/~maw48/pmpu/)

Re: [PATCH] Prevent changing a memory size of Domain-0 evenif users make a careless mistake

[John Levon]

On Fri, Apr 04, 2008 at 11:20:10AM +0100, Jan Beulich wrote:

> Each domain, in my opinion, should also be able to protect itself from
> being ballooned down too much. We have been carrying a respective
> patch for quite a while. Since originally it wasn't written by me, I never

FWIW, we have a similar approach in Solaris.

regards,
john

Re: [PATCH] Prevent changing a memory size of Domain-0 evenif users make a careless mistake

[Jeremy Fitzhardinge]

Jan Beulich wrote:
>>>> Masaki Kanno <kanno.masaki@jp.fujitsu.com> 04.04.08 12:06 >>>
>>>>         
>> If users accidentally change a memory size of Domain-0 to very small 
>> memory size by xm mem-set command, users will be not able to operate 
>> Domain-0.  I think that Domain-0 is important for Xen, so I'd like to 
>> prevent the accident by xm mem-set command. 
>>     
>
> Each domain, in my opinion, should also be able to protect itself from
> being ballooned down too much. We have been carrying a respective
> patch for quite a while. Since originally it wasn't written by me, I never
> tried to push it. Nevertheless, I'm showing it below to see whether
> others would think it makes sense.
>
> Jan
>
> From: ksrinivasan@novell.com 
> Subject: Don't allow ballooning down a domain below a reasonable limit.
> References: 172482
>
> Reasonable is hard to judge; we don't want to disallow small domains.
> But the system needs a reasonable amount of memory to perform its
> duties, set up tables, etc. If on the other hand, the admin is able
> to set up and boot up correctly a very small domain, there's no point
> in forcing it to be larger.
> We end up with some kind of logarithmic function, approximated.
>   

Hm, I've been bitten by this myself quite a lot lately, so I'm 
sympathetic to a patch like this.  In the 2.6 pvops balloon driver, I'm 
using hotplug memory to extend the page structures, rather than relying 
on statically preallocating them at boot.  This means that max_pfn isn't 
terribly meaningful, since there's no fixed upper limit.

I was thinking along the lines of having the balloon thread pay 
attention to how much free (lowmem) memory is actually available, and 
stop processing if it drops below some threshold.  That gives the VM 
some time to deal with the memory pressure (swap things out, etc), 
without making things OOM-killer critical.  And if the VM can't free up 
any more memory, then we can't force it beyond that.   The processing 
rate could be proportional to the amount of free memory rather than a 
hard go/no-go switch to make things a bit smoother.

    J