Re: [PATCH] x86: set dom0's default maximum reservation to the initial number of pages

[David Vrabel <david.vrabel@citrix.com>]

On 21/03/12 08:36, Jan Beulich wrote:
>>>> On 20.03.12 at 19:21, David Vrabel <david.vrabel@citrix.com> wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> If a maximum reservation for dom0 is not explictly given (i.e., no
>> dom0_mem=max:MMM command line option), then set the maximum
>> reservation to the initial number of pages.  This is what most people
>> seem to expect when they specify dom0_mem=512M (i.e., exactly 512 MB
>> and no more).
> 
> I think I had seen this before (not sure whether it came from you),
> and had already NAK-ed the idea back then. Please let's not play
> with the meaning of existing hypervisor options, unless the "most"
> in your description can validly be replaced by "all". In this case, _I_
> am using the option in its current sense almost everywhere
> (expecting to be able to balloon up beyond the initial allocation),
> and it can also be useful in hotplug capable machines. If someone
> wants a particular maximum, (s)he should add the respective
> command line option.

XenServer uses dom0_mem=752M and expects it to set the maximum
reservation to 752M (if it doesn't it grinds to a halt rather
spectacularly as it runs out of memory due to the extra memory used for
page tables etc.).  So whilst I appreciate your desire to not have to
change any command lines, I don't want to either.

The dom0_mem option has been historically confusing.  The min, and max
options don't affect the initial number of pages in a useful way
(min:MMM and max:MMM are no different to simply specifying MMM).

So I think we need to decide what behaviour is the best in the long term.

The two options are:

The current behaviour:
                      | Initial | Max       |
dom0_mem=III          | III     | Unlimited |
dom0_mem=III,max:MMM  | III     | MMM       |
dom0_mem=max:MMM      | MMM     | MMM       |

or this proposal:

                      | Initial | Max       |
dom0_mem=III          | III     | III       |
dom0_mem=III,max:MMM  | III     | MMM       |
dom0_mem=max:MMM      | MMM     | MMM       |

Hmm. Having enumerated the two options, the current behaviour looks more
sensible to me now...

The docs still need updating and I think the min:MMM option should be
deprecated.

>> This change means that with Linux 3.0.5 and later kernels,
>> dom0_mem=512M has the same result as older, 'classic Xen' kernels. The
>> older kernels used the initial number of pages to set the maximum
>> number of pages and did not query the hypervisor for the maximum
>> reservation.
> 
> If the pv-ops kernels behave differently in this aspect than the forward
> ported ones, then it should really be the newer one to get adjusted to
> the original behavior (if so intended), unless the old behavior was
> completely insane. Adjusting the hypervisor for a shortcoming(?) in a
> particular implementation of a Dom0 kernel doesn't seem to be the right
> route - please also consider non-Linux Dom0 kernels that might expect
> the current behavior.

The old behaviour did not allow dom0 to balloon up beyond the initial
allocation so we don't what to go back to that.

>> It is still possible to have a larger reservation by explicitly
>> specifying dom0_mem=max:MMM.
> 
> I am not intending to update dozens of command lines.

Looks like I will be though...

>> Keir,
>>
>> Suggest waiting for an Ack from Konrad (I think it results in the
>> behaviour we want but would prefer it if Konrad confirmed).
>>
>> Also consider for 4.1.
>>
>> Thanks.
>>
>> David
>>
>>  docs/misc/xen-command-line.markdown |    8 +++++++-
>>  xen/arch/x86/domain_build.c         |   10 ++++++++++
>>  2 files changed, 17 insertions(+), 1 deletions(-)
>>
>> diff --git a/docs/misc/xen-command-line.markdown 
>> b/docs/misc/xen-command-line.markdown
>> index beb8462..0798700 100644
>> --- a/docs/misc/xen-command-line.markdown
>> +++ b/docs/misc/xen-command-line.markdown
>> @@ -221,12 +221,18 @@ Specify the total size for dom0.
>>  ### dom0\_mem (x86)
>>  > `= List of ( min:<value> | max: <value> | <value> )`
>>  
>> -each `<value>` is a size parameter.  If the size is positive, it represents an 
>> absolute value.  If the size is negative, the size specified is subtracted 
>> from the total available memory.
>> +Specify the amount of memory for the initial domain (dom0) and the maximum 
>> reservation (the maximum amount of memory that dom0 can be increased or 
>> ballooned to).
>> +
>> +Each `<value>` is a size parameter.  If the size is positive, it represents 
>> an absolute value.  If the size is negative, the size specified is subtracted 
>> from the total available memory.
>>  
>>  * `min:<value>` specifies the minimum amount of memory allocated to dom0.
>>  * `max:<value>` specifies the maximum amount of memory allocated to dom0.
>>  * `<value>` specified the exact amount of memory allocated to dom0.
>>  
>> +If `max:<value>` is specified then this sets the maximum reservation, 
>> otherwise the maximum reservation is set to the amount of memory allocated to 
>> dom0.
>> +
>> +For example, with `dom0_mem=512M`, dom0 starts with 512 MB and cannot 
>> balloon up any more. With `dom0_mem=512M,max:2G`, dom0 starts with 512 MB of 
>> memory and can balloon up to 2 GB.
>> +
>>  ### dom0\_shadow
>>  ### dom0\_vcpus\_pin
>>  > `= <boolean>`
>> diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
>> index b3c5d4c..0c09abc 100644
>> --- a/xen/arch/x86/domain_build.c
>> +++ b/xen/arch/x86/domain_build.c
>> @@ -253,6 +253,16 @@ static unsigned long __init compute_dom0_nr_pages(
>>      }
>>  #endif
>>  
>> +    /*
>> +     * Set dom0's maximum reservation.
>> +     *
>> +     * If no maximum was set with dom0_mem=max:MMM, then the maximum
>> +     * is the same as the initial number of pages.  This is so
>> +     * dom0_mem=MMM gives the behaviour most people expect (i.e., this
>> +     * much RAM and no more).
>> +     */
>> +    if ( max_pages == LONG_MAX )
>> +        max_pages = nr_pages;
>>      d->max_pages = min_t(unsigned long, max_pages, UINT_MAX);
>>  
>>      return nr_pages;