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

[Mark Williamson <mark.williamson@xxxxxxxxxxxx>]

> 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@xxxxxxxxxx> wrote:
> >>>> Masaki Kanno <kanno.masaki@xxxxxxxxxxxxxx> 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@xxxxxxxxxx
> > 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@xxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxxxxxx
> > http://lists.xensource.com/xen-devel
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel