Re: [Xen-devel] [PATCH v3 2/3] xen/x86: add dom0 memory sizing variants

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.12.18 at 12:44, <jgross@xxxxxxxx> wrote:
> Today the memory size of dom0 can be specified only in terms of bytes
> (either an absolute value or "host-mem - value"). When dom0 shouldn't
> be auto-ballooned this requires nearly always a manual adaption of the
> Xen boot parameters to reflect the actual host memory size.
> 
> Add more possibilities to specify memory sizes. Today we have:
> 
> dom0_mem= List of ( min:<size> | max:<size> | <size> )
> 
> with <size> being a positive or negative size value (e.g. 1G).
> 
> Modify that to:
> 
> dom0_mem= List of ( min:<sz> | max:<sz> | <sz> )
> <sz>: <size> | [<size>+]<frac>%
> <frac>: integer value < 100
> 
> With the following semantics:
> 
> <frac>% specifies a fraction of host memory size in percent.
> <sz> is a percentage of host memory plus an offset.
> 
> So <sz> being 1G+25% on a 256G host would result in 65G.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

Once again
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
yet once again with a remark:

> -static long __init parse_amt(const char *s, const char **ps)
> +static int __init parse_amt(const char *s, const char **ps, struct memsize 
> *sz)
>  {
> -    long pages = parse_size_and_unit((*s == '-') ? s+1 : s, ps) >> 
> PAGE_SHIFT;
> -    return (*s == '-') ? -pages : pages;
> +    unsigned long val;
> +    struct memsize tmp = { };
> +    unsigned int items = 0;
> +
> +    tmp.minus = (*s == '-');
> +    if ( tmp.minus )
> +        s++;
> +
> +    do
> +    {
> +        if ( !isdigit(*s) )
> +            return -EINVAL;
> +
> +        val = parse_size_and_unit(s, ps);
> +        s = *ps;
> +        if ( *s == '%' )
> +        {
> +            if ( val >= 100 )
> +                return -EINVAL;
> +            tmp.percent = val;
> +            s++;
> +            items++; /* No other item allowed. */

Isn't this unnecessary with ...

> +        }
> +        else
> +        {
> +            /* <size> item must be first one. */
> +            if ( items )
> +                return -EINVAL;
> +            tmp.nr_pages = val >> PAGE_SHIFT;
> +        }
> +        items++;

... this? However, allowing <frac>%+<size> would apparently have
been easy (and add further flexibility).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel