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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.11.18 at 17:40, <jgross@xxxxxxxx> wrote:
> @@ -777,13 +777,20 @@ The amount of memory will be at least the minimum but 
> never more than
>  the maximum (i.e., `max` overrides the `min` option).  If there isn't
>  enough memory then as much as possible is allocated.
>  
> -`max:<size>` also sets the maximum reservation (the maximum amount of
> +`max:<sz>` also sets the maximum reservation (the maximum amount of
>  memory dom0 can balloon up to).  If this is omitted then the maximum
>  reservation is unlimited.
>  
>  For example, to set dom0's initial memory allocation to 512MB but
>  allow it to balloon up as far as 1GB use `dom0_mem=512M,max:1G`
>  
> +> `<sz>` is: `<size> | [<size>+]<frac>%`
> +> `<frac>` is an integer < 100

It is probably fine to state it like this here, but the implementation
should take care of not allowing <size>+<frac>% to be greater
than available memory.

> @@ -50,7 +55,28 @@ static unsigned long __init parse_amt(const char *s, const 
> char **ps,
>                                        unsigned long avail)
>  {
>      unsigned int minus = (*s == '-') ? 1 : 0;
> -    unsigned long pages = parse_size_and_unit(s + minus, ps) >> PAGE_SHIFT;
> +    unsigned long val, pages = 0;
> +
> +    /* Avoid accessing s[-1] in case value starts with '%'. */
> +    if ( *s == '%' )
> +        return 0;
> +
> +    s += minus;
> +    while ( isdigit(*s) )
> +    {
> +        val = parse_size_and_unit(s, ps);
> +        s = *ps;
> +        if ( *s == '%' && isdigit(*(s - 1)) && val < (100 << 10) )

In the comment above you talk about s[-1] - why do you use a more
complicated (even if just slightly) expression here?

> +        {
> +            pages += (val >> 10) * avail / 100;

The scaling by 10 bits here and above make it rather desirable to
slightly change parse_size_and_unit() instead: I think it could
easily be made not do the scaling if it finds % as the next input
character. Otherwise at least a brief comment should be put here.

Despite the comments - I very much like the added flexibility.

Jan



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